From abeac0edf6e6e27a13a41d5c849d69c59cd5cf35 Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Sat, 29 Jun 2024 10:44:45 +0300 Subject: [PATCH] fix(dryRun): Remove (install|upgrade).DryRun and start using DryRunOption Signed-off-by: Suleiman Dibirov --- cmd/helm/template.go | 6 ++++-- cmd/helm/upgrade.go | 1 - pkg/action/install.go | 24 +++++++++++++++--------- pkg/action/install_test.go | 12 ++++++------ pkg/action/upgrade.go | 9 ++++++--- pkg/action/upgrade_test.go | 6 +++--- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/cmd/helm/template.go b/cmd/helm/template.go index b53ed6b1c..a7c983530 100644 --- a/cmd/helm/template.go +++ b/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) diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 23472619d..f7fffb4b9 100644 --- a/cmd/helm/upgrade.go +++ b/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 diff --git a/pkg/action/install.go b/pkg/action/install.go index 6dce3ccbb..e259c9472 100644 --- a/pkg/action/install.go +++ b/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 diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 69b9cbc48..8c136e4c8 100644 --- a/pkg/action/install_test.go +++ b/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" diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 6d26a754e..f472b6ee9 100644 --- a/pkg/action/upgrade.go +++ b/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 { diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 78b4347e3..d81d89084 100644 --- a/pkg/action/upgrade_test.go +++ b/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())