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

This prepares volume plugins to run things in containers instead of running
them on the host.

pkg/volume/git_repo/BUILD

@@ -13,12 +13,12 @@ go_library(
         "git_repo.go",
     ],
     deps = [
+        "//pkg/util/mount:go_default_library",
         "//pkg/util/strings:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/util: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",
     ],
 )
 
@@ -27,14 +27,13 @@ go_test(
     srcs = ["git_repo_test.go"],
     library = ":go_default_library",
     deps = [
+        "//pkg/util/mount:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/empty_dir:go_default_library",
         "//pkg/volume/testing:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
-        "//vendor/k8s.io/utils/exec/testing:go_default_library",
     ],
 )
 

pkg/volume/git_repo/git_repo.go

@@ -24,10 +24,10 @@ import (
 
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/util/mount"
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
-	"k8s.io/utils/exec"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -100,7 +100,8 @@ func (plugin *gitRepoPlugin) NewMounter(spec *volume.Spec, pod *v1.Pod, opts vol
 		source:   spec.Volume.GitRepo.Repository,
 		revision: spec.Volume.GitRepo.Revision,
 		target:   spec.Volume.GitRepo.Directory,
-		exec:     exec.New(),
+		mounter:  plugin.host.GetMounter(plugin.GetPluginName()),
+		exec:     plugin.host.GetExec(plugin.GetPluginName()),
 		opts:     opts,
 	}, nil
 }
@@ -149,7 +150,8 @@ type gitRepoVolumeMounter struct {
 	source   string
 	revision string
 	target   string
-	exec     exec.Interface
+	mounter  mount.Interface
+	exec     mount.Exec
 	opts     volume.VolumeOptions
 }
 
@@ -195,7 +197,7 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 	if len(b.target) != 0 {
 		args = append(args, b.target)
 	}
-	if output, err := b.execCommand("git", args, dir); err != nil {
+	if output, err := b.execGit(args, dir); err != nil {
 		return fmt.Errorf("failed to exec 'git %s': %s: %v",
 			strings.Join(args, " "), output, err)
 	}
@@ -225,10 +227,10 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 		return fmt.Errorf("unexpected directory contents: %v", files)
 	}
 
-	if output, err := b.execCommand("git", []string{"checkout", b.revision}, subdir); err != nil {
+	if output, err := b.execGit([]string{"checkout", b.revision}, subdir); err != nil {
 		return fmt.Errorf("failed to exec 'git checkout %s': %s: %v", b.revision, output, err)
 	}
-	if output, err := b.execCommand("git", []string{"reset", "--hard"}, subdir); err != nil {
+	if output, err := b.execGit([]string{"reset", "--hard"}, subdir); err != nil {
 		return fmt.Errorf("failed to exec 'git reset --hard': %s: %v", output, err)
 	}
 
@@ -242,10 +244,10 @@ func (b *gitRepoVolumeMounter) getMetaDir() string {
 	return path.Join(b.plugin.host.GetPodPluginDir(b.podUID, utilstrings.EscapeQualifiedNameForDisk(gitRepoPluginName)), b.volName)
 }
 
-func (b *gitRepoVolumeMounter) execCommand(command string, args []string, dir string) ([]byte, error) {
+func (b *gitRepoVolumeMounter) execGit(args []string, dir string) ([]byte, error) {
-	cmd := b.exec.Command(command, args...)
+	// run git -C <dir> <args>
-	cmd.SetDir(dir)
+	fullArgs := append([]string{"-C", dir}, args...)
-	return cmd.CombinedOutput()
+	return b.exec.Run("git", fullArgs...)
 }
 
 // gitRepoVolumeUnmounter cleans git repo volumes.

pkg/volume/git_repo/git_repo_test.go

@@ -28,11 +28,16 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/volume"
 	"k8s.io/kubernetes/pkg/volume/empty_dir"
 	volumetest "k8s.io/kubernetes/pkg/volume/testing"
-	"k8s.io/utils/exec"
+)
-	fakeexec "k8s.io/utils/exec/testing"
+
+const (
+	gitUrl            = "https://github.com/kubernetes/kubernetes.git"
+	revision          = "2a30ce65c5ab586b98916d83385c5983edd353a1"
+	gitRepositoryName = "kubernetes"
 )
 
 func newTestHost(t *testing.T) (string, volume.VolumeHost) {
@@ -62,23 +67,18 @@ func TestCanSupport(t *testing.T) {
 }
 
 // Expected command
-type expectedCommand struct {
+type expectedCommand []string
-	// The git command
+
-	cmd []string
+type testScenario struct {
-	// The dir of git command is executed
+	name              string
-	dir string
+	vol               *v1.Volume
+	repositoryDir     string
+	expecteds         []expectedCommand
+	isExpectedFailure bool
 }
 
 func TestPlugin(t *testing.T) {
-	gitUrl := "https://github.com/kubernetes/kubernetes.git"
+	scenarios := []testScenario{
-	revision := "2a30ce65c5ab586b98916d83385c5983edd353a1"
-
-	scenarios := []struct {
-		name              string
-		vol               *v1.Volume
-		expecteds         []expectedCommand
-		isExpectedFailure bool
-	}{
 		{
 			name: "target-dir",
 			vol: &v1.Volume{
@@ -91,19 +91,11 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
+			repositoryDir: "target_dir",
 			expecteds: []expectedCommand{
-				{
+				[]string{"git", "-C", "volume-dir", "clone", gitUrl, "target_dir"},
-					cmd: []string{"git", "clone", gitUrl, "target_dir"},
+				[]string{"git", "-C", "volume-dir/target_dir", "checkout", revision},
-					dir: "",
+				[]string{"git", "-C", "volume-dir/target_dir", "reset", "--hard"},
-				},
-				{
-					cmd: []string{"git", "checkout", revision},
-					dir: "/target_dir",
-				},
-				{
-					cmd: []string{"git", "reset", "--hard"},
-					dir: "/target_dir",
-				},
 			},
 			isExpectedFailure: false,
 		},
@@ -118,11 +110,9 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
+			repositoryDir: "target_dir",
 			expecteds: []expectedCommand{
-				{
+				[]string{"git", "-C", "volume-dir", "clone", gitUrl, "target_dir"},
-					cmd: []string{"git", "clone", gitUrl, "target_dir"},
-					dir: "",
-				},
 			},
 			isExpectedFailure: false,
 		},
@@ -136,11 +126,9 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
+			repositoryDir: "kubernetes",
 			expecteds: []expectedCommand{
-				{
+				[]string{"git", "-C", "volume-dir", "clone", gitUrl},
-					cmd: []string{"git", "clone", gitUrl},
-					dir: "",
-				},
 			},
 			isExpectedFailure: false,
 		},
@@ -156,19 +144,11 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
+			repositoryDir: "kubernetes",
 			expecteds: []expectedCommand{
-				{
+				[]string{"git", "-C", "volume-dir", "clone", gitUrl},
-					cmd: []string{"git", "clone", gitUrl},
+				[]string{"git", "-C", "volume-dir/kubernetes", "checkout", revision},
-					dir: "",
+				[]string{"git", "-C", "volume-dir/kubernetes", "reset", "--hard"},
-				},
-				{
-					cmd: []string{"git", "checkout", revision},
-					dir: "/kubernetes",
-				},
-				{
-					cmd: []string{"git", "reset", "--hard"},
-					dir: "/kubernetes",
-				},
 			},
 			isExpectedFailure: false,
 		},
@@ -184,19 +164,11 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
+			repositoryDir: "",
 			expecteds: []expectedCommand{
-				{
+				[]string{"git", "-C", "volume-dir", "clone", gitUrl, "."},
-					cmd: []string{"git", "clone", gitUrl, "."},
+				[]string{"git", "-C", "volume-dir", "checkout", revision},
-					dir: "",
+				[]string{"git", "-C", "volume-dir", "reset", "--hard"},
-				},
-				{
-					cmd: []string{"git", "checkout", revision},
-					dir: "",
-				},
-				{
-					cmd: []string{"git", "reset", "--hard"},
-					dir: "",
-				},
 			},
 			isExpectedFailure: false,
 		},
@@ -214,12 +186,7 @@ func TestPlugin(t *testing.T) {
 
 }
 
-func doTestPlugin(scenario struct {
+func doTestPlugin(scenario testScenario, t *testing.T) []error {
-	name              string
-	vol               *v1.Volume
-	expecteds         []expectedCommand
-	isExpectedFailure bool
-}, t *testing.T) []error {
 	allErrs := []error{}
 
 	plugMgr := volume.VolumePluginMgr{}
@@ -311,73 +278,42 @@ func doTestPlugin(scenario struct {
 	return allErrs
 }
 
-func doTestSetUp(scenario struct {
+func doTestSetUp(scenario testScenario, mounter volume.Mounter) []error {
-	name              string
-	vol               *v1.Volume
-	expecteds         []expectedCommand
-	isExpectedFailure bool
-}, mounter volume.Mounter) []error {
 	expecteds := scenario.expecteds
 	allErrs := []error{}
 
-	// Construct combined outputs from expected commands
+	var commandLog []expectedCommand
-	var fakeOutputs []fakeexec.FakeCombinedOutputAction
+	execCallback := func(cmd string, args ...string) ([]byte, error) {
-	var fcmd fakeexec.FakeCmd
+		if len(args) < 2 {
-	for _, expected := range expecteds {
+			return nil, fmt.Errorf("expected at least 2 arguments, got %q", args)
-		if expected.cmd[1] == "clone" {
-			fakeOutputs = append(fakeOutputs, func() ([]byte, error) {
-				// git clone, it creates new dir/files
-				os.MkdirAll(path.Join(fcmd.Dirs[0], expected.dir), 0750)
-				return []byte{}, nil
-			})
-		} else {
-			// git checkout || git reset, they create nothing
-			fakeOutputs = append(fakeOutputs, func() ([]byte, error) {
-				return []byte{}, nil
-			})
 		}
+		if args[0] != "-C" {
+			return nil, fmt.Errorf("expected the first argument to be \"-C\", got %q", args[0])
+		}
+		// command is 'git -C <dir> <command> <args>
+		gitDir := args[1]
+		gitCommand := args[2]
+		if gitCommand == "clone" {
+			// Clone creates a directory
+			if scenario.repositoryDir != "" {
+				os.MkdirAll(path.Join(gitDir, scenario.repositoryDir), 0750)
+			}
+		}
+		// add the command to log with de-randomized gitDir
+		args[1] = strings.Replace(gitDir, mounter.GetPath(), "volume-dir", 1)
+		cmdline := append([]string{cmd}, args...)
+		commandLog = append(commandLog, cmdline)
+		return []byte{}, nil
 	}
-	fcmd = fakeexec.FakeCmd{
-		CombinedOutputScript: fakeOutputs,
-	}
-
-	// Construct fake exec outputs from fcmd
-	var fakeAction []fakeexec.FakeCommandAction
-	for i := 0; i < len(expecteds); i++ {
-		fakeAction = append(fakeAction, func(cmd string, args ...string) exec.Cmd {
-			return fakeexec.InitFakeCmd(&fcmd, cmd, args...)
-		})
-
-	}
-	fake := fakeexec.FakeExec{
-		CommandScript: fakeAction,
-	}
-
 	g := mounter.(*gitRepoVolumeMounter)
-	g.exec = &fake
+	g.mounter = &mount.FakeMounter{}
+	g.exec = mount.NewFakeExec(execCallback)
 
 	g.SetUp(nil)
 
-	if fake.CommandCalls != len(expecteds) {
+	if !reflect.DeepEqual(expecteds, commandLog) {
 		allErrs = append(allErrs,
-			fmt.Errorf("unexpected command calls in scenario: expected %d, saw: %d", len(expecteds), fake.CommandCalls))
+			fmt.Errorf("unexpected commands: %v, expected: %v", commandLog, expecteds))
-	}
-	var expectedCmds [][]string
-	for _, expected := range expecteds {
-		expectedCmds = append(expectedCmds, expected.cmd)
-	}
-	if !reflect.DeepEqual(expectedCmds, fcmd.CombinedOutputLog) {
-		allErrs = append(allErrs,
-			fmt.Errorf("unexpected commands: %v, expected: %v", fcmd.CombinedOutputLog, expectedCmds))
-	}
-
-	var expectedPaths []string
-	for _, expected := range expecteds {
-		expectedPaths = append(expectedPaths, g.GetPath()+expected.dir)
-	}
-	if len(fcmd.Dirs) != len(expectedPaths) || !reflect.DeepEqual(expectedPaths, fcmd.Dirs) {
-		allErrs = append(allErrs,
-			fmt.Errorf("unexpected directories: %v, expected: %v", fcmd.Dirs, expectedPaths))
 	}
 
 	return allErrs