From 195c87c13e16e3bc91002453930b9d60f16ee5ff Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 14 May 2019 09:24:12 -0700 Subject: [PATCH 1/4] Remove build dep for helm deploy On master, skaffold deploy fails with following error. ``` skaffold deploy -f examples/helm-deployment/skaffold.yaml Starting deploy... Error: release: "skaffold-helm" not found Helm release skaffold-helm not installed. Installing... FATA[0002] deploying skaffold-helm: matching build results to chart values: no build present for gcr.io/k8s-skaffold/skaffold-helm ``` This is because in #922, we removed deploy triggering the build. Running deploy should use the default tag i.e. "latest" when depoying images. --- pkg/skaffold/deploy/helm.go | 11 ++++++----- pkg/skaffold/deploy/helm_test.go | 5 ++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index a4b825a8aa3..9c1c9f1e41c 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -401,18 +401,19 @@ func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r lates func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) { imageToBuildResult := map[string]build.Artifact{} - for _, build := range builds { - imageToBuildResult[build.ImageName] = build + for _, b := range builds { + imageToBuildResult[b.ImageName] = b } paramToBuildResult := map[string]build.Artifact{} for param, imageName := range params { newImageName := util.SubstituteDefaultRepoIntoImage(h.defaultRepo, imageName) - build, ok := imageToBuildResult[newImageName] + b, ok := imageToBuildResult[newImageName] if !ok { - return nil, fmt.Errorf("no build present for %s", imageName) + logrus.Warnf("no build present for %s. Continuing with %s", imageName, imageName) + b = build.Artifact{ImageName: imageName, Tag: imageName} } - paramToBuildResult[param] = build + paramToBuildResult[param] = b } return paramToBuildResult, nil } diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index 37c36d1bf5f..77cb1715618 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -212,7 +212,7 @@ spec: containers: - name: skaffold-helm image: gcr.io/nick-cloudbuild/skaffold-helm:f759510436c8fd6f7ffa13dd9e9d85e64bec8d2bfd12c5aa3fb9af1288eccdab - imagePullPolicy: + imagePullPolicy: command: ["/bin/bash", "-c", "--" ] args: ["while true; do sleep 30; done;"] resources: @@ -301,11 +301,10 @@ func TestHelmDeploy(t *testing.T) { builds: testBuilds, }, { - description: "deploy error unmatched parameter", + description: "deploy should not error for unmatched parameter", cmd: &MockHelm{t: t}, runContext: makeRunContext(testDeployConfigParameterUnmatched, false), builds: testBuilds, - shouldErr: true, }, { description: "deploy success remote chart with skipBuildDependencies", From 410422148b8d62f0df5fb27b4026ee4688ef6cd1 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 14 May 2019 09:49:26 -0700 Subject: [PATCH 2/4] fix method sig --- pkg/skaffold/deploy/helm.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index 9c1c9f1e41c..e52a84ba093 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -162,10 +162,7 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates color.Red.Fprintf(out, "Helm release %s not installed. Installing...\n", releaseName) isInstalled = false } - params, err := h.joinTagsToBuildResult(builds, r.Values) - if err != nil { - return nil, errors.Wrap(err, "matching build results to chart values") - } + params := h.joinTagsToBuildResult(builds, r.Values) var setOpts []string for k, v := range params { @@ -399,7 +396,7 @@ func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r lates return nil } -func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) { +func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) map[string]build.Artifact { imageToBuildResult := map[string]build.Artifact{} for _, b := range builds { imageToBuildResult[b.ImageName] = b @@ -415,7 +412,7 @@ func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map } paramToBuildResult[param] = b } - return paramToBuildResult, nil + return paramToBuildResult } func evaluateReleaseName(nameTemplate string) (string, error) { From 890f7c8dff745309ea7dbea84d5efdab8a162777 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 14 May 2019 10:55:00 -0700 Subject: [PATCH 3/4] address balopt's review comment --- pkg/skaffold/deploy/helm.go | 17 ++++++++++++----- pkg/skaffold/deploy/helm_test.go | 9 ++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index e52a84ba093..d1c80c53688 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -162,7 +162,10 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates color.Red.Fprintf(out, "Helm release %s not installed. Installing...\n", releaseName) isInstalled = false } - params := h.joinTagsToBuildResult(builds, r.Values) + params, err := h.joinTagsToBuildResult(builds, r.Values) + if err != nil { + return nil, errors.Wrap(err, "matching build results to chart values") + } var setOpts []string for k, v := range params { @@ -396,7 +399,7 @@ func (h *HelmDeployer) deleteRelease(ctx context.Context, out io.Writer, r lates return nil } -func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) map[string]build.Artifact { +func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map[string]string) (map[string]build.Artifact, error) { imageToBuildResult := map[string]build.Artifact{} for _, b := range builds { imageToBuildResult[b.ImageName] = b @@ -407,12 +410,16 @@ func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map newImageName := util.SubstituteDefaultRepoIntoImage(h.defaultRepo, imageName) b, ok := imageToBuildResult[newImageName] if !ok { - logrus.Warnf("no build present for %s. Continuing with %s", imageName, imageName) - b = build.Artifact{ImageName: imageName, Tag: imageName} + if len(builds) == 0 { + logrus.Warnf("no build artifacts present. Assuming skaffold deploy. Continuing with %s", imageName) + b = build.Artifact{ImageName: imageName, Tag: imageName} + } else { + return nil, fmt.Errorf("no build present for %s", imageName) + } } paramToBuildResult[param] = b } - return paramToBuildResult + return paramToBuildResult, nil } func evaluateReleaseName(nameTemplate string) (string, error) { diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index 77cb1715618..b5cf380ece0 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -301,10 +301,17 @@ func TestHelmDeploy(t *testing.T) { builds: testBuilds, }, { - description: "deploy should not error for unmatched parameter", + description: "deploy should not error for unmatched parameter when no builds present", + cmd: &MockHelm{t: t}, + runContext: makeRunContext(testDeployConfigParameterUnmatched, false), + builds: nil, + }, + { + description: "deploy should error for unmatched parameter when builds present", cmd: &MockHelm{t: t}, runContext: makeRunContext(testDeployConfigParameterUnmatched, false), builds: testBuilds, + shouldErr: true, }, { description: "deploy success remote chart with skipBuildDependencies", From df8a2a00bb8e029fa8a8ba9dce8324773e533eb1 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 20 May 2019 15:15:28 -0700 Subject: [PATCH 4/4] address warn to debug --- pkg/skaffold/deploy/helm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index d1c80c53688..bce2e824d5c 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -411,7 +411,7 @@ func (h *HelmDeployer) joinTagsToBuildResult(builds []build.Artifact, params map b, ok := imageToBuildResult[newImageName] if !ok { if len(builds) == 0 { - logrus.Warnf("no build artifacts present. Assuming skaffold deploy. Continuing with %s", imageName) + logrus.Debugf("no build artifacts present. Assuming skaffold deploy. Continuing with %s", imageName) b = build.Artifact{ImageName: imageName, Tag: imageName} } else { return nil, fmt.Errorf("no build present for %s", imageName)