Skip to content

Commit

Permalink
switch ordering of params and pass nil over empty map
Browse files Browse the repository at this point in the history
  • Loading branch information
nkubala committed Jul 17, 2020
1 parent c3f8690 commit dadfc8e
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 33 deletions.
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions pkg/skaffold/deploy/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand All @@ -947,7 +947,7 @@ func TestHelmDependencies(t *testing.T) {
SkipBuildDependencies: test.skipBuildDependencies,
Remote: test.remote,
}},
}, false))
}, false), nil)

deps, err := deployer.Dependencies()

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 14 additions & 14 deletions pkg/skaffold/deploy/kubectl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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()

Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -406,7 +406,7 @@ spec:
Namespace: testNamespace,
AddSkaffoldLabels: true,
},
})
}, nil)

// Deploy one manifest
err := deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{
Expand Down Expand Up @@ -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{
Expand All @@ -491,7 +491,7 @@ func TestDependencies(t *testing.T) {
},
},
},
})
}, nil)
dependencies, err := k.Dependencies()

t.CheckNoError(err)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -674,7 +674,7 @@ func TestGCSManifests(t *testing.T) {
Namespace: testNamespace,
SkipRender: test.skipRender,
},
})
}, nil)

err := k.Deploy(context.Background(), ioutil.Discard, nil).GetError()

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/kustomize.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
16 changes: 8 additions & 8 deletions pkg/skaffold/deploy/kustomize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/runner/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit dadfc8e

Please sign in to comment.