diff --git a/pkg/volume/git_repo/BUILD b/pkg/volume/git_repo/BUILD index b726378cf1b17..cb4197cc3053a 100644 --- a/pkg/volume/git_repo/BUILD +++ b/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", ], ) @@ -29,14 +29,13 @@ go_test( embed = [":go_default_library"], importpath = "k8s.io/kubernetes/pkg/volume/git_repo", 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", ], ) diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index 1948971e94ea7..779bdc5e93ce7 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/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) { - cmd := b.exec.Command(command, args...) - cmd.SetDir(dir) - return cmd.CombinedOutput() +func (b *gitRepoVolumeMounter) execGit(args []string, dir string) ([]byte, error) { + // run git -C + fullArgs := append([]string{"-C", dir}, args...) + return b.exec.Run("git", fullArgs...) } // gitRepoVolumeUnmounter cleans git repo volumes. diff --git a/pkg/volume/git_repo/git_repo_test.go b/pkg/volume/git_repo/git_repo_test.go index 8c6a41a1c1e3b..46e4c13379cc0 100644 --- a/pkg/volume/git_repo/git_repo_test.go +++ b/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 { - // The git command - cmd []string - // The dir of git command is executed - dir string +type expectedCommand []string + +type testScenario struct { + name string + vol *v1.Volume + repositoryDir string + expecteds []expectedCommand + isExpectedFailure bool } func TestPlugin(t *testing.T) { - gitUrl := "https://github.com/kubernetes/kubernetes.git" - revision := "2a30ce65c5ab586b98916d83385c5983edd353a1" - - scenarios := []struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool - }{ + scenarios := []testScenario{ { name: "target-dir", vol: &v1.Volume{ @@ -91,19 +91,11 @@ func TestPlugin(t *testing.T) { }, }, }, + repositoryDir: "target_dir", expecteds: []expectedCommand{ - { - cmd: []string{"git", "clone", gitUrl, "target_dir"}, - dir: "", - }, - { - cmd: []string{"git", "checkout", revision}, - dir: "/target_dir", - }, - { - cmd: []string{"git", "reset", "--hard"}, - dir: "/target_dir", - }, + []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"}, }, isExpectedFailure: false, }, @@ -118,11 +110,9 @@ func TestPlugin(t *testing.T) { }, }, }, + repositoryDir: "target_dir", expecteds: []expectedCommand{ - { - cmd: []string{"git", "clone", gitUrl, "target_dir"}, - dir: "", - }, + []string{"git", "-C", "volume-dir", "clone", gitUrl, "target_dir"}, }, isExpectedFailure: false, }, @@ -136,11 +126,9 @@ func TestPlugin(t *testing.T) { }, }, }, + repositoryDir: "kubernetes", expecteds: []expectedCommand{ - { - cmd: []string{"git", "clone", gitUrl}, - dir: "", - }, + []string{"git", "-C", "volume-dir", "clone", gitUrl}, }, isExpectedFailure: false, }, @@ -156,19 +144,11 @@ func TestPlugin(t *testing.T) { }, }, }, + repositoryDir: "kubernetes", expecteds: []expectedCommand{ - { - cmd: []string{"git", "clone", gitUrl}, - dir: "", - }, - { - cmd: []string{"git", "checkout", revision}, - dir: "/kubernetes", - }, - { - cmd: []string{"git", "reset", "--hard"}, - dir: "/kubernetes", - }, + []string{"git", "-C", "volume-dir", "clone", gitUrl}, + []string{"git", "-C", "volume-dir/kubernetes", "checkout", revision}, + []string{"git", "-C", "volume-dir/kubernetes", "reset", "--hard"}, }, isExpectedFailure: false, }, @@ -184,19 +164,11 @@ func TestPlugin(t *testing.T) { }, }, }, + repositoryDir: "", expecteds: []expectedCommand{ - { - cmd: []string{"git", "clone", gitUrl, "."}, - dir: "", - }, - { - cmd: []string{"git", "checkout", revision}, - dir: "", - }, - { - cmd: []string{"git", "reset", "--hard"}, - dir: "", - }, + []string{"git", "-C", "volume-dir", "clone", gitUrl, "."}, + []string{"git", "-C", "volume-dir", "checkout", revision}, + []string{"git", "-C", "volume-dir", "reset", "--hard"}, }, isExpectedFailure: false, }, @@ -214,12 +186,7 @@ func TestPlugin(t *testing.T) { } -func doTestPlugin(scenario struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool -}, t *testing.T) []error { +func doTestPlugin(scenario testScenario, t *testing.T) []error { allErrs := []error{} plugMgr := volume.VolumePluginMgr{} @@ -311,73 +278,42 @@ func doTestPlugin(scenario struct { return allErrs } -func doTestSetUp(scenario struct { - name string - vol *v1.Volume - expecteds []expectedCommand - isExpectedFailure bool -}, mounter volume.Mounter) []error { +func doTestSetUp(scenario testScenario, mounter volume.Mounter) []error { expecteds := scenario.expecteds allErrs := []error{} - // 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 - }) + 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 + 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) { - 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) { + if !reflect.DeepEqual(expecteds, commandLog) { allErrs = append(allErrs, - fmt.Errorf("unexpected directories: %v, expected: %v", fcmd.Dirs, expectedPaths)) + fmt.Errorf("unexpected commands: %v, expected: %v", commandLog, expecteds)) } return allErrs