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

pkg/volume/git_repo/BUILD

@@ -14,12 +14,12 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/pkg/volume/git_repo",
     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",
     ],
 )
 
@@ -28,13 +28,14 @@ go_test(
     srcs = ["git_repo_test.go"],
     embed = [":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,8 +100,7 @@ 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,
-		mounter:  plugin.host.GetMounter(plugin.GetPluginName()),
-		exec:     plugin.host.GetExec(plugin.GetPluginName()),
+		exec:     exec.New(),
 		opts:     opts,
 	}, nil
 }
@@ -150,8 +149,7 @@ type gitRepoVolumeMounter struct {
 	source   string
 	revision string
 	target   string
-	mounter  mount.Interface
-	exec     mount.Exec
+	exec     exec.Interface
 	opts     volume.VolumeOptions
 }
 
@@ -197,7 +195,7 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 	if len(b.target) != 0 {
 		args = append(args, b.target)
 	}
-	if output, err := b.execGit(args, dir); err != nil {
+	if output, err := b.execCommand("git", args, dir); err != nil {
 		return fmt.Errorf("failed to exec 'git %s': %s: %v",
 			strings.Join(args, " "), output, err)
 	}
@@ -227,10 +225,10 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
 		return fmt.Errorf("unexpected directory contents: %v", files)
 	}
 
-	if output, err := b.execGit([]string{"checkout", b.revision}, subdir); err != nil {
+	if output, err := b.execCommand("git", []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.execGit([]string{"reset", "--hard"}, subdir); err != nil {
+	if output, err := b.execCommand("git", []string{"reset", "--hard"}, subdir); err != nil {
 		return fmt.Errorf("failed to exec 'git reset --hard': %s: %v", output, err)
 	}
 
@@ -244,10 +242,10 @@ func (b *gitRepoVolumeMounter) getMetaDir() string {
 	return path.Join(b.plugin.host.GetPodPluginDir(b.podUID, utilstrings.EscapeQualifiedNameForDisk(gitRepoPluginName)), b.volName)
 }
 
-func (b *gitRepoVolumeMounter) execGit(args []string, dir string) ([]byte, error) {
-	// run git -C <dir> <args>
-	fullArgs := append([]string{"-C", dir}, args...)
-	return b.exec.Run("git", fullArgs...)
+func (b *gitRepoVolumeMounter) execCommand(command string, args []string, dir string) ([]byte, error) {
+	cmd := b.exec.Command(command, args...)
+	cmd.SetDir(dir)
+	return cmd.CombinedOutput()
 }
 
 // gitRepoVolumeUnmounter cleans git repo volumes.

pkg/volume/git_repo/git_repo_test.go

@@ -28,16 +28,11 @@ 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"
-)
-
-const (
-	gitUrl            = "https://github.com/kubernetes/kubernetes.git"
-	revision          = "2a30ce65c5ab586b98916d83385c5983edd353a1"
-	gitRepositoryName = "kubernetes"
+	"k8s.io/utils/exec"
+	fakeexec "k8s.io/utils/exec/testing"
 )
 
 func newTestHost(t *testing.T) (string, volume.VolumeHost) {
@@ -67,18 +62,23 @@ func TestCanSupport(t *testing.T) {
 }
 
 // Expected command
-type expectedCommand []string
-
-type testScenario struct {
-	name              string
-	vol               *v1.Volume
-	repositoryDir     string
-	expecteds         []expectedCommand
-	isExpectedFailure bool
+type expectedCommand struct {
+	// The git command
+	cmd []string
+	// The dir of git command is executed
+	dir string
 }
 
 func TestPlugin(t *testing.T) {
-	scenarios := []testScenario{
+	gitUrl := "https://github.com/kubernetes/kubernetes.git"
+	revision := "2a30ce65c5ab586b98916d83385c5983edd353a1"
+
+	scenarios := []struct {
+		name              string
+		vol               *v1.Volume
+		expecteds         []expectedCommand
+		isExpectedFailure bool
+	}{
 		{
 			name: "target-dir",
 			vol: &v1.Volume{
@@ -91,11 +91,19 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
-			repositoryDir: "target_dir",
 			expecteds: []expectedCommand{
-				[]string{"git", "-C", "volume-dir", "clone", gitUrl, "target_dir"},
-				[]string{"git", "-C", "volume-dir/target_dir", "checkout", revision},
-				[]string{"git", "-C", "volume-dir/target_dir", "reset", "--hard"},
+				{
+					cmd: []string{"git", "clone", gitUrl, "target_dir"},
+					dir: "",
+				},
+				{
+					cmd: []string{"git", "checkout", revision},
+					dir: "/target_dir",
+				},
+				{
+					cmd: []string{"git", "reset", "--hard"},
+					dir: "/target_dir",
+				},
 			},
 			isExpectedFailure: false,
 		},
@@ -110,9 +118,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"},
+					dir: "",
+				},
 			},
 			isExpectedFailure: false,
 		},
@@ -126,9 +136,11 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
-			repositoryDir: "kubernetes",
 			expecteds: []expectedCommand{
-				[]string{"git", "-C", "volume-dir", "clone", gitUrl},
+				{
+					cmd: []string{"git", "clone", gitUrl},
+					dir: "",
+				},
 			},
 			isExpectedFailure: false,
 		},
@@ -144,11 +156,19 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
-			repositoryDir: "kubernetes",
 			expecteds: []expectedCommand{
-				[]string{"git", "-C", "volume-dir", "clone", gitUrl},
-				[]string{"git", "-C", "volume-dir/kubernetes", "checkout", revision},
-				[]string{"git", "-C", "volume-dir/kubernetes", "reset", "--hard"},
+				{
+					cmd: []string{"git", "clone", gitUrl},
+					dir: "",
+				},
+				{
+					cmd: []string{"git", "checkout", revision},
+					dir: "/kubernetes",
+				},
+				{
+					cmd: []string{"git", "reset", "--hard"},
+					dir: "/kubernetes",
+				},
 			},
 			isExpectedFailure: false,
 		},
@@ -164,11 +184,19 @@ func TestPlugin(t *testing.T) {
 					},
 				},
 			},
-			repositoryDir: "",
 			expecteds: []expectedCommand{
-				[]string{"git", "-C", "volume-dir", "clone", gitUrl, "."},
-				[]string{"git", "-C", "volume-dir", "checkout", revision},
-				[]string{"git", "-C", "volume-dir", "reset", "--hard"},
+				{
+					cmd: []string{"git", "clone", gitUrl, "."},
+					dir: "",
+				},
+				{
+					cmd: []string{"git", "checkout", revision},
+					dir: "",
+				},
+				{
+					cmd: []string{"git", "reset", "--hard"},
+					dir: "",
+				},
 			},
 			isExpectedFailure: false,
 		},
@@ -186,7 +214,12 @@ func TestPlugin(t *testing.T) {
 
 }
 
-func doTestPlugin(scenario testScenario, t *testing.T) []error {
+func doTestPlugin(scenario struct {
+	name              string
+	vol               *v1.Volume
+	expecteds         []expectedCommand
+	isExpectedFailure bool
+}, t *testing.T) []error {
 	allErrs := []error{}
 
 	plugMgr := volume.VolumePluginMgr{}
@@ -278,42 +311,73 @@ func doTestPlugin(scenario testScenario, t *testing.T) []error {
 	return allErrs
 }
 
-func doTestSetUp(scenario testScenario, mounter volume.Mounter) []error {
+func doTestSetUp(scenario struct {
+	name              string
+	vol               *v1.Volume
+	expecteds         []expectedCommand
+	isExpectedFailure bool
+}, mounter volume.Mounter) []error {
 	expecteds := scenario.expecteds
 	allErrs := []error{}
 
-	var commandLog []expectedCommand
-	execCallback := func(cmd string, args ...string) ([]byte, error) {
-		if len(args) < 2 {
-			return nil, fmt.Errorf("expected at least 2 arguments, got %q", args)
-		}
-		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)
-			}
+	// Construct combined outputs from expected commands
+	var fakeOutputs []fakeexec.FakeCombinedOutputAction
+	var fcmd fakeexec.FakeCmd
+	for _, expected := range expecteds {
+		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
+			})
 		}
-		// 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.mounter = &mount.FakeMounter{}
-	g.exec = mount.NewFakeExec(execCallback)
+	g.exec = &fake
 
 	g.SetUp(nil)
 
-	if !reflect.DeepEqual(expecteds, commandLog) {
+	if fake.CommandCalls != len(expecteds) {
+		allErrs = append(allErrs,
+			fmt.Errorf("unexpected command calls in scenario: expected %d, saw: %d", len(expecteds), fake.CommandCalls))
+	}
+	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 commands: %v, expected: %v", commandLog, expecteds))
+			fmt.Errorf("unexpected directories: %v, expected: %v", fcmd.Dirs, expectedPaths))
 	}
 
 	return allErrs