From 26ceb9ec0de6088833b19a8116457f64d19574e3 Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Thu, 16 Jul 2020 12:08:04 -0700 Subject: [PATCH 1/8] add fallback to kubectl kustomize if kustomize binary isn't present in user's PATH --- pkg/skaffold/deploy/kubectl/cli.go | 5 ++ pkg/skaffold/deploy/kustomize.go | 29 ++++++++- pkg/skaffold/deploy/kustomize_test.go | 89 +++++++++++++++++++-------- 3 files changed, 94 insertions(+), 29 deletions(-) diff --git a/pkg/skaffold/deploy/kubectl/cli.go b/pkg/skaffold/deploy/kubectl/cli.go index d13345acb77..0c4d4742f9a 100644 --- a/pkg/skaffold/deploy/kubectl/cli.go +++ b/pkg/skaffold/deploy/kubectl/cli.go @@ -73,6 +73,11 @@ func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList) return nil } +// Kustomize runs `kubectl kustomize` with the provided args +func (c *CLI) Kustomize(ctx context.Context, args []string) ([]byte, error) { + return c.RunOut(ctx, "kustomize", c.args(nil, args...)...) +} + // ReadManifests reads a list of manifests in yaml format. func (c *CLI) ReadManifests(ctx context.Context, manifests []string) (ManifestList, error) { var list []string diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 6f5d03ea30e..ec3930d1dc7 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -47,6 +47,7 @@ var ( DefaultKustomizePath = "." kustomizeFilePaths = []string{"kustomization.yaml", "kustomization.yml", "Kustomization"} basePath = "base" + kustomizeBinaryCheck = kustomizeBinaryExists // For testing ) // kustomization is the content of a kustomization.yaml file. @@ -100,9 +101,13 @@ type KustomizeDeployer struct { BuildArgs []string globalConfig string addSkaffoldLabels bool + useKubectl bool } func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { + // if user has kustomize binary, prioritize that over kubectl kustomize + useKubectl := !kustomizeBinaryCheck() + return &KustomizeDeployer{ KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, kubectl: deploy.CLI{ @@ -114,9 +119,20 @@ func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { BuildArgs: runCtx.Cfg.Deploy.KustomizeDeploy.BuildArgs, globalConfig: runCtx.Opts.GlobalConfig, addSkaffoldLabels: runCtx.Opts.AddSkaffoldLabels, + useKubectl: useKubectl, } } +// Check for existence of kustomize binary in user's PATH +func kustomizeBinaryExists() bool { + cmd := exec.Command("which", "kustomize") + if err := util.RunCmd(cmd); err != nil { + return false + } + + return true +} + // Labels returns the labels specific to kustomize. func (k *KustomizeDeployer) Labels() map[string]string { return map[string]string{ @@ -366,8 +382,16 @@ func pathExistsLocally(filename string, workingDir string) (bool, os.FileMode) { func (k *KustomizeDeployer) readManifests(ctx context.Context) (deploy.ManifestList, error) { var manifests deploy.ManifestList for _, kustomizePath := range k.KustomizePaths { - cmd := exec.CommandContext(ctx, "kustomize", buildCommandArgs(k.BuildArgs, kustomizePath)...) - out, err := util.RunCmdOut(cmd) + var out []byte + var err error + + if k.useKubectl { + out, err = k.kubectl.Kustomize(ctx, buildCommandArgs(k.BuildArgs, kustomizePath)) + } else { + cmd := exec.CommandContext(ctx, "kustomize", append([]string{"build"}, buildCommandArgs(k.BuildArgs, kustomizePath)...)...) + out, err = util.RunCmdOut(cmd) + } + if err != nil { return nil, fmt.Errorf("kustomize build: %w", err) } @@ -382,7 +406,6 @@ func (k *KustomizeDeployer) readManifests(ctx context.Context) (deploy.ManifestL func buildCommandArgs(buildArgs []string, kustomizePath string) []string { var args []string - args = append(args, "build") if len(buildArgs) > 0 { for _, v := range buildArgs { diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 5c51eb31281..6b0b95c60bf 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -34,12 +34,13 @@ import ( func TestKustomizeDeploy(t *testing.T) { tests := []struct { - description string - cfg *latest.KustomizeDeploy - builds []build.Artifact - commands util.Command - shouldErr bool - forceDeploy bool + description string + cfg *latest.KustomizeDeploy + builds []build.Artifact + commands util.Command + shouldErr bool + forceDeploy bool + useMockKustomizeCheck bool }{ { description: "no manifest", @@ -47,7 +48,8 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRun("which kustomize"). + AndRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build .", ""), }, { @@ -56,7 +58,8 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRun("which kustomize"). + AndRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build .", deploymentWebYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), builds: []build.Artifact{{ @@ -71,7 +74,8 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"a", "b"}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRun("which kustomize"). + AndRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build a", deploymentWebYAML). AndRunOut("kustomize build b", deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), @@ -87,10 +91,41 @@ func TestKustomizeDeploy(t *testing.T) { }, forceDeploy: true, }, + { + description: "built-in kubectl kustomize", + cfg: &latest.KustomizeDeploy{ + KustomizePaths: []string{"a", "b"}, + }, + commands: testutil. + CmdRunOut("kubectl version --client -ojson", kubectlVersion118). + AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize a", deploymentWebYAML). + AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize b", deploymentAppYAML). + AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), + builds: []build.Artifact{ + { + ImageName: "leeroy-web", + Tag: "leeroy-web:123", + }, + { + ImageName: "leeroy-app", + Tag: "leeroy-app:123", + }, + }, + forceDeploy: true, + useMockKustomizeCheck: true, + }, + } + + mockKustomizeCheck := func() bool { + return false } + for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) + if test.useMockKustomizeCheck { + t.Override(&kustomizeBinaryCheck, mockKustomizeCheck) + } t.NewTempDir(). Chdir() @@ -131,7 +166,8 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: []string{tmpDir.Root()}, }, commands: testutil. - CmdRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). + CmdRun("which kustomize"). + AndRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). AndRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), }, { @@ -140,7 +176,8 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: tmpDir.Paths("a", "b"), }, commands: testutil. - CmdRunOut("kustomize build "+tmpDir.Path("a"), deploymentWebYAML). + CmdRun("which kustomize"). + AndRunOut("kustomize build "+tmpDir.Path("a"), deploymentWebYAML). AndRunOut("kustomize build "+tmpDir.Path("b"), deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), }, @@ -150,7 +187,8 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: []string{tmpDir.Root()}, }, commands: testutil. - CmdRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). + CmdRun("which kustomize"). + AndRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). AndRunErr("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -", errors.New("BUG")), shouldErr: true, }, @@ -159,11 +197,9 @@ func TestKustomizeCleanup(t *testing.T) { cfg: &latest.KustomizeDeploy{ KustomizePaths: []string{tmpDir.Root()}, }, - commands: testutil.CmdRunOutErr( - "kustomize build "+tmpDir.Root(), - "", - errors.New("BUG"), - ), + commands: testutil. + CmdRun("which kustomize"). + AndRunOutErr("kustomize build "+tmpDir.Root(), "", errors.New("BUG")), shouldErr: true, }, } @@ -415,49 +451,49 @@ func TestKustomizeBuildCommandArgs(t *testing.T) { description: "no BuildArgs, empty KustomizePaths ", buildArgs: []string{}, kustomizePath: "", - expectedArgs: []string{"build"}, + expectedArgs: nil, }, { description: "One BuildArg, empty KustomizePaths", buildArgs: []string{"--foo"}, kustomizePath: "", - expectedArgs: []string{"build", "--foo"}, + expectedArgs: []string{"--foo"}, }, { description: "no BuildArgs, non-empty KustomizePaths", buildArgs: []string{}, kustomizePath: "foo", - expectedArgs: []string{"build", "foo"}, + expectedArgs: []string{"foo"}, }, { description: "One BuildArg, non-empty KustomizePaths", buildArgs: []string{"--foo"}, kustomizePath: "bar", - expectedArgs: []string{"build", "--foo", "bar"}, + expectedArgs: []string{"--foo", "bar"}, }, { description: "Multiple BuildArg, empty KustomizePaths", buildArgs: []string{"--foo", "--bar"}, kustomizePath: "", - expectedArgs: []string{"build", "--foo", "--bar"}, + expectedArgs: []string{"--foo", "--bar"}, }, { description: "Multiple BuildArg with spaces, empty KustomizePaths", buildArgs: []string{"--foo bar", "--baz"}, kustomizePath: "", - expectedArgs: []string{"build", "--foo", "bar", "--baz"}, + expectedArgs: []string{"--foo", "bar", "--baz"}, }, { description: "Multiple BuildArg with spaces, non-empty KustomizePaths", buildArgs: []string{"--foo bar", "--baz"}, kustomizePath: "barfoo", - expectedArgs: []string{"build", "--foo", "bar", "--baz", "barfoo"}, + expectedArgs: []string{"--foo", "bar", "--baz", "barfoo"}, }, { description: "Multiple BuildArg no spaces, non-empty KustomizePaths", buildArgs: []string{"--foo", "bar", "--baz"}, kustomizePath: "barfoo", - expectedArgs: []string{"build", "--foo", "bar", "--baz", "barfoo"}, + expectedArgs: []string{"--foo", "bar", "--baz", "barfoo"}, }, } @@ -646,7 +682,8 @@ spec: testutil.Run(t, test.description, func(t *testutil.T) { var kustomizationPaths []string fakeCmd := testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112) + CmdRun("which kustomize"). + AndRunOut("kubectl version --client -ojson", kubectlVersion112) for _, kustomizationCall := range test.kustomizations { fakeCmd.AndRunOut("kustomize build "+kustomizationCall.folder, kustomizationCall.buildResult) kustomizationPaths = append(kustomizationPaths, kustomizationCall.folder) From 0c8e998b53b750e5c1650a68f3aad216ce5fb5e9 Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Thu, 23 Jul 2020 13:09:54 -0700 Subject: [PATCH 2/8] use exec.LookPath("kustomize") over "which kustomize", add kubectl version check --- pkg/skaffold/deploy/kustomize.go | 24 +++++++++++++++++++++--- pkg/skaffold/deploy/kustomize_test.go | 25 +++++++++---------------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index ec3930d1dc7..b5955744cbc 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -106,7 +106,7 @@ type KustomizeDeployer struct { func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { // if user has kustomize binary, prioritize that over kubectl kustomize - useKubectl := !kustomizeBinaryCheck() + useKubectl := !kustomizeBinaryCheck() && kubectlVersionCheck(runCtx) return &KustomizeDeployer{ KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, @@ -125,14 +125,32 @@ func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { // Check for existence of kustomize binary in user's PATH func kustomizeBinaryExists() bool { - cmd := exec.Command("which", "kustomize") - if err := util.RunCmd(cmd); err != nil { + if _, err := exec.LookPath("kustomize"); err != nil { return false } return true } +// Check that kubectl version is valid to use kubectl kustomize +func kubectlVersionCheck(runCtx *runcontext.RunContext) bool { + kubectl := deploy.CLI{ + CLI: kubectl.NewFromRunContext(runCtx), + Flags: runCtx.Cfg.Deploy.KustomizeDeploy.Flags, + ForceDeploy: runCtx.Opts.Force, + } + + gt, err := kubectl.CompareVersionTo(context.Background(), 1, 14) + if err != nil { + return false + } + if gt == 1 { + return true + } + + return false +} + // Labels returns the labels specific to kustomize. func (k *KustomizeDeployer) Labels() map[string]string { return map[string]string{ diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 6b0b95c60bf..4434ef7f1df 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -48,8 +48,7 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build .", ""), }, { @@ -58,8 +57,7 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build .", deploymentWebYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), builds: []build.Artifact{{ @@ -74,8 +72,7 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"a", "b"}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion112). AndRunOut("kustomize build a", deploymentWebYAML). AndRunOut("kustomize build b", deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), @@ -98,6 +95,7 @@ func TestKustomizeDeploy(t *testing.T) { }, commands: testutil. CmdRunOut("kubectl version --client -ojson", kubectlVersion118). + AndRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize a", deploymentWebYAML). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize b", deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), @@ -166,8 +164,7 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: []string{tmpDir.Root()}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). + CmdRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). AndRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), }, { @@ -176,8 +173,7 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: tmpDir.Paths("a", "b"), }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kustomize build "+tmpDir.Path("a"), deploymentWebYAML). + CmdRunOut("kustomize build "+tmpDir.Path("a"), deploymentWebYAML). AndRunOut("kustomize build "+tmpDir.Path("b"), deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -"), }, @@ -187,8 +183,7 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: []string{tmpDir.Root()}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). + CmdRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). AndRunErr("kubectl --context kubecontext --namespace testNamespace delete --ignore-not-found=true -f -", errors.New("BUG")), shouldErr: true, }, @@ -198,8 +193,7 @@ func TestKustomizeCleanup(t *testing.T) { KustomizePaths: []string{tmpDir.Root()}, }, commands: testutil. - CmdRun("which kustomize"). - AndRunOutErr("kustomize build "+tmpDir.Root(), "", errors.New("BUG")), + CmdRunOutErr("kustomize build "+tmpDir.Root(), "", errors.New("BUG")), shouldErr: true, }, } @@ -682,8 +676,7 @@ spec: testutil.Run(t, test.description, func(t *testutil.T) { var kustomizationPaths []string fakeCmd := testutil. - CmdRun("which kustomize"). - AndRunOut("kubectl version --client -ojson", kubectlVersion112) + CmdRunOut("kubectl version --client -ojson", kubectlVersion112) for _, kustomizationCall := range test.kustomizations { fakeCmd.AndRunOut("kustomize build "+kustomizationCall.folder, kustomizationCall.buildResult) kustomizationPaths = append(kustomizationPaths, kustomizationCall.folder) From 2fa2c999e7e27b1f6845f63bbf17a8f5b3fa693d Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Wed, 9 Sep 2020 11:12:05 -0700 Subject: [PATCH 3/8] fix kubectl version double call, nits --- pkg/skaffold/deploy/kustomize.go | 47 ++++++++++++--------------- pkg/skaffold/deploy/kustomize_test.go | 21 ++++++------ 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 93812fe8dd5..af0a8917103 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -95,30 +95,31 @@ type secretGenerator struct { type KustomizeDeployer struct { *latest.KustomizeDeploy - kubectl deploy.CLI - insecureRegistries map[string]bool - labels map[string]string - BuildArgs []string - globalConfig string - useKubectl bool + kubectl deploy.CLI + insecureRegistries map[string]bool + labels map[string]string + BuildArgs []string + globalConfig string + useKubectlKustomize bool } func NewKustomizeDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KustomizeDeployer { + kubectl := deploy.CLI{ + CLI: kubectl.NewFromRunContext(runCtx), + Flags: runCtx.Cfg.Deploy.KustomizeDeploy.Flags, + ForceDeploy: runCtx.Opts.Force, + } // if user has kustomize binary, prioritize that over kubectl kustomize - useKubectl := !kustomizeBinaryCheck() && kubectlVersionCheck(runCtx) + useKubectlKustomize := !kustomizeBinaryCheck() && kubectlVersionCheck(kubectl) return &KustomizeDeployer{ - KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, - kubectl: deploy.CLI{ - CLI: kubectl.NewFromRunContext(runCtx), - Flags: runCtx.Cfg.Deploy.KustomizeDeploy.Flags, - ForceDeploy: runCtx.Opts.Force, - }, - insecureRegistries: runCtx.InsecureRegistries, - BuildArgs: runCtx.Cfg.Deploy.KustomizeDeploy.BuildArgs, - globalConfig: runCtx.Opts.GlobalConfig, - labels: labels, - useKubectl: useKubectl, + KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, + kubectl: kubectl, + insecureRegistries: runCtx.InsecureRegistries, + BuildArgs: runCtx.Cfg.Deploy.KustomizeDeploy.BuildArgs, + globalConfig: runCtx.Opts.GlobalConfig, + labels: labels, + useKubectlKustomize: useKubectlKustomize, } } @@ -132,13 +133,7 @@ func kustomizeBinaryExists() bool { } // Check that kubectl version is valid to use kubectl kustomize -func kubectlVersionCheck(runCtx *runcontext.RunContext) bool { - kubectl := deploy.CLI{ - CLI: kubectl.NewFromRunContext(runCtx), - Flags: runCtx.Cfg.Deploy.KustomizeDeploy.Flags, - ForceDeploy: runCtx.Opts.Force, - } - +func kubectlVersionCheck(kubectl deploy.CLI) bool { gt, err := kubectl.CompareVersionTo(context.Background(), 1, 14) if err != nil { return false @@ -390,7 +385,7 @@ func (k *KustomizeDeployer) readManifests(ctx context.Context) (deploy.ManifestL var out []byte var err error - if k.useKubectl { + if k.useKubectlKustomize { out, err = k.kubectl.Kustomize(ctx, buildCommandArgs(k.BuildArgs, kustomizePath)) } else { cmd := exec.CommandContext(ctx, "kustomize", append([]string{"build"}, buildCommandArgs(k.BuildArgs, kustomizePath)...)...) diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 53441c0adf3..40e5c7568d6 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -34,13 +34,13 @@ import ( func TestKustomizeDeploy(t *testing.T) { tests := []struct { - description string - cfg *latest.KustomizeDeploy - builds []build.Artifact - commands util.Command - shouldErr bool - forceDeploy bool - useMockKustomizeCheck bool + description string + cfg *latest.KustomizeDeploy + builds []build.Artifact + commands util.Command + shouldErr bool + forceDeploy bool + kustomizeCmdAbsent bool }{ { description: "no manifest", @@ -95,7 +95,6 @@ func TestKustomizeDeploy(t *testing.T) { }, commands: testutil. CmdRunOut("kubectl version --client -ojson", kubectlVersion118). - AndRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize a", deploymentWebYAML). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize b", deploymentAppYAML). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), @@ -109,8 +108,8 @@ func TestKustomizeDeploy(t *testing.T) { Tag: "leeroy-app:123", }, }, - forceDeploy: true, - useMockKustomizeCheck: true, + forceDeploy: true, + kustomizeCmdAbsent: true, }, } @@ -121,7 +120,7 @@ func TestKustomizeDeploy(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) - if test.useMockKustomizeCheck { + if test.kustomizeCmdAbsent { t.Override(&kustomizeBinaryCheck, mockKustomizeCheck) } t.NewTempDir(). From c5a49d646b11f67286313f32ab043079951b313d Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Wed, 9 Sep 2020 11:35:22 -0700 Subject: [PATCH 4/8] trying to kickoff travis From 61796f3435640dfbc7b801af3a02b493009d267e Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Wed, 9 Sep 2020 12:07:48 -0700 Subject: [PATCH 5/8] remove buildargs --- pkg/skaffold/deploy/kustomize.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index f8f7f0363e7..725b6099d34 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -96,7 +96,6 @@ type KustomizeDeployer struct { kubectl deploy.CLI insecureRegistries map[string]bool labels map[string]string - BuildArgs []string globalConfig string useKubectlKustomize bool } @@ -108,11 +107,10 @@ func NewKustomizeDeployer(cfg Config, labels map[string]string) *KustomizeDeploy useKubectlKustomize := !kustomizeBinaryCheck() && kubectlVersionCheck(kubectl) return &KustomizeDeployer{ - KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, + KustomizeDeploy: cfg.Pipeline().Deploy.KustomizeDeploy, kubectl: kubectl, - insecureRegistries: runCtx.InsecureRegistries, - BuildArgs: runCtx.Cfg.Deploy.KustomizeDeploy.BuildArgs, - globalConfig: runCtx.Opts.GlobalConfig, + insecureRegistries: cfg.GetInsecureRegistries(), + globalConfig: cfg.GlobalConfig(), labels: labels, useKubectlKustomize: useKubectlKustomize, } From 9d6967d359cfb366c014fbc00227999114befe14 Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Wed, 9 Sep 2020 12:39:04 -0700 Subject: [PATCH 6/8] fix new test and kpt deployer kustomize call --- pkg/skaffold/deploy/kpt.go | 2 +- pkg/skaffold/deploy/kustomize_test.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/deploy/kpt.go b/pkg/skaffold/deploy/kpt.go index 6beb10a34d7..a66b1480bb3 100644 --- a/pkg/skaffold/deploy/kpt.go +++ b/pkg/skaffold/deploy/kpt.go @@ -226,7 +226,7 @@ func (k *KptDeployer) kustomizeBuild(ctx context.Context) error { return nil } - cmd := exec.CommandContext(ctx, "kustomize", buildCommandArgs([]string{"-o", filepath.Join(pipeline, k.Dir)}, k.Dir)...) + cmd := exec.CommandContext(ctx, "kustomize", append([]string{"build"}, buildCommandArgs([]string{"-o", filepath.Join(pipeline, k.Dir)}, k.Dir)...)...) if _, err := util.RunCmdOut(cmd); err != nil { return err } diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 0d5b276511c..a453c0abe8d 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -93,22 +93,23 @@ func TestKustomizeDeploy(t *testing.T) { }, { description: "built-in kubectl kustomize", - cfg: &latest.KustomizeDeploy{ + kustomize: latest.KustomizeDeploy{ KustomizePaths: []string{"a", "b"}, }, commands: testutil. CmdRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize a", deploymentWebYAML). AndRunOut("kubectl --context kubecontext --namespace testNamespace kustomize b", deploymentAppYAML). + AndRunInputOut("kubectl --context kubecontext --namespace testNamespace get -f - --ignore-not-found -ojson", deploymentWebYAMLv1+"\n---\n"+deploymentAppYAMLv1, ""). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), builds: []build.Artifact{ { ImageName: "leeroy-web", - Tag: "leeroy-web:123", + Tag: "leeroy-web:v1", }, { ImageName: "leeroy-app", - Tag: "leeroy-app:123", + Tag: "leeroy-app:v1", }, }, forceDeploy: true, From 1853c63abc8dd3ad557b9e2328fa3288b61d3fd6 Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Wed, 9 Sep 2020 15:18:47 -0700 Subject: [PATCH 7/8] add another mock kustomize check to fake true --- pkg/skaffold/deploy/kustomize_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index a453c0abe8d..551ab90e752 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -33,6 +33,14 @@ import ( "github.com/GoogleContainerTools/skaffold/testutil" ) +func kustomizeAbsent() bool { + return false +} + +func kustomizePresent() bool { + return true +} + func TestKustomizeDeploy(t *testing.T) { tests := []struct { description string @@ -117,15 +125,11 @@ func TestKustomizeDeploy(t *testing.T) { }, } - mockKustomizeCheck := func() bool { - return false - } - for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) if test.kustomizeCmdAbsent { - t.Override(&kustomizeBinaryCheck, mockKustomizeCheck) + t.Override(&kustomizeBinaryCheck, kustomizeAbsent) } t.NewTempDir(). Chdir() @@ -198,6 +202,7 @@ func TestKustomizeCleanup(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) + t.Override(&kustomizeBinaryCheck, kustomizePresent) k := NewKustomizeDeployer(&kustomizeConfig{ workingDir: tmpDir.Root(), From 095909f759523bab533cce78cabdf35b52658f2a Mon Sep 17 00:00:00 2001 From: Marlon Gamez Date: Mon, 21 Sep 2020 14:46:25 -0700 Subject: [PATCH 8/8] cleanup some functions and tests --- pkg/skaffold/deploy/kustomize.go | 11 ++----- pkg/skaffold/deploy/kustomize_test.go | 44 +++++++++++---------------- 2 files changed, 21 insertions(+), 34 deletions(-) diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 725b6099d34..2eb0329403b 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -118,11 +118,9 @@ func NewKustomizeDeployer(cfg Config, labels map[string]string) *KustomizeDeploy // Check for existence of kustomize binary in user's PATH func kustomizeBinaryExists() bool { - if _, err := exec.LookPath("kustomize"); err != nil { - return false - } + _, err := exec.LookPath("kustomize") - return true + return err == nil } // Check that kubectl version is valid to use kubectl kustomize @@ -131,11 +129,8 @@ func kubectlVersionCheck(kubectl deploy.CLI) bool { if err != nil { return false } - if gt == 1 { - return true - } - return false + return gt == 1 } // Deploy runs `kubectl apply` on the manifest generated by kustomize. diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 551ab90e752..2aea72b428a 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -33,23 +33,15 @@ import ( "github.com/GoogleContainerTools/skaffold/testutil" ) -func kustomizeAbsent() bool { - return false -} - -func kustomizePresent() bool { - return true -} - func TestKustomizeDeploy(t *testing.T) { tests := []struct { - description string - kustomize latest.KustomizeDeploy - builds []build.Artifact - commands util.Command - shouldErr bool - forceDeploy bool - kustomizeCmdAbsent bool + description string + kustomize latest.KustomizeDeploy + builds []build.Artifact + commands util.Command + shouldErr bool + forceDeploy bool + kustomizeCmdPresent bool }{ { description: "no manifest", @@ -57,8 +49,9 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kustomize build .", ""), + kustomizeCmdPresent: true, }, { description: "deploy success", @@ -66,7 +59,7 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"."}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kustomize build .", deploymentWebYAML). AndRunInputOut("kubectl --context kubecontext --namespace testNamespace get -f - --ignore-not-found -ojson", deploymentWebYAMLv1, ""). AndRun("kubectl --context kubecontext --namespace testNamespace apply -f - --force --grace-period=0"), @@ -74,7 +67,8 @@ func TestKustomizeDeploy(t *testing.T) { ImageName: "leeroy-web", Tag: "leeroy-web:v1", }}, - forceDeploy: true, + forceDeploy: true, + kustomizeCmdPresent: true, }, { description: "deploy success with multiple kustomizations", @@ -82,7 +76,7 @@ func TestKustomizeDeploy(t *testing.T) { KustomizePaths: []string{"a", "b"}, }, commands: testutil. - CmdRunOut("kubectl version --client -ojson", kubectlVersion112). + CmdRunOut("kubectl version --client -ojson", kubectlVersion118). AndRunOut("kustomize build a", deploymentWebYAML). AndRunOut("kustomize build b", deploymentAppYAML). AndRunInputOut("kubectl --context kubecontext --namespace testNamespace get -f - --ignore-not-found -ojson", deploymentWebYAMLv1+"\n---\n"+deploymentAppYAMLv1, ""). @@ -97,7 +91,8 @@ func TestKustomizeDeploy(t *testing.T) { Tag: "leeroy-app:v1", }, }, - forceDeploy: true, + forceDeploy: true, + kustomizeCmdPresent: true, }, { description: "built-in kubectl kustomize", @@ -120,17 +115,14 @@ func TestKustomizeDeploy(t *testing.T) { Tag: "leeroy-app:v1", }, }, - forceDeploy: true, - kustomizeCmdAbsent: true, + forceDeploy: true, }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) - if test.kustomizeCmdAbsent { - t.Override(&kustomizeBinaryCheck, kustomizeAbsent) - } + t.Override(&kustomizeBinaryCheck, func() bool { return test.kustomizeCmdPresent }) t.NewTempDir(). Chdir() @@ -202,7 +194,7 @@ func TestKustomizeCleanup(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) - t.Override(&kustomizeBinaryCheck, kustomizePresent) + t.Override(&kustomizeBinaryCheck, func() bool { return true }) k := NewKustomizeDeployer(&kustomizeConfig{ workingDir: tmpDir.Root(),