cephfs: do not run modprobe if support is compiled into the kernel

By reading the contents of /proc/filesystems, and checking if "ceph" is included there, running "modprobe ceph" can be skipped. Fixes: #4376 Signed-off-by: Niels de Vos <[email protected]>
ceph · Jan 17, 2024 · ab87045 · ab87045
1 parent b79dc5d
commit ab87045
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 14 deletions.
diff --git a/internal/cephfs/mounter/kernel.go b/internal/cephfs/mounter/kernel.go
@@ -19,6 +19,8 @@ package mounter
 import (
 	"context"
 	"fmt"
+	"os"
+	"strings"
 
 	"github.com/ceph/ceph-csi/internal/cephfs/store"
 	"github.com/ceph/ceph-csi/internal/util"
@@ -27,13 +29,47 @@ import (
 const (
 	volumeMounterKernel = "kernel"
 	netDev              = "_netdev"
+	kernelModule        = "ceph"
 )
 
-type KernelMounter struct{}
+// testErrorf can be set by unit test for enhanced error reporting.
+var testErrorf = func(fmt string, args ...any) { /* do nothing */ }
 
-func mountKernel(ctx context.Context, mountPoint string, cr *util.Credentials, volOptions *store.VolumeOptions) error {
-	if err := execCommandErr(ctx, "modprobe", "ceph"); err != nil {
-		return err
+type KernelMounter interface {
+	Mount(
+		ctx context.Context,
+		mountPoint string,
+		cr *util.Credentials,
+		volOptions *store.VolumeOptions,
+	) error
+
+	Name() string
+}
+
+type kernelMounter struct {
+	// needsModprobe indicates that the ceph kernel module is not loaded in
+	// the kernel yet (or compiled into it)
+	needsModprobe bool
+}
+
+func NewKernelMounter() KernelMounter {
+	return &kernelMounter{
+		needsModprobe: !filesystemSupported(kernelModule),
+	}
+}
+
+func (m *kernelMounter) mountKernel(
+	ctx context.Context,
+	mountPoint string,
+	cr *util.Credentials,
+	volOptions *store.VolumeOptions,
+) error {
+	if m.needsModprobe {
+		if err := execCommandErr(ctx, "modprobe", kernelModule); err != nil {
+			return err
+		}
+
+		m.needsModprobe = false
 	}
 
 	args := []string{
@@ -68,7 +104,7 @@ func mountKernel(ctx context.Context, mountPoint string, cr *util.Credentials, v
 	return err
 }
 
-func (m *KernelMounter) Mount(
+func (m *kernelMounter) Mount(
 	ctx context.Context,
 	mountPoint string,
 	cr *util.Credentials,
@@ -78,7 +114,26 @@ func (m *KernelMounter) Mount(
 		return err
 	}
 
-	return mountKernel(ctx, mountPoint, cr, volOptions)
+	return m.mountKernel(ctx, mountPoint, cr, volOptions)
 }
 
-func (m *KernelMounter) Name() string { return "Ceph kernel client" }
+func (m *kernelMounter) Name() string { return "Ceph kernel client" }
+
+// filesystemSupported checks if the passed name of the filesystem is included
+// in /proc/filesystems.
+func filesystemSupported(fs string) bool {
+	// /proc/filesystems contains a list of all supported filesystems,
+	// either compiled into the kernel, or as loadable module.
+	data, err := os.ReadFile("/proc/filesystems")
+	if err != nil {
+		testErrorf("failed to read /proc/filesystems: %v", err)
+
+		return false
+	}
+
+	// The format of /proc/filesystems is one filesystem per line, an
+	// optional keyword ("nodev") followed by a tab and the name of the
+	// filesystem. Matching <tab>ceph<eol> for the ceph kernel module that
+	// supports CephFS.
+	return strings.Contains(string(data), "\t"+fs+"\n")
+}
diff --git a/internal/cephfs/mounter/kernel_test.go b/internal/cephfs/mounter/kernel_test.go
@@ -0,0 +1,38 @@
+/*
+Copyright 2024 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 mounter
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFilesystemSupported(t *testing.T) {
+	t.Parallel()
+
+	testErrorf = func(fmt string, args ...any) {
+		t.Errorf(fmt, args...)
+	}
+
+	// "proc" is always a supported filesystem, we detect supported
+	// filesystems by reading from it
+	assert.True(t, filesystemSupported("proc"))
+
+	// "nonefs" is a made-up name, and does not exist
+	assert.False(t, filesystemSupported("nonefs"))
+}
diff --git a/internal/cephfs/mounter/volumemounter.go b/internal/cephfs/mounter/volumemounter.go
@@ -131,7 +131,7 @@ func New(volOptions *store.VolumeOptions) (VolumeMounter, error) {
 	case volumeMounterFuse:
 		return &FuseMounter{}, nil
 	case volumeMounterKernel:
-		return &KernelMounter{}, nil
+		return NewKernelMounter(), nil
 	}
 
 	return nil, fmt.Errorf("unknown mounter '%s'", chosenMounter)

diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go
@@ -801,7 +801,7 @@ func (ns *NodeServer) setMountOptions(
 		}
 		volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, configuredMountOptions)
 		volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, mountOptions...)
-	case *mounter.KernelMounter:
+	case mounter.KernelMounter:
 		configuredMountOptions = ns.kernelMountOptions
 		// override of kernelMountOptions are set
 		if kernelMountOptions != "" {
@@ -821,7 +821,7 @@ func (ns *NodeServer) setMountOptions(
 			if !csicommon.MountOptionContains(strings.Split(volOptions.FuseMountOptions, ","), readOnly) {
 				volOptions.FuseMountOptions = util.MountOptionsAdd(volOptions.FuseMountOptions, readOnly)
 			}
-		case *mounter.KernelMounter:
+		case mounter.KernelMounter:
 			if !csicommon.MountOptionContains(strings.Split(volOptions.KernelMountOptions, ","), readOnly) {
 				volOptions.KernelMountOptions = util.MountOptionsAdd(volOptions.KernelMountOptions, readOnly)
 			}

diff --git a/internal/cephfs/nodeserver_test.go b/internal/cephfs/nodeserver_test.go
@@ -77,7 +77,7 @@ func Test_setMountOptions(t *testing.T) {
 		{
 			name: "KernelMountOptions set in cluster-1 config and not set in CLI",
 			ns:   &NodeServer{},
-			mnt:  mounter.VolumeMounter(&mounter.KernelMounter{}),
+			mnt:  mounter.VolumeMounter(mounter.NewKernelMounter()),
 			volOptions: &store.VolumeOptions{
 				ClusterID: "cluster-1",
 			},
@@ -97,7 +97,7 @@ func Test_setMountOptions(t *testing.T) {
 			ns: &NodeServer{
 				kernelMountOptions: cliKernelMountOptions,
 			},
-			mnt: mounter.VolumeMounter(&mounter.KernelMounter{}),
+			mnt: mounter.VolumeMounter(mounter.NewKernelMounter()),
 			volOptions: &store.VolumeOptions{
 				ClusterID: "cluster-1",
 			},
@@ -119,7 +119,7 @@ func Test_setMountOptions(t *testing.T) {
 			ns: &NodeServer{
 				kernelMountOptions: cliKernelMountOptions,
 			},
-			mnt: mounter.VolumeMounter(&mounter.KernelMounter{}),
+			mnt: mounter.VolumeMounter(mounter.NewKernelMounter()),
 			volOptions: &store.VolumeOptions{
 				ClusterID: "cluster-2",
 			},
@@ -162,7 +162,7 @@ func Test_setMountOptions(t *testing.T) {
 				if !strings.Contains(tc.volOptions.FuseMountOptions, tc.want) {
 					t.Errorf("Set FuseMountOptions = %v Required FuseMountOptions = %v", tc.volOptions.FuseMountOptions, tc.want)
 				}
-			case *mounter.KernelMounter:
+			case mounter.KernelMounter:
 				if !strings.Contains(tc.volOptions.KernelMountOptions, tc.want) {
 					t.Errorf("Set KernelMountOptions = %v Required KernelMountOptions = %v", tc.volOptions.KernelMountOptions, tc.want)
 				}