Repository: rook/rook
Pull request #13862 (2024-03-04 15:10:46)

csi: update CSIDriverOption params during saving cluster config (backport #13836)
This is an automatic backport of pull request #13836 done by [Mergify](https://mergify.com).

---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

mergify[bot]: Cherry-pick of 330f604cca1384b763bce629626b75fce6f73bd4 has failed:
```
On branch mergify/bp/release-1.13/pr-13836
Your branch is up to date with 'origin/release-1.13'.

You are currently cherry-picking commit 330f604cc.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   pkg/operator/ceph/csi/cluster_config.go
	both modified:   pkg/operator/ceph/csi/cluster_config_test.go

no changes added to commit (use "git add" and/or "git commit -a")
```


To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Patch:
diff --git a/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md b/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md
index 83c6fffc2..37dc1b2da 100644
--- a/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md
+++ b/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md
@@ -143,6 +143,9 @@ gateway:
       # hostname: example.com
 ```
 
+This will create a service with the endpoint `192.168.39.182` on port `80`, pointing to the Ceph object external gateway.
+All the other settings from the gateway section will be ignored, except for `securePort`.
+
 ## Zone Settings
 
 The [zone](../../Storage-Configuration/Object-Storage-RGW/ceph-object-multisite.md) settings allow the object store to join custom created [ceph-object-zone](ceph-object-zone-crd.md).
diff --git a/deploy/charts/rook-ceph-cluster/values.yaml b/deploy/charts/rook-ceph-cluster/values.yaml
index aaf10ef0c..403420f54 100644
--- a/deploy/charts/rook-ceph-cluster/values.yaml
+++ b/deploy/charts/rook-ceph-cluster/values.yaml
@@ -440,7 +440,7 @@ cephBlockPools:
       replicated:
         size: 3
       # Enables collecting RBD per-image IO statistics by enabling dynamic OSD performance counters. Defaults to false.
-      # For reference: https://docs.ceph.com/docs/latest/mgr/prometheus/#rbd-io-statistics
+      # For reference: https://docs.ceph.com/docs/master/mgr/prometheus/#rbd-io-statistics
       # enableRBDStats: true
     storageClass:
       enabled: true
@@ -460,16 +460,16 @@ cephBlockPools:
       parameters:
         # (optional) mapOptions is a comma-separated list of map options.
         # For krbd options refer
-        # https://docs.ceph.com/docs/latest/man/8/rbd/#kernel-rbd-krbd-options
+        # https://docs.ceph.com/docs/master/man/8/rbd/#kernel-rbd-krbd-options
         # For nbd options refer
-        # https://docs.ceph.com/docs/latest/man/8/rbd-nbd/#options
+        # https://docs.ceph.com/docs/master/man/8/rbd-nbd/#options
         # mapOptions: lock_on_read,queue_depth=1024
 
         # (optional) unmapOptions is a comma-separated list of unmap options.
         # For krbd options refer
-        # https://docs.ceph.com/docs/latest/man/8/rbd/#kernel-rbd-krbd-options
+        # https://docs.ceph.com/docs/master/man/8/rbd/#kernel-rbd-krbd-options
         # For nbd options refer
-        # https://docs.ceph.com/docs/latest/man/8/rbd-nbd/#options
+        # https://docs.ceph.com/docs/master/man/8/rbd-nbd/#options
         # unmapOptions: force
 
         # RBD image format. Defaults to "2".
@@ -651,16 +651,16 @@ cephObjectStores:
 #
 #    # (optional) mapOptions is a comma-separated list of map options.
 #    # For krbd options refer
-#    # https://docs.ceph.com/docs/latest/man/8/rbd/#kernel-rbd-krbd-options
+#    # https://docs.ceph.com/docs/master/man/8/rbd/#kernel-rbd-krbd-options
 #    # For nbd options refer
-#    # https://docs.ceph.com/docs/latest/man/8/rbd-nbd/#options
+#    # https://docs.ceph.com/docs/master/man/8/rbd-nbd/#options
 #    # mapOptions: lock_on_read,queue_depth=1024
 #
 #    # (optional) unmapOptions is a comma-separated list of unmap options.
 #    # For krbd options refer
-#    # https://docs.ceph.com/docs/latest/man/8/rbd/#kernel-rbd-krbd-options
+#    # https://docs.ceph.com/docs/master/man/8/rbd/#kernel-rbd-krbd-options
 #    # For nbd options refer
-#    # https://docs.ceph.com/docs/latest/man/8/rbd-nbd/#options
+#    # https://docs.ceph.com/docs/master/man/8/rbd-nbd/#options
 #    # unmapOptions: force
 #
 #    # RBD image format. Defaults to "2".
diff --git a/deploy/examples/create-external-cluster-resources.py b/deploy/examples/create-external-cluster-resources.py
index 61039c9eb..5ffef28d3 100644
--- a/deploy/examples/create-external-cluster-resources.py
+++ b/deploy/examples/create-external-cluster-resources.py
@@ -699,6 +699,14 @@ class RadosJSON:
         q_leader_details = q_leader_matching_list[0]
         # get the address vector of the quorum-leader
         q_leader_addrvec = q_leader_details.get("public_addrs", {}).get("addrvec", [])
+        # if the quorum-leader has only one address in the address-vector
+        # and it is of type 'v2' (ie; with <IP>:3300),
+        # raise an exception to make user aware that
+        # they have to enable 'v1' (ie; with <IP>:6789) type as well
+        if len(q_leader_addrvec) == 1 and q_leader_addrvec[0]["type"] == "v2":
+            raise ExecutionFailureException(
+                "Only 'v2' address type is enabled, user should also enable 'v1' type as well"
+            )
         ip_addr = str(q_leader_details["public_addr"].split("/")[0])
 
         if self._arg_parser.v2_port_enable:
diff --git a/images/ceph/Dockerfile b/images/ceph/Dockerfile
index 268926856..26f6dbb05 100644
--- a/images/ceph/Dockerfile
+++ b/images/ceph/Dockerfile
@@ -20,7 +20,7 @@ ARG S5CMD_VERSION
 ARG S5CMD_ARCH
 
 # install 'ip' tool for Multus
-RUN dnf install -y --repo baseos --setopt=install_weak_deps=False iproute && dnf clean all
+RUN dnf install -y --setopt=install_weak_deps=False iproute && dnf clean all
 
 # Install the s5cmd package to interact with s3 gateway
 RUN curl --fail -sSL -o /s5cmd.tar.gz https://github.com/peak/s5cmd/releases/download/v${S5CMD_VERSION}/s5cmd_${S5CMD_VERSION}_${S5CMD_ARCH}.tar.gz && \
diff --git a/pkg/operator/ceph/csi/cluster_config.go b/pkg/operator/ceph/csi/cluster_config.go
index 97928b9bf..2f0d41c70 100644
--- a/pkg/operator/ceph/csi/cluster_config.go
+++ b/pkg/operator/ceph/csi/cluster_config.go
@@ -127,9 +127,10 @@ func MonEndpoints(mons map[string]*cephclient.MonInfo, requireMsgr2 bool) []stri
 // the cluster-to-mon mapping required to configure ceph csi.
 func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *CsiClusterConfigEntry) (string, error) {
 	var (
-		cc     csiClusterConfig
-		centry CsiClusterConfigEntry
-		found  bool
+		cc                                   csiClusterConfig
+		centry                               CsiClusterConfigEntry
+		found                                bool
+		kernelMountOptions, fuseMountOptions string
 	)
 
 	cc, err := parseCsiClusterConfig(curr)
@@ -137,6 +138,12 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 		return "", errors.Wrap(err, "failed to parse current csi cluster config")
 	}
 
+	// set mount options here to avoid doing nil checks multiple times.
+	if newCsiClusterConfigEntry != nil && newCsiClusterConfigEntry.CephFS != nil {
+		kernelMountOptions = newCsiClusterConfigEntry.CephFS.KernelMountOptions
+		fuseMountOptions = newCsiClusterConfigEntry.CephFS.FuseMountOptions
+	}
+
 	// Regardless of which controllers call updateCsiClusterConfig(), the values will be preserved since
 	// a lock is acquired for the update operation. So concurrent updates (rare event) will block and
 	// wait for the other update to complete. Monitors and Subvolumegroup will be updated
@@ -147,6 +154,12 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 			// update default clusterID's entry
 			if clusterKey == centry.Namespace {
 				centry.Monitors = newCsiClusterConfigEntry.Monitors
+				centry.ReadAffinity = newCsiClusterConfigEntry.ReadAffinity
+				if centry.CephFS == nil {
+					centry.CephFS = &CsiCephFSSpec{}
+				}
+				centry.CephFS.KernelMountOptions = kernelMountOptions
+				centry.CephFS.FuseMountOptions = fuseMountOptions
 				cc[i] = centry
 			}
 		}
@@ -160,15 +173,28 @@ 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 != nil && (newCsiClusterConfigEntry.CephFS.SubvolumeGroup != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "") {
 				centry.CephFS = newCsiClusterConfigEntry.CephFS
+			} else {
+				if centry.CephFS == nil {
+					centry.CephFS = &CsiCephFSSpec{}
+				}
+				centry.CephFS.KernelMountOptions = kernelMountOptions
+				centry.CephFS.FuseMountOptions = fuseMountOptions
 			}
+			// update nfs netNamespaceFilePath only when specified.
 			if newCsiClusterConfigEntry.NFS != nil && newCsiClusterConfigEntry.NFS.NetNamespaceFilePath != "" {
 				centry.NFS = newCsiClusterConfigEntry.NFS
 			}
+			// update radosNamespace and rbd netNamespaceFilePath only when either is specified.
 			if newCsiClusterConfigEntry.RBD != nil && (newCsiClusterConfigEntry.RBD.RadosNamespace != "" || newCsiClusterConfigEntry.RBD.NetNamespaceFilePath != "") {
 				centry.RBD = newCsiClusterConfigEntry.RBD
 			}
+			if newCsiClusterConfigEntry.ReadAffinity != nil && len(newCsiClusterConfigEntry.ReadAffinity.CrushLocationLabels) != 0 {
+				centry.ReadAffinity = newCsiClusterConfigEntry.ReadAffinity
+			}
 			// This maintains backward compatibility for existing clusters, from now on the
 			// preferred way is to use RBD.RadosNamespace
 			if newCsiClusterConfigEntry.RadosNamespace != "" {
@@ -186,16 +212,20 @@ func updateCsiClusterConfig(curr, clusterKey string, newCsiClusterConfigEntry *C
 			centry.ClusterID = clusterKey
 			centry.Namespace = newCsiClusterConfigEntry.Namespace
 			centry.Monitors = newCsiClusterConfigEntry.Monitors
-			if newCsiClusterConfigEntry.RBD != nil && (newCsiClusterConfigEntry.RBD.RadosNamespace != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "") {
+			if newCsiClusterConfigEntry.RBD != nil && (newCsiClusterConfigEntry.RBD.RadosNamespace != "" || newCsiClusterConfigEntry.RBD.NetNamespaceFilePath != "") {
 				centry.RBD = newCsiClusterConfigEntry.RBD
 			}
 			// Add a condition not to fill with empty values
-			if newCsiClusterConfigEntry.CephFS != nil && (newCsiClusterConfigEntry.CephFS.SubvolumeGroup != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "") {
+			if newCsiClusterConfigEntry.CephFS != nil && (newCsiClusterConfigEntry.CephFS.SubvolumeGroup != "" || newCsiClusterConfigEntry.CephFS.NetNamespaceFilePath != "" ||
+				newCsiClusterConfigEntry.CephFS.KernelMountOptions != "" || newCsiClusterConfigEntry.CephFS.FuseMountOptions != "") {
 				centry.CephFS = newCsiClusterConfigEntry.CephFS
 			}
 			if newCsiClusterConfigEntry.NFS != nil && newCsiClusterConfigEntry.NFS.NetNamespaceFilePath != "" {
 				centry.NFS = newCsiClusterConfigEntry.NFS
 			}
+			if newCsiClusterConfigEntry.ReadAffinity != nil && len(newCsiClusterConfigEntry.ReadAffinity.CrushLocationLabels) != 0 {
+				centry.ReadAffinity = newCsiClusterConfigEntry.ReadAffinity
+			}
 			cc = append(cc, centry)
 		}
 	}
@@ -248,6 +278,19 @@ func SaveClusterConfig(clientset kubernetes.Interface, clusterNamespace string,
 	}
 	logger.Debugf("using %q for csi configmap namespace", csiNamespace)
 
+	if newCsiClusterConfigEntry != nil {
+		// set CSIDriverOptions
+		newCsiClusterConfigEntry.ReadAffinity = &cephv1.ReadAffinitySpec{
+			Enabled:             clusterInfo.CSIDriverSpec.ReadAffinity.Enabled,
+			CrushLocationLabels: clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels,
+		}
+		if newCsiClusterConfigEntry.CephFS == nil {
+			newCsiClusterConfigEntry.CephFS = &CsiCephFSSpec{}
+		}
+		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 bc406f8df..3c307b7d3 100644
--- a/pkg/operator/ceph/csi/cluster_config_test.go
+++ b/pkg/operator/ceph/csi/cluster_config_test.go
@@ -38,6 +38,14 @@ func TestUpdateCsiClusterConfig(t *testing.T) {
 			RadosNamespace:       "rook-ceph-1",
 		},
 	}
+	csiClusterConfigEntryMountOptions := CsiClusterConfigEntry{
+		Namespace: "rook-ceph-1",
+		Monitors:  []string{"1.2.3.4:5000"},
+		CephFS: &CsiCephFSSpec{
+			KernelMountOptions: "ms_mode=crc",
+			FuseMountOptions:   "debug",
+		},
+	}
 	csiClusterConfigEntry2 := CsiClusterConfigEntry{
 		Namespace: "rook-ceph-2",
 		Monitors:  []string{"20.1.1.1:5000", "20.1.1.2:5000", "20.1.1.3:5000"},
@@ -72,6 +80,28 @@ 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 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)
@@ -128,6 +158,32 @@ func TestUpdateCsiClusterConfig(t *testing.T) {
 
 	})
 
+	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("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)