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

pkg/volume/iscsi/BUILD

@@ -22,7 +22,6 @@ go_library(
         "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 
@@ -42,8 +41,6 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//vendor/k8s.io/client-go/util/testing:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
-        "//vendor/k8s.io/utils/exec/testing:go_default_library",
     ],
 )
 

pkg/volume/iscsi/iscsi.go

@@ -28,17 +28,15 @@ import (
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
 	ioutil "k8s.io/kubernetes/pkg/volume/util"
-	"k8s.io/utils/exec"
 )
 
 // This is the primary entrypoint for volume plugins.
 func ProbeVolumePlugins() []volume.VolumePlugin {
-	return []volume.VolumePlugin{&iscsiPlugin{nil, exec.New()}}
+	return []volume.VolumePlugin{&iscsiPlugin{nil}}
 }
 
 type iscsiPlugin struct {
 	host volume.VolumeHost
-	exe  exec.Interface
 }
 
 var _ volume.VolumePlugin = &iscsiPlugin{}
@@ -154,6 +152,7 @@ func (plugin *iscsiPlugin) newMounterInternal(spec *volume.Spec, podUID types.UI
 		fsType:       iscsi.FSType,
 		readOnly:     readOnly,
 		mounter:      &mount.SafeFormatAndMount{Interface: mounter, Exec: exec},
+		exec:         exec,
 		deviceUtil:   ioutil.NewDeviceHandler(ioutil.NewIOHandler()),
 		mountOptions: volume.MountOptionFromSpec(spec),
 	}, nil
@@ -173,14 +172,10 @@ func (plugin *iscsiPlugin) newUnmounterInternal(volName string, podUID types.UID
 			plugin:  plugin,
 		},
 		mounter: mounter,
+		exec:    exec,
 	}, nil
 }
 
-func (plugin *iscsiPlugin) execCommand(command string, args []string) ([]byte, error) {
-	cmd := plugin.exe.Command(command, args...)
-	return cmd.CombinedOutput()
-}
-
 func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
 	iscsiVolume := &v1.Volume{
 		Name: volumeName,
@@ -222,6 +217,7 @@ type iscsiDiskMounter struct {
 	readOnly     bool
 	fsType       string
 	mounter      *mount.SafeFormatAndMount
+	exec         mount.Exec
 	deviceUtil   ioutil.DeviceUtil
 	mountOptions []string
 }
@@ -259,6 +255,7 @@ func (b *iscsiDiskMounter) SetUpAt(dir string, fsGroup *int64) error {
 type iscsiDiskUnmounter struct {
 	*iscsiDisk
 	mounter mount.Interface
+	exec    mount.Exec
 }
 
 var _ volume.Unmounter = &iscsiDiskUnmounter{}

pkg/volume/iscsi/iscsi_util.go

@@ -46,7 +46,7 @@ var (
 
 func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error {
 	if b.chap_discovery {
-		out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", "discovery.sendtargets.auth.authmethod", "-v", "CHAP"})
+		out, err := b.exec.Run("iscsiadm", "-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", "discovery.sendtargets.auth.authmethod", "-v", "CHAP")
 		if err != nil {
 			return fmt.Errorf("iscsi: failed to update discoverydb with CHAP, output: %v", string(out))
 		}
@@ -54,7 +54,7 @@ func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error {
 		for _, k := range chap_st {
 			v := b.secret[k]
 			if len(v) > 0 {
-				out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", k, "-v", v})
+				out, err := b.exec.Run("iscsiadm", "-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", k, "-v", v)
 				if err != nil {
 					return fmt.Errorf("iscsi: failed to update discoverydb key %q with value %q error: %v", k, v, string(out))
 				}
@@ -66,7 +66,7 @@ func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error {
 
 func updateISCSINode(b iscsiDiskMounter, tp string) error {
 	if b.chap_session {
-		out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", "node.session.auth.authmethod", "-v", "CHAP"})
+		out, err := b.exec.Run("iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", "node.session.auth.authmethod", "-v", "CHAP")
 		if err != nil {
 			return fmt.Errorf("iscsi: failed to update node with CHAP, output: %v", string(out))
 		}
@@ -74,7 +74,7 @@ func updateISCSINode(b iscsiDiskMounter, tp string) error {
 		for _, k := range chap_sess {
 			v := b.secret[k]
 			if len(v) > 0 {
-				out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", k, "-v", v})
+				out, err := b.exec.Run("iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", k, "-v", v)
 				if err != nil {
 					return fmt.Errorf("iscsi: failed to update node session key %q with value %q error: %v", k, v, string(out))
 				}
@@ -196,7 +196,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error {
 	var iscsiTransport string
 	var lastErr error
 
-	out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", b.Iface, "-o", "show"})
+	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "show")
 	if err != nil {
 		glog.Errorf("iscsi: could not read iface %s error: %s", b.Iface, string(out))
 		return err
@@ -222,7 +222,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error {
 	for _, tp := range bkpPortal {
 		// Rescan sessions to discover newly mapped LUNs. Do not specify the interface when rescanning
 		// to avoid establishing additional sessions to the same target.
-		out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-R"})
+		out, err := b.exec.Run("iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-R")
 		if err != nil {
 			glog.Errorf("iscsi: failed to rescan session with error: %s (%v)", string(out), err)
 		}
@@ -238,17 +238,17 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error {
 		exist := waitForPathToExist(&devicePath, 1, iscsiTransport)
 		if exist == false {
 			// build discoverydb and discover iscsi target
-			b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "new"})
+			b.exec.Run("iscsiadm", "-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "new")
 			// update discoverydb with CHAP secret
 			err = updateISCSIDiscoverydb(b, tp)
 			if err != nil {
 				lastErr = fmt.Errorf("iscsi: failed to update discoverydb to portal %s error: %v", tp, err)
 				continue
 			}
-			out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "--discover"})
+			out, err := b.exec.Run("iscsiadm", "-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "--discover")
 			if err != nil {
 				// delete discoverydb record
-				b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "delete"})
+				b.exec.Run("iscsiadm", "-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "delete")
 				lastErr = fmt.Errorf("iscsi: failed to sendtargets to portal %s output: %s, err %v", tp, string(out), err)
 				continue
 			}
@@ -259,10 +259,10 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error {
 				continue
 			}
 			// login to iscsi target
-			out, err = b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "--login"})
+			out, err = b.exec.Run("iscsiadm", "-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "--login")
 			if err != nil {
 				// delete the node record from database
-				b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-I", b.Iface, "-T", b.Iqn, "-o", "delete"})
+				b.exec.Run("iscsiadm", "-m", "node", "-p", tp, "-I", b.Iface, "-T", b.Iqn, "-o", "delete")
 				lastErr = fmt.Errorf("iscsi: failed to attach disk: Error: %s (%v)", string(out), err)
 				continue
 			}
@@ -283,7 +283,7 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error {
 
 	if len(devicePaths) == 0 {
 		// delete cloned iface
-		b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", b.Iface, "-o", "delete"})
+		b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "delete")
 		glog.Errorf("iscsi: failed to get any path for iscsi disk, last err seen:\n%v", lastErr)
 		return fmt.Errorf("failed to get any path for iscsi disk, last err seen:\n%v", lastErr)
 	}
@@ -381,13 +381,13 @@ func (util *ISCSIUtil) DetachDisk(c iscsiDiskUnmounter, mntPath string) error {
 					delete = append(delete, []string{"-I", iface}...)
 				}
 				glog.Infof("iscsi: log out target %s iqn %s iface %s", portal, iqn, iface)
-				out, err := c.plugin.execCommand("iscsiadm", logout)
+				out, err := c.exec.Run("iscsiadm", logout...)
 				if err != nil {
 					glog.Errorf("iscsi: failed to detach disk Error: %s", string(out))
 				}
 				// Delete the node record
 				glog.Infof("iscsi: delete node record target %s iqn %s", portal, iqn)
-				out, err = c.plugin.execCommand("iscsiadm", delete)
+				out, err = c.exec.Run("iscsiadm", delete...)
 				if err != nil {
 					glog.Errorf("iscsi: failed to delete node record Error: %s", string(out))
 				}
@@ -396,7 +396,7 @@ func (util *ISCSIUtil) DetachDisk(c iscsiDiskUnmounter, mntPath string) error {
 			// If the iface is not created via iscsi plugin, skip to delete
 			if initiatorName != "" && found && iface == (portals[0]+":"+c.volName) {
 				delete := []string{"-m", "iface", "-I", iface, "-o", "delete"}
-				out, err := c.plugin.execCommand("iscsiadm", delete)
+				out, err := c.exec.Run("iscsiadm", delete...)
 				if err != nil {
 					glog.Errorf("iscsi: failed to delete iface Error: %s", string(out))
 				}
@@ -504,7 +504,7 @@ func parseIscsiadmShow(output string) (map[string]string, error) {
 func cloneIface(b iscsiDiskMounter, newIface string) error {
 	var lastErr error
 	// get pre-configured iface records
-	out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", b.Iface, "-o", "show"})
+	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "show")
 	if err != nil {
 		lastErr = fmt.Errorf("iscsi: failed to show iface records: %s (%v)", string(out), err)
 		return lastErr
@@ -518,16 +518,16 @@ func cloneIface(b iscsiDiskMounter, newIface string) error {
 	// update initiatorname
 	params["iface.initiatorname"] = b.InitiatorName
 	// create new iface
-	out, err = b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", newIface, "-o", "new"})
+	out, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "new")
 	if err != nil {
 		lastErr = fmt.Errorf("iscsi: failed to create new iface: %s (%v)", string(out), err)
 		return lastErr
 	}
 	// update new iface records
 	for key, val := range params {
-		_, err = b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", newIface, "-o", "update", "-n", key, "-v", val})
+		_, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "update", "-n", key, "-v", val)
 		if err != nil {
-			b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", newIface, "-o", "delete"})
+			b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "delete")
 			lastErr = fmt.Errorf("iscsi: failed to update iface records: %s (%v). iface(%s) will be used", string(out), err, b.Iface)
 			break
 		}

pkg/volume/iscsi/iscsi_util_test.go

@@ -18,6 +18,7 @@ package iscsi
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -25,8 +26,6 @@ import (
 
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/utils/exec"
-	fakeexec "k8s.io/utils/exec/testing"
 )
 
 func TestGetDevicePrefixRefCount(t *testing.T) {
@@ -247,117 +246,117 @@ func TestParseIscsiadmShow(t *testing.T) {
 }
 
 func TestClonedIface(t *testing.T) {
-	fcmd := fakeexec.FakeCmd{
+	cmdCount := 0
-		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+	fakeExec := mount.NewFakeExec(func(cmd string, args ...string) ([]byte, error) {
+		cmdCount++
+		if cmd != "iscsiadm" {
+			t.Errorf("iscsiadm command expected, got %q", cmd)
+		}
+		switch cmdCount {
+		case 1:
 			// iscsiadm -m iface -I <iface> -o show
-			func() ([]byte, error) {
+			return []byte("iface.ipaddress = <empty>\niface.transport_name = tcp\niface.initiatorname = <empty>\n"), nil
-				return []byte("iface.ipaddress = <empty>\niface.transport_name = tcp\niface.initiatorname = <empty>\n"), nil
+
-			},
+		case 2:
 			// iscsiadm -m iface -I <newIface> -o new
-			func() ([]byte, error) { return []byte("New interface 192.168.1.10:pv0001 added"), nil },
+			return []byte("New interface 192.168.1.10:pv0001 added"), nil
+		case 3:
 			// iscsiadm -m iface -I <newIface> -o update -n <key> -v <val>
-			func() ([]byte, error) { return []byte(""), nil },
+			return []byte(""), nil
-			func() ([]byte, error) { return []byte(""), nil },
+		case 4:
-		},
+			return []byte(""), nil
-	}
+		}
-	fexec := fakeexec.FakeExec{
+		return nil, fmt.Errorf("Unexpected exec call nr %d: %s", cmdCount, cmd)
-		CommandScript: []fakeexec.FakeCommandAction{
+	})
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-		},
-	}
 	plugins := []volume.VolumePlugin{
 		&iscsiPlugin{
 			host: nil,
-			exe:  &fexec,
 		},
 	}
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
 			plugin: plugin.(*iscsiPlugin)},
+		exec: fakeExec,
 	}
 	newIface := "192.168.1.10:pv0001"
 	cloneIface(fakeMounter, newIface)
-	if fcmd.CombinedOutputCalls != 4 {
+	if cmdCount != 4 {
-		t.Errorf("expected 4 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+		t.Errorf("expected 4 CombinedOutput() calls, got %d", cmdCount)
 	}
 
 }
 
 func TestClonedIfaceShowError(t *testing.T) {
-	fcmd := fakeexec.FakeCmd{
+	cmdCount := 0
-		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+	fakeExec := mount.NewFakeExec(func(cmd string, args ...string) ([]byte, error) {
-			// iscsiadm -m iface -I <iface> -o show, return test error
+		cmdCount++
-			func() ([]byte, error) { return []byte(""), errors.New("test error") },
+		if cmd != "iscsiadm" {
-		},
+			t.Errorf("iscsiadm command expected, got %q", cmd)
-	}
+		}
-	fexec := fakeexec.FakeExec{
+		// iscsiadm -m iface -I <iface> -o show, return test error
-		CommandScript: []fakeexec.FakeCommandAction{
+		return []byte(""), errors.New("test error")
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+	})
-		},
-	}
 	plugins := []volume.VolumePlugin{
 		&iscsiPlugin{
 			host: nil,
-			exe:  &fexec,
 		},
 	}
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
 			plugin: plugin.(*iscsiPlugin)},
+		exec: fakeExec,
 	}
 	newIface := "192.168.1.10:pv0001"
 	cloneIface(fakeMounter, newIface)
-	if fcmd.CombinedOutputCalls != 1 {
+	if cmdCount != 1 {
-		t.Errorf("expected 1 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+		t.Errorf("expected 1 CombinedOutput() calls, got %d", cmdCount)
 	}
 
 }
 
 func TestClonedIfaceUpdateError(t *testing.T) {
-	fcmd := fakeexec.FakeCmd{
+	cmdCount := 0
-		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+	fakeExec := mount.NewFakeExec(func(cmd string, args ...string) ([]byte, error) {
+		cmdCount++
+		if cmd != "iscsiadm" {
+			t.Errorf("iscsiadm command expected, got %q", cmd)
+		}
+		switch cmdCount {
+		case 1:
 			// iscsiadm -m iface -I <iface> -o show
-			func() ([]byte, error) {
+			return []byte("iface.ipaddress = <empty>\niface.transport_name = tcp\niface.initiatorname = <empty>\n"), nil
-				return []byte("iface.ipaddress = <empty>\niface.transport_name = tcp\niface.initiatorname = <empty>\n"), nil
+
-			},
+		case 2:
 			// iscsiadm -m iface -I <newIface> -o new
-			func() ([]byte, error) { return []byte("New interface 192.168.1.10:pv0001 added"), nil },
+			return []byte("New interface 192.168.1.10:pv0001 added"), nil
+		case 3:
 			// iscsiadm -m iface -I <newIface> -o update -n <key> -v <val>
-			func() ([]byte, error) { return []byte(""), nil },
+			return []byte(""), nil
-			func() ([]byte, error) { return []byte(""), errors.New("test error") },
+		case 4:
+			return []byte(""), errors.New("test error")
+		case 5:
 			// iscsiadm -m iface -I <newIface> -o delete
-			func() ([]byte, error) { return []byte(""), nil },
+			return []byte(""), nil
-		},
+		}
-	}
+		return nil, fmt.Errorf("Unexpected exec call nr %d: %s", cmdCount, cmd)
-	fexec := fakeexec.FakeExec{
+	})
-		CommandScript: []fakeexec.FakeCommandAction{
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
-		},
-	}
 	plugins := []volume.VolumePlugin{
 		&iscsiPlugin{
 			host: nil,
-			exe:  &fexec,
 		},
 	}
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
 			plugin: plugin.(*iscsiPlugin)},
+		exec: fakeExec,
 	}
 	newIface := "192.168.1.10:pv0001"
 	cloneIface(fakeMounter, newIface)
-	if fcmd.CombinedOutputCalls != 5 {
+	if cmdCount != 5 {
-		t.Errorf("expected 5 CombinedOutput() calls, got %d", fcmd.CombinedOutputCalls)
+		t.Errorf("expected 5 CombinedOutput() calls, got %d", cmdCount)
 	}
 
 }