diff --git a/controllers/storagecluster/cephconfig.go b/controllers/storagecluster/cephconfig.go index e65c698d86..c07b736a40 100644 --- a/controllers/storagecluster/cephconfig.go +++ b/controllers/storagecluster/cephconfig.go @@ -25,6 +25,7 @@ const ( rookOverrideConfigMapName = "rook-config-override" globalSectionKey = "global" publicNetworkKey = "public_network" + targetPgPerOSDKey = "mon_target_pg_per_osd" ) var ( @@ -66,6 +67,30 @@ func (obj *ocsCephConfig) ensureCreated(r *StorageClusterReconciler, sc *ocsv1.S Data: rookConfigOverrideData, } _, err := ctrl.CreateOrUpdate(context.Background(), r.Client, rookConfigOverrideCM, func() error { + // mon_target_pg_per_osd=200 added only during new CM creation, as setting it on existing clusters can cause data movement. + if rookConfigOverrideCM.ObjectMeta.CreationTimestamp.IsZero() { + updatedConfig, err := updateRookConfig(rookConfigOverrideData["config"], globalSectionKey, targetPgPerOSDKey, "200") + if err != nil { + return err + } + rookConfigOverrideData["config"] = updatedConfig + } else { + // Ensure if mon_target_pg_per_osd=200 added during new CM creation is not removed during updates. + if configData, exists := rookConfigOverrideCM.Data["config"]; exists && configData != "" { + cfg, err := ini.Load([]byte(configData)) + if err != nil { + return fmt.Errorf("failed to parse existing config: %w", err) + } + if val := cfg.Section(globalSectionKey).Key(targetPgPerOSDKey).String(); val != "" { + updatedConfig, err := updateRookConfig(rookConfigOverrideData["config"], globalSectionKey, targetPgPerOSDKey, val) + if err != nil { + return fmt.Errorf("failed to update Rook config during update: %w", err) + } + rookConfigOverrideData["config"] = updatedConfig + } + } + } + if !reflect.DeepEqual(rookConfigOverrideCM.Data, rookConfigOverrideData) { r.Log.Info("updating rook config override configmap", "ConfigMap", klog.KRef(sc.Namespace, rookOverrideConfigMapName)) rookConfigOverrideCM.Data = rookConfigOverrideData diff --git a/controllers/storagecluster/cephconfig_test.go b/controllers/storagecluster/cephconfig_test.go index 1747db2c24..1c385925ce 100644 --- a/controllers/storagecluster/cephconfig_test.go +++ b/controllers/storagecluster/cephconfig_test.go @@ -158,3 +158,77 @@ func TestDualStack(t *testing.T) { } } + +func TestMonTargetPGPerOSD(t *testing.T) { + testTable := []struct { + label string + existingConfig string + reconcileCount int + expectedValue string + shouldExist bool + }{ + { + label: "Fresh install - should set mon_target_pg_per_osd=200", + existingConfig: "", + reconcileCount: 2, + expectedValue: "200", + shouldExist: true, + }, + { + label: "Existing cluster - should not add mon_target_pg_per_osd", + existingConfig: `[global] +some_other_key = value +`, + reconcileCount: 1, + expectedValue: "", + shouldExist: false, + }, + } + + for i, testCase := range testTable { + t.Logf("Case #%+v: %s", i+1, testCase.label) + r := createFakeStorageClusterReconciler(t) + sc := &api.StorageCluster{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test"}, + } + + // Create existing ConfigMap if specified + if testCase.existingConfig != "" { + existingCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: rookOverrideConfigMapName, + Namespace: sc.Namespace, + CreationTimestamp: metav1.Now(), + }, + Data: map[string]string{ + "config": testCase.existingConfig, + }, + } + err := r.Client.Create(context.TODO(), existingCM) + assert.NilError(t, err, "failed to create existing configmap") + } + + cephConfigReconciler := &ocsCephConfig{} + for j := 0; j < testCase.reconcileCount; j++ { + _, err := cephConfigReconciler.ensureCreated(&r, sc) + assert.NilError(t, err, "reconcile %d failed", j+1) + + // Verify ConfigMap after each reconcile + configMap := &corev1.ConfigMap{} + err = r.Client.Get(context.TODO(), types.NamespacedName{Name: rookOverrideConfigMapName, Namespace: sc.Namespace}, configMap) + assert.NilError(t, err, "expected to find configmap") + + cfg, err := ini.Load([]byte(configMap.Data["config"])) + assert.NilError(t, err, "expected ini string to load") + + sect, err := cfg.GetSection(globalSectionKey) + assert.NilError(t, err, "expected section to exist") + + keyFound := sect.HasKey(targetPgPerOSDKey) + assert.Equal(t, keyFound, testCase.shouldExist, "mon_target_pg_per_osd key existence mismatch") + if testCase.shouldExist { + assert.Equal(t, sect.Key(targetPgPerOSDKey).Value(), testCase.expectedValue, "mon_target_pg_per_osd value mismatch") + } + } + } +}