Repository: helm/helm
Pull request #6363 (2019-09-05 19:05:29)

Remove run test subcommand and --cleanup flag, print test status
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/master/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

Related to #6020. This PR removes the `helm test run` subcommand and `--cleanup` flag. The output of `helm test --help` is now:

```
The test command runs the tests for a release.

The argument this command takes is the name of a deployed release.
The tests to be run are defined in the chart that was installed.

Usage:
  helm test [RELEASE] [flags]

Flags:
  -h, --help               help for test
      --timeout duration   time to wait for any individual Kubernetes operation (like Jobs for hooks) (default 5m0s)
```

More output has also been added during execution of `helm test`. The output of `helm test <release>` now looks like:

```
Pod nginx-test-bar pending
Pod nginx-test-bar pending
Pod nginx-test-bar succeeded
Pod nginx-test-foo pending
Pod nginx-test-foo pending
Pod nginx-test-foo succeeded
TEST SUITE:     nginx-test-foo
Last Started:   2019-09-05 15:04:12.33368 -0400 EDT m=+5.107841576
Last Completed: 2019-09-05 15:04:14.147617 -0400 EDT m=+6.921723894
Phase:          Succeeded
TEST SUITE:     nginx-test-bar
Last Started:   2019-09-05 15:04:08.31585 -0400 EDT m=+1.090131819
Last Completed: 2019-09-05 15:04:12.314482 -0400 EDT m=+5.088644403
Phase:          Succeeded
```

For now in order to perform test resource cleanup, users will need to set `helm.sh/hook-delete-policy`, manually delete resources with kubectl, or set [TTL for Job resources](https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/). The hook labels proposal in https://github.com/helm/helm/issues/6365 should improve on the ergonomics of using kubectl for cleanup if we don't implement something directly in helm first.

**Special notes for your reviewer**:

**If applicable**:
- [ ] this PR contains documentation
- [ ] this PR contains unit tests
- [ ] this PR has been tested for backwards compatibility


bacongobbler: If at all possible, can you (the next core maintainer to review this PR) please hold on merging this until I get back next week? I want to take a closer look at the ramifications of removing the `--cleanup` flag when it comes to the user experience. I want to make sure we have those user stories captured and well-understood before we decide to remove a fairly significant feature of `helm test` in favour of an alternative approach (hook annotations).
jlegrone: /hold
unguiculus: If I understand it correctly, #3211 is fixed in v3?
jlegrone: > If I understand it correctly, #3211 is fixed in v3?

Yes, I wasn't aware of that issue but it should be closed by https://github.com/helm/helm/pull/6054.
bacongobbler: All that being said, once I rebased this PR on master, everything seems to work as described. Thank you!

Patch:
diff --git a/cmd/helm/root.go b/cmd/helm/root.go
index e28e8bdfb..678409a52 100644
--- a/cmd/helm/root.go
+++ b/cmd/helm/root.go
@@ -139,7 +139,7 @@ __helm_custom_func()
 {
 	__helm_debug "${FUNCNAME[0]}: last_command is $last_command"
     case ${last_command} in
-		helm_uninstall | helm_history | helm_status | helm_test_run |\
+		helm_uninstall | helm_history | helm_status | helm_test |\
 	    helm_upgrade | helm_rollback | helm_get_*)
             __helm_list_releases
             return
diff --git a/cmd/helm/status.go b/cmd/helm/status.go
index b1475be38..92e12dbe0 100644
--- a/cmd/helm/status.go
+++ b/cmd/helm/status.go
@@ -97,13 +97,15 @@ func (s statusPrinter) WriteTable(out io.Writer) error {
 	fmt.Fprintf(out, "REVISION: %d\n", s.release.Version)
 
 	executions := executionsByHookEvent(s.release)
-	if tests, ok := executions[release.HookTest]; ok {
+	if tests, ok := executions[release.HookTest]; !ok || len(tests) == 0 {
+		fmt.Fprintln(out, "TEST SUITE: None")
+	} else {
 		for _, h := range tests {
 			// Don't print anything if hook has not been initiated
 			if h.LastRun.StartedAt.IsZero() {
 				continue
 			}
-			fmt.Fprintf(out, "TEST SUITE:     %s\n%s\n%s\n%s\n\n",
+			fmt.Fprintf(out, "TEST SUITE:     %s\n%s\n%s\n%s\n",
 				h.Name,
 				fmt.Sprintf("Last Started:   %s", h.LastRun.StartedAt.Format(time.ANSIC)),
 				fmt.Sprintf("Last Completed: %s", h.LastRun.CompletedAt.Format(time.ANSIC)),


Test patch:
diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go
index 0c5631abd..a49816305 100644
--- a/cmd/helm/release_testing.go
+++ b/cmd/helm/release_testing.go
@@ -18,28 +18,43 @@ package main
 
 import (
 	"io"
+	"time"
 
 	"github.com/spf13/cobra"
 
+	"helm.sh/helm/v3/cmd/helm/require"
 	"helm.sh/helm/v3/pkg/action"
+	"helm.sh/helm/v3/pkg/cli/output"
 )
 
 const releaseTestHelp = `
-The test command consists of multiple subcommands around running tests on a release.
-
-Example usage:
-    $ helm test run [RELEASE]
+The test command runs the tests for a release.
 
+The argument this command takes is the name of a deployed release.
+The tests to be run are defined in the chart that was installed.
 `
 
 func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
+	client := action.NewReleaseTesting(cfg)
+	var outfmt output.Format
+
 	cmd := &cobra.Command{
-		Use:   "test",
-		Short: "test a release or cleanup test artifacts",
+		Use:   "test [RELEASE]",
+		Short: "run tests for a release",
 		Long:  releaseTestHelp,
+		Args:  require.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			rel, err := client.Run(args[0])
+			if err != nil {
+				return err
+			}
+
+			return outfmt.Write(out, &statusPrinter{rel, settings.Debug})
+		},
 	}
-	cmd.AddCommand(
-		newReleaseTestRunCmd(cfg, out),
-	)
+
+	f := cmd.Flags()
+	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
+
 	return cmd
 }
diff --git a/cmd/helm/release_testing_run.go b/cmd/helm/release_testing_run.go
deleted file mode 100644
index 6dc769e83..000000000
--- a/cmd/helm/release_testing_run.go
+++ /dev/null
@@ -1,53 +0,0 @@
-/*
-Copyright The Helm Authors.
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package main
-
-import (
-	"io"
-	"time"
-
-	"github.com/spf13/cobra"
-
-	"helm.sh/helm/v3/cmd/helm/require"
-	"helm.sh/helm/v3/pkg/action"
-)
-
-const releaseTestRunHelp = `
-The test command runs the tests for a release.
-
-The argument this command takes is the name of a deployed release.
-The tests to be run are defined in the chart that was installed.
-`
-
-func newReleaseTestRunCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
-	client := action.NewReleaseTesting(cfg)
-
-	cmd := &cobra.Command{
-		Use:   "run [RELEASE]",
-		Short: "run tests for a release",
-		Long:  releaseTestRunHelp,
-		Args:  require.ExactArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return client.Run(args[0])
-		},
-	}
-
-	f := cmd.Flags()
-	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
-	f.BoolVar(&client.Cleanup, "cleanup", false, "delete test pods upon completion")
-
-	return cmd
-}
diff --git a/cmd/helm/testdata/output/get-release.txt b/cmd/helm/testdata/output/get-release.txt
index e12ed19cd..47b249244 100644
--- a/cmd/helm/testdata/output/get-release.txt
+++ b/cmd/helm/testdata/output/get-release.txt
@@ -3,6 +3,7 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
 USER-SUPPLIED VALUES:
 name: value
 
diff --git a/cmd/helm/testdata/output/install-and-replace.txt b/cmd/helm/testdata/output/install-and-replace.txt
index 0a9aa1803..039d6aef6 100644
--- a/cmd/helm/testdata/output/install-and-replace.txt
+++ b/cmd/helm/testdata/output/install-and-replace.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-name-template.txt b/cmd/helm/testdata/output/install-name-template.txt
index 70d9b71b9..67e06d92b 100644
--- a/cmd/helm/testdata/output/install-name-template.txt
+++ b/cmd/helm/testdata/output/install-name-template.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-no-hooks.txt b/cmd/helm/testdata/output/install-no-hooks.txt
index 0a9aa1803..039d6aef6 100644
--- a/cmd/helm/testdata/output/install-no-hooks.txt
+++ b/cmd/helm/testdata/output/install-no-hooks.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-multiple-values-files.txt b/cmd/helm/testdata/output/install-with-multiple-values-files.txt
index 201e74927..406e522a9 100644
--- a/cmd/helm/testdata/output/install-with-multiple-values-files.txt
+++ b/cmd/helm/testdata/output/install-with-multiple-values-files.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-multiple-values.txt b/cmd/helm/testdata/output/install-with-multiple-values.txt
index 201e74927..406e522a9 100644
--- a/cmd/helm/testdata/output/install-with-multiple-values.txt
+++ b/cmd/helm/testdata/output/install-with-multiple-values.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-timeout.txt b/cmd/helm/testdata/output/install-with-timeout.txt
index 9115085db..19952e3c2 100644
--- a/cmd/helm/testdata/output/install-with-timeout.txt
+++ b/cmd/helm/testdata/output/install-with-timeout.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-values-file.txt b/cmd/helm/testdata/output/install-with-values-file.txt
index 201e74927..406e522a9 100644
--- a/cmd/helm/testdata/output/install-with-values-file.txt
+++ b/cmd/helm/testdata/output/install-with-values-file.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-values.txt b/cmd/helm/testdata/output/install-with-values.txt
index 201e74927..406e522a9 100644
--- a/cmd/helm/testdata/output/install-with-values.txt
+++ b/cmd/helm/testdata/output/install-with-values.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install-with-wait.txt b/cmd/helm/testdata/output/install-with-wait.txt
index a5609edb7..7ce22d4ec 100644
--- a/cmd/helm/testdata/output/install-with-wait.txt
+++ b/cmd/helm/testdata/output/install-with-wait.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/install.txt b/cmd/helm/testdata/output/install.txt
index 0a9aa1803..039d6aef6 100644
--- a/cmd/helm/testdata/output/install.txt
+++ b/cmd/helm/testdata/output/install.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/schema.txt b/cmd/helm/testdata/output/schema.txt
index 7424e723e..22a94b3f4 100644
--- a/cmd/helm/testdata/output/schema.txt
+++ b/cmd/helm/testdata/output/schema.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/status-with-notes.txt b/cmd/helm/testdata/output/status-with-notes.txt
index f460b343d..e992ce91e 100644
--- a/cmd/helm/testdata/output/status-with-notes.txt
+++ b/cmd/helm/testdata/output/status-with-notes.txt
@@ -3,5 +3,6 @@ LAST DEPLOYED: Sat Jan 16 00:00:00 2016
 NAMESPACE: default
 STATUS: deployed
 REVISION: 0
+TEST SUITE: None
 NOTES:
 release notes
diff --git a/cmd/helm/testdata/output/status-with-test-suite.txt b/cmd/helm/testdata/output/status-with-test-suite.txt
index 79ea4e442..58c67e103 100644
--- a/cmd/helm/testdata/output/status-with-test-suite.txt
+++ b/cmd/helm/testdata/output/status-with-test-suite.txt
@@ -7,9 +7,7 @@ TEST SUITE:     passing-test
 Last Started:   Mon Jan  2 15:04:05 2006
 Last Completed: Mon Jan  2 15:04:07 2006
 Phase:          Succeeded
-
 TEST SUITE:     failing-test
 Last Started:   Mon Jan  2 15:10:05 2006
 Last Completed: Mon Jan  2 15:10:07 2006
 Phase:          Failed
-
diff --git a/cmd/helm/testdata/output/status.txt b/cmd/helm/testdata/output/status.txt
index 9d89842db..a326c3db0 100644
--- a/cmd/helm/testdata/output/status.txt
+++ b/cmd/helm/testdata/output/status.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Sat Jan 16 00:00:00 2016
 NAMESPACE: default
 STATUS: deployed
 REVISION: 0
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/subchart-schema-cli.txt b/cmd/helm/testdata/output/subchart-schema-cli.txt
index 7424e723e..22a94b3f4 100644
--- a/cmd/helm/testdata/output/subchart-schema-cli.txt
+++ b/cmd/helm/testdata/output/subchart-schema-cli.txt
@@ -3,3 +3,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-install-timeout.txt b/cmd/helm/testdata/output/upgrade-with-install-timeout.txt
index e49fb7dc4..5d8d3a4ea 100644
--- a/cmd/helm/testdata/output/upgrade-with-install-timeout.txt
+++ b/cmd/helm/testdata/output/upgrade-with-install-timeout.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 2
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-install.txt b/cmd/helm/testdata/output/upgrade-with-install.txt
index bf8b8c403..af61212bd 100644
--- a/cmd/helm/testdata/output/upgrade-with-install.txt
+++ b/cmd/helm/testdata/output/upgrade-with-install.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 2
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-reset-values.txt b/cmd/helm/testdata/output/upgrade-with-reset-values.txt
index dfb8fec5c..01f1c0ac8 100644
--- a/cmd/helm/testdata/output/upgrade-with-reset-values.txt
+++ b/cmd/helm/testdata/output/upgrade-with-reset-values.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 5
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-reset-values2.txt b/cmd/helm/testdata/output/upgrade-with-reset-values2.txt
index 34fbe3d07..fdd1d2db7 100644
--- a/cmd/helm/testdata/output/upgrade-with-reset-values2.txt
+++ b/cmd/helm/testdata/output/upgrade-with-reset-values2.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 6
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-timeout.txt b/cmd/helm/testdata/output/upgrade-with-timeout.txt
index 7b4181f09..be3a42368 100644
--- a/cmd/helm/testdata/output/upgrade-with-timeout.txt
+++ b/cmd/helm/testdata/output/upgrade-with-timeout.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 4
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade-with-wait.txt b/cmd/helm/testdata/output/upgrade-with-wait.txt
index f47ac718d..500d07a11 100644
--- a/cmd/helm/testdata/output/upgrade-with-wait.txt
+++ b/cmd/helm/testdata/output/upgrade-with-wait.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 3
+TEST SUITE: None
diff --git a/cmd/helm/testdata/output/upgrade.txt b/cmd/helm/testdata/output/upgrade.txt
index 0f070cce5..bea42db54 100644
--- a/cmd/helm/testdata/output/upgrade.txt
+++ b/cmd/helm/testdata/output/upgrade.txt
@@ -4,3 +4,4 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 3
+TEST SUITE: None
diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go
index 0f594512b..fbd36bef8 100644
--- a/pkg/action/release_testing.go
+++ b/pkg/action/release_testing.go
@@ -17,9 +17,6 @@ limitations under the License.
 package action
 
 import (
-	"bytes"
-	"fmt"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -31,10 +28,8 @@ import (
 //
 // It provides the implementation of 'helm test'.
 type ReleaseTesting struct {
-	cfg *Configuration
-
+	cfg     *Configuration
 	Timeout time.Duration
-	Cleanup bool
 }
 
 // NewReleaseTesting creates a new ReleaseTesting object with the given configuration.
@@ -45,43 +40,25 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting {
 }
 
 // Run executes 'helm test' against the given release.
-func (r *ReleaseTesting) Run(name string) error {
+func (r *ReleaseTesting) Run(name string) (*release.Release, error) {
 	if err := r.cfg.KubeClient.IsReachable(); err != nil {
-		return err
+		return nil, err
 	}
 
 	if err := validateReleaseName(name); err != nil {
-		return errors.Errorf("releaseTest: Release name is invalid: %s", name)
+		return nil, errors.Errorf("releaseTest: Release name is invalid: %s", name)
 	}
 
 	// finds the non-deleted release with the given name
 	rel, err := r.cfg.Releases.Last(name)
 	if err != nil {
-		return err
+		return rel, err
 	}
 
 	if err := r.cfg.execHook(rel, release.HookTest, r.Timeout); err != nil {
 		r.cfg.Releases.Update(rel)
-		return err
-	}
-
-	if r.Cleanup {
-		var manifestsToDelete strings.Builder
-		for _, h := range rel.Hooks {
-			for _, e := range h.Events {
-				if e == release.HookTest {
-					fmt.Fprintf(&manifestsToDelete, "\n---\n%s", h.Manifest)
-				}
-			}
-		}
-		hooks, err := r.cfg.KubeClient.Build(bytes.NewBufferString(manifestsToDelete.String()), false)
-		if err != nil {
-			return fmt.Errorf("unable to build test hooks: %v", err)
-		}
-		if _, errs := r.cfg.KubeClient.Delete(hooks); errs != nil {
-			return fmt.Errorf("unable to delete test hooks: %v", joinErrors(errs))
-		}
+		return rel, err
 	}
 
-	return r.cfg.Releases.Update(rel)
+	return rel, r.cfg.Releases.Update(rel)
 }