From dadfc8ee495559127744d15a848f58bd34cf81f4 Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Fri, 17 Jul 2020 10:02:33 -0700 Subject: [PATCH] switch ordering of params and pass nil over empty map --- pkg/skaffold/deploy/helm.go | 2 +- pkg/skaffold/deploy/helm_test.go | 10 +++++----- pkg/skaffold/deploy/kubectl.go | 2 +- pkg/skaffold/deploy/kubectl_test.go | 28 +++++++++++++-------------- pkg/skaffold/deploy/kustomize.go | 2 +- pkg/skaffold/deploy/kustomize_test.go | 16 +++++++-------- pkg/skaffold/runner/new.go | 6 +++--- 7 files changed, 33 insertions(+), 33 deletions(-) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index a40ed6cf21b..eb76c084221 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -82,7 +82,7 @@ type HelmDeployer struct { } // NewHelmDeployer returns a configured HelmDeployer -func NewHelmDeployer(labels map[string]string, runCtx *runcontext.RunContext) *HelmDeployer { +func NewHelmDeployer(runCtx *runcontext.RunContext, labels map[string]string) *HelmDeployer { return &HelmDeployer{ HelmDeploy: runCtx.Cfg.Deploy.HelmDeploy, kubeContext: runCtx.KubeContext, diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index 67aeefb36a7..a9aa4687960 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -769,7 +769,7 @@ func TestHelmDeploy(t *testing.T) { event.InitializeState(test.runContext.Cfg, "test", true, true, true) - deployer := NewHelmDeployer(map[string]string{}, test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) deployer.pkgTmpDir = tmpDir result := deployer.Deploy(context.Background(), ioutil.Discard, test.builds) @@ -838,7 +838,7 @@ func TestHelmCleanup(t *testing.T) { event.InitializeState(test.runContext.Cfg, "test", true, true, true) - deployer := NewHelmDeployer(map[string]string{}, test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) err := deployer.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) t.CheckDeepEqual(test.expectedWarnings, fakeWarner.Warnings) @@ -936,7 +936,7 @@ func TestHelmDependencies(t *testing.T) { tmpDir := t.NewTempDir(). Touch(test.files...) - deployer := NewHelmDeployer(map[string]string{}, makeRunContext(latest.HelmDeploy{ + deployer := NewHelmDeployer(makeRunContext(latest.HelmDeploy{ Releases: []latest.HelmRelease{{ Name: "skaffold-helm", ChartPath: tmpDir.Root(), @@ -947,7 +947,7 @@ func TestHelmDependencies(t *testing.T) { SkipBuildDependencies: test.skipBuildDependencies, Remote: test.remote, }}, - }, false)) + }, false), nil) deps, err := deployer.Dependencies() @@ -1119,7 +1119,7 @@ func TestHelmRender(t *testing.T) { file = t.NewTempDir().Path(test.outputFile) } - deployer := NewHelmDeployer(map[string]string{}, test.runContext) + deployer := NewHelmDeployer(test.runContext, nil) t.Override(&util.DefaultExecCommand, test.commands) err := deployer.Render(context.Background(), ioutil.Discard, test.builds, true, file) diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index f772105433d..98835844452 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -58,7 +58,7 @@ type KubectlDeployer struct { // NewKubectlDeployer returns a new KubectlDeployer for a DeployConfig filled // with the needed configuration for `kubectl apply` -func NewKubectlDeployer(labels map[string]string, runCtx *runcontext.RunContext) *KubectlDeployer { +func NewKubectlDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KubectlDeployer { return &KubectlDeployer{ KubectlDeploy: runCtx.Cfg.Deploy.KubectlDeploy, workingDir: runCtx.WorkingDir, diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 18874dee71e..5b1f9116597 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -191,7 +191,7 @@ func TestKubectlDeploy(t *testing.T) { Touch("empty.ignored"). Chdir() - k := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -205,7 +205,7 @@ func TestKubectlDeploy(t *testing.T) { Namespace: testNamespace, Force: test.forceDeploy, }, - }) + }, nil) err := k.Deploy(context.Background(), ioutil.Discard, test.builds).GetError() @@ -275,7 +275,7 @@ func TestKubectlCleanup(t *testing.T) { Write("deployment.yaml", deploymentWebYAML). Chdir() - k := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -288,7 +288,7 @@ func TestKubectlCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) @@ -330,7 +330,7 @@ func TestKubectlDeployerRemoteCleanup(t *testing.T) { Write("deployment.yaml", deploymentWebYAML). Chdir() - k := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -343,7 +343,7 @@ func TestKubectlDeployerRemoteCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckNoError(err) @@ -392,7 +392,7 @@ spec: cfg := &latest.KubectlDeploy{ Manifests: []string{tmpDir.Path("deployment-app.yaml"), "deployment-web.yaml"}, } - deployer := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + deployer := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: tmpDir.Root(), Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -406,7 +406,7 @@ spec: Namespace: testNamespace, AddSkaffoldLabels: true, }, - }) + }, nil) // Deploy one manifest err := deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ @@ -481,7 +481,7 @@ func TestDependencies(t *testing.T) { Touch("00/b.yaml", "00/a.yaml"). Chdir() - k := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKubectlDeployer(&runcontext.RunContext{ Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ @@ -491,7 +491,7 @@ func TestDependencies(t *testing.T) { }, }, }, - }) + }, nil) dependencies, err := k.Dependencies() t.CheckNoError(err) @@ -606,7 +606,7 @@ spec: AndRunOut("kubectl --context kubecontext create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), test.input)) defaultRepo := config.StringOrUndefined{} defaultRepo.Set("gcr.io/project") - deployer := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + deployer := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -622,7 +622,7 @@ spec: AddSkaffoldLabels: true, DefaultRepo: defaultRepo, }, - }) + }, nil) var b bytes.Buffer err := deployer.Render(context.Background(), &b, test.builds, true, "") t.CheckNoError(err) @@ -660,7 +660,7 @@ func TestGCSManifests(t *testing.T) { if err := ioutil.WriteFile(manifestTmpDir+"/deployment.yaml", []byte(deploymentWebYAML), os.ModePerm); err != nil { t.Fatal(err) } - k := NewKubectlDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -674,7 +674,7 @@ func TestGCSManifests(t *testing.T) { Namespace: testNamespace, SkipRender: test.skipRender, }, - }) + }, nil) err := k.Deploy(context.Background(), ioutil.Discard, nil).GetError() diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 8452b52cdaf..2898848cf97 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -102,7 +102,7 @@ type KustomizeDeployer struct { addSkaffoldLabels bool } -func NewKustomizeDeployer(labels map[string]string, runCtx *runcontext.RunContext) *KustomizeDeployer { +func NewKustomizeDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KustomizeDeployer { return &KustomizeDeployer{ KustomizeDeploy: runCtx.Cfg.Deploy.KustomizeDeploy, kubectl: deploy.CLI{ diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index e583f21b1ad..444b9147319 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -94,7 +94,7 @@ func TestKustomizeDeploy(t *testing.T) { t.NewTempDir(). Chdir() - k := NewKustomizeDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKustomizeDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -108,7 +108,7 @@ func TestKustomizeDeploy(t *testing.T) { Namespace: testNamespace, Force: test.forceDeploy, }, - }) + }, nil) err := k.Deploy(context.Background(), ioutil.Discard, test.builds).GetError() t.CheckError(test.shouldErr, err) @@ -171,7 +171,7 @@ func TestKustomizeCleanup(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.commands) - k := NewKustomizeDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKustomizeDeployer(&runcontext.RunContext{ WorkingDir: tmpDir.Root(), Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -184,7 +184,7 @@ func TestKustomizeCleanup(t *testing.T) { Opts: config.SkaffoldOptions{ Namespace: testNamespace, }, - }) + }, nil) err := k.Cleanup(context.Background(), ioutil.Discard) t.CheckError(test.shouldErr, err) @@ -385,7 +385,7 @@ func TestDependenciesForKustomization(t *testing.T) { tmpDir.Write(path, contents) } - k := NewKustomizeDeployer(map[string]string{}, &runcontext.RunContext{ + k := NewKustomizeDeployer(&runcontext.RunContext{ Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ @@ -396,7 +396,7 @@ func TestDependenciesForKustomization(t *testing.T) { }, }, KubeContext: testKubeContext, - }) + }, nil) deps, err := k.Dependencies() t.CheckErrorAndDeepEqual(test.shouldErr, err, tmpDir.Paths(test.expected...), deps) @@ -636,7 +636,7 @@ spec: t.NewTempDir(). Chdir() - k := NewKustomizeDeployer(test.labels, &runcontext.RunContext{ + k := NewKustomizeDeployer(&runcontext.RunContext{ WorkingDir: ".", Cfg: latest.Pipeline{ Deploy: latest.DeployConfig{ @@ -652,7 +652,7 @@ spec: Namespace: testNamespace, AddSkaffoldLabels: true, }, - }) + }, test.labels) var b bytes.Buffer err := k.Render(context.Background(), &b, test.builds, true, "") t.CheckError(test.shouldErr, err) diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index f59147cef5c..1fffece8357 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -195,15 +195,15 @@ func getDeployer(runCtx *runcontext.RunContext) deploy.Deployer { var deployers deploy.DeployerMux if runCtx.Cfg.Deploy.HelmDeploy != nil { - deployers = append(deployers, deploy.NewHelmDeployer(labels, runCtx)) + deployers = append(deployers, deploy.NewHelmDeployer(runCtx, labels)) } if runCtx.Cfg.Deploy.KubectlDeploy != nil { - deployers = append(deployers, deploy.NewKubectlDeployer(labels, runCtx)) + deployers = append(deployers, deploy.NewKubectlDeployer(runCtx, labels)) } if runCtx.Cfg.Deploy.KustomizeDeploy != nil { - deployers = append(deployers, deploy.NewKustomizeDeployer(labels, runCtx)) + deployers = append(deployers, deploy.NewKustomizeDeployer(runCtx, labels)) } // avoid muxing overhead when only a single deployer is configured