From cd68809edc30d27c84afb7d474b1e5eebe6aeabc Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Tue, 2 Jul 2024 07:48:37 +0300 Subject: [PATCH] Revert "fix(dryRun): Remove (install|upgrade).DryRun and start using DryRunOption" This reverts commit abeac0edf6e6e27a13a41d5c849d69c59cd5cf35. 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, 24 insertions(+), 34 deletions(-) diff --git a/cmd/helm/template.go b/cmd/helm/template.go index a7c983530..b53ed6b1c 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -86,12 +86,10 @@ 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.DryRunOption = "client" - if validate { - client.DryRunOption = "server" - } + client.ClientOnly = !validate 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 f7fffb4b9..23472619d 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -131,6 +131,7 @@ 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 e259c9472..6dce3ccbb 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -69,8 +69,10 @@ 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. @@ -97,7 +99,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 markAsClientOnly is false + // (for things like templating). These are ignored if ClientOnly is false KubeVersion *chartutil.KubeVersion APIVersions chartutil.VersionSet // Used by helm template to render charts with .Release.IsUpgrade. Ignored if Dry-Run is false @@ -226,7 +228,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.isClientOnly() { + if !i.ClientOnly { if err := i.cfg.KubeClient.IsReachable(); err != nil { return nil, err } @@ -252,7 +254,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.isClientOnly() && !i.SkipCRDs && len(crds) > 0 { + if crds := chrt.CRDObjects(); !i.ClientOnly && !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.") @@ -261,7 +263,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma } } - if i.isClientOnly() { + if i.ClientOnly { // 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() @@ -274,7 +276,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.isClientOnly() && len(i.APIVersions) > 0 { + } else if !i.ClientOnly && len(i.APIVersions) > 0 { i.cfg.Log("API Version list given outside of client only mode, this list will be ignored") } @@ -341,7 +343,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.isClientOnly() && !isUpgrade && len(resources) > 0 { + if !i.ClientOnly && !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") @@ -425,20 +427,12 @@ 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.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" { + if i.DryRun || 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 8c136e4c8..69b9cbc48 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.markAsClientOnly() + instAction.ClientOnly = true 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.markAsClientOnly() + instAction.DryRun = true 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.markAsClientOnly() + instAction.DryRun = true 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.DryRunOption = "" + instAction.DryRun = false 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.markAsClientOnly() + instAction.DryRun = true 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.markAsClientOnly() + instAction.DryRun = true 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 f472b6ee9..6d26a754e 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -70,6 +70,8 @@ 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 @@ -182,17 +184,12 @@ 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.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" { + if u.DryRun || 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 d81d89084..78b4347e3 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.markAsClientOnly() + upAction.DryRun = true 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 there is an error when HideSecret True but not in a dry-run mode - upAction.DryRunOption = "" + // Ensure in a dry run mode when using HideSecret + upAction.DryRun = false vals = map[string]interface{}{} ctx, done = context.WithCancel(context.Background())