Merge pull request #62394 from jsafrane/revert-git-changes

Automatic merge from submit-queue (batch tested with PRs 60476, 62462, 61391, 62535, 62394). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

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

This reverts commit c578542ad7 (PR #51098). The PR added support for containerized git, on the other hand it required git 1.8.5. This breaks git volumes on older distros (CentOS 7, Ubuntu 14.04) that have old git.

Git volumes are getting deprecated (https://github.com/kubernetes/kubernetes/issues/60999) so we should restore it to the last working state and not touch it any longer.

**Release note**:

```release-note
gitRepo volumes in pods no longer require git 1.8.5 or newer, older git versions are supported too now.
```

I'd like to cherry-pick it into 1.10.

/sig storage

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)
+	// 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
+			})
 		}
-		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.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 commands: %v, expected: %v", commandLog, expecteds))
+			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 directories: %v, expected: %v", fcmd.Dirs, expectedPaths))
 	}
 
 	return allErrs