Repository: rook/rook
Pull request #13836 (2024-02-29 12:28:00)

csi: update CSIDriverOption params during saving cluster config
During cluster creation, csi config map was first filled with mon ips and without CSIDriverOptions.
This commit makes sure CSIDriverOptions are added at the begining when the entry is first created.

Updates:
- #13835

#### Without fix (kernelmountoptions and readaffinity options are filled in later)
```
[User@localhost ~]:rook$ k get cm rook-ceph-csi-config -o yaml -w
apiVersion: v1
data:
  csi-cluster-config-json: '[]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-02-29T13:43:36Z"
  name: rook-ceph-csi-config
  namespace: rook-ceph
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: false
    controller: true
    kind: Deployment
    name: rook-ceph-operator
    uid: 17c87c42-f130-481e-996d-a9750b5b21df
  resourceVersion: "10112"
  uid: 7e37de42-06fb-403b-b4a9-f0a8bfc2bb3e
---
apiVersion: v1
data:
  csi-cluster-config-json: '[{"clusterID":"rook-ceph","monitors":["192.168.39.252:6789"],"cephFS":{"netNamespaceFilePath":"","subvolumeGroup":"","kernelMountOptions":"","fuseMountOptions":""},"rbd":{"netNamespaceFilePath":"","radosNamespace":""},"nfs":{"netNamespaceFilePath":""},"readAffinity":{"enabled":false,"crushLocationLabels":null},"namespace":"rook-ceph"}]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-02-29T13:43:36Z"
  name: rook-ceph-csi-config
  namespace: rook-ceph
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: false
    controller: true
    kind: Deployment
    name: rook-ceph-operator
    uid: 17c87c42-f130-481e-996d-a9750b5b21df
  resourceVersion: "10199"
  uid: 7e37de42-06fb-403b-b4a9-f0a8bfc2bb3e
---
apiVersion: v1
data:
  csi-cluster-config-json: '[{"clusterID":"rook-ceph","monitors":["192.168.39.252:6789"],"cephFS":{"netNamespaceFilePath":"","subvolumeGroup":"","kernelMountOptions":"ms_mode=prefer-crc","fuseMountOptions":""},"rbd":{"netNamespaceFilePath":"","radosNamespace":""},"nfs":{"netNamespaceFilePath":""},"readAffinity":{"enabled":false,"crushLocationLabels":["test"]},"namespace":"rook-ceph"}]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-02-29T13:43:36Z"
  name: rook-ceph-csi-config
  namespace: rook-ceph
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: false
    controller: true
    kind: Deployment
    name: rook-ceph-operator
    uid: 17c87c42-f130-481e-996d-a9750b5b21df
  resourceVersion: "10306"
  uid: 7e37de42-06fb-403b-b4a9-f0a8bfc2bb3e
```

#### With fix
```
apiVersion: v1
data:
  csi-cluster-config-json: '[]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-02-29T13:39:13Z"
  name: rook-ceph-csi-config
  namespace: rook-ceph
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: false
    controller: true
    kind: Deployment
    name: rook-ceph-operator
    uid: 74331958-4fc2-492c-b3d7-ccffc9d29e48
  resourceVersion: "124843"
  uid: 8b4f509f-bbb4-4b94-9397-5c75f840e33b
---
apiVersion: v1
data:
  csi-cluster-config-json: '[{"clusterID":"rook-ceph","monitors":["10.108.161.177:6789"],"cephFS":{"netNamespaceFilePath":"","subvolumeGroup":"","kernelMountOptions":"mds_mode=prefer-crc","fuseMountOptions":""},"rbd":{"netNamespaceFilePath":"","radosNamespace":""},"nfs":{"netNamespaceFilePath":""},"readAffinity":{"enabled":false,"crushLocationLabels":["w"]},"namespace":"rook-ceph"}]'
kind: ConfigMap
metadata:
  creationTimestamp: "2024-02-29T13:39:13Z"
  name: rook-ceph-csi-config
  namespace: rook-ceph
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: false
    controller: true
    kind: Deployment
    name: rook-ceph-operator
    uid: 74331958-4fc2-492c-b3d7-ccffc9d29e48
  resourceVersion: "124946"
  uid: 8b4f509f-bbb4-4b94-9397-5c75f840e33b
```

**Checklist:**

- [ ] **Commit Message Formatting**: Commit titles and messages follow guidelines in the [developer guide](https://rook.io/docs/rook/latest/Contributing/development-flow/#commit-structure).
- [ ] Reviewed the developer guide on [Submitting a Pull Request](https://rook.io/docs/rook/latest/Contributing/development-flow/#submitting-a-pull-request)
- [ ] [Pending release notes](https://github.com/rook/rook/blob/master/PendingReleaseNotes.md) updated with breaking and/or notable changes for the next minor release.
- [ ] Documentation has been updated, if necessary.
- [ ] Unit tests have been added, if necessary.
- [ ] Integration tests have been added, if necessary.



Patch:
diff --git a/pkg/operator/ceph/csi/cluster_config.go b/pkg/operator/ceph/csi/cluster_config.go
index 7775b0e9c..6b9a9ee59 100644
--- a/pkg/operator/ceph/csi/cluster_config.go
+++ b/pkg/operator/ceph/csi/cluster_config.go
@@ -170,6 +170,9 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 			// update default clusterID's entry
 			if clusterKey == centry.Namespace {
 				centry.Monitors = newCsiClusterConfigEntry.Monitors
+				centry.ReadAffinity = newCsiClusterConfigEntry.ReadAffinity
+				centry.CephFS.KernelMountOptions = newCsiClusterConfigEntry.CephFS.KernelMountOptions
+				centry.CephFS.FuseMountOptions = newCsiClusterConfigEntry.CephFS.FuseMountOptions
 				cc[i] = centry
 			}
 		}
@@ -183,12 +186,19 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 				break
 			}
 			centry.Monitors = newCsiClusterConfigEntry.Monitors
+			// update subvolumegroup and cephfs netNamespaceFilePath only when either is specified
+			// while always updating kernel and fuse mount options.
 			if newCsiClusterConfigEntry.CephFS.SubvolumeGroup != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "" {
 				centry.CephFS = newCsiClusterConfigEntry.CephFS
+			} else {
+				centry.CephFS.KernelMountOptions = newCsiClusterConfigEntry.CephFS.KernelMountOptions
+				centry.CephFS.FuseMountOptions = newCsiClusterConfigEntry.CephFS.FuseMountOptions
 			}
+			// update nfs netNamespaceFilePath only when specified.
 			if newCsiClusterConfigEntry.NFS.NetNamespaceFilePath != "" {
 				centry.NFS = newCsiClusterConfigEntry.NFS
 			}
+			// update radosNamespace and rbd netNamespaceFilePath only when either is specified.
 			if newCsiClusterConfigEntry.RBD.RadosNamespace != "" || newCsiClusterConfigEntry.RBD.NetNamespaceFilePath != "" {
 				centry.RBD = newCsiClusterConfigEntry.RBD
 			}
@@ -207,16 +217,9 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 			centry.ClusterID = clusterKey
 			centry.Namespace = newCsiClusterConfigEntry.Namespace
 			centry.Monitors = newCsiClusterConfigEntry.Monitors
-			if newCsiClusterConfigEntry.RBD.RadosNamespace != "" || newCsiClusterConfigEntry.RBD.NetNamespaceFilePath != "" {
-				centry.RBD = newCsiClusterConfigEntry.RBD
-			}
-			// Add a condition not to fill with empty values
-			if newCsiClusterConfigEntry.CephFS.SubvolumeGroup != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "" {
-				centry.CephFS = newCsiClusterConfigEntry.CephFS
-			}
-			if newCsiClusterConfigEntry.NFS.NetNamespaceFilePath != "" {
-				centry.NFS = newCsiClusterConfigEntry.NFS
-			}
+			centry.RBD = newCsiClusterConfigEntry.RBD
+			centry.CephFS = newCsiClusterConfigEntry.CephFS
+			centry.NFS = newCsiClusterConfigEntry.NFS
 			if len(newCsiClusterConfigEntry.ReadAffinity.CrushLocationLabels) != 0 {
 				centry.ReadAffinity = newCsiClusterConfigEntry.ReadAffinity
 			}
@@ -273,6 +276,15 @@ func SaveClusterConfig(clientset kubernetes.Interface, clusterNamespace string,
 	}
 	logger.Debugf("using %q for csi configmap namespace", csiNamespace)
 
+	if newCsiClusterConfigEntry != nil {
+		// set CSIDriverOptions
+		newCsiClusterConfigEntry.ReadAffinity.Enabled = clusterInfo.CSIDriverSpec.ReadAffinity.Enabled
+		newCsiClusterConfigEntry.ReadAffinity.CrushLocationLabels = clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels
+
+		newCsiClusterConfigEntry.CephFS.KernelMountOptions = clusterInfo.CSIDriverSpec.CephFS.KernelMountOptions
+		newCsiClusterConfigEntry.CephFS.FuseMountOptions = clusterInfo.CSIDriverSpec.CephFS.FuseMountOptions
+	}
+
 	configMutex.Lock()
 	defer configMutex.Unlock()
 


Test patch:
diff --git a/pkg/operator/ceph/csi/cluster_config_test.go b/pkg/operator/ceph/csi/cluster_config_test.go
index 9a87c39fc..3698e0fb1 100644
--- a/pkg/operator/ceph/csi/cluster_config_test.go
+++ b/pkg/operator/ceph/csi/cluster_config_test.go
@@ -74,6 +74,16 @@ func TestUpdateCsiClusterConfig(t *testing.T) {
 			},
 		},
 	}
+	csiClusterConfigEntryMountOptions := CSIClusterConfigEntry{
+		Namespace: "rook-ceph-1",
+		ClusterInfo: cephcsi.ClusterInfo{
+			Monitors: []string{"1.2.3.4:5000"},
+			CephFS: cephcsi.CephFS{
+				KernelMountOptions: "ms_mode=crc",
+				FuseMountOptions:   "debug",
+			},
+		},
+	}
 	csiClusterConfigEntry2 := CSIClusterConfigEntry{
 		Namespace: "rook-ceph-2",
 		ClusterInfo: cephcsi.ClusterInfo{
@@ -123,6 +133,17 @@ func TestUpdateCsiClusterConfig(t *testing.T) {
 		assert.Equal(t, 2, len(cc[0].Monitors))
 	})
 
+	t.Run("add mount options to the current cluster", func(t *testing.T) {
+		configWithMountOptions, err := updateCsiClusterConfig(s, "rook-ceph-1", &csiClusterConfigEntryMountOptions)
+		assert.NoError(t, err)
+		cc, err := parseCsiClusterConfig(configWithMountOptions)
+		assert.NoError(t, err)
+		assert.Equal(t, 1, len(cc))
+		assert.Equal(t, "rook-ceph-1", cc[0].ClusterID)
+		assert.Equal(t, csiClusterConfigEntryMountOptions.CephFS.KernelMountOptions, cc[0].CephFS.KernelMountOptions)
+		assert.Equal(t, csiClusterConfigEntryMountOptions.CephFS.FuseMountOptions, cc[0].CephFS.FuseMountOptions)
+	})
+
 	t.Run("add a 2nd cluster with 3 mons", func(t *testing.T) {
 		s, err = updateCsiClusterConfig(s, "beta", &csiClusterConfigEntry2)
 		assert.NoError(t, err)
@@ -178,6 +199,19 @@ func TestUpdateCsiClusterConfig(t *testing.T) {
 		assert.Equal(t, "my-group", cc[2].CephFS.SubvolumeGroup)
 	})
 
+	t.Run("update mount options in presence of subvolumegroup", func(t *testing.T) {
+		sMntOptionUpdate, err := updateCsiClusterConfig(s, "baba", &csiClusterConfigEntryMountOptions)
+		assert.NoError(t, err)
+		cc, err := parseCsiClusterConfig(sMntOptionUpdate)
+		assert.NoError(t, err)
+		assert.Equal(t, 3, len(cc))
+		assert.Equal(t, "baba", cc[2].ClusterID)
+		assert.Equal(t, "my-group", cc[2].CephFS.SubvolumeGroup)
+		assert.Equal(t, csiClusterConfigEntryMountOptions.CephFS.KernelMountOptions, cc[2].CephFS.KernelMountOptions)
+		assert.Equal(t, csiClusterConfigEntryMountOptions.CephFS.FuseMountOptions, cc[2].CephFS.FuseMountOptions)
+
+	})
+
 	t.Run("add a 4th mon to the 3rd cluster and subvolumegroup is preserved", func(t *testing.T) {
 		csiClusterConfigEntry3.Monitors = append(csiClusterConfigEntry3.Monitors, "10.11.12.13:5000")
 		s, err = updateCsiClusterConfig(s, "baba", &csiClusterConfigEntry3)