diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 1ea10ba84..203f61f49 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -9,7 +9,7 @@ ## Features -RBD - - Support for configuring read affinity for individuals cluster within the ceph-csi-config ConfigMap in [PR](https://github.com/ceph/ceph-csi/pull/4165) + +- Support for CephFS kernel and fuse mount options per cluster in [PR](https://github.com/ceph/ceph-csi/pull/4245) diff --git a/deploy/csi-config-map-sample.yaml b/deploy/csi-config-map-sample.yaml index 706980825..d44f96905 100644 --- a/deploy/csi-config-map-sample.yaml +++ b/deploy/csi-config-map-sample.yaml @@ -25,7 +25,7 @@ kind: ConfigMap # path for the Ceph cluster identified by the , This will be used # by the CephFS CSI plugin to execute the mount -t in the # The "cephFS.kernelMountOptions" fields are comma separated mount options -# for `Ceph kernel client`. Setting this will override the kernelmountoptions +# for `Ceph Kernel client`. Setting this will override the kernelmountoptions # command line flag. # The "cephFS.fuseMountOptions" fields are common separated mount options # for `Ceph FUSE driver`. Setting this will override the fusemountoptions diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index 94a9cb492..5d353cc66 100644 --- a/internal/cephfs/nodeserver.go +++ b/internal/cephfs/nodeserver.go @@ -311,34 +311,11 @@ func (ns *NodeServer) mount( log.DebugLog(ctx, "cephfs: mounting volume %s with %s", volID, mnt.Name()) - var mountOptions []string - if m := volCap.GetMount(); m != nil { - mountOptions = m.GetMountFlags() - } + err = ns.setMountOptions(mnt, volOptions, volCap, util.CsiConfigFile) + if err != nil { + log.ErrorLog(ctx, "failed to set mount options for volume %s: %v", volID, err) - switch mnt.(type) { - case *mounter.FuseMounter: - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, ns.fuseMountOptions) - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, mountOptions...) - case *mounter.KernelMounter: - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, ns.kernelMountOptions) - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, mountOptions...) - } - - const readOnly = "ro" - - if volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || - volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { - switch mnt.(type) { - case *mounter.FuseMounter: - if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) { - volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, readOnly) - } - case *mounter.KernelMounter: - if !csicommon.MountOptionContains(strings.Split(volOptions.KernelMountOptions, ","), readOnly) { - volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, readOnly) - } - } + return status.Error(codes.Internal, err.Error()) } if err = mnt.Mount(ctx, stagingTargetPath, cr, volOptions); err != nil { @@ -779,3 +756,67 @@ func (ns *NodeServer) NodeGetVolumeStats( return nil, status.Errorf(codes.InvalidArgument, "targetpath %q is not a directory or device", targetPath) } + +// setMountOptions updates the kernel/fuse mount options from CSI config file if it exists. +// If not, it falls back to returning the kernelMountOptions/fuseMountOptions from the command line. +func (ns *NodeServer) setMountOptions( + mnt mounter.VolumeMounter, + volOptions *store.VolumeOptions, + volCap *csi.VolumeCapability, + csiConfigFile string, +) error { + var ( + configuredMountOptions string + kernelMountOptions string + fuseMountOptions string + mountOptions []string + err error + ) + if m := volCap.GetMount(); m != nil { + mountOptions = m.GetMountFlags() + } + + if volOptions.ClusterID != "" { + kernelMountOptions, fuseMountOptions, err = util.GetCephFSMountOptions(csiConfigFile, volOptions.ClusterID) + if err != nil { + return err + } + } + + switch mnt.(type) { + case *mounter.FuseMounter: + configuredMountOptions = ns.fuseMountOptions + // override if fuseMountOptions are set + if fuseMountOptions != "" { + configuredMountOptions = fuseMountOptions + } + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, configuredMountOptions) + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, mountOptions...) + case *mounter.KernelMounter: + configuredMountOptions = ns.kernelMountOptions + // override of kernelMountOptions are set + if kernelMountOptions != "" { + configuredMountOptions = kernelMountOptions + } + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, configuredMountOptions) + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, mountOptions...) + } + + const readOnly = "ro" + + if volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY || + volCap.AccessMode.Mode == csi.VolumeCapability_AccessMode_SINGLE_NODE_READER_ONLY { + switch mnt.(type) { + case *mounter.FuseMounter: + if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) { + volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, readOnly) + } + case *mounter.KernelMounter: + if !csicommon.MountOptionContains(strings.Split(volOptions.KernelMountOptions, ","), readOnly) { + volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, readOnly) + } + } + } + + return nil +} diff --git a/internal/cephfs/nodeserver_test.go b/internal/cephfs/nodeserver_test.go new file mode 100644 index 000000000..e610d95a6 --- /dev/null +++ b/internal/cephfs/nodeserver_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2023 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cephfs + +import ( + "encoding/json" + "os" + "strings" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + + "github.com/ceph/ceph-csi/internal/cephfs/mounter" + "github.com/ceph/ceph-csi/internal/cephfs/store" + "github.com/ceph/ceph-csi/internal/util" +) + +func Test_setMountOptions(t *testing.T) { + t.Parallel() + + cliKernelMountOptions := "noexec,nodev" + cliFuseMountOptions := "default_permissions,auto_cache" + + configKernelMountOptions := "crc" + configFuseMountOptions := "allow_other" + + csiConfig := []util.ClusterInfo{ + { + ClusterID: "cluster-1", + CephFS: util.CephFS{ + KernelMountOptions: configKernelMountOptions, + FuseMountOptions: configFuseMountOptions, + }, + }, + { + ClusterID: "cluster-2", + CephFS: util.CephFS{ + KernelMountOptions: "", + FuseMountOptions: "", + }, + }, + } + + csiConfigFileContent, err := json.Marshal(csiConfig) + if err != nil { + t.Errorf("failed to marshal csi config info %v", err) + } + tmpConfPath := t.TempDir() + "/ceph-csi.json" + t.Logf("path = %s", tmpConfPath) + err = os.WriteFile(tmpConfPath, csiConfigFileContent, 0o600) + if err != nil { + t.Errorf("failed to write %s file content: %v", util.CsiConfigFile, err) + } + + tests := []struct { + name string + ns NodeServer + mnt mounter.VolumeMounter + volOptions *store.VolumeOptions + want string + }{ + { + name: "KernelMountOptions set in cluster-1 config and not set in CLI", + ns: NodeServer{}, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configKernelMountOptions, + }, + { + name: "FuseMountOptions set in cluster-1 config and not set in CLI", + ns: NodeServer{}, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configFuseMountOptions, + }, + { + name: "KernelMountOptions set in cluster-1 config and set in CLI", + ns: NodeServer{ + kernelMountOptions: cliKernelMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configKernelMountOptions, + }, + { + name: "FuseMountOptions not set in cluster-2 config and set in CLI", + ns: NodeServer{ + fuseMountOptions: cliFuseMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-1", + }, + want: configFuseMountOptions, + }, + { + name: "KernelMountOptions not set in cluster-2 config and set in CLI", + ns: NodeServer{ + kernelMountOptions: cliKernelMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.KernelMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-2", + }, + want: cliKernelMountOptions, + }, + { + name: "FuseMountOptions not set in cluster-1 config and set in CLI", + ns: NodeServer{ + fuseMountOptions: cliFuseMountOptions, + }, + mnt: mounter.VolumeMounter(&mounter.FuseMounter{}), + volOptions: &store.VolumeOptions{ + ClusterID: "cluster-2", + }, + want: cliFuseMountOptions, + }, + } + + volCap := &csi.VolumeCapability{ + AccessMode: &csi.VolumeCapability_AccessMode{}, + } + + for _, tt := range tests { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := tc.ns.setMountOptions(tc.mnt, tc.volOptions, volCap, tmpConfPath) + if err != nil { + t.Errorf("setMountOptions() = %v", err) + } + + switch tc.mnt.(type) { + case *mounter.FuseMounter: + if !strings.Contains(tc.volOptions.FuseMountOptions, tc.want) { + t.Errorf("Set FuseMountOptions = %v Required FuseMountOptions = %v", tc.volOptions.FuseMountOptions, tc.want) + } + case *mounter.KernelMounter: + if !strings.Contains(tc.volOptions.KernelMountOptions, tc.want) { + t.Errorf("Set KernelMountOptions = %v Required KernelMountOptions = %v", tc.volOptions.KernelMountOptions, tc.want) + } + } + }) + } +}