diff --git a/pkg/skaffold/deploy/helm/helm.go b/pkg/skaffold/deploy/helm/helm.go index 3102fd8d952..344477564e5 100644 --- a/pkg/skaffold/deploy/helm/helm.go +++ b/pkg/skaffold/deploy/helm/helm.go @@ -285,40 +285,42 @@ func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Art // Group releases by their dependency level to deploy them in the correct order. levelGroups := groupReleasesByLevel(deploymentOrder, dependencyGraph) - g, levelCtx := errgroup.WithContext(ctx) - + var concurrency int if h.Concurrency == nil || *h.Concurrency == 1 { - g.SetLimit(1) + concurrency = 1 olog.Entry(ctx).Infof("Installing %d releases sequentially", len(h.Releases)) } else { - g.SetLimit(*h.Concurrency) + if *h.Concurrency == 0 { + concurrency = -1 + } else { + concurrency = *h.Concurrency + } olog.Entry(ctx).Infof("Installing %d releases concurrently", len(h.Releases)) } releaseNameToRelease := make(map[string]latest.HelmRelease) for _, r := range h.Releases { - releaseName, err := util.ExpandEnvTemplateOrFail(r.Name, nil) - if err != nil { - return fmt.Errorf("cannot parse the release name template: %w", err) - } - releaseNameToRelease[releaseName] = r + releaseNameToRelease[r.Name] = r } // Process each level in order for level, releases := range levelGroups { if len(levelGroups) > 1 { - olog.Entry(ctx).Infof("Installing level %d/%d releases (%d releases)", level, len(levelGroups), len(releases)) + olog.Entry(ctx).Infof("Installing level %d/%d releases (%d releases)", level+1, len(levelGroups), len(releases)) } else { olog.Entry(ctx).Infof("Installing releases (%d releases)", len(releases)) } + g, levelCtx := errgroup.WithContext(ctx) + g.SetLimit(concurrency) + // sort releases in the same level, this is merely to ensure that the series of helm commands are in order for // consistency in unit testing. sort.Strings(releases) // Deploy releases in current level - for _, releaseName := range releases { - release := releaseNameToRelease[releaseName] + for _, name := range releases { + release := releaseNameToRelease[name] g.Go(func() error { chartVersion, err := util.ExpandEnvTemplateOrFail(release.Version, nil) @@ -336,6 +338,11 @@ func (h *Deployer) Deploy(ctx context.Context, out io.Writer, builds []graph.Art return helm.UserErr(fmt.Sprintf("cannot expand chart path %q", release.ChartPath), err) } + releaseName, err := util.ExpandEnvTemplateOrFail(release.Name, nil) + if err != nil { + return helm.UserErr(fmt.Sprintf("cannot expand release name %q", release.Name), err) + } + m, results, err := h.deployRelease(levelCtx, out, releaseName, release, builds, h.bV, chartVersion, repo) if err != nil { return helm.UserErr(fmt.Sprintf("deploying %q", releaseName), err) diff --git a/pkg/skaffold/deploy/helm/helm_test.go b/pkg/skaffold/deploy/helm/helm_test.go index 15034222854..248d0015261 100644 --- a/pkg/skaffold/deploy/helm/helm_test.go +++ b/pkg/skaffold/deploy/helm/helm_test.go @@ -66,6 +66,21 @@ var testDeployConfig = latest.LegacyHelmDeploy{ }}, } +var testDeployWithDependsOnConfig = latest.LegacyHelmDeploy{ + Releases: []latest.HelmRelease{{ + Name: "skaffold-helm-{{.FOO}}", + ChartPath: "examples/test", + Overrides: schemautil.HelmOverrides{Values: map[string]interface{}{"foo": "bar"}}, + SetValues: map[string]string{ + "some.key": "somevalue", + }, + DependsOn: []string{"other-{{.FOO}}"}, + }, { + Name: "other-{{.FOO}}", + ChartPath: "examples/test", + }}, +} + var testDeployNamespacedConfig = latest.LegacyHelmDeploy{ Releases: []latest.HelmRelease{{ Name: "skaffold-helm", @@ -446,6 +461,24 @@ func TestHelmDeploy(t *testing.T) { builds: testBuilds, expectedNamespaces: []string{""}, }, + { + description: "helm3.1 deploy with dependsOn success", + commands: testutil. + CmdRunWithOutput("helm version --client", version31). + AndRun("helm --kube-context kubecontext get all other-FOOBAR --kubeconfig kubeconfig"). + AndRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). + AndRunEnv("helm --kube-context kubecontext upgrade other-FOOBAR examples/test --post-renderer SKAFFOLD-BINARY --kubeconfig kubeconfig", + []string{"SKAFFOLD_FILENAME=test.yaml", "SKAFFOLD_CMDLINE=filter --kube-context kubecontext --build-artifacts TMPFILE --kubeconfig kubeconfig"}). + AndRunWithOutput("helm --kube-context kubecontext get all other-FOOBAR --template {{.Release.Manifest}} --kubeconfig kubeconfig", validDeployYaml). + AndRun("helm --kube-context kubecontext get all skaffold-helm-FOOBAR --kubeconfig kubeconfig"). + AndRun("helm --kube-context kubecontext dep build examples/test --kubeconfig kubeconfig"). + AndRunEnv("helm --kube-context kubecontext upgrade skaffold-helm-FOOBAR examples/test --post-renderer SKAFFOLD-BINARY --set some.key=somevalue -f skaffold-overrides.yaml --kubeconfig kubeconfig", + []string{"SKAFFOLD_FILENAME=test.yaml", "SKAFFOLD_CMDLINE=filter --kube-context kubecontext --build-artifacts TMPFILE --kubeconfig kubeconfig"}). + AndRunWithOutput("helm --kube-context kubecontext get all skaffold-helm-FOOBAR --template {{.Release.Manifest}} --kubeconfig kubeconfig", validDeployYaml), + helm: testDeployWithDependsOnConfig, + builds: testBuilds, + expectedNamespaces: []string{""}, + }, { description: "helm3.1 namespaced context deploy success", commands: testutil. diff --git a/pkg/skaffold/deploy/helm/util.go b/pkg/skaffold/deploy/helm/util.go index dbb8e599cbd..e225758179a 100644 --- a/pkg/skaffold/deploy/helm/util.go +++ b/pkg/skaffold/deploy/helm/util.go @@ -19,19 +19,13 @@ package helm import ( "fmt" - "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/helm" "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest" - "github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/util" ) func BuildDependencyGraph(releases []latest.HelmRelease) (map[string][]string, error) { dependencyGraph := make(map[string][]string) for _, r := range releases { - releaseName, err := util.ExpandEnvTemplateOrFail(r.Name, nil) - if err != nil { - return nil, helm.UserErr(fmt.Sprintf("cannot expand release name %q", r.Name), err) - } - dependencyGraph[releaseName] = r.DependsOn + dependencyGraph[r.Name] = r.DependsOn } return dependencyGraph, nil diff --git a/pkg/skaffold/deploy/helm/util_test.go b/pkg/skaffold/deploy/helm/util_test.go index 4336f24f30b..a7921f0a810 100644 --- a/pkg/skaffold/deploy/helm/util_test.go +++ b/pkg/skaffold/deploy/helm/util_test.go @@ -67,11 +67,15 @@ func TestBuildDependencyGraph(t *testing.T) { }, }, { - description: "invalid release name template", + description: "simple dependency graph with placeholder in name", releases: []latest.HelmRelease{ - {Name: "{{.Invalid}}"}, + {Name: "release1-{{.Service}}", DependsOn: []string{"release2-{{.Service}}"}}, + {Name: "release2-{{.Service}}", DependsOn: []string{}}, + }, + expected: map[string][]string{ + "release1-{{.Service}}": {"release2-{{.Service}}"}, + "release2-{{.Service}}": {}, }, - shouldErr: true, }, }