From 76c33e0f851d6199cc786805c021e0a20ba5ebb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 5 Nov 2019 18:45:16 +0100 Subject: [PATCH 1/2] Revert "ceph: osd hide 'restorecon' when necessary" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit d224fbdef1a7711df8ab0ad6d49b298e7f9b92db. Since https://github.com/ceph/ceph/pull/31421, the monkey patching of 'restorecon' is not needed anymore. Signed-off-by: Sébastien Han --- pkg/daemon/ceph/osd/daemon.go | 5 --- pkg/daemon/ceph/osd/volume.go | 48 ++------------------------- pkg/daemon/ceph/osd/volume_test.go | 46 ------------------------- pkg/operator/ceph/cluster/mon/env.go | 11 +----- pkg/operator/ceph/cluster/osd/spec.go | 1 - 5 files changed, 3 insertions(+), 108 deletions(-) diff --git a/pkg/daemon/ceph/osd/daemon.go b/pkg/daemon/ceph/osd/daemon.go index 72b4482c0..f80223863 100644 --- a/pkg/daemon/ceph/osd/daemon.go +++ b/pkg/daemon/ceph/osd/daemon.go @@ -57,11 +57,6 @@ func StartOSD(context *clusterd.Context, osdType, osdID, osdUUID, lvPath string, return fmt.Errorf("failed to update lvm configuration file, %+v", err) // fail return here as validation provided by ceph-volume } - // Hide restorecon command, only when hostnetworking is enabled - if err := replaceRestoreconCommand(); err != nil { - return fmt.Errorf("failed to hide 'restorecon' command. %+v", err) - } - var volumeGroupName string if pvcBackedOSD { volumeGroupName, err = getVolumeGroupName(lvPath) diff --git a/pkg/daemon/ceph/osd/volume.go b/pkg/daemon/ceph/osd/volume.go index 97660b626..7b102a92d 100644 --- a/pkg/daemon/ceph/osd/volume.go +++ b/pkg/daemon/ceph/osd/volume.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "os" "path" "strconv" "strings" @@ -38,11 +37,8 @@ import ( // These are not constants because they are used by the tests var ( - cephConfigDir = "/var/lib/ceph" - lvmConfPath = "/etc/lvm/lvm.conf" - restoreconPath = "/usr/sbin/restorecon" - restoreconPathNewPath = restoreconPath + ".old" - redHatReleaseFile = "/etc/redhat-release" + cephConfigDir = "/var/lib/ceph" + lvmConfPath = "/etc/lvm/lvm.conf" ) const ( @@ -53,10 +49,6 @@ const ( dbDeviceFlag = "--db-devices" cephVolumeCmd = "ceph-volume" cephVolumeMinDBSize = 1024 // 1GB - restoreconNewContent = `#!/usr/bin/env bash -echo "restorecon command was replaced with a no-op." -echo "original restorecon command is now at %s" -` ) func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *DeviceOsdMapping) ([]oposd.OSDInfo, error) { @@ -81,10 +73,6 @@ func (a *OsdAgent) configureCVDevices(context *clusterd.Context, devices *Device if err := updateLVMConfig(context, a.pvcBacked); err != nil { return nil, fmt.Errorf("failed to update lvm configuration file, %+v", err) // fail return here as validation provided by ceph-volume } - // Hide restorecon command, only when hostnetworking is enabled - if err := replaceRestoreconCommand(); err != nil { - return nil, fmt.Errorf("failed to hide 'restorecon' command. %+v", err) - } if a.pvcBacked { if lv, err = a.initializeBlockPVC(context, devices); err != nil { return nil, fmt.Errorf("failed to initialize devices. %+v", err) @@ -178,38 +166,6 @@ func updateLVMConfig(context *clusterd.Context, onPVC bool) error { return nil } -func replaceRestoreconCommand() error { - // Check if Host Networking is enabled - hostnetworking := os.Getenv("ROOK_HOST_NETWORKING") - if hostnetworking == "false" { - logger.Debugf("ROOK_HOST_NETWORKING is %q, not replacing 'restorecon' command", hostnetworking) - return nil - } - - // Check whether we are running on RHEL - // The existence of /etc/redhat-release should be enough - _, err := os.Stat(redHatReleaseFile) - if os.IsNotExist(err) { - logger.Debugf("%q does not exist, not replacing 'restorecon' command, only doing this on red hat systems", redHatReleaseFile) - return nil - } - - logger.Debugf("renaming %q to %q", restoreconPath, restoreconPathNewPath) - err = os.Rename(restoreconPath, restoreconPathNewPath) - if err != nil { - return fmt.Errorf("failed to rename %q to %q. %+v", restoreconPath, restoreconPathNewPath, err) - } - - logger.Debugf("writing new content to %q", restoreconPath) - err = ioutil.WriteFile(restoreconPath, []byte(fmt.Sprintf(restoreconNewContent, restoreconPathNewPath)), 0755) - if err != nil { - return fmt.Errorf("failed to write new content of restorecon to %q. %+v", restoreconPath, err) - } - - logger.Infof("Successfully replaced restorecon command to %q", restoreconPathNewPath) - return nil -} - func (a *OsdAgent) initializeDevices(context *clusterd.Context, devices *DeviceOsdMapping) error { storeFlag := "--bluestore" if a.storeConfig.StoreType == config.Filestore { diff --git a/pkg/daemon/ceph/osd/volume_test.go b/pkg/daemon/ceph/osd/volume_test.go index f47f15d03..e9c8b3412 100644 --- a/pkg/daemon/ceph/osd/volume_test.go +++ b/pkg/daemon/ceph/osd/volume_test.go @@ -18,9 +18,6 @@ package osd import ( "fmt" - "io/ioutil" - "os" - "strings" "testing" "github.com/rook/rook/pkg/clusterd" @@ -314,46 +311,3 @@ func TestGetDatabaseSize(t *testing.T) { assert.Equal(t, 0, getDatabaseSize(0, 0)) assert.Equal(t, 2048, getDatabaseSize(4096, 2048)) } - -func TestHideRestoreconCommand(t *testing.T) { - os.Setenv("ROOK_HOST_NETWORKING", "false") - defer os.Setenv("ROOK_HOST_NETWORKING", "") - - // Should not run if ROOK_HOST_NETWORKING is false - err := replaceRestoreconCommand() - assert.NoError(t, err) - - // Should not run if /etc/redhat-release does not exist - os.Setenv("ROOK_HOST_NETWORKING", "true") - err = replaceRestoreconCommand() - assert.NoError(t, err) - - // Should run now - // Fake redhat-release file - f, err := ioutil.TempFile("", "redhat-release") - assert.NoError(t, err) - defer f.Close() - defer os.Remove(f.Name()) - redHatReleaseFile = f.Name() - assert.FileExists(t, redHatReleaseFile) - - // Fake restorecon command - ff, err := ioutil.TempFile("", "restorecon") - defer ff.Close() - defer os.Remove(ff.Name()) - assert.NoError(t, err) - restoreconPath = ff.Name() - assert.FileExists(t, restoreconPath) - - restoreconPathNewPath = restoreconPath + ".old" - defer os.Remove(restoreconPathNewPath) - err = replaceRestoreconCommand() - assert.NoError(t, err) - assert.FileExists(t, restoreconPathNewPath, "restoreconPath is %q and restoreconPathNewPath is %q", restoreconPath, restoreconPathNewPath) - - r, err := ioutil.ReadFile(restoreconPath) - assert.NoError(t, err) - - b := strings.Contains(string(r), "restorecon command was replaced with a no-op") - assert.True(t, b, restoreconPath) -} diff --git a/pkg/operator/ceph/cluster/mon/env.go b/pkg/operator/ceph/cluster/mon/env.go index 8ca05f8e7..4ca440cec 100644 --- a/pkg/operator/ceph/cluster/mon/env.go +++ b/pkg/operator/ceph/cluster/mon/env.go @@ -16,11 +16,7 @@ limitations under the License. package mon -import ( - "strconv" - - v1 "k8s.io/api/core/v1" -) +import v1 "k8s.io/api/core/v1" // ClusterNameEnvVar is the cluster name environment var func ClusterNameEnvVar(name string) v1.EnvVar { @@ -44,8 +40,3 @@ func AdminSecretEnvVar() v1.EnvVar { ref := &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: AppName}, Key: adminSecretName} return v1.EnvVar{Name: "ROOK_ADMIN_SECRET", ValueFrom: &v1.EnvVarSource{SecretKeyRef: ref}} } - -// ClusterHostNetworking is the value of the hostnetworking spec -func ClusterHostNetworking(name bool) v1.EnvVar { - return v1.EnvVar{Name: "ROOK_HOST_NETWORKING", Value: strconv.FormatBool(name)} -} diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index 5d8277fff..f93d54016 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -555,7 +555,6 @@ func (c *Cluster) getConfigEnvVars(storeConfig config.StoreConfig, dataDir, node opmon.EndpointEnvVar(), opmon.SecretEnvVar(), opmon.AdminSecretEnvVar(), - opmon.ClusterHostNetworking(c.Network.IsHost()), k8sutil.ConfigDirEnvVar(dataDir), k8sutil.ConfigOverrideEnvVar(), {Name: "ROOK_FSID", ValueFrom: &v1.EnvVarSource{ From a0fdf1bb5f42228c387b300d8e9fa421381cf5c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Tue, 5 Nov 2019 18:39:59 +0100 Subject: [PATCH 2/2] ceph: add CEPH_VOLUME_SKIP_RESTORECON env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will tell ceph-volume not to run any restorecon commands. This relies on https://github.com/ceph/ceph/pull/31421 Signed-off-by: Sébastien Han --- pkg/operator/ceph/cluster/osd/spec.go | 17 +++++++++++------ pkg/operator/ceph/cluster/osd/spec_test.go | 8 ++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/operator/ceph/cluster/osd/spec.go b/pkg/operator/ceph/cluster/osd/spec.go index f93d54016..ed0b3978a 100644 --- a/pkg/operator/ceph/cluster/osd/spec.go +++ b/pkg/operator/ceph/cluster/osd/spec.go @@ -260,11 +260,6 @@ func (c *Cluster) makeDeployment(osdProps osdProperties, osd OSDInfo) (*apps.Dep Name: "run-udev", MountPath: "/run/udev"}) - // Activate verbose mode for ceph-volume on activate - envVars = append(envVars, []v1.EnvVar{ - {Name: "CEPH_VOLUME_DEBUG", Value: "1"}, - }...) - } else if osd.IsDirectory { // config for dir-based osds is gotten from the commandline or from the mon database doConfigInit = false @@ -563,9 +558,12 @@ func (c *Cluster) getConfigEnvVars(storeConfig config.StoreConfig, dataDir, node Key: "fsid", }, }}, - {Name: "CEPH_VOLUME_DEBUG", Value: "1"}, k8sutil.NodeEnvVar(), } + + // Append ceph-volume environment variables + envVars = append(envVars, cephVolumeEnvVar()...) + if storeConfig.StoreType != "" { envVars = append(envVars, v1.EnvVar{Name: osdStoreEnvVarName, Value: storeConfig.StoreType}) } @@ -868,3 +866,10 @@ func (c *Cluster) osdPrepareResources(osdClaimName string) v1.ResourceRequiremen }, } } + +func cephVolumeEnvVar() []v1.EnvVar { + return []v1.EnvVar{ + {Name: "CEPH_VOLUME_DEBUG", Value: "1"}, + {Name: "CEPH_VOLUME_SKIP_RESTORECON", Value: "1"}, + } +} diff --git a/pkg/operator/ceph/cluster/osd/spec_test.go b/pkg/operator/ceph/cluster/osd/spec_test.go index 8ec17afe5..d43e1d67e 100644 --- a/pkg/operator/ceph/cluster/osd/spec_test.go +++ b/pkg/operator/ceph/cluster/osd/spec_test.go @@ -365,3 +365,11 @@ func TestOsdPrepareResources(t *testing.T) { assert.Equal(t, "0", r.Limits.Memory().String()) assert.Equal(t, "250", r.Requests.Memory().String()) } + +func TestCephVolumeEnvVar(t *testing.T) { + cvEnv := cephVolumeEnvVar() + assert.Equal(t, "CEPH_VOLUME_DEBUG", cvEnv[0].Name) + assert.Equal(t, "1", cvEnv[0].Value) + assert.Equal(t, "CEPH_VOLUME_SKIP_RESTORECON", cvEnv[1].Name) + assert.Equal(t, "1", cvEnv[1].Value) +}