fix(dryRun): Remove (install|upgrade).DryRun and start using DryRunOption

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>

cmd/helm/template.go

@@ -86,10 +86,12 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 			if client.DryRunOption == "" {
 				client.DryRunOption = "true"
 			}
-			client.DryRun = true
 			client.ReleaseName = "release-name"
 			client.Replace = true // Skip the name check
-			client.ClientOnly = !validate
+			client.DryRunOption = "client"
+			if validate {
+				client.DryRunOption = "server"
+			}
 			client.APIVersions = chartutil.VersionSet(extraAPIs)
 			client.IncludeCRDs = includeCrds
 			rel, err := runInstall(args, client, valueOpts, out)

cmd/helm/upgrade.go

@@ -131,7 +131,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 					instClient.CreateNamespace = createNamespace
 					instClient.ChartPathOptions = client.ChartPathOptions
 					instClient.Force = client.Force
-					instClient.DryRun = client.DryRun
 					instClient.DryRunOption = client.DryRunOption
 					instClient.DisableHooks = client.DisableHooks
 					instClient.SkipCRDs = client.SkipCRDs

pkg/action/install.go

@@ -69,10 +69,8 @@ type Install struct {
 
 	ChartPathOptions
 
-	ClientOnly      bool
 	Force           bool
 	CreateNamespace bool
-	DryRun          bool
 	DryRunOption    string
 	// HideSecret can be set to true when DryRun is enabled in order to hide
 	// Kubernetes Secrets in the output. It cannot be used outside of DryRun.
@@ -99,7 +97,7 @@ type Install struct {
 	Labels                   map[string]string
 	// KubeVersion allows specifying a custom kubernetes version to use and
 	// APIVersions allows a manual set of supported API Versions to be passed
-	// (for things like templating). These are ignored if ClientOnly is false
+	// (for things like templating). These are ignored if markAsClientOnly is false
 	KubeVersion *chartutil.KubeVersion
 	APIVersions chartutil.VersionSet
 	// Used by helm template to render charts with .Release.IsUpgrade. Ignored if Dry-Run is false
@@ -228,7 +226,7 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 // proceeds in the background.
 func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
 	// Check reachability of cluster unless in client-only mode (e.g. `helm template` without `--validate`)
-	if !i.ClientOnly {
+	if !i.isClientOnly() {
 		if err := i.cfg.KubeClient.IsReachable(); err != nil {
 			return nil, err
 		}
@@ -254,7 +252,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 
 	// Pre-install anything in the crd/ directory. We do this before Helm
 	// contacts the upstream server and builds the capabilities object.
-	if crds := chrt.CRDObjects(); !i.ClientOnly && !i.SkipCRDs && len(crds) > 0 {
+	if crds := chrt.CRDObjects(); !i.isClientOnly() && !i.SkipCRDs && len(crds) > 0 {
 		// On dry run, bail here
 		if i.isDryRun() {
 			i.cfg.Log("WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.")
@@ -263,7 +261,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 		}
 	}
 
-	if i.ClientOnly {
+	if i.isClientOnly() {
 		// Add mock objects in here so it doesn't use Kube API server
 		// NOTE(bacongobbler): used for `helm template`
 		i.cfg.Capabilities = chartutil.DefaultCapabilities.Copy()
@@ -276,7 +274,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 		mem := driver.NewMemory()
 		mem.SetNamespace(i.Namespace)
 		i.cfg.Releases = storage.Init(mem)
-	} else if !i.ClientOnly && len(i.APIVersions) > 0 {
+	} else if !i.isClientOnly() && len(i.APIVersions) > 0 {
 		i.cfg.Log("API Version list given outside of client only mode, this list will be ignored")
 	}
 
@@ -343,7 +341,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
 	// we'll end up in a state where we will delete those resources upon
 	// deleting the release because the manifest will be pointing at that
 	// resource
-	if !i.ClientOnly && !isUpgrade && len(resources) > 0 {
+	if !i.isClientOnly() && !isUpgrade && len(resources) > 0 {
 		toBeAdopted, err = existingResourceConflict(resources, rel.Name, rel.Namespace)
 		if err != nil {
 			return nil, errors.Wrap(err, "Unable to continue with install")
@@ -427,12 +425,20 @@ func (i *Install) performInstallCtx(ctx context.Context, rel *release.Release, t
 
 // isDryRun returns true if Upgrade is set to run as a DryRun
 func (i *Install) isDryRun() bool {
-	if i.DryRun || i.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" {
+	if i.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" {
 		return true
 	}
 	return false
 }
 
+func (i *Install) isClientOnly() bool {
+	return i.DryRunOption == "client" || i.DryRunOption == "true"
+}
+
+func (i *Install) markAsClientOnly() {
+	i.DryRunOption = "client"
+}
+
 func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.ResourceList, resources kube.ResourceList) (*release.Release, error) {
 	var err error
 	// pre-install hooks

pkg/action/install_test.go

@@ -129,7 +129,7 @@ func TestInstallReleaseWithValues(t *testing.T) {
 func TestInstallReleaseClientOnly(t *testing.T) {
 	is := assert.New(t)
 	instAction := installAction(t)
-	instAction.ClientOnly = true
+	instAction.markAsClientOnly()
 	instAction.Run(buildChart(), nil) // disregard output
 
 	is.Equal(instAction.cfg.Capabilities, chartutil.DefaultCapabilities)
@@ -235,7 +235,7 @@ func TestInstallRelease_WithChartAndDependencyAllNotes(t *testing.T) {
 func TestInstallRelease_DryRun(t *testing.T) {
 	is := assert.New(t)
 	instAction := installAction(t)
-	instAction.DryRun = true
+	instAction.markAsClientOnly()
 	vals := map[string]interface{}{}
 	res, err := instAction.Run(buildChart(withSampleTemplates()), vals)
 	if err != nil {
@@ -260,7 +260,7 @@ func TestInstallRelease_DryRunHiddenSecret(t *testing.T) {
 	instAction := installAction(t)
 
 	// First perform a normal dry-run with the secret and confirm its presence.
-	instAction.DryRun = true
+	instAction.markAsClientOnly()
 	vals := map[string]interface{}{}
 	res, err := instAction.Run(buildChart(withSampleSecret(), withSampleTemplates()), vals)
 	if err != nil {
@@ -287,7 +287,7 @@ func TestInstallRelease_DryRunHiddenSecret(t *testing.T) {
 	is.Equal(res2.Info.Description, "Dry run complete")
 
 	// Ensure there is an error when HideSecret True but not in a dry-run mode
-	instAction.DryRun = false
+	instAction.DryRunOption = ""
 	vals = map[string]interface{}{}
 	_, err = instAction.Run(buildChart(withSampleSecret(), withSampleTemplates()), vals)
 	if err == nil {
@@ -299,7 +299,7 @@ func TestInstallRelease_DryRunHiddenSecret(t *testing.T) {
 func TestInstallRelease_DryRun_Lookup(t *testing.T) {
 	is := assert.New(t)
 	instAction := installAction(t)
-	instAction.DryRun = true
+	instAction.markAsClientOnly()
 	vals := map[string]interface{}{}
 
 	mockChart := buildChart(withSampleTemplates())
@@ -319,7 +319,7 @@ func TestInstallRelease_DryRun_Lookup(t *testing.T) {
 func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) {
 	is := assert.New(t)
 	instAction := installAction(t)
-	instAction.DryRun = true
+	instAction.markAsClientOnly()
 	vals := map[string]interface{}{}
 	_, err := instAction.Run(buildChart(withSampleIncludingIncorrectTemplates()), vals)
 	expectedErr := "\"hello/templates/incorrect\" at <.Values.bad.doh>: nil pointer evaluating interface {}.doh"

pkg/action/upgrade.go

@@ -70,8 +70,6 @@ type Upgrade struct {
 	WaitForJobs bool
 	// DisableHooks disables hook processing if set to true.
 	DisableHooks bool
-	// DryRun controls whether the operation is prepared, but not executed.
-	DryRun bool
 	// DryRunOption controls whether the operation is prepared, but not executed with options on whether or not to interact with the remote cluster.
 	DryRunOption string
 	// HideSecret can be set to true when DryRun is enabled in order to hide
@@ -184,12 +182,17 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart.
 
 // isDryRun returns true if Upgrade is set to run as a DryRun
 func (u *Upgrade) isDryRun() bool {
-	if u.DryRun || u.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" {
+	if u.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" {
 		return true
 	}
 	return false
 }
 
+// markAsClientOnly sets Upgrade.DryRunOption to "client"
+func (u *Upgrade) markAsClientOnly() {
+	u.DryRunOption = "client"
+}
+
 // prepareUpgrade builds an upgraded release for an upgrade operation.
 func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, *release.Release, error) {
 	if chart == nil {

pkg/action/upgrade_test.go

@@ -546,7 +546,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) {
 	rel.Info.Status = release.StatusDeployed
 	req.NoError(upAction.cfg.Releases.Create(rel))
 
-	upAction.DryRun = true
+	upAction.markAsClientOnly()
 	vals := map[string]interface{}{}
 
 	ctx, done := context.WithCancel(context.Background())
@@ -577,8 +577,8 @@ func TestUpgradeRelease_DryRun(t *testing.T) {
 	is.Equal(lastRelease.Info.Status, release.StatusDeployed)
 	is.Equal(1, lastRelease.Version)
 
-	// Ensure in a dry run mode when using HideSecret
-	upAction.DryRun = false
+	// Ensure there is an error when HideSecret True but not in a dry-run mode
+	upAction.DryRunOption = ""
 	vals = map[string]interface{}{}
 
 	ctx, done = context.WithCancel(context.Background())