update to other reconcilers and tests

3scale · May 18, 2022 · e85defe · e85defe
1 parent d8bb561
commit e85defe
Show file tree

Hide file tree

Showing 10 changed files with 551 additions and 124 deletions.
diff --git a/pkg/3scale/amp/operator/apicast_reconciler.go b/pkg/3scale/amp/operator/apicast_reconciler.go
@@ -66,47 +66,30 @@ func (r *ApicastReconciler) Reconcile() (reconcile.Result, error) {
 		return reconcile.Result{}, err
 	}
 
-	opts := []reconcilers.DCMutateFn{
-		reconcilers.DeploymentConfigContainerResourcesMutator,
-		reconcilers.DeploymentConfigAffinityMutator,
-		reconcilers.DeploymentConfigTolerationsMutator,
-		apicastLogLevelEnvVarMutator,
-		apicastTracingConfigEnvVarsMutator,
-		apicastEnvironmentEnvVarMutator,
-		apicastHTTPSEnvVarMutator,
-		apicastProxyConfigurationsEnvVarMutator,
-		apicastVolumeMountsMutator,
-		apicastVolumesMutator,
-		apicastCustomPolicyAnnotationsMutator,  // Should be always after volume mutator
-		apicastTracingConfigAnnotationsMutator, // Should be always after volume mutator
-		apicastCustomEnvAnnotationsMutator,     // Should be always after volume
-		portsMutator,
-		apicastPodTemplateEnvConfigMapAnnotationsMutator,
-	}
+	stagingOpts := getApicastsGenericMutators()
 
 	if !metav1.HasAnnotation(r.apiManager.ObjectMeta, disableApicastStagingInstancesSyncing) || r.apiManager.Annotations[disableApicastStagingInstancesSyncing] != "true" {
-		opts = append(opts, reconcilers.DeploymentConfigReplicasMutator)
+		stagingOpts = append(stagingOpts, reconcilers.DeploymentConfigReplicasMutator)
 	}
 
 	// Staging DC
-	stagingDCMutator := reconcilers.DeploymentConfigMutator(
-		opts...
-	)
-	err = r.ReconcileDeploymentConfig(apicast.StagingDeploymentConfig(), stagingDCMutator)
+	err = r.ReconcileDeploymentConfig(apicast.StagingDeploymentConfig(), reconcilers.DeploymentConfigMutator(stagingOpts...))
 	if err != nil {
 		return reconcile.Result{}, err
 	}
 
 	// add apicast production env var mutator
-	apicastProductionOpts := append(opts, apicastProductionWorkersEnvVarMutator)
+	productionOpts := getApicastsGenericMutators()
+
+	productionOpts = append(productionOpts, apicastProductionWorkersEnvVarMutator)
 
 	if !metav1.HasAnnotation(r.apiManager.ObjectMeta, disableApicastProductionInstancesSyncing) || r.apiManager.Annotations[disableApicastProductionInstancesSyncing] != "true" {
-		apicastProductionOpts = append(opts, reconcilers.DeploymentConfigReplicasMutator)
+		productionOpts = append(productionOpts, reconcilers.DeploymentConfigReplicasMutator)
 	}
 
 	// Production DC
 	productionDCMutator := reconcilers.DeploymentConfigMutator(
-		apicastProductionOpts...
+		productionOpts...
 	)
 
 	err = r.ReconcileDeploymentConfig(apicast.ProductionDeploymentConfig(), productionDCMutator)
@@ -509,3 +492,23 @@ func Apicast(apimanager *appsv1alpha1.APIManager, cl client.Client) (*component.
 	}
 	return component.NewApicast(opts), nil
 }
+
+func getApicastsGenericMutators() []reconcilers.DCMutateFn {
+	return []reconcilers.DCMutateFn{
+		reconcilers.DeploymentConfigContainerResourcesMutator,
+		reconcilers.DeploymentConfigAffinityMutator,
+		reconcilers.DeploymentConfigTolerationsMutator,
+		apicastLogLevelEnvVarMutator,
+		apicastTracingConfigEnvVarsMutator,
+		apicastEnvironmentEnvVarMutator,
+		apicastHTTPSEnvVarMutator,
+		apicastProxyConfigurationsEnvVarMutator,
+		apicastVolumeMountsMutator,
+		apicastVolumesMutator,
+		apicastCustomPolicyAnnotationsMutator,  // Should be always after volume mutator
+		apicastTracingConfigAnnotationsMutator, // Should be always after volume mutator
+		apicastCustomEnvAnnotationsMutator,     // Should be always after volume
+		portsMutator,
+		apicastPodTemplateEnvConfigMapAnnotationsMutator,
+	}
+}
diff --git a/pkg/3scale/amp/operator/apicast_reconciler_test.go b/pkg/3scale/amp/operator/apicast_reconciler_test.go
@@ -555,7 +555,6 @@ func TestApicastReconcilerDisableReplicaSyncingAnnotations(t *testing.T) {
 	var (
 		namespace                  = "operator-unittest"
 		log                        = logf.Log.WithName("operator_test")
-		oneValue             int64 = 1
 		twoValue             int32 = 2
 	)
 	ctx := context.TODO()
@@ -575,15 +574,20 @@ func TestApicastReconcilerDisableReplicaSyncingAnnotations(t *testing.T) {
 		objName  string
 		obj      runtime.Object
 		apimanager *appsv1alpha1.APIManager
+		annotation string
+		annotationValue string
+		expectedAmountOfReplicas int32
+		validatingFunction func(*appsv1alpha1.APIManager, *appsv1.DeploymentConfig, string, string, int32) bool
 	}{
-		{"apicastStagingDisableReplicaSyncingTrueExisting", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "true")},
-		{"apicastStagingDisableReplicaSyncingTrueExisting", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "dummy")},
-		{"apicastStagingDisableReplicaSyncingTrueExisting", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "false")},
-		{"apicastStagingDisableReplicaSyncingTrueExisting", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator("dummyAnnotation", "true")},
-		{"apicastProductionDisableReplicaSyncingTrueExisting", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "true")},
-		{"apicastProductionDisableReplicaSyncingTrueExisting", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "dummy")},
-		{"apicastProductionDisableReplicaSyncingTrueExisting", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "false")},
-		{"apicastProductionDisableReplicaSyncingTrueExisting", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator("dummyAnnotation", "true")},
+		{"apicast-staging-DC-annotation not present", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator("someAnnotation", "false"), disableApicastStagingInstancesSyncing, "dummy", int32(1), confirmReplicasWhenAnnotationIsNotPresent},
+		{"apicast-staging-DC-annotation false", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "false"), disableApicastStagingInstancesSyncing, "false", int32(1), confirmReplicasWhenAnnotationPresent},
+		{"apicast-staging-DC-annotation true", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "true"), disableApicastStagingInstancesSyncing, "true", int32(2), confirmReplicasWhenAnnotationPresent},
+		{"apicast-staging-DC-annotation true of dummy value", "apicast-staging", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastStagingInstancesSyncing, "true"), disableApicastStagingInstancesSyncing, "someDummyValue", int32(1), confirmReplicasWhenAnnotationPresent},		
+
+		{"apicast-production-DC-annotation not present", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator("someAnnotation", "false"), disableApicastProductionInstancesSyncing, "dummy", int32(1), confirmReplicasWhenAnnotationIsNotPresent},
+		{"apicast-production-DC-annotation false", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "false"), disableApicastProductionInstancesSyncing, "false", int32(1), confirmReplicasWhenAnnotationPresent},
+		{"apicast-production-DC-annotation true", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "true"), disableApicastProductionInstancesSyncing, "true", int32(2), confirmReplicasWhenAnnotationPresent},
+		{"apicast-production-DC-annotation true of dummy value", "apicast-production", &appsv1.DeploymentConfig{}, apiManagerCreator(disableApicastProductionInstancesSyncing, "true"), disableApicastProductionInstancesSyncing, "someDummyValue", int32(1), confirmReplicasWhenAnnotationPresent},	
 	}
 
 	for _, tc := range cases {
@@ -632,32 +636,9 @@ func TestApicastReconcilerDisableReplicaSyncingAnnotations(t *testing.T) {
 				subT.Errorf("error fetching object %s: %v", tc.objName, err)
 			}
 
-			// if annotation is not present sync the values
-			if !metav1.HasAnnotation(tc.apimanager.ObjectMeta, disableApicastStagingInstancesSyncing) {
-				if dc.Spec.Replicas != int32(oneValue) {
-					subT.Errorf("value of expteced replicas does not match for %s. expected: 1 actual: %v", tc.objName, dc.Spec.Replicas)
-				}
-			}
-
-			// if annotation is present but it's value is a dummy value sync the values
-			if !metav1.HasAnnotation(tc.apimanager.ObjectMeta, disableApicastStagingInstancesSyncing) || tc.apimanager.Annotations[disableApicastStagingInstancesSyncing] == "dummy" {
-				if dc.Spec.Replicas != int32(oneValue) {
-					subT.Errorf("value of expteced replicas does not match for %s. expected: 1 actual: %v", tc.objName, dc.Spec.Replicas)
-				}
-			}
-
-			// if annotation is present and it's value is true, do not sync values
-			if metav1.HasAnnotation(tc.apimanager.ObjectMeta, disableApicastStagingInstancesSyncing) && tc.apimanager.Annotations[disableApicastStagingInstancesSyncing] == "true" {
-				if dc.Spec.Replicas != twoValue {
-					subT.Errorf("value of expteced replicas does not match for %s. expected: 2 actual: %v", tc.objName, dc.Spec.Replicas)
-				}
-			}
-
-			// if annotation is present and it's value is false, do not sync values
-			if metav1.HasAnnotation(tc.apimanager.ObjectMeta, disableApicastStagingInstancesSyncing) && tc.apimanager.Annotations[disableApicastStagingInstancesSyncing] == "false" {
-				if dc.Spec.Replicas != int32(oneValue) {
-					subT.Errorf("value of expteced replicas does not match for %s. expected: 1 actual: %v", tc.objName, dc.Spec.Replicas)
-				}
+			correct := tc.validatingFunction(tc.apimanager, dc, tc.annotation, tc.annotationValue, tc.expectedAmountOfReplicas)
+			if !correct {
+				subT.Errorf("value of expteced replicas does not match for %s. expected: %v actual: %v", tc.objName, tc.expectedAmountOfReplicas, dc.Spec.Replicas)
 			}
 		})
 	}

diff --git a/pkg/3scale/amp/operator/backend_reconciler.go b/pkg/3scale/amp/operator/backend_reconciler.go
@@ -5,7 +5,6 @@ import (
 	"github.com/3scale/3scale-operator/pkg/3scale/amp/component"
 	"github.com/3scale/3scale-operator/pkg/reconcilers"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
@@ -14,6 +13,13 @@ type BackendReconciler struct {
 	*BaseAPIManagerLogicReconciler
 }
 
+const (
+	disableBackendListenerInstancesSyncing = "apps.3scale.net/backend-listener-replica-field"
+	disableBackendWorkerInstancesSyncing = "apps.3scale.net/backend-worker-replica-field"
+	disableCronInstancesSyncing = "apps.3scale.net/cron-replica-field"
+)
+
+
 func NewBackendReconciler(baseAPIManagerLogicReconciler *BaseAPIManagerLogicReconciler) *BackendReconciler {
 	return &BackendReconciler{
 		BaseAPIManagerLogicReconciler: baseAPIManagerLogicReconciler,
@@ -27,29 +33,17 @@ func (r *BackendReconciler) Reconcile() (reconcile.Result, error) {
 	}
 
 	// Cron DC
-	err = r.ReconcileDeploymentConfig(backend.CronDeploymentConfig(), reconcilers.GenericDeploymentConfigMutator())
+	cronConfigMutator := reconcilers.GetConfigMutators(r.apiManager.Annotations, disableCronInstancesSyncing)
+	err = r.ReconcileDeploymentConfig(backend.CronDeploymentConfig(), reconcilers.DeploymentConfigMutator(cronConfigMutator...))
 	if err != nil {
 		return reconcile.Result{}, err
 	}
 
-	// Listerner DC
-	// Do not reconcile backend listener replicas if running with manualHPA annotation
-	manualHPA := "false"
-
-	if metav1.HasAnnotation(r.apiManager.ObjectMeta, "manualHPA") {
-		manualHPA = r.apiManager.Annotations["manualHPA"]
-	}
-
-	if manualHPA == "true" {
-		err = r.ReconcileDeploymentConfig(backend.ListenerDeploymentConfig(), reconcilers.ManualHPADeploymentConfigMutator())
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-	} else {
-		err = r.ReconcileDeploymentConfig(backend.ListenerDeploymentConfig(), reconcilers.GenericDeploymentConfigMutator())
-		if err != nil {
-			return reconcile.Result{}, err
-		}
+	// Listener DC
+	listenerConfigMutator := reconcilers.GetConfigMutators(r.apiManager.Annotations, disableBackendListenerInstancesSyncing)
+	err = r.ReconcileDeploymentConfig(backend.ListenerDeploymentConfig(), reconcilers.DeploymentConfigMutator(listenerConfigMutator...))
+	if err != nil {
+		return reconcile.Result{}, err
 	}
 
 	// Listener Service
@@ -65,17 +59,10 @@ func (r *BackendReconciler) Reconcile() (reconcile.Result, error) {
 	}
 
 	// Worker DC
-	// Do not reconcile backend worker replicas if running with manualHPA annotation
-	if manualHPA == "true" {
-		err = r.ReconcileDeploymentConfig(backend.WorkerDeploymentConfig(), reconcilers.ManualHPADeploymentConfigMutator())
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-	} else {
-		err = r.ReconcileDeploymentConfig(backend.WorkerDeploymentConfig(), reconcilers.GenericDeploymentConfigMutator())
-		if err != nil {
-			return reconcile.Result{}, err
-		}
+	workerConfigMutator := reconcilers.GetConfigMutators(r.apiManager.Annotations, disableBackendWorkerInstancesSyncing)
+	err = r.ReconcileDeploymentConfig(backend.WorkerDeploymentConfig(), reconcilers.DeploymentConfigMutator(workerConfigMutator...))
+	if err != nil {
+		return reconcile.Result{}, err
 	}
 
 	// Environment ConfigMap