Remote helm charts should not be upgraded by default

pkg/skaffold/deploy/helm.go

@@ -217,17 +217,34 @@ func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r lates
 	if !isInstalled {
 		args = append(args, "install", "--name", releaseName)
 		args = append(args, h.Flags.Install...)
-	} else if !h.shouldUpgradeOnChange(r) {
-		logrus.Infof("Release %s already installed...\n", releaseName)
-		return []Artifact{}, nil
 	} else {
-		args = append(args, "upgrade", releaseName)
-		args = append(args, h.Flags.Upgrade...)
-		if h.forceDeploy {
-			args = append(args, "--force")
+
+		var shouldUpgrade bool
+		if r.UpgradeOnChange != nil {
+			shouldUpgrade = *r.UpgradeOnChange
+
+			if !shouldUpgrade {
+				logrus.Infof("Release %s already installed...\n", releaseName)
+			}
+		} else {
+			shouldUpgrade = !r.Remote
+
+			if !shouldUpgrade {
+				logrus.Infof("Release %s not upgraded since remote (see `upgradeOnChange`)...\n", releaseName)
+			}
 		}
-		if r.RecreatePods {
-			args = append(args, "--recreate-pods")
+
+		if shouldUpgrade {
+			args = append(args, "upgrade", releaseName)
+			args = append(args, h.Flags.Upgrade...)
+			if h.forceDeploy {
+				args = append(args, "--force")
+			}
+			if r.RecreatePods {
+				args = append(args, "--recreate-pods")
+			}
+		} else {
+			return []Artifact{}, nil
 		}
 	}
 
@@ -546,11 +563,3 @@ func expandPaths(paths []string) []string {
 
 	return paths
 }
-
-func (h *HelmDeployer) shouldUpgradeOnChange(r latest.HelmRelease) bool {
-	if r.UpgradeOnChange != nil {
-		return *r.UpgradeOnChange
-	}
-
-	return !r.Remote
-}

pkg/skaffold/deploy/helm_test.go

@@ -940,38 +940,6 @@ func TestGetSetFileValues(t *testing.T) {
 	}
 }
 
-func TestShouldUpgradeOnChange(t *testing.T) {
-	trueBool := true
-	tests := []struct {
-		description string
-		input       latest.HelmRelease
-		expected    bool
-	}{
-		{
-			description: "Default `UpgradeOnChange: false` when `Remote: true`",
-			input:       latest.HelmRelease{Remote: true, UpgradeOnChange: nil},
-			expected:    false,
-		},
-		{
-			description: "Default `UpgradeOnChange: true` when `Remote: false`",
-			input:       latest.HelmRelease{Remote: false, UpgradeOnChange: nil},
-			expected:    true,
-		},
-		{
-			description: "Uses upgradeOnChange value regardless of Remote value",
-			input:       latest.HelmRelease{Remote: true, UpgradeOnChange: &trueBool},
-			expected:    true,
-		},
-	}
-	for _, test := range tests {
-		testutil.Run(t, test.description, func(t *testutil.T) {
-			deployer := NewHelmDeployer(&runcontext.RunContext{})
-
-			t.CheckDeepEqual(test.expected, deployer.shouldUpgradeOnChange(test.input))
-		})
-	}
-}
-
 func makeRunContext(deploy latest.HelmDeploy, force bool) *runcontext.RunContext {
 	pipeline := latest.Pipeline{}
 	pipeline.Deploy.DeployType.HelmDeploy = &deploy