From 20f6af8a4d578380f1888bb14ac9a956722c2efa Mon Sep 17 00:00:00 2001 From: travisn Date: Tue, 30 Apr 2019 11:44:53 -0600 Subject: [PATCH 1/2] remove obsolete flex driver version checks The flex driver no longer needs to check whether a kubelet restart is needed or whether the namespace-specific driver can be supported. The copy of k8s 1.13 types can also be removed since we have updated to a newer k8s client. Signed-off-by: travisn --- cmd/rookflex/cmd/init.go | 36 ++----------------- pkg/daemon/ceph/agent/flexvolume/server.go | 34 ++---------------- .../ceph/provisioner/provisioner_test.go | 6 ++-- 3 files changed, 8 insertions(+), 68 deletions(-) diff --git a/cmd/rookflex/cmd/init.go b/cmd/rookflex/cmd/init.go index b7a8477a9..47312abc4 100644 --- a/cmd/rookflex/cmd/init.go +++ b/cmd/rookflex/cmd/init.go @@ -51,9 +51,9 @@ func initPlugin(cmd *cobra.Command, args []string) error { rookEnableFSGroup = true } - status := DriverStatus{ + status := flexvolume.DriverStatus{ Status: flexvolume.StatusSuccess, - Capabilities: &DriverCapabilities{ + Capabilities: &flexvolume.DriverCapabilities{ Attach: false, // Required for any mount performed on a host running selinux SELinuxRelabel: rookEnableSelinuxRelabeling, @@ -66,35 +66,3 @@ func initPlugin(cmd *cobra.Command, args []string) error { os.Exit(0) return nil } - -// FIX: After we move to the k8s 1.13 dependencies, we can remove this type -// and reference the type in k8s.io/kubernetes/pkg/volume/flexvolume/driver-call.go -type DriverStatus struct { - // Status of the callout. One of "Success", "Failure" or "Not supported". - Status string `json:"status"` - // Reason for success/failure. - Message string `json:"message,omitempty"` - // Path to the device attached. This field is valid only for attach calls. - // ie: /dev/sdx - DevicePath string `json:"device,omitempty"` - // Cluster wide unique name of the volume. - VolumeName string `json:"volumeName,omitempty"` - // Represents volume is attached on the node - Attached bool `json:"attached,omitempty"` - // Returns capabilities of the driver. - // By default we assume all the capabilities are supported. - // If the plugin does not support a capability, it can return false for that capability. - Capabilities *DriverCapabilities `json:",omitempty"` - // Returns the actual size of the volume after resizing is done, the size is in bytes. - ActualVolumeSize int64 `json:"volumeNewSize,omitempty"` -} - -// FIX: After we move to the k8s 1.13 dependencies, we can remove this type -// and reference the type in k8s.io/kubernetes/pkg/volume/flexvolume/driver-call.go -type DriverCapabilities struct { - Attach bool `json:"attach"` - SELinuxRelabel bool `json:"selinuxRelabel"` - SupportsMetrics bool `json:"supportsMetrics"` - FSGroup bool `json:"fsGroup"` - RequiresFSResize bool `json:"requiresFSResize"` -} diff --git a/pkg/daemon/ceph/agent/flexvolume/server.go b/pkg/daemon/ceph/agent/flexvolume/server.go index 69edfff35..132a5b8ed 100644 --- a/pkg/daemon/ceph/agent/flexvolume/server.go +++ b/pkg/daemon/ceph/agent/flexvolume/server.go @@ -28,8 +28,6 @@ import ( "strings" "time" - "k8s.io/apimachinery/pkg/util/version" - "github.com/coreos/pkg/capnslog" "github.com/rook/rook/pkg/clusterd" "github.com/rook/rook/pkg/operator/k8sutil" @@ -43,7 +41,6 @@ const ( flexvolumeDriverFileName = "rookflex" flexMountPath = "/flexmnt/%s~%s" usrBinDir = "/usr/local/bin/" - serverVersionV180 = "v1.8.0" ) var logger = capnslog.NewPackageLogger("github.com/rook/rook", "flexvolume") @@ -102,12 +99,6 @@ func (s *FlexvolumeServer) Start(driverVendor, driverName string) error { go rpc.Accept(listener) logger.Infof("Listening on unix socket for Kubernetes volume attach commands: %s", unixSocketFile) - - // flexvolume driver was installed OK. If running on pre 1.8 Kubernetes, then remind the user - // to restart the Kubelet. We do this last so that it's the last message in the log, making it - // harder for the user to miss. - checkIfKubeletRestartRequired(s.context) - return nil } @@ -130,19 +121,9 @@ func (s *FlexvolumeServer) StopAll() { // RookDriverName return the Kubernetes version appropriate Rook driver name func RookDriverName(context *clusterd.Context) (string, error) { - kubeVersion, err := k8sutil.GetK8SVersion(context.Clientset) - if err != nil { - return "", fmt.Errorf("Error getting server version: %v", err) - } - // K8s 1.7 returns an error when trying to run multiple drivers under the same rook.io provider, - // so we will fall back to the rook driver name in that case. - if kubeVersion.AtLeast(version.MustParseSemantic(serverVersionV180)) { - // the driver name needs to be the same as the namespace so that we can support multiple namespaces - // without the drivers conflicting with each other - return os.Getenv(k8sutil.PodNamespaceEnvVar), nil - } - // fall back to the rook driver name where multiple system namespaces are not supported - return FlexDriverName, nil + // the driver name needs to be the same as the namespace so that we can support multiple namespaces + // without the drivers conflicting with each other + return os.Getenv(k8sutil.PodNamespaceEnvVar), nil } // TouchFlexDrivers causes k8s to reload the flex volumes. Needed periodically due to a k8s race condition with flex driver loading. @@ -210,15 +191,6 @@ func copyFile(src, dest string) error { return destFile.Sync() } -func checkIfKubeletRestartRequired(context *clusterd.Context) { - kubeVersion, err := k8sutil.GetK8SVersion(context.Clientset) - if err != nil || kubeVersion.LessThan(version.MustParseSemantic(serverVersionV180)) { - logger.Warning("NOTE: The Kubelet must be restarted on this node since this pod appears to " + - "be running on a Kubernetes version prior to 1.8. More details can be found in the Rook docs at " + - "https://rook.io/docs/rook/master/common-issues.html#kubelet-restart") - } -} - // Gets the flex driver info (vendor, driver name) from a given path where the flex driver exists. // The given path may look something like this: // /usr/libexec/kubernetes/kubelet-plugins/volume/exec/rook.io~rook/rook/ diff --git a/pkg/operator/ceph/provisioner/provisioner_test.go b/pkg/operator/ceph/provisioner/provisioner_test.go index 4a7b5f02f..314975fa8 100644 --- a/pkg/operator/ceph/provisioner/provisioner_test.go +++ b/pkg/operator/ceph/provisioner/provisioner_test.go @@ -41,7 +41,7 @@ func TestProvisionImage(t *testing.T) { clientset := test.New(3) namespace := "ns" configDir, _ := ioutil.TempDir("", "") - os.Setenv("POD_NAMESPACE", "rook-system") + os.Setenv("POD_NAMESPACE", "rook-ceph") defer os.Setenv("POD_NAMESPACE", "") defer os.RemoveAll(configDir) executor := &exectest.MockExecutor{ @@ -78,7 +78,7 @@ func TestProvisionImage(t *testing.T) { assert.Equal(t, "pvc-uid-1-1", pv.Name) assert.NotNil(t, pv.Spec.PersistentVolumeSource.FlexVolume) assert.Equal(t, v1.PersistentVolumeReclaimRetain, pv.Spec.PersistentVolumeReclaimPolicy) - assert.Equal(t, "foo.io/rook", pv.Spec.PersistentVolumeSource.FlexVolume.Driver) + assert.Equal(t, "foo.io/rook-ceph", pv.Spec.PersistentVolumeSource.FlexVolume.Driver) assert.Equal(t, "ext3", pv.Spec.PersistentVolumeSource.FlexVolume.FSType) assert.Equal(t, "testCluster", pv.Spec.PersistentVolumeSource.FlexVolume.Options["clusterNamespace"]) assert.Equal(t, "class-1", pv.Spec.PersistentVolumeSource.FlexVolume.Options["storageClass"]) @@ -94,7 +94,7 @@ func TestProvisionImage(t *testing.T) { assert.Equal(t, "pvc-uid-1-1", pv.Name) assert.NotNil(t, pv.Spec.PersistentVolumeSource.FlexVolume) assert.Equal(t, v1.PersistentVolumeReclaimRecycle, pv.Spec.PersistentVolumeReclaimPolicy) - assert.Equal(t, "foo.io/rook", pv.Spec.PersistentVolumeSource.FlexVolume.Driver) + assert.Equal(t, "foo.io/rook-ceph", pv.Spec.PersistentVolumeSource.FlexVolume.Driver) assert.Equal(t, "ext3", pv.Spec.PersistentVolumeSource.FlexVolume.FSType) assert.Equal(t, "testCluster", pv.Spec.PersistentVolumeSource.FlexVolume.Options["clusterNamespace"]) assert.Equal(t, "class-1", pv.Spec.PersistentVolumeSource.FlexVolume.Options["storageClass"]) From 7cdfcc395a82c81c30b25907bd9f81c6cfdac419 Mon Sep 17 00:00:00 2001 From: travisn Date: Tue, 30 Apr 2019 13:01:36 -0600 Subject: [PATCH 2/2] write flex settings to config file instead of env vars The flex settings cannot be passed to the driver with environment variables. There is no context available for returning the flex settings except that the flex driver will look in a config file in the same directory. These settings must be valid or else the driver will return the default settings. Signed-off-by: travisn --- PendingReleaseNotes.md | 1 + cmd/rookflex/cmd/init.go | 30 ++------ pkg/daemon/ceph/agent/flexvolume/server.go | 71 +++++++++++++++++++ .../ceph/agent/flexvolume/server_test.go | 15 ++++ 4 files changed, 93 insertions(+), 24 deletions(-) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 91174c586..f82942775 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -36,6 +36,7 @@ Jobs, and StatefulSets. These identify the Rook version which last modified the resource and the Ceph version which Rook has detected in the pod(s) being run by the resource. - OSDs provisioned by `ceph-volume` now supports `metadataDevice` and `databaseSizeMB` options. +- The flex driver can be configured to properly disable SELinux relabeling and FSGroup with the settings in operator.yaml. ## Breaking Changes diff --git a/cmd/rookflex/cmd/init.go b/cmd/rookflex/cmd/init.go index 47312abc4..91bd56e1b 100644 --- a/cmd/rookflex/cmd/init.go +++ b/cmd/rookflex/cmd/init.go @@ -17,13 +17,11 @@ limitations under the License. package cmd import ( - "encoding/json" "os" - "strconv" + "path/filepath" - "github.com/rook/rook/pkg/operator/ceph/agent" + "github.com/rook/rook/pkg/daemon/ceph/agent/flexvolume" "github.com/spf13/cobra" - "k8s.io/kubernetes/pkg/volume/flexvolume" ) var ( @@ -39,28 +37,12 @@ func init() { } func initPlugin(cmd *cobra.Command, args []string) error { - rookEnableSelinuxRelabeling, err := strconv.ParseBool(os.Getenv(agent.RookEnableSelinuxRelabelingEnv)) + executable, err := os.Executable() if err != nil { - // Don't log any errors to stdout as this will break the init. Just default the value to true. - rookEnableSelinuxRelabeling = true + return err } - - rookEnableFSGroup, err := strconv.ParseBool(os.Getenv(agent.RookEnableFSGroupEnv)) - if err != nil { - // Don't log any errors to stdout as this will break the init. Just default the value to true. - rookEnableFSGroup = true - } - - status := flexvolume.DriverStatus{ - Status: flexvolume.StatusSuccess, - Capabilities: &flexvolume.DriverCapabilities{ - Attach: false, - // Required for any mount performed on a host running selinux - SELinuxRelabel: rookEnableSelinuxRelabeling, - FSGroup: rookEnableFSGroup, - }, - } - if err := json.NewEncoder(os.Stdout).Encode(&status); err != nil { + settings := flexvolume.LoadFlexSettings(filepath.Dir(executable)) + if _, err := os.Stdout.WriteString(string(settings)); err != nil { return err } os.Exit(0) diff --git a/pkg/daemon/ceph/agent/flexvolume/server.go b/pkg/daemon/ceph/agent/flexvolume/server.go index 132a5b8ed..6c22d2ff5 100644 --- a/pkg/daemon/ceph/agent/flexvolume/server.go +++ b/pkg/daemon/ceph/agent/flexvolume/server.go @@ -18,19 +18,24 @@ limitations under the License. package flexvolume import ( + "encoding/json" "fmt" "io" + "io/ioutil" "net" "net/rpc" "os" "path" "regexp" + "strconv" "strings" "time" "github.com/coreos/pkg/capnslog" "github.com/rook/rook/pkg/clusterd" + "github.com/rook/rook/pkg/operator/ceph/agent" "github.com/rook/rook/pkg/operator/k8sutil" + "k8s.io/kubernetes/pkg/volume/flexvolume" ) const ( @@ -41,6 +46,7 @@ const ( flexvolumeDriverFileName = "rookflex" flexMountPath = "/flexmnt/%s~%s" usrBinDir = "/usr/local/bin/" + settingsFilename = "flex.config" ) var logger = capnslog.NewPackageLogger("github.com/rook/rook", "flexvolume") @@ -138,6 +144,49 @@ func TouchFlexDrivers(vendor, driverName string) { } } +// Encode the flex settings in json +func generateFlexSettings(enableSELinuxRelabeling, enableFSGroup bool) ([]byte, error) { + status := flexvolume.DriverStatus{ + Status: flexvolume.StatusSuccess, + Capabilities: &flexvolume.DriverCapabilities{ + Attach: false, + // Required for any mount performed on a host running selinux + SELinuxRelabel: enableSELinuxRelabeling, + FSGroup: enableFSGroup, + }, + } + result, err := json.Marshal(status) + if err != nil { + return nil, fmt.Errorf("Invalid flex settings. %+v", err) + } + return result, nil +} + +// The flex settings must be loaded from a file next to the flex driver since there is context +// that can be used other than the directory where the flex driver is running. +// This method cannot write to stdout since it is running in the context of the kubelet +// which only expects the json settings to be output. +func LoadFlexSettings(directory string) []byte { + // Load the settings from the expected config file, ensure they are valid settings, then return them in + // a json string to the caller + var status flexvolume.DriverStatus + if output, err := ioutil.ReadFile(path.Join(directory, settingsFilename)); err == nil { + if err := json.Unmarshal(output, &status); err == nil { + if output, err = json.Marshal(status); err == nil { + return output + } + } + } + + // If there is an error loading settings, set the defaults + settings, err := generateFlexSettings(true, true) + if err != nil { + // Never expect this to happen since we'll validate settings in the build + return nil + } + return settings +} + func configureFlexVolume(driverFile, driverDir, driverName string) error { // copying flex volume if _, err := os.Stat(driverDir); os.IsNotExist(err) { @@ -168,6 +217,28 @@ func configureFlexVolume(driverFile, driverDir, driverName string) error { return fmt.Errorf("failed to rename %s to %s: %+v", destFile, finalDestFile, err) } + // Write the flex configuration + enableSELinuxRelabeling, err := strconv.ParseBool(os.Getenv(agent.RookEnableSelinuxRelabelingEnv)) + if err != nil { + logger.Errorf("invalid value for disabling SELinux relabeling. %+v", err) + enableSELinuxRelabeling = true + } + enableFSGroup, err := strconv.ParseBool(os.Getenv(agent.RookEnableFSGroupEnv)) + if err != nil { + logger.Errorf("invalid value for disabling fs group. %+v", err) + enableFSGroup = true + } + settings, err := generateFlexSettings(enableSELinuxRelabeling, enableFSGroup) + if err != nil { + logger.Errorf("invalid flex settings. %+v", err) + } else { + if err := ioutil.WriteFile(path.Join(driverDir, settingsFilename), settings, 0644); err != nil { + logger.Errorf("failed to write settings file %s. %+v", settingsFilename, err) + } else { + logger.Debugf("flex settings: %s", string(settings)) + } + } + return nil } diff --git a/pkg/daemon/ceph/agent/flexvolume/server_test.go b/pkg/daemon/ceph/agent/flexvolume/server_test.go index 856308505..14baebaf3 100644 --- a/pkg/daemon/ceph/agent/flexvolume/server_test.go +++ b/pkg/daemon/ceph/agent/flexvolume/server_test.go @@ -18,12 +18,15 @@ limitations under the License. package flexvolume import ( + "encoding/json" "io/ioutil" "os" "path" "testing" + "github.com/rook/rook/pkg/operator/ceph/agent" "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/volume/flexvolume" ) func TestConfigureFlexVolume(t *testing.T) { @@ -35,10 +38,22 @@ func TestConfigureFlexVolume(t *testing.T) { driverName := "rook" os.Setenv("POD_NAMESPACE", driverName) defer os.Setenv("POD_NAMESPACE", "") + os.Setenv(agent.RookEnableSelinuxRelabelingEnv, "false") + defer os.Setenv(agent.RookEnableSelinuxRelabelingEnv, "") + os.Setenv(agent.RookEnableFSGroupEnv, "false") + defer os.Setenv(agent.RookEnableFSGroupEnv, "") err := configureFlexVolume(driverFile, driverDir, driverName) assert.Nil(t, err) _, err = os.Stat(path.Join(driverDir, "rook")) assert.False(t, os.IsNotExist(err)) + + // verify the non-default settings + settings := LoadFlexSettings(driverDir) + var status flexvolume.DriverStatus + err = json.Unmarshal(settings, &status) + assert.Nil(t, err) + assert.False(t, status.Capabilities.FSGroup) + assert.False(t, status.Capabilities.SELinuxRelabel) } func TestGetFlexDriverInfo(t *testing.T) {