Merge pull request #51103 from jsafrane/exec-scaleio

Automatic merge from submit-queue (batch tested with PRs 50872, 51103, 51220, 51285, 50841)

ScaleIO:  Use VolumeHost.GetExec() to execute stuff in volume plugins

This PR updates ScaleIO volume plugin to use `VolumeHost.GetExec()` to execute utilities like mkfs and lsblk instead of simple `os/exec` + to use a fresh `mounter` for every `SetUp` / `TearDown` calls, as they may be different each time.

This prepares the volume plugin to run these utilities in containers instead of running them on the host + makes the volume plugin more independent and less hardcoded.

See proposal in https://github.com/kubernetes/community/pull/589.

Note that this PR does **not** change place where the utilities are executed - `VolumeHost.GetExec()` still leads directly to `os/exec`. It will be changed when the aforementioned proposal is merged and implemented.

**Special notes for your reviewer**:

* I needed to pass `mount.Exec` interface from the place where it is available down to `SioClient` where it's needed to execute stuff.


@kubernetes/sig-storage-pr-reviews 
/assign @vladimirvivien @rootfs 

**Release note**:
```release-note
NONE
```

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

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

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)
 	}

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
 }

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
@@ -384,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
@@ -416,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
@@ -450,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
@@ -479,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

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{