From 7ed0371a0142e817a56ab67902a713c29caa1ff4 Mon Sep 17 00:00:00 2001 From: Nitin Goyal Date: Mon, 20 Jan 2025 18:20:45 +0530 Subject: [PATCH 1/2] Avoid reconciling rook-ceph-operator-config via the external controller The rook-ceph-operator-config ConfigMap is managed by the OCSInitialization controller and should only be reconciled by it. This ensures consistency and prevents duplicate handling. It should be reconciled from OCSInitialization only. Signed-off-by: Nitin Goyal --- .../storagecluster/external_resources.go | 36 -------------- .../storagecluster/external_resources_test.go | 47 ++----------------- 2 files changed, 5 insertions(+), 78 deletions(-) diff --git a/controllers/storagecluster/external_resources.go b/controllers/storagecluster/external_resources.go index 94ddc09307..93681d1aed 100644 --- a/controllers/storagecluster/external_resources.go +++ b/controllers/storagecluster/external_resources.go @@ -36,11 +36,6 @@ const ( cephRgwTLSSecretKey = "ceph-rgw-tls-cert" ) -const ( - rookCephOperatorConfigName = "rook-ceph-operator-config" - rookEnableCephFSCSIKey = "ROOK_CSI_ENABLE_CEPHFS" -) - // store the name of the rados-namespace var radosNamespaceName string @@ -59,29 +54,6 @@ type ExternalResource struct { type ocsExternalResources struct{} -// setRookCSICephFS function enables or disables the 'ROOK_CSI_ENABLE_CEPHFS' key -func (r *StorageClusterReconciler) setRookCSICephFS( - enableDisableFlag bool, instance *ocsv1.StorageCluster) error { - rookCephOperatorConfig := &corev1.ConfigMap{} - err := r.Client.Get(context.TODO(), - types.NamespacedName{Name: rookCephOperatorConfigName, Namespace: instance.ObjectMeta.Namespace}, - rookCephOperatorConfig) - if err != nil { - r.Log.Error(err, "Unable to get RookCeph ConfigMap.", "RookCephConfigMap", klog.KRef(instance.Namespace, rookCephOperatorConfigName)) - return err - } - enableDisableFlagStr := fmt.Sprintf("%v", enableDisableFlag) - if rookCephOperatorConfig.Data == nil { - rookCephOperatorConfig.Data = map[string]string{} - } - // if the current state of 'ROOK_CSI_ENABLE_CEPHFS' flag is same, just return - if rookCephOperatorConfig.Data[rookEnableCephFSCSIKey] == enableDisableFlagStr { - return nil - } - rookCephOperatorConfig.Data[rookEnableCephFSCSIKey] = enableDisableFlagStr - return r.Client.Update(context.TODO(), rookCephOperatorConfig) -} - func checkEndpointReachable(endpoint string, timeout time.Duration) error { rxp := regexp.MustCompile(`^http[s]?://`) // remove any http or https protocols from the endpoint string @@ -288,8 +260,6 @@ func (r *StorageClusterReconciler) createExternalStorageClusterResources(instanc Kind: instance.Kind, Name: instance.Name, } - // this flag sets the 'ROOK_CSI_ENABLE_CEPHFS' flag - enableRookCSICephFS := false // this stores only the StorageClasses specified in the Secret availableSCCs := []StorageClassConfiguration{} @@ -372,7 +342,6 @@ func (r *StorageClusterReconciler) createExternalStorageClusterResources(instanc var err error if d.Name == cephFsStorageClassName { scc = newCephFilesystemStorageClassConfiguration(instance) - enableRookCSICephFS = true } else if d.Name == cephRbdStorageClassName { scc = newCephBlockPoolStorageClassConfiguration(instance) } else if strings.HasPrefix(d.Name, cephRbdRadosNamespaceStorageClassNamePrefix) { // ceph-rbd-rados-namespace- @@ -421,11 +390,6 @@ func (r *StorageClusterReconciler) createExternalStorageClusterResources(instanc return err } - if err = r.setRookCSICephFS(enableRookCSICephFS, instance); err != nil { - r.Log.Error(err, "Failed to set RookEnableCephFSCSIKey to EnableRookCSICephFS.", "RookEnableCephFSCSIKey", rookEnableCephFSCSIKey, "EnableRookCSICephFS", enableRookCSICephFS) - return err - } - if rgwEndpoint != "" { if err := checkEndpointReachable(rgwEndpoint, 5*time.Second); err != nil { r.Log.Error(err, "RGW endpoint is not reachable.", "RGWEndpoint", rgwEndpoint) diff --git a/controllers/storagecluster/external_resources_test.go b/controllers/storagecluster/external_resources_test.go index c6bb4fdd0d..5cbf9755f9 100644 --- a/controllers/storagecluster/external_resources_test.go +++ b/controllers/storagecluster/external_resources_test.go @@ -86,18 +86,6 @@ func TestEnsureExternalStorageClusterResources(t *testing.T) { assertExpectedExternalResources(t, reconciler) } -func newRookCephOperatorConfig(namespace string) *corev1.ConfigMap { - config := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: rookCephOperatorConfigName, - Namespace: namespace, - }, - } - data := make(map[string]string) - config.Data = data - return config -} - func createExternalCephClusterSecret(extResources []ExternalResource) (*corev1.Secret, error) { jsonBlob, err := json.Marshal(extResources) if err != nil { @@ -156,9 +144,8 @@ func createExternalClusterReconcilerFromCustomResources( if err != nil { t.Fatalf("failed to create external secret: %v", err) } - rookCephConfig := newRookCephOperatorConfig("") reconciler := createFakeInitializationStorageClusterReconciler(t, &nbv1.NooBaa{}) - clientObjs := []client.Object{cr, externalSecret, rookCephConfig} + clientObjs := []client.Object{cr, externalSecret} for _, obj := range clientObjs { if err = reconciler.Client.Create(context.TODO(), obj); err != nil { t.Fatalf("failed to create a needed runtime object: %v", err) @@ -292,14 +279,12 @@ func TestOptionalExternalStorageClusterResources(t *testing.T) { expectedRookCephConfigVal string }{ { - label: "RemoveRGW", - resourceToBeRemoved: "ceph-rgw", - expectedRookCephConfigVal: "true", + label: "RemoveRGW", + resourceToBeRemoved: "ceph-rgw", }, { - label: "RemoveCephFS", - resourceToBeRemoved: "cephfs", - expectedRookCephConfigVal: "false", + label: "RemoveCephFS", + resourceToBeRemoved: "cephfs", }, } @@ -314,34 +299,12 @@ func TestOptionalExternalStorageClusterResources(t *testing.T) { assertExpectedExternalResources(t, reconciler) // make sure we are missing the provided resource assertMissingExternalResource(t, reconciler, testParam.resourceToBeRemoved) - // make sure that we have expected rook ceph config value - assertRookCephOperatorConfigValue(t, reconciler, testParam.expectedRookCephConfigVal) // make sure about the availability of 'CephObjectStore' according to the resource removed assertCephObjectStore(t, reconciler, testParam.resourceToBeRemoved) }) } } -func assertRookCephOperatorConfigValue(t *testing.T, reconciler StorageClusterReconciler, checkValue string) { - request := reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: "ocsinit", - Namespace: "", - }, - } - sc := &api.StorageCluster{} - err := reconciler.Client.Get(context.TODO(), request.NamespacedName, sc) - assert.NoError(t, err) - rookCephOperatorConfig := &corev1.ConfigMap{} - err = reconciler.Client.Get(context.TODO(), - types.NamespacedName{Name: rookCephOperatorConfigName, Namespace: sc.ObjectMeta.Namespace}, - rookCephOperatorConfig) - assert.NoErrorf(t, err, "Unable to get '%s' config", rookCephOperatorConfigName) - assert.Truef(t, - rookCephOperatorConfig.Data[rookEnableCephFSCSIKey] == checkValue, - "'%s' key is supposed to be '%s'", rookEnableCephFSCSIKey, checkValue) -} - func assertMissingExternalResource(t *testing.T, reconciler StorageClusterReconciler, resourceName string) { request := reconcile.Request{ NamespacedName: types.NamespacedName{ From 09896a11e30a31ffea9a9bdf808da52290d31feb Mon Sep 17 00:00:00 2001 From: Nitin Goyal Date: Mon, 20 Jan 2025 18:34:33 +0530 Subject: [PATCH 2/2] Remove ROOK_CSI_ENABLE_CEPHFS key from rook-ceph-operator-config CM The ROOK_CSI_ENABLE_CEPHFS key was set in the external mode within the rook-ceph-operator-config ConfigMap. However, it should have been removed when transitioning from rook-ceph-operator-config to ocs-operator-config. The rook-ceph-operator-config ConfigMap is now retained only for customer modifications. This code can be safely removed in the next release, as it only require to run once to delete the key. Signed-off-by: Nitin Goyal --- controllers/ocsinitialization/ocsinitialization_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/controllers/ocsinitialization/ocsinitialization_controller.go b/controllers/ocsinitialization/ocsinitialization_controller.go index cc2b9f8b33..638c7e8079 100644 --- a/controllers/ocsinitialization/ocsinitialization_controller.go +++ b/controllers/ocsinitialization/ocsinitialization_controller.go @@ -445,6 +445,9 @@ func (r *OCSInitializationReconciler) ensureRookCephOperatorConfigExists(initial return err // nolint:revive } + // TODO: remove this in the next release, look at commit msg for more info + delete(rookCephOperatorConfig.Data, "ROOK_CSI_ENABLE_CEPHFS") + return nil })