From e290cee720582dd983e89dc6c53c8774eaa4ffa7 Mon Sep 17 00:00:00 2001 From: Steve Leon Date: Mon, 9 Oct 2017 16:25:49 -0700 Subject: [PATCH] Executing modprobe in agent pod Fixes https://github.com/rook/rook/issues/1062 --- cmd/rookflex/cmd/init.go | 23 -------- pkg/agent/flexvolume/controller_test.go | 4 ++ pkg/agent/flexvolume/manager/ceph/manager.go | 29 +++++++++- .../flexvolume/manager/ceph/manager_test.go | 58 +++++++++++++++++++ pkg/agent/flexvolume/types.go | 1 + pkg/operator/agent/agent.go | 24 ++++++++ pkg/operator/agent/agent_test.go | 4 +- tests/scripts/kubeadm-install.sh | 1 - tests/scripts/kubeadm.sh | 3 - tests/scripts/minikube.sh | 1 - 10 files changed, 117 insertions(+), 31 deletions(-) diff --git a/cmd/rookflex/cmd/init.go b/cmd/rookflex/cmd/init.go index 72224894e..81fcf4431 100644 --- a/cmd/rookflex/cmd/init.go +++ b/cmd/rookflex/cmd/init.go @@ -19,16 +19,10 @@ package cmd import ( "fmt" "os" - "os/exec" - "github.com/rook/rook/pkg/util/sys" "github.com/spf13/cobra" ) -const ( - moduleName = "rbd" -) - var ( initCmd = &cobra.Command{ Use: "init", @@ -42,23 +36,6 @@ func init() { } func initPlugin(cmd *cobra.Command, args []string) error { - - hasSingleMajor := false - // check to see if the rbd kernel module has single_major support - out, err := exec.Command("modinfo", "-F", "parm", moduleName).Output() - if err == nil { - hasSingleMajor = sys.Grep(string(out), "^single_major") != "" - } - - opts := []string{moduleName} - if hasSingleMajor { - opts = append(opts, "single_major=Y") - } - - // load the rbd kernel module with options - // TODO: should this fail if modprobe fails? - exec.Command("modprobe", opts...).Run() - fmt.Println(`{"status":"Success", "capabilities": {"attach": false}}`) os.Exit(0) return nil diff --git a/pkg/agent/flexvolume/controller_test.go b/pkg/agent/flexvolume/controller_test.go index b4f3dc8e8..2889b5249 100644 --- a/pkg/agent/flexvolume/controller_test.go +++ b/pkg/agent/flexvolume/controller_test.go @@ -681,6 +681,10 @@ func TestGetCRDNameFromMountDirInvalid(t *testing.T) { type FakeVolumeManager struct{} +func (f *FakeVolumeManager) Init() error { + return nil +} + func (f *FakeVolumeManager) Attach(image, pool, clusterName string) (string, error) { return fmt.Sprintf("/%s/%s/%s", image, pool, clusterName), nil } diff --git a/pkg/agent/flexvolume/manager/ceph/manager.go b/pkg/agent/flexvolume/manager/ceph/manager.go index 621aa470d..4e4b76fdd 100644 --- a/pkg/agent/flexvolume/manager/ceph/manager.go +++ b/pkg/agent/flexvolume/manager/ceph/manager.go @@ -37,6 +37,7 @@ import ( const ( findDevicePathMaxRetries = 10 + rbdKernelModuleName = "rbd" ) var logger = capnslog.NewPackageLogger("github.com/rook/rook", "ceph-volumeattacher") @@ -51,10 +52,36 @@ type devicePathFinder struct{} // NewVolumeManager create attacher for ceph volumes func NewVolumeManager(context *clusterd.Context) *VolumeManager { - return &VolumeManager{ + vm := &VolumeManager{ context: context, devicePathFinder: &devicePathFinder{}, } + vm.Init() + return vm +} + +// Init the ceph volume manager +func (vm *VolumeManager) Init() error { + + // check to see if the rbd kernel module has single_major support + hasSingleMajor, err := sys.CheckKernelModuleParam(rbdKernelModuleName, "single_major", vm.context.Executor) + if err != nil { + logger.Noticef("failed %s single_major check, assuming it's unsupported: %+v", rbdKernelModuleName, err) + hasSingleMajor = false + } + + opts := []string{} + if hasSingleMajor { + opts = append(opts, "single_major=Y") + } + + // load the rbd kernel module with options + // TODO: should this fail if modprobe fails? + if err := sys.LoadKernelModule(rbdKernelModuleName, opts, vm.context.Executor); err != nil { + logger.Noticef("failed to load kernel module %s: %+v", rbdKernelModuleName, err) + } + + return nil } // Attach a ceph image to the node diff --git a/pkg/agent/flexvolume/manager/ceph/manager_test.go b/pkg/agent/flexvolume/manager/ceph/manager_test.go index 14960dbe4..2e50e2718 100644 --- a/pkg/agent/flexvolume/manager/ceph/manager_test.go +++ b/pkg/agent/flexvolume/manager/ceph/manager_test.go @@ -45,6 +45,64 @@ func (f *fakeDevicePathFinder) FindDevicePath(image, pool, clusterName string) ( return response, nil } +func TestInitLoadRBDModSingleMajor(t *testing.T) { + + modInfoCalled := false + modprobeCalled := false + + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + assert.Equal(t, "modinfo", command) + assert.Equal(t, "rbd", args[2]) + modInfoCalled = true + return "single_major:Use a single major number for all rbd devices (default: false) (bool)", nil + }, + MockExecuteCommand: func(debug bool, actionName string, command string, args ...string) error { + assert.Equal(t, "modprobe", command) + assert.Equal(t, "rbd", args[0]) + assert.Equal(t, "single_major=Y", args[1]) + modprobeCalled = true + return nil + }, + } + + context := &clusterd.Context{ + Executor: executor, + } + NewVolumeManager(context) + assert.True(t, modInfoCalled) + assert.True(t, modprobeCalled) +} + +func TestInitLoadRBDModNoSingleMajor(t *testing.T) { + + modInfoCalled := false + modprobeCalled := false + + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(debug bool, actionName string, command string, args ...string) (string, error) { + assert.Equal(t, "modinfo", command) + assert.Equal(t, "rbd", args[2]) + modInfoCalled = true + return "", nil + }, + MockExecuteCommand: func(debug bool, actionName string, command string, args ...string) error { + assert.Equal(t, "modprobe", command) + assert.Equal(t, 1, len(args)) + assert.Equal(t, "rbd", args[0]) + modprobeCalled = true + return nil + }, + } + + context := &clusterd.Context{ + Executor: executor, + } + NewVolumeManager(context) + assert.True(t, modInfoCalled) + assert.True(t, modprobeCalled) +} + func TestAttach(t *testing.T) { clientset := test.New(3) clusterName := "testCluster" diff --git a/pkg/agent/flexvolume/types.go b/pkg/agent/flexvolume/types.go index 3f76379b3..0c8f34e84 100644 --- a/pkg/agent/flexvolume/types.go +++ b/pkg/agent/flexvolume/types.go @@ -26,6 +26,7 @@ const ( // VolumeManager handles flexvolume plugin storage operations type VolumeManager interface { + Init() error Attach(image, pool, clusterName string) (string, error) Detach(image, pool, clusterName string) error } diff --git a/pkg/operator/agent/agent.go b/pkg/operator/agent/agent.go index 3b0799d66..27e8c9714 100644 --- a/pkg/operator/agent/agent.go +++ b/pkg/operator/agent/agent.go @@ -106,6 +106,14 @@ func (a *Agent) createAgentDaemonSet(namespace string) error { Name: "libmodules", MountPath: "/lib/modules", }, + { + Name: "modprobe", + MountPath: "/sbin/modprobe", + }, + { + Name: "modinfo", + MountPath: "/sbin/modinfo", + }, }, Env: []v1.EnvVar{ k8sutil.NamespaceEnvVar(), @@ -146,6 +154,22 @@ func (a *Agent) createAgentDaemonSet(namespace string) error { }, }, }, + { + Name: "modprobe", + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/sbin/modprobe", + }, + }, + }, + { + Name: "modinfo", + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/sbin/modinfo", + }, + }, + }, }, HostNetwork: true, Tolerations: []v1.Toleration{ diff --git a/pkg/operator/agent/agent_test.go b/pkg/operator/agent/agent_test.go index 5b80d61f1..0826270b8 100644 --- a/pkg/operator/agent/agent_test.go +++ b/pkg/operator/agent/agent_test.go @@ -70,9 +70,9 @@ func TestStartAgentDaemonset(t *testing.T) { assert.Equal(t, "rook-agent", agentDS.Name) assert.True(t, *agentDS.Spec.Template.Spec.Containers[0].SecurityContext.Privileged) volumes := agentDS.Spec.Template.Spec.Volumes - assert.Equal(t, 4, len(volumes)) + assert.Equal(t, 6, len(volumes)) volumeMounts := agentDS.Spec.Template.Spec.Containers[0].VolumeMounts - assert.Equal(t, 4, len(volumeMounts)) + assert.Equal(t, 6, len(volumeMounts)) envs := agentDS.Spec.Template.Spec.Containers[0].Env assert.Equal(t, 2, len(envs)) image := agentDS.Spec.Template.Spec.Containers[0].Image diff --git a/tests/scripts/kubeadm-install.sh b/tests/scripts/kubeadm-install.sh index 0d9400877..b983f9878 100755 --- a/tests/scripts/kubeadm-install.sh +++ b/tests/scripts/kubeadm-install.sh @@ -15,7 +15,6 @@ then sudo mkdir -p /usr/libexec/kubernetes/kubelet-plugins/volume/exec/rook.io~rook cat << EOF | sudo tee -a /usr/libexec/kubernetes/kubelet-plugins/volume/exec/rook.io~rook/rook #!/bin/bash -modprobe rbd single_major=Y > /dev/null echo -ne '{"status": "Success", "capabilities": {"attach": false}}' >&1 exit 0 EOF diff --git a/tests/scripts/kubeadm.sh b/tests/scripts/kubeadm.sh index 3954eed7f..c196ae690 100755 --- a/tests/scripts/kubeadm.sh +++ b/tests/scripts/kubeadm.sh @@ -141,9 +141,6 @@ case "${1:-}" in exit 1 ;; esac - sudo cp ${scriptdir}/kubeadm-rbd /bin/rbd - sudo chmod +x /bin/rbd - docker pull ceph/base || true ;; wait) if [ "$#" -eq 2 ]; then diff --git a/tests/scripts/minikube.sh b/tests/scripts/minikube.sh index 549357427..bf1e13b04 100755 --- a/tests/scripts/minikube.sh +++ b/tests/scripts/minikube.sh @@ -6,7 +6,6 @@ source "${scriptdir}/../../build/common.sh" init_flexvolume() { cat < ~/rook' #!/bin/bash -modprobe rbd single_major=Y > /dev/null echo -ne '{"status": "Success", "capabilities": {"attach": false}}' >&1 exit 0 EOF