From cc39368964fb01d57380d8e66f6ddb24bfd41e45 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 27 Aug 2025 15:57:28 -0600 Subject: [PATCH 1/3] csi: update client profile when changing The csi client profile CR needed to be updated if the spec changed. Previously, the client profile was never being updated because it was calling Update() on the existing CR retreived from Get(), rather than with the updated spec. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/csi/config.go | 62 ++++++++------------------------- 1 file changed, 15 insertions(+), 47 deletions(-) diff --git a/pkg/operator/ceph/csi/config.go b/pkg/operator/ceph/csi/config.go index d853d2b9b..4d1bf54af 100644 --- a/pkg/operator/ceph/csi/config.go +++ b/pkg/operator/ceph/csi/config.go @@ -55,26 +55,7 @@ func CreateUpdateClientProfileRadosNamespace(ctx context.Context, c client.Clien }, } - err := c.Get(ctx, types.NamespacedName{Name: csiOpClientProfile.Name, Namespace: csiOpClientProfile.Namespace}, csiOpClientProfile) - if err != nil { - if apierrors.IsNotFound(err) { - err = c.Create(ctx, csiOpClientProfile) - if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi clientProfile cr for RBD %q", csiOpClientProfile.Name) - } - logger.Infof("successfully created ceph-csi clientProfile CR for RBD %q", csiOpClientProfile.Name) - return nil - } - return err - } - - err = c.Update(ctx, csiOpClientProfile) - if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi clientProfile cr for RBD %q", csiOpClientProfile.Name) - } - logger.Infof("successfully updated ceph-csi clientProfile CR for RBD %q", csiOpClientProfile.Name) - - return nil + return createUpdateClientProfile(c, clusterInfo, csiOpClientProfile) } func CreateUpdateClientProfileSubVolumeGroup(ctx context.Context, c client.Client, clusterInfo *cephclient.ClusterInfo, cephFilesystemSubVolumeGroupName, clusterID string) error { @@ -82,26 +63,7 @@ func CreateUpdateClientProfileSubVolumeGroup(ctx context.Context, c client.Clien csiOpClientProfile := generateProfileSubVolumeGroupSpec(clusterInfo, cephFilesystemSubVolumeGroupName, clusterID) - err := c.Get(ctx, types.NamespacedName{Name: csiOpClientProfile.Name, Namespace: csiOpClientProfile.Namespace}, csiOpClientProfile) - if err != nil { - if apierrors.IsNotFound(err) { - err = c.Create(ctx, csiOpClientProfile) - if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi clientProfile cr for subVolGrp %q", csiOpClientProfile.Name) - } - logger.Infof("successfully created ceph-csi clientProfile CR for subVolGrp %q", csiOpClientProfile.Name) - return nil - } - return err - } - - err = c.Update(ctx, csiOpClientProfile) - if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi clientProfile cr for subVolGrp %q", csiOpClientProfile.Name) - } - logger.Infof("successfully updated ceph-csi clientProfile CR for subVolGrp %q", csiOpClientProfile.Name) - - return nil + return createUpdateClientProfile(c, clusterInfo, csiOpClientProfile) } func generateProfileSubVolumeGroupSpec(clusterInfo *cephclient.ClusterInfo, cephFilesystemSubVolumeGroupName, clusterID string) *csiopv1.ClientProfile { @@ -189,24 +151,30 @@ func CreateDefaultClientProfile(c client.Client, clusterInfo *cephclient.Cluster }, } - err := c.Get(clusterInfo.Context, types.NamespacedName{Name: csiOpClientProfile.Name, Namespace: csiOpClientProfile.Namespace}, csiOpClientProfile) + return createUpdateClientProfile(c, clusterInfo, csiOpClientProfile) +} + +func createUpdateClientProfile(c client.Client, clusterInfo *cephclient.ClusterInfo, clientProfile *csiopv1.ClientProfile) error { + existingCsiOpClientProfile := &csiopv1.ClientProfile{} + err := c.Get(clusterInfo.Context, types.NamespacedName{Name: clientProfile.Name, Namespace: clientProfile.Namespace}, existingCsiOpClientProfile) if err != nil { if apierrors.IsNotFound(err) { - err = c.Create(clusterInfo.Context, csiOpClientProfile) + err = c.Create(clusterInfo.Context, clientProfile) if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi for default clientProfile CR %q", csiOpClientProfile.Name) + return errors.Wrapf(err, "failed to create ceph-csi for clientProfile CR %q", clientProfile.Name) } - logger.Infof("successfully created ceph-csi for default clientProfile CR %q", csiOpClientProfile.Name) + logger.Infof("successfully created ceph-csi for clientProfile CR %q", clientProfile.Name) return nil } return err } - err = c.Update(clusterInfo.Context, csiOpClientProfile) + existingCsiOpClientProfile.Spec = clientProfile.Spec + err = c.Update(clusterInfo.Context, existingCsiOpClientProfile) if err != nil { - return errors.Wrapf(err, "failed to create ceph-csi for default clientProfile CR %q", csiOpClientProfile.Name) + return errors.Wrapf(err, "failed to create ceph-csi for clientProfile CR %q", clientProfile.Name) } - logger.Infof("successfully updated ceph-csi for default clientProfile CR %q", csiOpClientProfile.Name) + logger.Infof("successfully updated ceph-csi for clientProfile CR %q", clientProfile.Name) return nil } From eeebfbf4961620eb12e517d61328454d77929061 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 27 Aug 2025 16:11:34 -0600 Subject: [PATCH 2/3] csi: set kernel mount options from env var if not in cluster spec If the cephfs kernel mount options are not specified in the cephcluster.spec.csi CR settings, fall back to the setting in the operator env var CSI_CEPHFS_KERNEL_MOUNT_OPTIONS. Signed-off-by: Travis Nielsen --- pkg/operator/ceph/controller/cluster_info.go | 4 ++++ .../ceph/controller/cluster_info_test.go | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/pkg/operator/ceph/controller/cluster_info.go b/pkg/operator/ceph/controller/cluster_info.go index 9e3fb75a8..010c4fbaa 100644 --- a/pkg/operator/ceph/controller/cluster_info.go +++ b/pkg/operator/ceph/controller/cluster_info.go @@ -168,6 +168,10 @@ func CreateOrLoadClusterInfo(clusterdContext *clusterd.Context, context context. if len(clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels) == 0 { clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels = strings.Split(topology.GetDefaultTopologyLabels(), ",") } + // if the csi spec does not specify mount options, apply them from the operator env vars + if cephClusterSpec.CSI.CephFS.KernelMountOptions == "" { + clusterInfo.CSIDriverSpec.CephFS.KernelMountOptions = k8sutil.GetOperatorSetting("CSI_CEPHFS_KERNEL_MOUNT_OPTIONS", "") + } // If an admin key was provided we don't need to load the other resources // Some people might want to give the admin key diff --git a/pkg/operator/ceph/controller/cluster_info_test.go b/pkg/operator/ceph/controller/cluster_info_test.go index d95394dac..2034199a5 100644 --- a/pkg/operator/ceph/controller/cluster_info_test.go +++ b/pkg/operator/ceph/controller/cluster_info_test.go @@ -139,4 +139,25 @@ func TestCreateClusterSecrets(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "testid", info.CephCred.Username) assert.Equal(t, "testkey", info.CephCred.Secret) + assert.Equal(t, "", info.CSIDriverSpec.CephFS.KernelMountOptions) + + // Verify the csi settings are loaded from an env var + t.Setenv("CSI_CEPHFS_KERNEL_MOUNT_OPTIONS", "ms_mode=crc") + info, _, _, err = CreateOrLoadClusterInfo(context, ctx, namespace, ownerInfo, cephClusterSpec) + assert.NoError(t, err) + assert.Equal(t, "ms_mode=crc", info.CSIDriverSpec.CephFS.KernelMountOptions) + + // Verify the csi settings are loaded from the cephClusterSpec if specified and override the env var + cephClusterSpec.CSI = cephv1.CSIDriverSpec{ + ReadAffinity: cephv1.ReadAffinitySpec{ + Enabled: true, + CrushLocationLabels: []string{"kubernetes.io/hostname"}, + }, + CephFS: cephv1.CSICephFSSpec{ + KernelMountOptions: "ms_mode=secure", + }, + } + info, _, _, err = CreateOrLoadClusterInfo(context, ctx, namespace, ownerInfo, cephClusterSpec) + assert.NoError(t, err) + assert.Equal(t, "ms_mode=secure", info.CSIDriverSpec.CephFS.KernelMountOptions) } From 6309cd367e562ec6d317810d7cc148bd8f065ad3 Mon Sep 17 00:00:00 2001 From: Travis Nielsen Date: Wed, 27 Aug 2025 16:30:47 -0600 Subject: [PATCH 3/3] csi: default to secure kernel mount options if network encrypted If the network is encrypted with msgr2, cephfs requires the mount options to include ms_mode=secure. Now this will be set by default if the mount options are not already set in the cephcluster CR or the operator env var CSI_CEPHFS_KERNEL_MOUNT_OPTIONS. Signed-off-by: Travis Nielsen --- pkg/apis/ceph.rook.io/v1/cluster.go | 11 +++++++ pkg/operator/ceph/cluster/cluster.go | 4 +-- pkg/operator/ceph/controller/cluster_info.go | 30 +++++++++++++------ .../ceph/controller/cluster_info_test.go | 13 ++++++++ 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/pkg/apis/ceph.rook.io/v1/cluster.go b/pkg/apis/ceph.rook.io/v1/cluster.go index 615ffc4ae..36e800e05 100644 --- a/pkg/apis/ceph.rook.io/v1/cluster.go +++ b/pkg/apis/ceph.rook.io/v1/cluster.go @@ -33,6 +33,17 @@ func (c *ClusterSpec) RequireMsgr2() bool { return false } +// RequireMsgr2 checks if the network settings require the msgr2 protocol +func (c *ClusterSpec) NetworkEncryptionEnabled() bool { + if c.Network.Connections == nil { + return false + } + if c.Network.Connections.Encryption == nil { + return false + } + return c.Network.Connections.Encryption.Enabled +} + func (c *ClusterSpec) IsStretchCluster() bool { return c.Mon.StretchCluster != nil && len(c.Mon.StretchCluster.Zones) > 0 } diff --git a/pkg/operator/ceph/cluster/cluster.go b/pkg/operator/ceph/cluster/cluster.go index 47097ae9b..1626f3df4 100755 --- a/pkg/operator/ceph/cluster/cluster.go +++ b/pkg/operator/ceph/cluster/cluster.go @@ -734,9 +734,7 @@ func (c *cluster) configureMsgr2() error { } monStore := config.GetMonStore(c.context, c.ClusterInfo) - encryptionEnabled := c.Spec.Network.Connections != nil && - c.Spec.Network.Connections.Encryption != nil && - c.Spec.Network.Connections.Encryption.Enabled + encryptionEnabled := c.Spec.NetworkEncryptionEnabled() if encryptionEnabled { logger.Infof("setting msgr2 encryption mode to %q", encryptionSetting) diff --git a/pkg/operator/ceph/controller/cluster_info.go b/pkg/operator/ceph/controller/cluster_info.go index 010c4fbaa..91ada14fe 100644 --- a/pkg/operator/ceph/controller/cluster_info.go +++ b/pkg/operator/ceph/controller/cluster_info.go @@ -163,15 +163,8 @@ func CreateOrLoadClusterInfo(clusterdContext *clusterd.Context, context context. // can run on remote executor clusterInfo.NetworkSpec = cephClusterSpec.Network - // update clusterInfo with cephClusterSpec.CSI. - clusterInfo.CSIDriverSpec = cephClusterSpec.CSI - if len(clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels) == 0 { - clusterInfo.CSIDriverSpec.ReadAffinity.CrushLocationLabels = strings.Split(topology.GetDefaultTopologyLabels(), ",") - } - // if the csi spec does not specify mount options, apply them from the operator env vars - if cephClusterSpec.CSI.CephFS.KernelMountOptions == "" { - clusterInfo.CSIDriverSpec.CephFS.KernelMountOptions = k8sutil.GetOperatorSetting("CSI_CEPHFS_KERNEL_MOUNT_OPTIONS", "") - } + // update clusterInfo with cephClusterSpec.CSI settings + clusterInfo.CSIDriverSpec = loadCsiSettings(cephClusterSpec) // If an admin key was provided we don't need to load the other resources // Some people might want to give the admin key @@ -195,6 +188,25 @@ func CreateOrLoadClusterInfo(clusterdContext *clusterd.Context, context context. return clusterInfo, maxMonID, monMapping, nil } +func loadCsiSettings(cephClusterSpec *cephv1.ClusterSpec) cephv1.CSIDriverSpec { + settings := cephClusterSpec.CSI + if len(settings.ReadAffinity.CrushLocationLabels) == 0 { + settings.ReadAffinity.CrushLocationLabels = strings.Split(topology.GetDefaultTopologyLabels(), ",") + } + // if the csi spec does not specify mount options, apply them from the operator env vars + if cephClusterSpec.CSI.CephFS.KernelMountOptions == "" { + settings.CephFS.KernelMountOptions = k8sutil.GetOperatorSetting("CSI_CEPHFS_KERNEL_MOUNT_OPTIONS", "") + // if the kernel mount options are not set and encryption is enabled, set the default to secure + if settings.CephFS.KernelMountOptions == "" { + if cephClusterSpec.NetworkEncryptionEnabled() { + logger.Info("setting default cephfs kernel mount options to 'ms_mode=secure' since encryption is enabled") + settings.CephFS.KernelMountOptions = "ms_mode=secure" + } + } + } + return settings +} + // create new cluster info (FSID, shared keys) func createNamedClusterInfo(context *clusterd.Context, namespace string) (*cephclient.ClusterInfo, error) { fsid, err := uuid.NewRandom() diff --git a/pkg/operator/ceph/controller/cluster_info_test.go b/pkg/operator/ceph/controller/cluster_info_test.go index 2034199a5..c57b7d996 100644 --- a/pkg/operator/ceph/controller/cluster_info_test.go +++ b/pkg/operator/ceph/controller/cluster_info_test.go @@ -141,7 +141,20 @@ func TestCreateClusterSecrets(t *testing.T) { assert.Equal(t, "testkey", info.CephCred.Secret) assert.Equal(t, "", info.CSIDriverSpec.CephFS.KernelMountOptions) + // Verify the csi settings default to secure if the network is encrypted + cephClusterSpec.Network = cephv1.NetworkSpec{ + Connections: &cephv1.ConnectionsSpec{ + Encryption: &cephv1.EncryptionSpec{ + Enabled: true, + }, + }, + } + info, _, _, err = CreateOrLoadClusterInfo(context, ctx, namespace, ownerInfo, cephClusterSpec) + assert.NoError(t, err) + assert.Equal(t, "ms_mode=secure", info.CSIDriverSpec.CephFS.KernelMountOptions) + // Verify the csi settings are loaded from an env var + cephClusterSpec.Network = cephv1.NetworkSpec{} t.Setenv("CSI_CEPHFS_KERNEL_MOUNT_OPTIONS", "ms_mode=crc") info, _, _, err = CreateOrLoadClusterInfo(context, ctx, namespace, ownerInfo, cephClusterSpec) assert.NoError(t, err)