single stage operandconfig creation

internal/controller/bootstrap/config_bridge.go

@@ -0,0 +1,43 @@
+//
+// Copyright 2022 IBM Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package bootstrap
+
+import (
+	apiv3 "github.com/IBM/ibm-common-service-operator/v4/api/v3"
+)
+
+// ConfigMergerFunc is a function type that merges base config with CommonService configs
+// 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
+}
+
+// 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 no merger is set, return base config unchanged
+	return baseConfig, nil
+}

internal/controller/bootstrap/config_bridge_test.go

@@ -0,0 +1,199 @@
+//
+// Copyright 2022 IBM Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package bootstrap
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	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
+}
+
+// TestSetConfigMerger_NilByDefault verifies that before SetConfigMerger is called
+// the package-level 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"},
+	}
+	cs := &apiv3.CommonService{}
+
+	result, err := b.mergeConfigs("base-config", cs)
+	require.NoError(t, err)
+	assert.Equal(t, "base-config", result,
+		"when no merger is set, mergeConfigs must return the base config unchanged")
+}
+
+// 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) {
+		called = true
+		return "merged-" + baseConfig, nil
+	})
+
+	b := &Bootstrap{
+		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
+	}
+	cs := &apiv3.CommonService{}
+
+	result, err := b.mergeConfigs("base", cs)
+	require.NoError(t, err)
+	assert.True(t, called, "the injected merger function must be called")
+	assert.Equal(t, "merged-base", result)
+}
+
+// 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) {
+		capturedNs = servicesNs
+		return baseConfig, nil
+	})
+
+	b := &Bootstrap{
+		CSData: apiv3.CSData{ServicesNs: "my-services-ns"},
+	}
+	cs := &apiv3.CommonService{}
+
+	_, err := b.mergeConfigs("config", cs)
+	require.NoError(t, err)
+	assert.Equal(t, "my-services-ns", capturedNs,
+		"mergeConfigs must pass Bootstrap.CSData.ServicesNs to the merger function")
+}
+
+// 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) {
+		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"
+
+	_, err := b.mergeConfigs("config", cs)
+	require.NoError(t, err)
+	require.NotNil(t, capturedCS)
+	assert.Equal(t, "common-service", capturedCS.Name)
+	assert.Equal(t, "ibm-common-services", capturedCS.Namespace)
+}
+
+// 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) {
+		return "", mergeErr
+	})
+
+	b := &Bootstrap{
+		CSData: apiv3.CSData{ServicesNs: "ibm-common-services"},
+	}
+	cs := &apiv3.CommonService{}
+
+	result, err := b.mergeConfigs("config", cs)
+	assert.Error(t, err)
+	assert.Equal(t, mergeErr, err)
+	assert.Empty(t, result)
+}
+
+// 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) {
+		return "first", nil
+	})
+	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)
+	require.NoError(t, err)
+	assert.Equal(t, "second", result,
+		"the second SetConfigMerger call must overwrite the first")
+}
+
+// TestMergeConfigs_SingleStageNoRaceCondition is the key integration test for the
+// Single-Stage Creation feature.  It verifies that a single mergeConfigs call
+// 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) {
+		// 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)
+	require.NoError(t, err)
+
+	// The result must already contain CS values — no second call needed.
+	assert.Equal(t, "base-opcon+cs-values", result,
+		"single mergeConfigs call must return complete config with CS values applied")
+	assert.NotEqual(t, "base-opcon", result,
+		"result must not be the bare base config (that would indicate an incomplete intermediate state)")
+}

internal/controller/bootstrap/init.go

@@ -319,7 +319,7 @@ func (b *Bootstrap) InitResources(instance *apiv3.CommonService, forceUpdateODLM
 		}
 
 		klog.Info("Installing/Updating OperandConfig")
-		if err := b.InstallOrUpdateOpcon(forceUpdateODLMCRs); err != nil {
+		if err := b.InstallOrUpdateOpcon(forceUpdateODLMCRs, instance); err != nil {
 			return err
 		}
 	}
@@ -369,7 +369,7 @@ func (b *Bootstrap) InitResources(instance *apiv3.CommonService, forceUpdateODLM
 		}
 
 		klog.Info("Installing/Updating OperandConfig")
-		if err := b.InstallOrUpdateOpcon(forceUpdateODLMCRs); err != nil {
+		if err := b.InstallOrUpdateOpcon(forceUpdateODLMCRs, instance); err != nil {
 			return err
 		}
 	}
@@ -960,7 +960,8 @@ func (b *Bootstrap) InstallOrUpdateOpreg(ctx context.Context, installPlanApprova
 }
 
 // InstallOrUpdateOpcon will install or update OperandConfig when Opcon CRD is existent
-func (b *Bootstrap) InstallOrUpdateOpcon(forceUpdateODLMCRs bool) error {
+// Now accepts CommonService instance with merged configurations
+func (b *Bootstrap) InstallOrUpdateOpcon(forceUpdateODLMCRs bool, csInstance *apiv3.CommonService) error {
 
 	var baseCon string
 	configs := []string{
@@ -983,7 +984,21 @@ func (b *Bootstrap) InstallOrUpdateOpcon(forceUpdateODLMCRs bool) error {
 		return err
 	}
 
-	if err := b.renderTemplate(concatenatedCon, b.CSData, forceUpdateODLMCRs); err != nil {
+	// Merge CommonService configurations with base templates
+	// before rendering to create complete OperandConfig in one step
+	finalConfig := concatenatedCon
+	if csInstance != nil {
+		klog.Info("Merging CommonService configurations with base OperandConfig")
+		mergedConfig, err := b.mergeConfigs(concatenatedCon, csInstance)
+		if err != nil {
+			klog.Errorf("Failed to merge configurations: %v", err)
+			return err
+		}
+		finalConfig = mergedConfig
+		klog.Info("Successfully merged configurations for single-stage OperandConfig creation")
+	}
+
+	if err := b.renderTemplate(finalConfig, b.CSData, forceUpdateODLMCRs); err != nil {
 		return err
 	}
 	return nil

internal/controller/commonservice_controller.go

@@ -248,38 +248,11 @@ func (r *CommonServiceReconciler) ReconcileMasterCR(ctx context.Context, instanc
 		return ctrl.Result{}, statusErr
 	}
 
-	// Apply new configs to CommonService CR
-	cs := util.NewUnstructured("operator.ibm.com", "CommonService", "v3")
-	if statusErr = r.Bootstrap.Client.Get(ctx, types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}, cs); statusErr != nil {
-		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, statusErr)
-		return ctrl.Result{}, statusErr
-	}
-	// Set "Pending" condition and "Updating" for phase when config CS CR
-	instance.SetPendingCondition(constant.MasterCR, apiv3.ConditionTypeReconciling, corev1.ConditionTrue, apiv3.ConditionReasonConfig, apiv3.ConditionMessageConfig)
-	instance.Status.Phase = apiv3.CRUpdating
-	newConfigs, serviceControllerMapping, statusErr := r.getNewConfigs(cs)
-	if statusErr != nil {
-		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, statusErr)
-		instance.SetErrorCondition(constant.MasterCR, apiv3.ConditionTypeError, corev1.ConditionTrue, apiv3.ConditionReasonError, statusErr.Error())
-		instance.Status.Phase = apiv3.CRFailed
-	}
-
-	if statusErr = r.Client.Status().Update(ctx, instance); statusErr != nil {
-		klog.Errorf("Fail to update %s/%s: %v", instance.Namespace, instance.Name, statusErr)
-		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.updateOperandConfig(ctx, newConfigs, serviceControllerMapping); statusErr != nil {
-		if statusErr := r.updatePhase(ctx, instance, apiv3.CRFailed); statusErr != nil {
-			klog.Error(statusErr)
-		}
-		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, statusErr)
-		return ctrl.Result{}, statusErr
-	} else if isEqual {
-		r.Recorder.Event(instance, corev1.EventTypeNormal, "Noeffect", fmt.Sprintf("No update, resource sizings in the OperandConfig %s/%s are larger than the profile from CommonService CR %s/%s", r.Bootstrap.CSData.OperatorNs, "common-service", instance.Namespace, instance.Name))
-	}
-
 	if isEqual, statusErr = r.updateOperatorConfig(ctx, instance.Spec.OperatorConfigs); statusErr != nil {
 		if statusErr := r.updatePhase(ctx, instance, apiv3.CRFailed); statusErr != nil {
 			klog.Error(statusErr)
@@ -371,11 +344,6 @@ func (r *CommonServiceReconciler) ReconcileGeneralCR(ctx context.Context, instan
 		return ctrl.Result{}, err
 	}
 
-	cs := util.NewUnstructured("operator.ibm.com", "CommonService", "v3")
-	if err := r.Bootstrap.Client.Get(ctx, types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace}, cs); err != nil {
-		klog.Errorf("Fail to reconcile %s/%s: %v", instance.Namespace, instance.Name, err)
-		return ctrl.Result{}, err
-	}
 	// Generate Issuer and Certificate CR
 	if err := r.Bootstrap.DeployCertManagerCR(); err != nil {
 		klog.Errorf("Failed to deploy cert manager CRs: %v", err)
@@ -386,7 +354,8 @@ func (r *CommonServiceReconciler) ReconcileGeneralCR(ctx context.Context, instan
 		return ctrl.Result{}, err
 	}
 
-	newConfigs, serviceControllerMapping, err := r.getNewConfigs(cs)
+	// Extract configurations using new extractor for subsequent updates
+	newConfigs, serviceControllerMapping, err := ExtractCommonServiceConfigs(instance, r.Bootstrap.CSData.ServicesNs)
 	if err != nil {
 		if err := r.updatePhase(ctx, instance, apiv3.CRFailed); err != nil {
 			klog.Error(err)
@@ -395,6 +364,7 @@ func (r *CommonServiceReconciler) ReconcileGeneralCR(ctx context.Context, instan
 		return ctrl.Result{}, err
 	}
 
+	// Update OperandConfig (single update for subsequent reconciliations)
 	isEqual, err := r.updateOperandConfig(ctx, newConfigs, serviceControllerMapping)
 	if err != nil {
 		if err := r.updatePhase(ctx, instance, apiv3.CRFailed); err != nil {
@@ -557,6 +527,10 @@ 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))
+	klog.Info("Configuration merger initialized for single-stage OperandConfig creation")
 
 	controller := ctrl.NewControllerManagedBy(mgr).
 		// AnnotationChangedPredicate is intended to be used in conjunction with the GenerationChangedPredicate