Skip to content

Commit

Permalink
Merge pull request #9426 from tapaskapadia/feat/lookup-dryrun
Browse files Browse the repository at this point in the history
feat(helm): add ability for --dry-run to do lookup functions
  • Loading branch information
mattfarina committed Jul 20, 2023
2 parents f2a0ceb + b7a2d47 commit 838b121
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 13 deletions.
27 changes: 26 additions & 1 deletion cmd/helm/install.go
Expand Up @@ -142,6 +142,9 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
}
client.SetRegistryClient(registryClient)

if client.DryRunOption == "" {
client.DryRunOption = "none"
}
rel, err := runInstall(args, client, valueOpts, out)
if err != nil {
return errors.Wrap(err, "INSTALLATION FAILED")
Expand All @@ -160,7 +163,8 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {

func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Install, valueOpts *values.Options) {
f.BoolVar(&client.CreateNamespace, "create-namespace", false, "create the release namespace if not present")
f.BoolVar(&client.DryRun, "dry-run", false, "simulate an install")
f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.")
f.Lookup("dry-run").NoOptDefVal = "client"
f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy")
f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during install")
f.BoolVar(&client.Replace, "replace", false, "re-use the given name, only if that name is a deleted release which remains in the history. This is unsafe in production")
Expand Down Expand Up @@ -269,6 +273,11 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options

client.Namespace = settings.Namespace()

// Validate DryRunOption member is one of the allowed values
if err := validateDryRunOptionFlag(client.DryRunOption); err != nil {
return nil, err
}

// Create context and prepare the handle of SIGTERM
ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)
Expand Down Expand Up @@ -309,3 +318,19 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st
}
return nil, cobra.ShellCompDirectiveNoFileComp
}

func validateDryRunOptionFlag(dryRunOptionFlagValue string) error {
// Validate dry-run flag value with a set of allowed value
allowedDryRunValues := []string{"false", "true", "none", "client", "server"}
isAllowed := false
for _, v := range allowedDryRunValues {
if dryRunOptionFlagValue == v {
isAllowed = true
break
}
}
if !isAllowed {
return errors.New("Invalid dry-run flag. Flag must one of the following: false, true, none, client, server")
}
return nil
}
3 changes: 3 additions & 0 deletions cmd/helm/template.go
Expand Up @@ -79,6 +79,9 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
}
client.SetRegistryClient(registryClient)

if client.DryRunOption == "" {
client.DryRunOption = "true"
}
client.DryRun = true
client.ReleaseName = "release-name"
client.Replace = true // Skip the name check
Expand Down
11 changes: 10 additions & 1 deletion cmd/helm/upgrade.go
Expand Up @@ -96,6 +96,9 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
}
client.SetRegistryClient(registryClient)

if client.DryRunOption == "" {
client.DryRunOption = "none"
}
// Fixes #7002 - Support reading values from STDIN for `upgrade` command
// Must load values AFTER determining if we have to call install so that values loaded from stdin are are not read twice
if client.Install {
Expand All @@ -112,6 +115,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
instClient.ChartPathOptions = client.ChartPathOptions
instClient.Force = client.Force
instClient.DryRun = client.DryRun
instClient.DryRunOption = client.DryRunOption
instClient.DisableHooks = client.DisableHooks
instClient.SkipCRDs = client.SkipCRDs
instClient.Timeout = client.Timeout
Expand Down Expand Up @@ -146,6 +150,10 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
if err != nil {
return err
}
// Validate dry-run flag value is one of the allowed values
if err := validateDryRunOptionFlag(client.DryRunOption); err != nil {
return err
}

p := getter.All(settings)
vals, err := valueOpts.MergeValues(p)
Expand Down Expand Up @@ -221,7 +229,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
f.BoolVar(&createNamespace, "create-namespace", false, "if --install is set, create the release namespace if not present")
f.BoolVarP(&client.Install, "install", "i", false, "if a release by this name doesn't already exist, run an install")
f.BoolVar(&client.Devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored")
f.BoolVar(&client.DryRun, "dry-run", false, "simulate an upgrade")
f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.")
f.Lookup("dry-run").NoOptDefVal = "client"
f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable")
f.MarkDeprecated("recreate-pods", "functionality will no longer be updated. Consult the documentation for other methods to recreate pods")
f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy")
Expand Down
12 changes: 5 additions & 7 deletions pkg/action/action.go
Expand Up @@ -103,7 +103,7 @@ type Configuration struct {
// TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed
//
// This code has to do with writing files to disk.
func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun, enableDNS bool) ([]*release.Hook, *bytes.Buffer, string, error) {
func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, interactWithRemote, enableDNS bool) ([]*release.Hook, *bytes.Buffer, string, error) {
hs := []*release.Hook{}
b := bytes.NewBuffer(nil)

Expand All @@ -121,12 +121,10 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
var files map[string]string
var err2 error

// A `helm template` or `helm install --dry-run` should not talk to the remote cluster.
// It will break in interesting and exotic ways because other data (e.g. discovery)
// is mocked. It is not up to the template author to decide when the user wants to
// connect to the cluster. So when the user says to dry run, respect the user's
// wishes and do not connect to the cluster.
if !dryRun && cfg.RESTClientGetter != nil {
// A `helm template` should not talk to the remote cluster. However, commands with the flag
//`--dry-run` with the value of `false`, `none`, or `server` should try to interact with the cluster.
// It may break in interesting and exotic ways because other data (e.g. discovery) is mocked.
if interactWithRemote && cfg.RESTClientGetter != nil {
restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
if err != nil {
return hs, b, "", err
Expand Down
14 changes: 13 additions & 1 deletion pkg/action/install.go
Expand Up @@ -73,6 +73,7 @@ type Install struct {
Force bool
CreateNamespace bool
DryRun bool
DryRunOption string
DisableHooks bool
Replace bool
Wait bool
Expand Down Expand Up @@ -232,6 +233,16 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
return nil, err
}

// Determine dry run behavior
if i.DryRun || i.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" {
i.DryRun = true
}

var interactWithRemote bool
if !i.DryRun || i.DryRunOption == "server" || i.DryRunOption == "none" || i.DryRunOption == "false" {
interactWithRemote = true
}

// 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 {
Expand Down Expand Up @@ -286,7 +297,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma
rel := i.createRelease(chrt, vals)

var manifestDoc *bytes.Buffer
rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun, i.EnableDNS)
rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithRemote, i.EnableDNS)
// Even for errors, attach this if available
if manifestDoc != nil {
rel.Manifest = manifestDoc.String()
Expand Down Expand Up @@ -500,6 +511,7 @@ func (i *Install) availableName() error {
if err := chartutil.ValidateReleaseName(start); err != nil {
return errors.Wrapf(err, "release name %q", start)
}
// On dry run, bail here
if i.DryRun {
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/action/install_test.go
Expand Up @@ -254,7 +254,7 @@ func TestInstallRelease_DryRun(t *testing.T) {
is.Equal(res.Info.Description, "Dry run complete")
}

// Regression test for #7955: Lookup must not connect to Kubernetes on a dry-run.
// Regression test for #7955
func TestInstallRelease_DryRun_Lookup(t *testing.T) {
is := assert.New(t)
instAction := installAction(t)
Expand Down
19 changes: 17 additions & 2 deletions pkg/action/upgrade.go
Expand Up @@ -71,8 +71,9 @@ type Upgrade struct {
// DisableHooks disables hook processing if set to true.
DisableHooks bool
// DryRun controls whether the operation is prepared, but not executed.
// If `true`, the upgrade is prepared but not performed.
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
// Force will, if set to `true`, ignore certain warnings and perform the upgrade anyway.
//
// This should be used with caution.
Expand Down Expand Up @@ -147,6 +148,12 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart.
if err := chartutil.ValidateReleaseName(name); err != nil {
return nil, errors.Errorf("release name is invalid: %s", name)
}

// Determine dry run behavior
if u.DryRun || u.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" {
u.DryRun = true
}

u.cfg.Log("preparing upgrade for %s", name)
currentRelease, upgradedRelease, err := u.prepareUpgrade(name, chart, vals)
if err != nil {
Expand All @@ -161,6 +168,7 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart.
return res, err
}

// Do not update for dry runs
if !u.DryRun {
u.cfg.Log("updating status for upgraded release for %s", name)
if err := u.cfg.Releases.Update(upgradedRelease); err != nil {
Expand Down Expand Up @@ -239,7 +247,13 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
return nil, nil, err
}

hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun, u.EnableDNS)
// Determine whether or not to interact with remote
var interactWithRemote bool
if !u.DryRun || u.DryRunOption == "server" {
interactWithRemote = true
}

hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithRemote, u.EnableDNS)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -317,6 +331,7 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR
return nil
})

// Run if it is a dry run
if u.DryRun {
u.cfg.Log("dry run for %s", upgradedRelease.Name)
if len(u.Description) > 0 {
Expand Down

0 comments on commit 838b121

Please sign in to comment.