Merge pull request #737 from 3scale/external-components-optional

External components optional

apis/apps/v1alpha1/apimanager_types.go

@@ -606,39 +606,30 @@ type HighAvailabilitySpec struct {
 
 type ExternalComponentsSpec struct {
 	// +optional
-	System ExternalSystemComponents `json:"system"`
+	System *ExternalSystemComponents `json:"system,omitempty"`
 	// +optional
-	Backend ExternalBackendComponents `json:"backend"`
+	Backend *ExternalBackendComponents `json:"backend,omitempty"`
 	// +optional
-	Zync ExternalZyncComponents `json:"zync"`
+	Zync *ExternalZyncComponents `json:"zync,omitempty"`
 }
 
 type ExternalSystemComponents struct {
-	ExternalDatabaseSpec `json:",omitempty"`
-	ExternalRedisSpec    `json:",omitempty"`
+	// +optional
+	Redis *bool `json:"redis,omitempty"`
+	// +optional
+	Database *bool `json:"database,omitempty"`
 }
 
 type ExternalBackendComponents struct {
-	ExternalRedisSpec `json:",omitempty"`
-}
-
-type ExternalZyncComponents struct {
-	ExternalDatabaseSpec `json:",omitempty"`
-}
-
-type ExternalDatabaseSpec struct {
 	// +optional
-	Database bool `json:"database"`
+	Redis *bool `json:"redis,omitempty"`
 }
 
-type ExternalRedisSpec struct {
+type ExternalZyncComponents struct {
 	// +optional
-	Redis bool `json:"redis"`
+	Database *bool `json:"database,omitempty"`
 }
 
-var ExternalDatabase ExternalDatabaseSpec = ExternalDatabaseSpec{true}
-var ExternalRedis ExternalRedisSpec = ExternalRedisSpec{true}
-
 type PodDisruptionBudgetSpec struct {
 	Enabled bool `json:"enabled,omitempty"`
 }
@@ -974,67 +965,93 @@ func (apimanager *APIManager) setZyncDefaults() bool {
 	return changed
 }
 
-func AllComponentsInternal() *ExternalComponentsSpec {
-	return &ExternalComponentsSpec{}
-}
+func (apimanager *APIManager) UpdateExternalComponentsFromHighAvailability() bool {
+	// The external components field is already populated. Nothing to do
+	if apimanager.Spec.ExternalComponents != nil {
+		return false
+	}
 
-func AllComponentsExternal() *ExternalComponentsSpec {
-	return &ExternalComponentsSpec{
-		System:  ExternalSystemComponents{ExternalDatabase, ExternalRedis},
-		Backend: ExternalBackendComponents{ExternalRedis},
-		Zync:    ExternalZyncComponents{ExternalDatabase},
+	updated := false
+	// When the info comes from the deprecated .spec.highAvailability field
+	e := mapHighAvailabilityToExternalComponents(apimanager)
+
+	if e != nil {
+		apimanager.Spec.ExternalComponents = e
+		updated = true
 	}
+
+	// Remove the deprecated field
+	if apimanager.Spec.HighAvailability != nil {
+		apimanager.Spec.HighAvailability = nil
+		updated = true
+	}
+
+	return updated
 }
 
-func (apimanager *APIManager) HighAvailabilityToExternalComponents() bool {
-	// The external components field is already populated. Nothing to do
+func (apimanager *APIManager) IsExternal(selector func(*ExternalComponentsSpec) bool) bool {
+	// ExternalComponents has precedence over HighAvailability
 	if apimanager.Spec.ExternalComponents != nil {
-		return false
+		return selector(apimanager.Spec.ExternalComponents)
 	}
 
-	apimanager.Spec.ExternalComponents = AllComponentsInternal()
+	// When the info comes from the deprecated .spec.highAvailability field
+	e := mapHighAvailabilityToExternalComponents(apimanager)
 
-	// The hight availability field is empty. Default to no external
-	// components
-	if apimanager.Spec.HighAvailability == nil {
-		return true
+	if e != nil {
+		return selector(e)
 	}
 
-	// HighAvailability is enabled. Default to all external components
-	if apimanager.Spec.HighAvailability.Enabled {
-		apimanager.Spec.ExternalComponents.System.Database = true
-		apimanager.Spec.ExternalComponents.System.Redis = true
-		apimanager.Spec.ExternalComponents.Backend.Redis = true
+	return false
+}
 
-		if apimanager.Spec.HighAvailability.ExternalZyncDatabaseEnabled != nil && *apimanager.Spec.HighAvailability.ExternalZyncDatabaseEnabled {
-			apimanager.Spec.ExternalComponents.Zync.Database = true
-		}
+func mapHighAvailabilityToExternalComponents(apiManager *APIManager) *ExternalComponentsSpec {
+	if apiManager.Spec.HighAvailability == nil {
+		return nil
 	}
 
-	// Remove the deprecated field
-	apimanager.Spec.HighAvailability = nil
-	return true
-}
+	if !apiManager.Spec.HighAvailability.Enabled {
+		return nil
+	}
 
-func (apimanager *APIManager) IsExternal(selector func(*ExternalComponentsSpec) bool) bool {
-	if apimanager.Spec.ExternalComponents == nil {
-		return false
+	trueVal := true
+
+	e := &ExternalComponentsSpec{
+		System:  &ExternalSystemComponents{Redis: &trueVal, Database: &trueVal},
+		Backend: &ExternalBackendComponents{Redis: &trueVal},
 	}
 
-	return selector(apimanager.Spec.ExternalComponents)
+	if apiManager.Spec.HighAvailability.ExternalZyncDatabaseEnabled != nil &&
+		*apiManager.Spec.HighAvailability.ExternalZyncDatabaseEnabled {
+		e.Zync = &ExternalZyncComponents{Database: &trueVal}
+	}
+
+	return e
+}
+
+func AllComponentsExternal() *ExternalComponentsSpec {
+	trueVal := true
+	return &ExternalComponentsSpec{
+		System:  &ExternalSystemComponents{Redis: &trueVal, Database: &trueVal},
+		Backend: &ExternalBackendComponents{Redis: &trueVal},
+		Zync:    &ExternalZyncComponents{Database: &trueVal},
+	}
 }
 
 func SystemDatabase(e *ExternalComponentsSpec) bool {
-	return e.System.Database
+	return e != nil && e.System != nil && e.System.Database != nil && *e.System.Database
 }
+
 func SystemRedis(e *ExternalComponentsSpec) bool {
-	return e.System.Redis
+	return e != nil && e.System != nil && e.System.Redis != nil && *e.System.Redis
 }
+
 func BackendRedis(e *ExternalComponentsSpec) bool {
-	return e.Backend.Redis
+	return e != nil && e.Backend != nil && e.Backend.Redis != nil && *e.Backend.Redis
 }
+
 func ZyncDatabase(e *ExternalComponentsSpec) bool {
-	return e.Zync.Database
+	return e != nil && e.Zync != nil && e.Zync.Database != nil && *e.Zync.Database
 }
 
 func (apimanager *APIManager) IsPDBEnabled() bool {

apis/apps/v1alpha1/apimanager_types_test.go

@@ -135,7 +135,9 @@ func TestZyncExternalDatabaseIsEnabled(t *testing.T) {
 					Enabled:                     true,
 					ExternalZyncDatabaseEnabled: &trueVal,
 				}
-				apimanager.Spec.ExternalComponents = AllComponentsExternal()
+				apimanager.Spec.ExternalComponents = &ExternalComponentsSpec{
+					Zync: &ExternalZyncComponents{Database: &trueVal},
+				}
 				return apimanager
 			},
 			true,

controllers/apps/apimanager_controller.go

@@ -205,17 +205,19 @@ func (r *APIManagerReconciler) apiManagerInstance(namespacedName types.Namespace
 }
 
 func (r *APIManagerReconciler) setAPIManagerDefaults(cr *appsv1alpha1.APIManager) (reconcile.Result, error) {
-	externalChanged := cr.HighAvailabilityToExternalComponents()
-	changed, err := cr.SetDefaults() // TODO check where to put this
+	updated := cr.UpdateExternalComponentsFromHighAvailability()
+
+	defaultsUpdated, err := cr.SetDefaults()
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+	updated = updated || defaultsUpdated
 
-	if changed || externalChanged {
+	if updated {
 		err = r.Client().Update(context.TODO(), cr)
 	}
 
-	return ctrl.Result{Requeue: changed}, err
+	return ctrl.Result{Requeue: updated}, err
 }
 
 func (r *APIManagerReconciler) reconcileAPIManagerLogic(cr *appsv1alpha1.APIManager) (reconcile.Result, error) {
@@ -320,12 +322,6 @@ func (r *APIManagerReconciler) validateApicastTLSCertificates(cr *appsv1alpha1.A
 }
 
 func (r *APIManagerReconciler) dependencyReconcilerForComponents(cr *appsv1alpha1.APIManager, baseAPIManagerLogicReconciler *operator.BaseAPIManagerLogicReconciler) operator.DependencyReconciler {
-	// Get the external components spec. Default to all internal if not set
-	componentsSpec := cr.Spec.ExternalComponents
-	if componentsSpec == nil {
-		componentsSpec = appsv1alpha1.AllComponentsInternal()
-	}
-
 	// Helper type that contains the constructors for a dependency reconciler
 	// whether it's external or internal
 	type constructors struct {
@@ -337,7 +333,7 @@ func (r *APIManagerReconciler) dependencyReconcilerForComponents(cr *appsv1alpha
 	// on whether it's external or internal
 	selectReconciler := func(cs constructors, selectIsExternal func(*appsv1alpha1.ExternalComponentsSpec) bool) operator.DependencyReconciler {
 		constructor := cs.Internal
-		if selectIsExternal(componentsSpec) {
+		if selectIsExternal(cr.Spec.ExternalComponents) {
 			constructor = cs.External
 		}