Fix resource loss during CS merge and improve OperandConfig creation

internal/controller/bootstrap/config_bridge.go

@@ -24,19 +24,16 @@ import (
 // This allows bootstrap to use controller merge logic without import cycles
 type ConfigMergerFunc func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error)
 
-// configMerger holds the merger function set by the reconciler
-var configMerger ConfigMergerFunc
-
-// SetConfigMerger sets the configuration merger function
-// Called during reconciler initialization to inject the merge logic
-func SetConfigMerger(merger ConfigMergerFunc) {
-	configMerger = merger
+// SetConfigMerger sets the configuration merger function on the Bootstrap instance.
+// Called during reconciler initialization to inject the merge logic.
+func (b *Bootstrap) SetConfigMerger(merger ConfigMergerFunc) {
+	b.configMerger = merger
 }
 
 // mergeConfigs calls the injected merger function if available
 func (b *Bootstrap) mergeConfigs(baseConfig string, cs *apiv3.CommonService) (string, error) {
-	if configMerger != nil {
-		return configMerger(baseConfig, cs, b.CSData.ServicesNs)
+	if b.configMerger != nil {
+		return b.configMerger(baseConfig, cs, b.CSData.ServicesNs)
 	}
 	// If no merger is set, return base config unchanged
 	return baseConfig, nil

internal/controller/bootstrap/config_bridge_test.go

@@ -26,20 +26,17 @@ import (
 	apiv3 "github.com/IBM/ibm-common-service-operator/v4/api/v3"
 )
 
-// resetConfigMerger clears the package-level configMerger between tests to
-// prevent state leaking across test cases.
-func resetConfigMerger() {
-	configMerger = nil
+// newTestBootstrap creates a Bootstrap instance for testing with the given ServicesNs.
+func newTestBootstrap(servicesNs string) *Bootstrap {
+	return &Bootstrap{
+		CSData: apiv3.CSData{ServicesNs: servicesNs},
+	}
 }
 
 // TestSetConfigMerger_NilByDefault verifies that before SetConfigMerger is called
-// the package-level merger is nil and mergeConfigs returns the base config unchanged.
+// the merger is nil and mergeConfigs returns the base config unchanged.
 func TestSetConfigMerger_NilByDefault(t *testing.T) {
-	resetConfigMerger()
-
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
+	b := newTestBootstrap("ibm-common-services")
 	cs := &apiv3.CommonService{}
 
 	result, err := b.mergeConfigs("base-config", cs)
@@ -51,18 +48,13 @@ func TestSetConfigMerger_NilByDefault(t *testing.T) {
 // TestSetConfigMerger_InjectsFunction verifies that SetConfigMerger stores the
 // provided function and that mergeConfigs delegates to it.
 func TestSetConfigMerger_InjectsFunction(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
 	called := false
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("ibm-common-services")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		called = true
 		return "merged-" + baseConfig, nil
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
 	cs := &apiv3.CommonService{}
 
 	result, err := b.mergeConfigs("base", cs)
@@ -74,18 +66,13 @@ func TestSetConfigMerger_InjectsFunction(t *testing.T) {
 // TestSetConfigMerger_PassesServicesNs verifies that mergeConfigs forwards the
 // Bootstrap's ServicesNs to the injected merger function.
 func TestSetConfigMerger_PassesServicesNs(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
 	var capturedNs string
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("my-services-ns")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		capturedNs = servicesNs
 		return baseConfig, nil
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "my-services-ns"},
-	}
 	cs := &apiv3.CommonService{}
 
 	_, err := b.mergeConfigs("config", cs)
@@ -97,18 +84,13 @@ func TestSetConfigMerger_PassesServicesNs(t *testing.T) {
 // TestSetConfigMerger_PassesCSInstance verifies that mergeConfigs forwards the
 // CommonService instance to the injected merger function.
 func TestSetConfigMerger_PassesCSInstance(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
 	var capturedCS *apiv3.CommonService
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("ibm-common-services")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		capturedCS = cs
 		return baseConfig, nil
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
 	cs := &apiv3.CommonService{}
 	cs.Name = "common-service"
 	cs.Namespace = "ibm-common-services"
@@ -123,17 +105,12 @@ func TestSetConfigMerger_PassesCSInstance(t *testing.T) {
 // TestSetConfigMerger_PropagatesError verifies that an error returned by the
 // injected merger function is propagated back to the caller of mergeConfigs.
 func TestSetConfigMerger_PropagatesError(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
 	mergeErr := errors.New("merge failed")
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("ibm-common-services")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		return "", mergeErr
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
 	cs := &apiv3.CommonService{}
 
 	result, err := b.mergeConfigs("config", cs)
@@ -145,19 +122,14 @@ func TestSetConfigMerger_PropagatesError(t *testing.T) {
 // TestSetConfigMerger_Overwrite verifies that calling SetConfigMerger a second
 // time replaces the previously registered function.
 func TestSetConfigMerger_Overwrite(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("ibm-common-services")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		return "first", nil
 	})
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		return "second", nil
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
 	cs := &apiv3.CommonService{}
 
 	result, err := b.mergeConfigs("config", cs)
@@ -171,21 +143,16 @@ func TestSetConfigMerger_Overwrite(t *testing.T) {
 // produces a complete merged result — there is no intermediate state where the
 // base config is returned before CS values are applied.
 func TestMergeConfigs_SingleStageNoRaceCondition(t *testing.T) {
-	resetConfigMerger()
-	defer resetConfigMerger()
-
 	// Simulate the real merger: it always returns a "complete" config that
 	// includes both base and CS values in one shot.
-	SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
+	b := newTestBootstrap("ibm-common-services")
+	b.SetConfigMerger(func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 		// In production this would be MergeConfigs(); here we just verify the
 		// contract: the result must differ from the bare base config, proving
 		// CS values were applied in the same call.
 		return baseConfig + "+cs-values", nil
 	})
 
-	b := &Bootstrap{
-		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
-	}
 	cs := &apiv3.CommonService{}
 
 	result, err := b.mergeConfigs("base-opcon", cs)

internal/controller/bootstrap/init.go

@@ -82,6 +82,7 @@ type Bootstrap struct {
 	MultiInstancesEnable bool
 	CSOperators          []CSOperator
 	CSData               apiv3.CSData
+	configMerger         ConfigMergerFunc
 }
 
 // CanI performs a SelfSubjectAccessReview (SSAR) to check whether the operator service account

internal/controller/commonservice_controller.go

@@ -248,10 +248,6 @@ func (r *CommonServiceReconciler) ReconcileMasterCR(ctx context.Context, instanc
 		return ctrl.Result{}, statusErr
 	}
 
-	// OperandConfig already created with complete configuration
-	// in InitResources, no need for second updateOperandConfig call
-	klog.Info("OperandConfig created with complete configuration")
-
 	var isEqual bool
 	if isEqual, statusErr = r.updateOperatorConfig(ctx, instance.Spec.OperatorConfigs); statusErr != nil {
 		if statusErr := r.updatePhase(ctx, instance, apiv3.CRFailed); statusErr != nil {
@@ -529,7 +525,7 @@ func isNonNoopOperandReconcile(operandRegistry *odlm.OperandRegistry) bool {
 func (r *CommonServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	// Set up configuration merger for single-stage OperandConfig creation
 	// This injects the merge logic into bootstrap without creating import cycles
-	bootstrap.SetConfigMerger(CreateMergerFunc(r))
+	r.Bootstrap.SetConfigMerger(CreateMergerFunc(r))
 	klog.Info("Configuration merger initialized for single-stage OperandConfig creation")
 
 	controller := ctrl.NewControllerManagedBy(mgr).

internal/controller/config_extractor.go

@@ -221,48 +221,58 @@ func extractSizeConfigs(
 	return sizeConfigs, serviceControllerMapping, nil
 }
 
-// extractCustomSizeConfigs extracts custom size configurations from services field
-func extractCustomSizeConfigs(cs *apiv3.CommonService, serviceControllerMapping map[string]string) ([]interface{}, map[string]string) {
-	var dest []interface{}
-
-	if cs.Spec.Services != nil {
-		for _, service := range cs.Spec.Services {
-			serviceMap := make(map[string]interface{})
-
-			// Convert service name
-			serviceMap["name"] = service.Name
-
-			// Convert service spec
-			if service.Spec != nil {
-				specMap := make(map[string]interface{})
-				for key, val := range service.Spec {
-					var rawValue interface{}
-					if err := json.Unmarshal(val.Raw, &rawValue); err == nil {
-						specMap[key] = rawValue
-					}
+// convertServiceConfigs converts CommonService service specs to []interface{} maps.
+// This is the shared conversion logic used by both extractCustomSizeConfigs and extractSizeTemplate.
+func convertServiceConfigs(services []apiv3.ServiceConfig) []interface{} {
+	if services == nil {
+		return nil
+	}
+	var result []interface{}
+	for _, service := range services {
+		serviceMap := make(map[string]interface{})
+		serviceMap["name"] = service.Name
+
+		if service.Spec != nil {
+			specMap := make(map[string]interface{})
+			for key, val := range service.Spec {
+				var rawValue interface{}
+				if err := json.Unmarshal(val.Raw, &rawValue); err == nil {
+					specMap[key] = rawValue
 				}
-				serviceMap["spec"] = specMap
 			}
+			serviceMap["spec"] = specMap
+		}
 
-			// Convert service resources
-			if service.Resources != nil {
-				var resources []interface{}
-				for _, res := range service.Resources {
-					var rawResource interface{}
-					if err := json.Unmarshal(res.Raw, &rawResource); err == nil {
-						resources = append(resources, rawResource)
-					}
+		if service.Resources != nil {
+			var resources []interface{}
+			for _, res := range service.Resources {
+				var rawResource interface{}
+				if err := json.Unmarshal(res.Raw, &rawResource); err == nil {
+					resources = append(resources, rawResource)
 				}
-				serviceMap["resources"] = resources
 			}
+			serviceMap["resources"] = resources
+		}
 
-			// Handle management strategy
+		if service.ManagementStrategy != "" {
+			serviceMap["managementStrategy"] = service.ManagementStrategy
+		}
+
+		result = append(result, serviceMap)
+	}
+	return result
+}
+
+// extractCustomSizeConfigs extracts custom size configurations from services field
+func extractCustomSizeConfigs(cs *apiv3.CommonService, serviceControllerMapping map[string]string) ([]interface{}, map[string]string) {
+	dest := convertServiceConfigs(cs.Spec.Services)
+
+	// Populate serviceControllerMapping from management strategies
+	if cs.Spec.Services != nil {
+		for _, service := range cs.Spec.Services {
 			if service.ManagementStrategy != "" {
-				serviceMap["managementStrategy"] = service.ManagementStrategy
 				serviceControllerMapping[service.Name] = service.ManagementStrategy
 			}
-
-			dest = append(dest, serviceMap)
 		}
 	}
 
@@ -272,41 +282,7 @@ func extractCustomSizeConfigs(cs *apiv3.CommonService, serviceControllerMapping
 // extractSizeTemplate applies a size template and merges with custom services
 func extractSizeTemplate(cs *apiv3.CommonService, sizeTemplate string, serviceControllerMapping map[string]string, opconNs string) ([]interface{}, map[string]string, error) {
 	// Convert custom services to []interface{}
-	var src []interface{}
-	if cs.Spec.Services != nil {
-		for _, service := range cs.Spec.Services {
-			serviceMap := make(map[string]interface{})
-			serviceMap["name"] = service.Name
-
-			if service.Spec != nil {
-				specMap := make(map[string]interface{})
-				for key, val := range service.Spec {
-					var rawValue interface{}
-					if err := json.Unmarshal(val.Raw, &rawValue); err == nil {
-						specMap[key] = rawValue
-					}
-				}
-				serviceMap["spec"] = specMap
-			}
-
-			if service.Resources != nil {
-				var resources []interface{}
-				for _, res := range service.Resources {
-					var rawResource interface{}
-					if err := json.Unmarshal(res.Raw, &rawResource); err == nil {
-						resources = append(resources, rawResource)
-					}
-				}
-				serviceMap["resources"] = resources
-			}
-
-			if service.ManagementStrategy != "" {
-				serviceMap["managementStrategy"] = service.ManagementStrategy
-			}
-
-			src = append(src, serviceMap)
-		}
-	}
+	src := convertServiceConfigs(cs.Spec.Services)
 
 	// Convert size template string to slice
 	sizes, err := convertStringToSlice(sizeTemplate)
@@ -367,5 +343,3 @@ func extractSizeTemplate(cs *apiv3.CommonService, sizeTemplate string, serviceCo
 
 	return sizes, serviceControllerMapping, nil
 }
-
-// Made with Bob

internal/controller/config_merger.go

@@ -157,12 +157,12 @@ func validateMergedConfig(config *odlm.OperandConfig) error {
 // MergeConfigs is a convenience function that combines extraction and merging
 // Used by bootstrap to create complete OperandConfig in single stage
 func MergeConfigs(
-	r *CommonServiceReconciler,
 	baseConfig string,
 	cs *apiv3.CommonService,
+	servicesNs string,
 ) (string, error) {
 	// Extract CommonService configurations
-	csConfigs, serviceControllerMapping, err := ExtractCommonServiceConfigs(cs, r.Bootstrap.CSData.ServicesNs)
+	csConfigs, serviceControllerMapping, err := ExtractCommonServiceConfigs(cs, servicesNs)
 	if err != nil {
 		return "", fmt.Errorf("failed to extract CommonService configs: %v", err)
 	}
@@ -172,7 +172,7 @@ func MergeConfigs(
 		baseConfig,
 		csConfigs,
 		serviceControllerMapping,
-		r.Bootstrap.CSData.ServicesNs,
+		servicesNs,
 	)
 	if err != nil {
 		return "", fmt.Errorf("failed to merge configurations: %v", err)
@@ -185,6 +185,6 @@ func MergeConfigs(
 // This is used to inject the merge logic into bootstrap without import cycles
 func CreateMergerFunc(r *CommonServiceReconciler) func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
 	return func(baseConfig string, cs *apiv3.CommonService, servicesNs string) (string, error) {
-		return MergeConfigs(r, baseConfig, cs)
+		return MergeConfigs(baseConfig, cs, servicesNs)
 	}
 }