From dbaf41e92a083d5c3aaacc3c47898a5a823b1d2c Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 Aug 2017 13:25:09 +0200 Subject: [PATCH 1/2] ScaleIO: use a fresh mounter for every SetUp/TearDown A volume plugin should not cache Mounter for a long time, it can get a different one with each SetUp/TearDown call. --- pkg/volume/scaleio/sio_plugin.go | 3 --- pkg/volume/scaleio/sio_volume.go | 10 ++++++---- pkg/volume/scaleio/sio_volume_test.go | 3 --- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/volume/scaleio/sio_plugin.go b/pkg/volume/scaleio/sio_plugin.go index 69035dd3ba9..d7758756216 100644 --- a/pkg/volume/scaleio/sio_plugin.go +++ b/pkg/volume/scaleio/sio_plugin.go @@ -23,7 +23,6 @@ import ( api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/util/keymutex" - "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" ) @@ -35,7 +34,6 @@ const ( type sioPlugin struct { host volume.VolumeHost - mounter mount.Interface volumeMtx keymutex.KeyMutex } @@ -53,7 +51,6 @@ var _ volume.VolumePlugin = &sioPlugin{} func (p *sioPlugin) Init(host volume.VolumeHost) error { p.host = host - p.mounter = host.GetMounter(p.GetPluginName()) p.volumeMtx = keymutex.NewKeyMutex() return nil } diff --git a/pkg/volume/scaleio/sio_volume.go b/pkg/volume/scaleio/sio_volume.go index 654d8fd58a6..f72b5a1adbb 100644 --- a/pkg/volume/scaleio/sio_volume.go +++ b/pkg/volume/scaleio/sio_volume.go @@ -93,7 +93,8 @@ func (v *sioVolume) SetUpAt(dir string, fsGroup *int64) error { return err } - notDevMnt, err := v.plugin.mounter.IsLikelyNotMountPoint(dir) + mounter := v.plugin.host.GetMounter(v.plugin.GetPluginName()) + notDevMnt, err := mounter.IsLikelyNotMountPoint(dir) if err != nil && !os.IsNotExist(err) { glog.Error(log("IsLikelyNotMountPoint test failed for dir %v", dir)) return err @@ -186,21 +187,22 @@ func (v *sioVolume) TearDownAt(dir string) error { v.plugin.volumeMtx.LockKey(v.volSpecName) defer v.plugin.volumeMtx.UnlockKey(v.volSpecName) - dev, _, err := mount.GetDeviceNameFromMount(v.plugin.mounter, dir) + mounter := v.plugin.host.GetMounter(v.plugin.GetPluginName()) + dev, _, err := mount.GetDeviceNameFromMount(mounter, dir) if err != nil { glog.Errorf(log("failed to get reference count for volume: %s", dir)) return err } glog.V(4).Info(log("attempting to unmount %s", dir)) - if err := util.UnmountPath(dir, v.plugin.mounter); err != nil { + if err := util.UnmountPath(dir, mounter); err != nil { glog.Error(log("teardown failed while unmounting dir %s: %v ", dir, err)) return err } glog.V(4).Info(log("dir %s unmounted successfully", dir)) // detach/unmap - deviceBusy, err := v.plugin.mounter.DeviceOpened(dev) + deviceBusy, err := mounter.DeviceOpened(dev) if err != nil { glog.Error(log("teardown unable to get status for device %s: %v", dev, err)) return err diff --git a/pkg/volume/scaleio/sio_volume_test.go b/pkg/volume/scaleio/sio_volume_test.go index 9d495d3e675..36e747c31fd 100644 --- a/pkg/volume/scaleio/sio_volume_test.go +++ b/pkg/volume/scaleio/sio_volume_test.go @@ -30,7 +30,6 @@ import ( "k8s.io/apimachinery/pkg/types" fakeclient "k8s.io/client-go/kubernetes/fake" utiltesting "k8s.io/client-go/util/testing" - "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" ) @@ -138,8 +137,6 @@ func TestVolumeMounterUnmounter(t *testing.T) { t.Errorf("Cannot assert plugin to be type sioPlugin") } - sioPlug.mounter = &mount.FakeMounter{} - vol := &api.Volume{ Name: testSioVolName, VolumeSource: api.VolumeSource{ From 158017cef71cec724477c8f512a94ce39c84db17 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 22 Aug 2017 13:27:59 +0200 Subject: [PATCH 2/2] ScaleIO: Use VolumeHost.GetExec() to execute utilities This prepares volume plugins to run things in containers instead of running them on the host. As consequence, a mount.Exec interface needs to be passed from VolumeHost down to SioClient. --- pkg/volume/scaleio/sio_client.go | 11 +++++++---- pkg/volume/scaleio/sio_mgr.go | 9 ++++++--- pkg/volume/scaleio/sio_mgr_test.go | 6 ++++-- pkg/volume/scaleio/sio_volume.go | 8 ++++---- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/pkg/volume/scaleio/sio_client.go b/pkg/volume/scaleio/sio_client.go index 8742905ed09..ac26868d324 100644 --- a/pkg/volume/scaleio/sio_client.go +++ b/pkg/volume/scaleio/sio_client.go @@ -21,7 +21,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path" "path/filepath" "regexp" @@ -30,6 +29,8 @@ import ( "sync" "time" + "k8s.io/kubernetes/pkg/util/mount" + sio "github.com/codedellemc/goscaleio" siotypes "github.com/codedellemc/goscaleio/types/v1" "github.com/golang/glog" @@ -77,13 +78,15 @@ type sioClient struct { inited bool diskRegex *regexp.Regexp mtx sync.Mutex + exec mount.Exec } -func newSioClient(gateway, username, password string, sslEnabled bool) (*sioClient, error) { +func newSioClient(gateway, username, password string, sslEnabled bool, exec mount.Exec) (*sioClient, error) { client := new(sioClient) client.gateway = gateway client.username = username client.password = password + client.exec = exec if sslEnabled { client.insecure = false client.certsEnabled = true @@ -296,7 +299,7 @@ func (c *sioClient) IID() (string, error) { if c.instanceID == "" { cmd := c.getSdcCmd() - output, err := exec.Command(cmd, "--query_guid").Output() + output, err := c.exec.Run(cmd, "--query_guid") if err != nil { glog.Error(log("drv_cfg --query_guid failed: %v", err)) return "", err @@ -355,7 +358,7 @@ func (c *sioClient) Devs() (map[string]string, error) { volumeMap := make(map[string]string) // grab the sdc tool output - out, err := exec.Command(c.getSdcCmd(), "--query_vols").Output() + out, err := c.exec.Run(c.getSdcCmd(), "--query_vols") if err != nil { glog.Error(log("sdc --query_vols failed: %v", err)) return nil, err diff --git a/pkg/volume/scaleio/sio_mgr.go b/pkg/volume/scaleio/sio_mgr.go index 83d5e498dc8..ecde665a1ac 100644 --- a/pkg/volume/scaleio/sio_mgr.go +++ b/pkg/volume/scaleio/sio_mgr.go @@ -20,6 +20,8 @@ import ( "errors" "strconv" + "k8s.io/kubernetes/pkg/util/mount" + "github.com/golang/glog" siotypes "github.com/codedellemc/goscaleio/types/v1" @@ -36,9 +38,10 @@ type storageInterface interface { type sioMgr struct { client sioInterface configData map[string]string + exec mount.Exec } -func newSioMgr(configs map[string]string) (*sioMgr, error) { +func newSioMgr(configs map[string]string, exec mount.Exec) (*sioMgr, error) { if configs == nil { return nil, errors.New("missing configuration data") } @@ -47,7 +50,7 @@ func newSioMgr(configs map[string]string) (*sioMgr, error) { configs[confKey.sdcRootPath] = defaultString(configs[confKey.sdcRootPath], sdcRootPath) configs[confKey.storageMode] = defaultString(configs[confKey.storageMode], "ThinProvisioned") - mgr := &sioMgr{configData: configs} + mgr := &sioMgr{configData: configs, exec: exec} return mgr, nil } @@ -67,7 +70,7 @@ func (m *sioMgr) getClient() (sioInterface, error) { certsEnabled := b glog.V(4).Info(log("creating new client for gateway %s", gateway)) - client, err := newSioClient(gateway, username, password, certsEnabled) + client, err := newSioClient(gateway, username, password, certsEnabled, m.exec) if err != nil { glog.Error(log("failed to create scaleio client: %v", err)) return nil, err diff --git a/pkg/volume/scaleio/sio_mgr_test.go b/pkg/volume/scaleio/sio_mgr_test.go index 3d580b6b99b..e2fe5c2002b 100644 --- a/pkg/volume/scaleio/sio_mgr_test.go +++ b/pkg/volume/scaleio/sio_mgr_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + "k8s.io/kubernetes/pkg/util/mount" + siotypes "github.com/codedellemc/goscaleio/types/v1" ) @@ -42,7 +44,7 @@ var ( ) func newTestMgr(t *testing.T) *sioMgr { - mgr, err := newSioMgr(fakeConfig) + mgr, err := newSioMgr(fakeConfig, mount.NewFakeExec(nil)) if err != nil { t.Error(err) } @@ -51,7 +53,7 @@ func newTestMgr(t *testing.T) *sioMgr { } func TestMgrNew(t *testing.T) { - mgr, err := newSioMgr(fakeConfig) + mgr, err := newSioMgr(fakeConfig, mount.NewFakeExec(nil)) if err != nil { t.Fatal(err) } diff --git a/pkg/volume/scaleio/sio_volume.go b/pkg/volume/scaleio/sio_volume.go index f72b5a1adbb..079df8f67f9 100644 --- a/pkg/volume/scaleio/sio_volume.go +++ b/pkg/volume/scaleio/sio_volume.go @@ -386,7 +386,7 @@ func (v *sioVolume) setSioMgr() error { return err } - mgr, err := newSioMgr(configData) + mgr, err := newSioMgr(configData, v.plugin.host.GetExec(v.plugin.GetPluginName())) if err != nil { glog.Error(log("failed to reset sio manager: %v", err)) return err @@ -418,7 +418,7 @@ func (v *sioVolume) resetSioMgr() error { return err } - mgr, err := newSioMgr(configData) + mgr, err := newSioMgr(configData, v.plugin.host.GetExec(v.plugin.GetPluginName())) if err != nil { glog.Error(log("failed to reset scaleio mgr: %v", err)) return err @@ -452,7 +452,7 @@ func (v *sioVolume) setSioMgrFromConfig() error { return err } - mgr, err := newSioMgr(data) + mgr, err := newSioMgr(data, v.plugin.host.GetExec(v.plugin.GetPluginName())) if err != nil { glog.Error(log("failed while setting scaleio mgr from config: %v", err)) return err @@ -481,7 +481,7 @@ func (v *sioVolume) setSioMgrFromSpec() error { return err } - mgr, err := newSioMgr(configData) + mgr, err := newSioMgr(configData, v.plugin.host.GetExec(v.plugin.GetPluginName())) if err != nil { glog.Error(log("failed to reset sio manager: %v", err)) return err