Skip to content

Commit

Permalink
List manifests in the order given by the user
Browse files Browse the repository at this point in the history 
Fix GoogleContainerTools#2645

Signed-off-by: David Gageot <david@gageot.net>
  • Loading branch information
@dgageot
dgageot committed Aug 27, 2019
1 parent fb2107f commit 879a1c9
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 48 deletions.
112 changes: 79 additions & 33 deletions pkg/skaffold/deploy/kubectl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,33 +56,18 @@ spec:

func TestKubectlDeploy(t *testing.T) {
tests := []struct {
description string
cfg *latest.KubectlDeploy
builds []build.Artifact
command util.Command
shouldErr bool
forceDeploy bool
expectedDependencies []string
description string
cfg *latest.KubectlDeploy
builds []build.Artifact
command util.Command
shouldErr bool
forceDeploy bool
}{
{
description: "no manifest",
cfg: &latest.KubectlDeploy{},
command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion),
},
{
description: "missing manifest file",
cfg: &latest.KubectlDeploy{
Manifests: []string{"missing.yaml"},
},
command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion),
},
{
description: "ignore non-manifest",
cfg: &latest.KubectlDeploy{
Manifests: []string{"*.ignored"},
},
command: testutil.FakeRunOut(t, "kubectl version --client -ojson", kubectlVersion),
},
{
description: "deploy success (forced)",
cfg: &latest.KubectlDeploy{
Expand All @@ -96,8 +81,7 @@ func TestKubectlDeploy(t *testing.T) {
ImageName: "leeroy-web",
Tag: "leeroy-web:123",
}},
forceDeploy: true,
expectedDependencies: []string{"deployment.yaml"},
forceDeploy: true,
},
{
description: "deploy success",
Expand All @@ -112,7 +96,6 @@ func TestKubectlDeploy(t *testing.T) {
ImageName: "leeroy-web",
Tag: "leeroy-web:123",
}},
expectedDependencies: []string{"deployment.yaml"},
},
{
description: "http manifest",
Expand All @@ -127,7 +110,6 @@ func TestKubectlDeploy(t *testing.T) {
ImageName: "leeroy-web",
Tag: "leeroy-web:123",
}},
expectedDependencies: []string{"deployment.yaml"},
},
{
description: "deploy command error",
Expand All @@ -142,8 +124,7 @@ func TestKubectlDeploy(t *testing.T) {
ImageName: "leeroy-web",
Tag: "leeroy-web:123",
}},
shouldErr: true,
expectedDependencies: []string{"deployment.yaml"},
shouldErr: true,
},
{
description: "additional flags",
Expand All @@ -163,8 +144,7 @@ func TestKubectlDeploy(t *testing.T) {
ImageName: "leeroy-web",
Tag: "leeroy-web:123",
}},
shouldErr: true,
expectedDependencies: []string{"deployment.yaml"},
shouldErr: true,
},
}
for _, test := range tests {
Expand All @@ -191,11 +171,8 @@ func TestKubectlDeploy(t *testing.T) {
},
})

dependencies, err := k.Dependencies()
t.CheckNoError(err)
t.CheckDeepEqual(test.expectedDependencies, dependencies)
err := k.Deploy(context.Background(), ioutil.Discard, test.builds, nil).GetError()

err = k.Deploy(context.Background(), ioutil.Discard, test.builds, nil).GetError()
t.CheckError(test.shouldErr, err)
})
}
Expand Down Expand Up @@ -410,3 +387,72 @@ spec:
t.CheckNoError(err)
})
}

func TestDependencies(t *testing.T) {
tests := []struct {
description string
manifests []string
expected []string
}{
{
description: "no manifest",
manifests: []string(nil),
expected: []string(nil),
},
{
description: "missing manifest file",
manifests: []string{"missing.yaml"},
expected: []string(nil),
},
{
description: "ignore non-manifest",
manifests: []string{"*.ignored"},
expected: []string(nil),
},
{
description: "single manifest",
manifests: []string{"deployment.yaml"},
expected: []string{"deployment.yaml"},
},
{
description: "keep manifests order",
manifests: []string{"01_name.yaml", "00_service.yaml"},
expected: []string{"01_name.yaml", "00_service.yaml"},
},
{
description: "sort children",
manifests: []string{"01/*.yaml", "00/*.yaml"},
expected: []string{"01/a.yaml", "01/b.yaml", "00/a.yaml", "00/b.yaml"},
},
{
description: "http manifest",
manifests: []string{"deployment.yaml", "http://remote.yaml"},
expected: []string{"deployment.yaml"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().
Touch("deployment.yaml", "01_name.yaml", "00_service.yaml", "empty.ignored").
Touch("01/a.yaml", "01/b.yaml").
Touch("00/b.yaml", "00/a.yaml").
Chdir()

k := NewKubectlDeployer(&runcontext.RunContext{
Cfg: latest.Pipeline{
Deploy: latest.DeployConfig{
DeployType: latest.DeployType{
KubectlDeploy: &latest.KubectlDeploy{
Manifests: test.manifests,
},
},
},
},
})
dependencies, err := k.Dependencies()

t.CheckNoError(err)
t.CheckDeepEqual(test.expected, dependencies)
})
}
}
53 changes: 39 additions & 14 deletions pkg/skaffold/util/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,22 +84,45 @@ func StrSliceInsert(sl []string, index int, insert []string) []string {
return newSlice
}

// fileList holds a list of files, with no duplicate.
type fileList struct {
files []string
seen map[string]bool
}

func (l *fileList) Add(file string) {
if l.seen[file] {
return
}

if l.seen == nil {
l.seen = make(map[string]bool)
}
l.seen[file] = true

l.files = append(l.files, file)
}

func (l *fileList) Files() []string {
return l.files
}

// ExpandPathsGlob expands paths according to filepath.Glob patterns
// Returns a list of unique files that match the glob patterns passed in.
func ExpandPathsGlob(workingDir string, paths []string) ([]string, error) {
expandedPaths := make(map[string]bool)
var list fileList

for _, p := range paths {
if filepath.IsAbs(p) {
// This is a absolute file reference
expandedPaths[p] = true
list.Add(p)
continue
}

path := filepath.Join(workingDir, p)

if _, err := os.Stat(path); err == nil {
// This is a file reference, so just add it
expandedPaths[path] = true
list.Add(path)
continue
}

Expand All @@ -112,25 +135,27 @@ func ExpandPathsGlob(workingDir string, paths []string) ([]string, error) {
}

for _, f := range files {
err := filepath.Walk(f, func(path string, info os.FileInfo, err error) error {
var filesInDirectory []string

if err := filepath.Walk(f, func(path string, info os.FileInfo, err error) error {
if !info.IsDir() {
expandedPaths[path] = true
filesInDirectory = append(filesInDirectory, path)
}

return nil
})
if err != nil {
}); err != nil {
return nil, errors.Wrap(err, "filepath walk")
}

// Make sure files inside a directory are listed in a consistent order
sort.Strings(filesInDirectory)
for _, file := range filesInDirectory {
list.Add(file)
}
}
}

var ret []string
for k := range expandedPaths {
ret = append(ret, k)
}
sort.Strings(ret)
return ret, nil
return list.Files(), nil
}

// BoolPtr returns a pointer to a bool
Expand Down
7 changes: 6 additions & 1 deletion pkg/skaffold/util/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestExpandPathsGlob(t *testing.T) {
out: []string{"dir/sub_dir/file"},
},
{
description: "match top level glob",
description: "top level glob",
in: []string{"dir*"},
out: []string{"dir/sub_dir/file", "dir_b/sub_dir_b/file"},
},
Expand All @@ -87,6 +87,11 @@ func TestExpandPathsGlob(t *testing.T) {
in: []string{"[]"},
shouldErr: true,
},
{
description: "keep top level order",
in: []string{"dir_b/*", "dir/*"},
out: []string{"dir_b/sub_dir_b/file", "dir/sub_dir/file"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
Expand Down

0 comments on commit 879a1c9

Please sign in to comment.