feat(controller): drop option to rollout Nginx once (or disable rollout)

api/v1alpha1/rpaasinstance_types.go

@@ -92,17 +92,6 @@ type RpaasInstanceSpec struct {
 	// +optional
 	TLSSessionResumption *TLSSessionResumption `json:"tlsSessionResumption,omitempty"`
 
-	// RolloutNginxOnce causes a rollout of the nginx object if changes are
-	// detected only once. After updating the nginx object the controller will
-	// set this value to false.
-	// +optional
-	RolloutNginxOnce bool `json:"rolloutNginxOnce,omitempty"`
-
-	// RolloutNginx causes the controller to always update the nginx object
-	// regardless of the default behavior on the controller.
-	// +optional
-	RolloutNginx bool `json:"rolloutNginx,omitempty"`
-
 	// AllowedUpstreams holds the endpoints to which the RpaasInstance should be able to access
 	// +optional
 	AllowedUpstreams []AllowedUpstream `json:"allowedUpstreams,omitempty"`

config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml

@@ -5873,15 +5873,6 @@ spec:
                       between explicit zero and not specified. Defaults to 1.
                     format: int32
                     type: integer
-                  rolloutNginx:
-                    description: RolloutNginx causes the controller to always update
-                      the nginx object regardless of the default behavior on the controller.
-                    type: boolean
-                  rolloutNginxOnce:
-                    description: RolloutNginxOnce causes a rollout of the nginx object
-                      if changes are detected only once. After updating the nginx
-                      object the controller will set this value to false.
-                    type: boolean
                   service:
                     description: Service to expose the nginx instance
                     properties:

config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml

@@ -5638,15 +5638,6 @@ spec:
                   between explicit zero and not specified. Defaults to 1.
                 format: int32
                 type: integer
-              rolloutNginx:
-                description: RolloutNginx causes the controller to always update the
-                  nginx object regardless of the default behavior on the controller.
-                type: boolean
-              rolloutNginxOnce:
-                description: RolloutNginxOnce causes a rollout of the nginx object
-                  if changes are detected only once. After updating the nginx object
-                  the controller will set this value to false.
-                type: boolean
               service:
                 description: Service to expose the nginx instance
                 properties:

controllers/controller.go

@@ -314,9 +314,6 @@ func (r *RpaasInstanceReconciler) reconcileSecretForSessionTickets(ctx context.C
 	}
 
 	if !enabled {
-		if !r.rolloutEnabled(instance) {
-			return nil
-		}
 		return r.Client.Delete(ctx, &secret)
 	}
 
@@ -353,9 +350,6 @@ func (r *RpaasInstanceReconciler) reconcileCronJobForSessionTickets(ctx context.
 	}
 
 	if !enabled {
-		if !r.rolloutEnabled(instance) {
-			return nil
-		}
 		return r.Client.Delete(ctx, &cj)
 	}
 
@@ -737,6 +731,7 @@ func (r *RpaasInstanceReconciler) reconcileNginx(ctx context.Context, instance *
 			logrus.Errorf("Failed to get nginx CR: %v", err)
 			return err
 		}
+
 		err = r.Client.Create(ctx, nginx)
 		if err != nil {
 			logrus.Errorf("Failed to create nginx CR: %v", err)
@@ -745,23 +740,13 @@ func (r *RpaasInstanceReconciler) reconcileNginx(ctx context.Context, instance *
 		return nil
 	}
 
-	// Update only replicas if rollout is not enabled to ensure HPAs work
-	// correctly.
-	if !r.rolloutEnabled(instance) {
-		nginx = found
-		nginx.Spec.Replicas = instance.Spec.Replicas
-	}
-
 	nginx.ObjectMeta.ResourceVersion = found.ObjectMeta.ResourceVersion
 	err = r.Client.Update(ctx, nginx)
 	if err != nil {
 		logrus.Errorf("Failed to update nginx CR: %v", err)
 	}
-	return err
-}
 
-func (r *RpaasInstanceReconciler) rolloutEnabled(instance *v1alpha1.RpaasInstance) bool {
-	return r.RolloutNginxEnabled || instance.Spec.RolloutNginx || instance.Spec.RolloutNginxOnce
+	return err
 }
 
 func (r *RpaasInstanceReconciler) reconcileCacheSnapshot(ctx context.Context, instance *v1alpha1.RpaasInstance, plan *v1alpha1.RpaasPlan) error {
@@ -773,14 +758,11 @@ func (r *RpaasInstanceReconciler) reconcileCacheSnapshot(ctx context.Context, in
 		return r.reconcileCacheSnapshotVolume(ctx, instance, plan)
 	}
 
-	if !r.rolloutEnabled(instance) {
-		return nil
-	}
-
 	err := r.destroyCacheSnapshotCronJob(ctx, instance)
 	if err != nil {
 		return err
 	}
+
 	return r.destroyCacheSnapshotVolume(ctx, instance)
 }
 

controllers/rpaasinstance_controller.go

@@ -190,10 +190,6 @@ func (r *RpaasInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, err
 	}
 
-	if err = r.resetRolloutOnce(ctx, instance); err != nil {
-		return ctrl.Result{}, err
-	}
-
 	return ctrl.Result{}, nil
 }
 
@@ -237,30 +233,6 @@ func (r *RpaasInstanceReconciler) refreshStatus(ctx context.Context, instance *e
 	return nil
 }
 
-func (r *RpaasInstanceReconciler) resetRolloutOnce(ctx context.Context, instance *extensionsv1alpha1.RpaasInstance) error {
-	if !instance.Spec.RolloutNginxOnce {
-		return nil
-	}
-
-	var rawInstance extensionsv1alpha1.RpaasInstance
-	if err := r.Client.Get(ctx, types.NamespacedName{
-		Name:      instance.Name,
-		Namespace: instance.Namespace,
-	}, &rawInstance); err != nil {
-		return err
-	}
-	if !rawInstance.Spec.RolloutNginxOnce {
-		return nil
-	}
-
-	rawInstance.Spec.RolloutNginxOnce = false
-	err := r.Client.Update(ctx, &rawInstance)
-	if err != nil {
-		return fmt.Errorf("failed to update rpaas instance rollout once: %v", err)
-	}
-	return nil
-}
-
 func (r *RpaasInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&extensionsv1alpha1.RpaasInstance{}).

internal/pkg/rpaas/k8s.go

@@ -253,7 +253,6 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e
 			Annotations: instance.Annotations,
 			Labels:      instance.Labels,
 		},
-		RolloutNginxOnce: true,
 	}
 
 	if config.Get().NamespacedInstances {
@@ -2230,18 +2229,17 @@ func (m *k8sRpaasManager) getEvents(ctx context.Context, nginx *nginxv1alpha1.Ng
 }
 
 func (m *k8sRpaasManager) patchInstance(ctx context.Context, originalInstance *v1alpha1.RpaasInstance, updatedInstance *v1alpha1.RpaasInstance) error {
-	updatedInstance.Spec.RolloutNginxOnce = true
-
 	originalData, err := json.Marshal(originalInstance)
 	if err != nil {
 		return err
 	}
+
 	updatedData, err := json.Marshal(updatedInstance)
 	if err != nil {
 		return err
 	}
-	data, err := jsonpatch.CreateMergePatch(originalData, updatedData)
 
+	data, err := jsonpatch.CreateMergePatch(originalData, updatedData)
 	if err != nil {
 		return err
 	}

internal/pkg/rpaas/k8s_test.go

@@ -3011,7 +3011,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3066,7 +3065,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3126,7 +3124,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3187,7 +3184,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3242,7 +3238,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3293,7 +3288,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3360,7 +3354,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},
@@ -3413,7 +3406,6 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 							"rpaas_instance":                          "r1",
 						},
 					},
-					RolloutNginxOnce: true,
 				},
 			},
 		},

main.go

@@ -32,7 +32,6 @@ type configOpts struct {
 	syncPeriod                 time.Duration
 	portRangeMin               int
 	portRangeMax               int
-	enableRollout              bool
 }
 
 func (o *configOpts) bindFlags(fs *flag.FlagSet) {
@@ -48,7 +47,6 @@ func (o *configOpts) bindFlags(fs *flag.FlagSet) {
 	fs.DurationVar(&o.syncPeriod, "sync-period", 10*time.Hour, "The resync period for reconciling manager resources.")
 	fs.StringVar(&o.namespace, "namespace", "", "Limit the observed RpaasInstance resources from specific namespace (empty means all namespaces)")
 
-	fs.BoolVar(&o.enableRollout, "enable-rollout", true, "Enable automatic rollout of nginx objects on rpaas-instance change.")
 	fs.IntVar(&o.portRangeMin, "port-range-min", 20000, "Allocated port range start")
 	fs.IntVar(&o.portRangeMax, "port-range-max", 30000, "Allocated port range end")
 }
@@ -92,13 +90,12 @@ func main() {
 	}
 
 	if err = (&controllers.RpaasInstanceReconciler{
-		Client:              mgr.GetClient(),
-		Log:                 ctrl.Log.WithName("controllers").WithName("RpaasInstance"),
-		Scheme:              mgr.GetScheme(),
-		RolloutNginxEnabled: opts.enableRollout,
-		PortRangeMin:        int32(opts.portRangeMin),
-		PortRangeMax:        int32(opts.portRangeMax),
-		ImageMetadata:       registry.NewImageMetadata(),
+		Client:        mgr.GetClient(),
+		Log:           ctrl.Log.WithName("controllers").WithName("RpaasInstance"),
+		Scheme:        mgr.GetScheme(),
+		PortRangeMin:  int32(opts.portRangeMin),
+		PortRangeMax:  int32(opts.portRangeMax),
+		ImageMetadata: registry.NewImageMetadata(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "RpaasInstance")
 		os.Exit(1)