Skip to content

Commit

Permalink
Merge pull request kubernetes#51098 from jsafrane/exec-git
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue. 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>.

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.

**Special notes for your reviewer**:

* instead of `cmd.SetDir(<dir>); cmd.Exec("git <command>")`, we do `cmd.Exec("git -C <dir> <command>")` - mounter.Exec does not have SetDir()
* there are lot of changes in the tests because a different exec interface is used.

@kubernetes/sig-storage-pr-reviews 

```release-note
gitRepo volumes in pods require git 1.8.5 or later
```

/assign @rootfs
  • Loading branch information
Kubernetes Submit Queue authored Jan 11, 2018
2 parents 807dab6 + c578542 commit 820ea04
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 136 deletions.
5 changes: 2 additions & 3 deletions pkg/volume/git_repo/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand All @@ -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",
],
)

Expand Down
22 changes: 12 additions & 10 deletions pkg/volume/git_repo/git_repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}

Expand All @@ -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 <dir> <args>
fullArgs := append([]string{"-C", dir}, args...)
return b.exec.Run("git", fullArgs...)
}

// gitRepoVolumeUnmounter cleans git repo volumes.
Expand Down
182 changes: 59 additions & 123 deletions pkg/volume/git_repo/git_repo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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{}
Expand Down Expand Up @@ -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 <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) {
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
Expand Down

0 comments on commit 820ea04

Please sign in to comment.