Skip to content

feat: add better support and messaging around using helm with skaffold apply #7149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion pkg/skaffold/runner/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,43 @@ func (d *deployerCtx) JSONParseConfig() v1.JSONParseConfig {

// GetDeployer creates a deployer from a given RunContext and deploy pipeline definitions.
func GetDeployer(ctx context.Context, runCtx *runcontext.RunContext, labeller *label.DefaultLabeller) (deploy.Deployer, error) {
deployerCfg := runCtx.Deployers()

if runCtx.Opts.Apply {
helmNamespaces := make(map[string]bool)
nonHelmDeployFound := false

for _, d := range deployerCfg {
if d.DockerDeploy != nil || d.KptDeploy != nil || d.KubectlDeploy != nil || d.KustomizeDeploy != nil {
nonHelmDeployFound = true
}

if d.HelmDeploy != nil {
for _, release := range d.HelmDeploy.Releases {
if release.Namespace != "" {
helmNamespaces[release.Namespace] = true
}
}
}
}

if len(helmNamespaces) > 1 || (nonHelmDeployFound && len(helmNamespaces) == 1) {
return nil, errors.New("skaffold apply called with conflicting namespaces set via skaffold.yaml. This is likely due to the use of the 'deploy.helm.releases.*.namespace' field which is not supported in apply. Remove the 'deploy.helm.releases.*.namespace' field(s) and run skaffold apply again")
}

if len(helmNamespaces) == 1 && !nonHelmDeployFound {
if runCtx.Opts.Namespace == "" {
// if skaffold --namespace flag not set, use the helm namespace value
for k := range helmNamespaces {
// map only has 1 (k,v) from length check in if condition
runCtx.Opts.Namespace = k
}
}
}

return getDefaultDeployer(runCtx, labeller)
}

deployerCfg := runCtx.Deployers()
localDeploy := false
remoteDeploy := false

Expand Down
53 changes: 53 additions & 0 deletions pkg/skaffold/runner/deployer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,59 @@ func TestGetDeployer(tOuter *testing.T) {
&kpt.Deployer{},
}, false),
},
{
description: "apply does not allow multiple deployers when a helm namespace is set",
apply: true,
cfg: latestV1.DeployType{
HelmDeploy: &latestV1.HelmDeploy{
Releases: []latestV1.HelmRelease{
{
Namespace: "foo",
},
},
},
KubectlDeploy: &latestV1.KubectlDeploy{},
},
shouldErr: true,
},
{
description: "apply does not allow multiple helm releases with different namespaces set",
apply: true,
cfg: latestV1.DeployType{
HelmDeploy: &latestV1.HelmDeploy{
Releases: []latestV1.HelmRelease{
{
Namespace: "foo",
},
{
Namespace: "bar",
},
},
},
},
shouldErr: true,
},
{
description: "apply does allow multiple helm releases with the same namespace set",
apply: true,
cfg: latestV1.DeployType{
HelmDeploy: &latestV1.HelmDeploy{
Releases: []latestV1.HelmRelease{
{
Namespace: "foo",
},
{
Namespace: "foo",
},
},
},
},
expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{
Pipelines: runcontext.NewPipelines([]latestV1.Pipeline{{}}),
}, &label.DefaultLabeller{}, &latestV1.KubectlDeploy{
Flags: latestV1.KubectlFlags{},
})).(deploy.Deployer),
},
}
for _, test := range tests {
testutil.Run(tOuter, test.description, func(t *testutil.T) {
Expand Down