Repository: helm/helm
Pull request #6519 (2019-09-27 15:31:53)

fix(pkg/chartutil): include values.schema.json in packaged chart
When playing around with new Helm 3 features, we noticed that `values.yaml` is not validated when running `helm install`, even though [the docs say it should be](https://v3.helm.sh/docs/topics/charts/#schema-files):

> Validation occurs when any of the following commands are invoked:
> 
> * `helm install`
> * `helm upgrade`
> * `helm lint`
> * `helm template`

The reason is that `values.schema.json` is not included in the tarball created by `helm package`, so `helm lint` cannot lint the tarball properly, even if it can lint the chart directory:

    $ helm lint my-chart            # Finds errors in values.yaml
    $ helm package my-chart
    $ helm lint my-chart-1.0.0.tgz  # Does not find errors in values.yaml

bacongobbler: Good catch. Thank you for the PR! Would you mind writing some unit tests under save_test.go to ensure this file is indeed packaged? Thanks!
lindhe: We would actually mind a little bit. Unless we have missed it, there seems to be no unit test for any of the other required files that are packaged (e.g., Chart.yaml), it would require some effort to implement it. Since we are new to the project, it would be a lot of trial-and error before we could get the tests in place. Maybe it's better to create a new PR after this one, that implements the entire suite of tests needed for the files?
bacongobbler: It shouldn't be too hard to add a `Schema` object [here](https://github.com/helm/helm/blob/15d277360e1406cbf708e5aef599bc7e0e685e9b/pkg/chartutil/save_test.go#L77-L89) and then check if `.Schema` hasn't changed, similar to how [we check the `.Files` object here](https://github.com/helm/helm/blob/15d277360e1406cbf708e5aef599bc7e0e685e9b/pkg/chartutil/save_test.go#L108-L110). That should be enough to cover the code introduced in this change.
bacongobbler: A test case for saving invalid JSON would also help capture the validation check.
SimonAlling: > It shouldn't be too hard to add a `Schema` object [here](https://github.com/helm/helm/blob/15d277360e1406cbf708e5aef599bc7e0e685e9b/pkg/chartutil/save_test.go#L77-L89) and then check if `.Schema` hasn't changed after it's been loaded, similar to how [we check the `.Files` object](https://github.com/helm/helm/blob/15d277360e1406cbf708e5aef599bc7e0e685e9b/pkg/chartutil/save_test.go#L108-L110). That should be enough to cover the code introduced in this change.

I absolutely agree.

> A test case for saving invalid JSON would also help capture the validation check.
> 
> https://github.com/helm/helm/pull/6519/files#diff-9ea0c476e7eb107c6b0f198af1a49caaR163

I'm not entirely sure about this, because as far as we can tell, an error returned from `writeTarContents` doesn't "propagate" to the user. Ergo, if I run `helm package` and something goes wrong such that any of the `return err` statements in `writeTarContents` is reached, then no tarball is created, but `helm package` still prints a success message:

    $ helm package my-chart
    Successfully packaged chart and saved it to: /[...]/my-chart-1.0.0.tgz
    $ ls my-chart-1.0.0.tgz
    ls: cannot access 'my-chart-1.0.0.tgz': No such file or directory

But that seems to us like a separate issue.

(We may not have time to work on this during the weekend, but hopefully on Monday.)
SimonAlling: I've added the requested test cases now, @bacongobbler. There was a particular confusing aspect of the code – namely, how the name `err` was used – that took me several hours to figure out, but I think that's gonna be a separate PR.
SimonAlling: Hi again, @bacongobbler. I rebased and force-pushed my commits because conflicts had arisen due to #5498. 
bacongobbler: Would you mind rebasing this PR? Once that's done I can merge this. Thanks.

Patch:
diff --git a/cmd/helm/get.go b/cmd/helm/get.go
index b9b38524d..d9c8a2d8c 100644
--- a/cmd/helm/get.go
+++ b/cmd/helm/get.go
@@ -71,7 +71,6 @@ func newGetCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	cmd.AddCommand(newGetValuesCmd(cfg, out))
 	cmd.AddCommand(newGetManifestCmd(cfg, out))
 	cmd.AddCommand(newGetHooksCmd(cfg, out))
-	cmd.AddCommand(newGetNotesCmd(cfg, out))
 
 	return cmd
 }
diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go
index c309edf71..509ab3241 100644
--- a/cmd/helm/helm.go
+++ b/cmd/helm/helm.go
@@ -22,9 +22,11 @@ import (
 	"log"
 	"os"
 	"strings"
+	"sync"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/klog"
 
 	// Import to initialize client auth plugins.
@@ -33,13 +35,18 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/gates"
+	"helm.sh/helm/v3/pkg/kube"
+	"helm.sh/helm/v3/pkg/storage"
+	"helm.sh/helm/v3/pkg/storage/driver"
 )
 
 // FeatureGateOCI is the feature gate for checking if `helm chart` and `helm registry` commands should work
 const FeatureGateOCI = gates.Gate("HELM_EXPERIMENTAL_OCI")
 
 var (
-	settings = cli.New()
+	settings   = cli.New()
+	config     genericclioptions.RESTClientGetter
+	configOnce sync.Once
 )
 
 func init() {
@@ -64,9 +71,13 @@ func initKubeLogs() {
 func main() {
 	initKubeLogs()
 
-	actionConfig, err := action.InitActionConfig(settings, false, os.Getenv("HELM_DRIVER"), debug)
+	actionConfig := new(action.Configuration)
+	cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:])
 
-	if err != nil {
+	// Initialize the rest of the actionConfig
+	initActionConfig(actionConfig, false)
+
+	if err := cmd.Execute(); err != nil {
 		debug("%+v", err)
 		switch e := err.(type) {
 		case pluginError:
@@ -75,13 +86,62 @@ func main() {
 			os.Exit(1)
 		}
 	}
+}
 
-	cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:])
+func initActionConfig(actionConfig *action.Configuration, allNamespaces bool) {
+	kc := kube.New(kubeConfig())
+	kc.Log = debug
 
-	if err := cmd.Execute(); err != nil {
-		debug("%+v", err)
-		os.Exit(1)
+	clientset, err := kc.Factory.KubernetesClientSet()
+	if err != nil {
+		// TODO return error
+		log.Fatal(err)
+	}
+	var namespace string
+	if !allNamespaces {
+		namespace = getNamespace()
+	}
+
+	var store *storage.Storage
+	switch os.Getenv("HELM_DRIVER") {
+	case "secret", "secrets", "":
+		d := driver.NewSecrets(clientset.CoreV1().Secrets(namespace))
+		d.Log = debug
+		store = storage.Init(d)
+	case "configmap", "configmaps":
+		d := driver.NewConfigMaps(clientset.CoreV1().ConfigMaps(namespace))
+		d.Log = debug
+		store = storage.Init(d)
+	case "memory":
+		d := driver.NewMemory()
+		store = storage.Init(d)
+	default:
+		// Not sure what to do here.
+		panic("Unknown driver in HELM_DRIVER: " + os.Getenv("HELM_DRIVER"))
+	}
+
+	actionConfig.RESTClientGetter = kubeConfig()
+	actionConfig.KubeClient = kc
+	actionConfig.Releases = store
+	actionConfig.Log = debug
+}
+
+func kubeConfig() genericclioptions.RESTClientGetter {
+	configOnce.Do(func() {
+		config = kube.GetConfig(settings.KubeConfig, settings.KubeContext, settings.Namespace)
+	})
+	return config
+}
+
+func getNamespace() string {
+	if settings.Namespace != "" {
+		return settings.Namespace
+	}
+
+	if ns, _, err := kubeConfig().ToRawKubeConfigLoader().Namespace(); err == nil {
+		return ns
 	}
+	return "default"
 }
 
 // wordSepNormalizeFunc changes all flags that contain "_" separators
diff --git a/cmd/helm/install.go b/cmd/helm/install.go
index e29fbbda1..2540558a5 100644
--- a/cmd/helm/install.go
+++ b/cmd/helm/install.go
@@ -205,7 +205,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options
 		}
 	}
 
-	client.Namespace = settings.Namespace()
+	client.Namespace = getNamespace()
 	return client.Run(chartRequested, vals)
 }
 
diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go
index 9a2e8d31c..13395c6d6 100644
--- a/cmd/helm/lint.go
+++ b/cmd/helm/lint.go
@@ -51,7 +51,7 @@ func newLintCmd(out io.Writer) *cobra.Command {
 			if len(args) > 0 {
 				paths = args
 			}
-			client.Namespace = settings.Namespace()
+			client.Namespace = getNamespace()
 			vals, err := valueOpts.MergeValues(getter.All(settings))
 			if err != nil {
 				return err
diff --git a/cmd/helm/list.go b/cmd/helm/list.go
index 50f9a7441..8c89425f5 100644
--- a/cmd/helm/list.go
+++ b/cmd/helm/list.go
@@ -19,7 +19,6 @@ package main
 import (
 	"fmt"
 	"io"
-	"os"
 	"strconv"
 
 	"github.com/gosuri/uitable"
@@ -70,7 +69,7 @@ func newListCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Args:    require.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if client.AllNamespaces {
-				action.InitActionConfig(settings, true, os.Getenv("HELM_DRIVER"), debug)
+				initActionConfig(cfg, true)
 			}
 			client.SetStateMask()
 
diff --git a/cmd/helm/get_notes.go b/cmd/helm/release_testing_run.go
similarity index 52%
rename from cmd/helm/get_notes.go
rename to cmd/helm/release_testing_run.go
index feab4e303..6dc769e83 100644
--- a/cmd/helm/get_notes.go
+++ b/cmd/helm/release_testing_run.go
@@ -1,11 +1,10 @@
 /*
 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
+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,
@@ -17,8 +16,8 @@ limitations under the License.
 package main
 
 import (
-	"fmt"
 	"io"
+	"time"
 
 	"github.com/spf13/cobra"
 
@@ -26,32 +25,29 @@ import (
 	"helm.sh/helm/v3/pkg/action"
 )
 
-var getNotesHelp = `
-This command shows notes provided by the chart of a named release.
+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 newGetNotesCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
-	client := action.NewGet(cfg)
+func newReleaseTestRunCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
+	client := action.NewReleaseTesting(cfg)
 
 	cmd := &cobra.Command{
-		Use:   "notes [flags] RELEASE_NAME",
-		Short: "displays the notes of the named release",
-		Long:  getNotesHelp,
+		Use:   "run [RELEASE]",
+		Short: "run tests for a release",
+		Long:  releaseTestRunHelp,
 		Args:  require.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			res, err := client.Run(args[0])
-			if err != nil {
-				return err
-			}
-			if len(res.Info.Notes) > 0 {
-				fmt.Fprintf(out, "NOTES:\n%s\n", res.Info.Notes)
-			}
-			return nil
+			return client.Run(args[0])
 		},
 	}
 
 	f := cmd.Flags()
-	f.IntVar(&client.Version, "revision", 0, "get the named release with revision")
+	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/root.go b/cmd/helm/root.go
index 678409a52..e28e8bdfb 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 |\
+		helm_uninstall | helm_history | helm_status | helm_test_run |\
 	    helm_upgrade | helm_rollback | helm_get_*)
             __helm_list_releases
             return
diff --git a/cmd/helm/status.go b/cmd/helm/status.go
index 92e12dbe0..b1475be38 100644
--- a/cmd/helm/status.go
+++ b/cmd/helm/status.go
@@ -97,15 +97,13 @@ 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 || len(tests) == 0 {
-		fmt.Fprintln(out, "TEST SUITE: None")
-	} else {
+	if tests, ok := executions[release.HookTest]; ok {
 		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",
+			fmt.Fprintf(out, "TEST SUITE:     %s\n%s\n%s\n%s\n\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)),
diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go
index 22dd23970..33631504f 100644
--- a/cmd/helm/upgrade.go
+++ b/cmd/helm/upgrade.go
@@ -71,7 +71,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 		Long:  upgradeDesc,
 		Args:  require.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			client.Namespace = settings.Namespace()
+			client.Namespace = getNamespace()
 
 			if client.Version == "" && client.Devel {
 				debug("setting version to >0.0.0-0")
diff --git a/pkg/action/action.go b/pkg/action/action.go
index 7cfe4a7d3..f784b3651 100644
--- a/pkg/action/action.go
+++ b/pkg/action/action.go
@@ -29,11 +29,9 @@ import (
 
 	"helm.sh/helm/v3/internal/experimental/registry"
 	"helm.sh/helm/v3/pkg/chartutil"
-	"helm.sh/helm/v3/pkg/cli"
 	"helm.sh/helm/v3/pkg/kube"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage"
-	"helm.sh/helm/v3/pkg/storage/driver"
 )
 
 // Timestamper is a function capable of producing a timestamp.Timestamper.
@@ -84,16 +82,6 @@ type Configuration struct {
 	Log func(string, ...interface{})
 }
 
-// RESTClientGetter gets the rest client
-type RESTClientGetter interface {
-	ToRESTConfig() (*rest.Config, error)
-	ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error)
-	ToRESTMapper() (meta.RESTMapper, error)
-}
-
-// DebugLog sets the logger that writes debug strings
-type DebugLog func(format string, v ...interface{})
-
 // capabilities builds a Capabilities from discovery information.
 func (c *Configuration) getCapabilities() (*chartutil.Capabilities, error) {
 	if c.Capabilities != nil {
@@ -209,46 +197,8 @@ func (c *Configuration) recordRelease(r *release.Release) {
 	}
 }
 
-// InitActionConfig initializes the action configuration
-func InitActionConfig(envSettings *cli.EnvSettings, allNamespaces bool, helmDriver string, log DebugLog) (*Configuration, error) {
-
-	var actionConfig Configuration
-	kubeconfig := envSettings.KubeConfig()
-
-	kc := kube.New(kubeconfig)
-	kc.Log = log
-
-	clientset, err := kc.Factory.KubernetesClientSet()
-	if err != nil {
-		return nil, err
-	}
-	var namespace string
-	if !allNamespaces {
-		namespace = envSettings.Namespace()
-	}
-
-	var store *storage.Storage
-	switch helmDriver {
-	case "secret", "secrets", "":
-		d := driver.NewSecrets(clientset.CoreV1().Secrets(namespace))
-		d.Log = log
-		store = storage.Init(d)
-	case "configmap", "configmaps":
-		d := driver.NewConfigMaps(clientset.CoreV1().ConfigMaps(namespace))
-		d.Log = log
-		store = storage.Init(d)
-	case "memory":
-		d := driver.NewMemory()
-		store = storage.Init(d)
-	default:
-		// Not sure what to do here.
-		panic("Unknown driver in HELM_DRIVER: " + helmDriver)
-	}
-
-	actionConfig.RESTClientGetter = kubeconfig
-	actionConfig.KubeClient = kc
-	actionConfig.Releases = store
-	actionConfig.Log = log
-
-	return &actionConfig, nil
+type RESTClientGetter interface {
+	ToRESTConfig() (*rest.Config, error)
+	ToDiscoveryClient() (discovery.CachedDiscoveryInterface, error)
+	ToRESTMapper() (meta.RESTMapper, error)
 }
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index 7aa78284d..72668ef48 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -35,6 +35,8 @@ const (
 	ChartfileName = "Chart.yaml"
 	// ValuesfileName is the default values file name.
 	ValuesfileName = "values.yaml"
+	// SchemafileName is the default values schema file name.
+	SchemafileName = "values.schema.json"
 	// TemplatesDir is the relative directory name for templates.
 	TemplatesDir = "templates"
 	// ChartsDir is the relative directory name for charts dependencies.
diff --git a/pkg/chartutil/save.go b/pkg/chartutil/save.go
index 1d90142c1..ca6eeaa19 100644
--- a/pkg/chartutil/save.go
+++ b/pkg/chartutil/save.go
@@ -19,6 +19,7 @@ package chartutil
 import (
 	"archive/tar"
 	"compress/gzip"
+	"encoding/json"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -56,6 +57,14 @@ func SaveDir(c *chart.Chart, dest string) error {
 		}
 	}
 
+	// Save values.schema.json if it exists
+	if c.Schema != nil {
+		filename := filepath.Join(outdir, SchemafileName)
+		if err := writeFile(filename, c.Schema); err != nil {
+			return err
+		}
+	}
+
 	// Save templates and files
 	for _, o := range [][]*chart.File{c.Templates, c.Files} {
 		for _, f := range o {
@@ -124,6 +133,7 @@ func Save(c *chart.Chart, outDir string) (string, error) {
 
 	if err := writeTarContents(twriter, c, ""); err != nil {
 		rollback = true
+		return filename, err
 	}
 	return filename, err
 }
@@ -149,6 +159,16 @@ func writeTarContents(out *tar.Writer, c *chart.Chart, prefix string) error {
 		return err
 	}
 
+	// Save values.schema.json if it exists
+	if c.Schema != nil {
+		if !json.Valid(c.Schema) {
+			return errors.New("Invalid JSON in " + SchemafileName)
+		}
+		if err := writeToTar(out, filepath.Join(base, SchemafileName), c.Schema); err != nil {
+			return err
+		}
+	}
+
 	// Save templates
 	for _, f := range c.Templates {
 		n := filepath.Join(base, f.Name)
diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go
index 76dc31bcf..bbe3e964b 100644
--- a/pkg/cli/environment.go
+++ b/pkg/cli/environment.go
@@ -26,26 +26,23 @@ import (
 	"fmt"
 	"os"
 	"strconv"
-	"sync"
 
 	"github.com/spf13/pflag"
 
-	"k8s.io/cli-runtime/pkg/genericclioptions"
-
 	"helm.sh/helm/v3/pkg/helmpath"
-	"helm.sh/helm/v3/pkg/kube"
 )
 
 // EnvSettings describes all of the environment settings.
 type EnvSettings struct {
-	namespace  string
-	kubeConfig string
-	config     genericclioptions.RESTClientGetter
-	configOnce sync.Once
+	// Namespace is the namespace scope.
+	Namespace string
+	// KubeConfig is the path to the kubeconfig file.
+	KubeConfig string
 	// KubeContext is the name of the kubeconfig context.
 	KubeContext string
 	// Debug indicates whether or not Helm is running in Debug mode.
 	Debug bool
+
 	// RegistryConfig is the path to the registry config file.
 	RegistryConfig string
 	// RepositoryConfig is the path to the repositories file.
@@ -57,9 +54,8 @@ type EnvSettings struct {
 }
 
 func New() *EnvSettings {
-
 	env := EnvSettings{
-		namespace:        os.Getenv("HELM_NAMESPACE"),
+		Namespace:        os.Getenv("HELM_NAMESPACE"),
 		PluginsDirectory: envOr("HELM_PLUGINS", helmpath.DataPath("plugins")),
 		RegistryConfig:   envOr("HELM_REGISTRY_CONFIG", helmpath.ConfigPath("registry.json")),
 		RepositoryConfig: envOr("HELM_REPOSITORY_CONFIG", helmpath.ConfigPath("repositories.yaml")),
@@ -71,8 +67,8 @@ func New() *EnvSettings {
 
 // AddFlags binds flags to the given flagset.
 func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) {
-	fs.StringVarP(&s.namespace, "namespace", "n", s.namespace, "namespace scope for this request")
-	fs.StringVar(&s.kubeConfig, "kubeconfig", "", "path to the kubeconfig file")
+	fs.StringVarP(&s.Namespace, "namespace", "n", s.Namespace, "namespace scope for this request")
+	fs.StringVar(&s.KubeConfig, "kubeconfig", "", "path to the kubeconfig file")
 	fs.StringVar(&s.KubeContext, "kube-context", "", "name of the kubeconfig context to use")
 	fs.BoolVar(&s.Debug, "debug", s.Debug, "enable verbose output")
 	fs.StringVar(&s.RegistryConfig, "registry-config", s.RegistryConfig, "path to the registry config file")
@@ -97,23 +93,3 @@ func (s *EnvSettings) EnvVars() map[string]string {
 		"HELM_REPOSITORY_CONFIG": s.RepositoryConfig,
 	}
 }
-
-//Namespace gets the namespace from the configuration
-func (s *EnvSettings) Namespace() string {
-	if s.namespace != "" {
-		return s.namespace
-	}
-
-	if ns, _, err := s.KubeConfig().ToRawKubeConfigLoader().Namespace(); err == nil {
-		return ns
-	}
-	return "default"
-}
-
-//KubeConfig gets the kubeconfig from EnvSettings
-func (s *EnvSettings) KubeConfig() genericclioptions.RESTClientGetter {
-	s.configOnce.Do(func() {
-		s.config = kube.GetConfig(s.kubeConfig, s.KubeContext, s.namespace)
-	})
-	return s.config
-}
diff --git a/pkg/release/mock.go b/pkg/release/mock.go
index 3e0bf0a6d..0ad18a8da 100644
--- a/pkg/release/mock.go
+++ b/pkg/release/mock.go
@@ -90,7 +90,6 @@ func Mock(opts *MockReleaseOptions) *Release {
 		LastDeployed:  date,
 		Status:        scode,
 		Description:   "Release mock",
-		Notes:         "Some mock release notes!",
 	}
 
 	return &Release{


Test patch:
diff --git a/cmd/helm/get_notes_test.go b/cmd/helm/get_notes_test.go
deleted file mode 100644
index a3ddea9a7..000000000
--- a/cmd/helm/get_notes_test.go
+++ /dev/null
@@ -1,38 +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 (
-	"testing"
-
-	"helm.sh/helm/v3/pkg/release"
-)
-
-func TestGetNotesCmd(t *testing.T) {
-	tests := []cmdTestCase{{
-		name:   "get notes of a deployed release",
-		cmd:    "get notes the-limerick",
-		golden: "output/get-notes.txt",
-		rels:   []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "the-limerick"})},
-	}, {
-		name:      "get notes without args",
-		cmd:       "get notes",
-		golden:    "output/get-notes-no-args.txt",
-		wantError: true,
-	}}
-	runTestCmd(t, tests)
-}
diff --git a/cmd/helm/release_testing.go b/cmd/helm/release_testing.go
index a49816305..0c5631abd 100644
--- a/cmd/helm/release_testing.go
+++ b/cmd/helm/release_testing.go
@@ -18,43 +18,28 @@ 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 runs the tests for a release.
+The test command consists of multiple subcommands around running tests on a release.
+
+Example usage:
+    $ helm test run [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 [RELEASE]",
-		Short: "run tests for a release",
+		Use:   "test",
+		Short: "test a release or cleanup test artifacts",
 		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})
-		},
 	}
-
-	f := cmd.Flags()
-	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
-
+	cmd.AddCommand(
+		newReleaseTestRunCmd(cfg, out),
+	)
 	return cmd
 }
diff --git a/cmd/helm/testdata/output/get-notes-no-args.txt b/cmd/helm/testdata/output/get-notes-no-args.txt
deleted file mode 100644
index 6523ce8fd..000000000
--- a/cmd/helm/testdata/output/get-notes-no-args.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-Error: "helm get notes" requires 1 argument
-
-Usage:  helm get notes [flags] RELEASE_NAME
diff --git a/cmd/helm/testdata/output/get-notes.txt b/cmd/helm/testdata/output/get-notes.txt
deleted file mode 100644
index e710c7801..000000000
--- a/cmd/helm/testdata/output/get-notes.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-NOTES:
-Some mock release notes!
diff --git a/cmd/helm/testdata/output/get-release.txt b/cmd/helm/testdata/output/get-release.txt
index f6c3b57eb..e12ed19cd 100644
--- a/cmd/helm/testdata/output/get-release.txt
+++ b/cmd/helm/testdata/output/get-release.txt
@@ -3,7 +3,6 @@ LAST DEPLOYED: Fri Sep  2 22:04:05 1977
 NAMESPACE: default
 STATUS: deployed
 REVISION: 1
-TEST SUITE: None
 USER-SUPPLIED VALUES:
 name: value
 
@@ -25,5 +24,3 @@ kind: Secret
 metadata:
   name: fixture
 
-NOTES:
-Some mock release notes!
diff --git a/cmd/helm/testdata/output/install-and-replace.txt b/cmd/helm/testdata/output/install-and-replace.txt
index 039d6aef6..0a9aa1803 100644
--- a/cmd/helm/testdata/output/install-and-replace.txt
+++ b/cmd/helm/testdata/output/install-and-replace.txt
@@ -3,4 +3,3 @@ 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 67e06d92b..70d9b71b9 100644
--- a/cmd/helm/testdata/output/install-name-template.txt
+++ b/cmd/helm/testdata/output/install-name-template.txt
@@ -3,4 +3,3 @@ 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 039d6aef6..0a9aa1803 100644
--- a/cmd/helm/testdata/output/install-no-hooks.txt
+++ b/cmd/helm/testdata/output/install-no-hooks.txt
@@ -3,4 +3,3 @@ 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 406e522a9..201e74927 100644
--- a/cmd/helm/testdata/output/install-with-multiple-values-files.txt
+++ b/cmd/helm/testdata/output/install-with-multiple-values-files.txt
@@ -3,4 +3,3 @@ 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 406e522a9..201e74927 100644
--- a/cmd/helm/testdata/output/install-with-multiple-values.txt
+++ b/cmd/helm/testdata/output/install-with-multiple-values.txt
@@ -3,4 +3,3 @@ 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 19952e3c2..9115085db 100644
--- a/cmd/helm/testdata/output/install-with-timeout.txt
+++ b/cmd/helm/testdata/output/install-with-timeout.txt
@@ -3,4 +3,3 @@ 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 406e522a9..201e74927 100644
--- a/cmd/helm/testdata/output/install-with-values-file.txt
+++ b/cmd/helm/testdata/output/install-with-values-file.txt
@@ -3,4 +3,3 @@ 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 406e522a9..201e74927 100644
--- a/cmd/helm/testdata/output/install-with-values.txt
+++ b/cmd/helm/testdata/output/install-with-values.txt
@@ -3,4 +3,3 @@ 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 7ce22d4ec..a5609edb7 100644
--- a/cmd/helm/testdata/output/install-with-wait.txt
+++ b/cmd/helm/testdata/output/install-with-wait.txt
@@ -3,4 +3,3 @@ 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 039d6aef6..0a9aa1803 100644
--- a/cmd/helm/testdata/output/install.txt
+++ b/cmd/helm/testdata/output/install.txt
@@ -3,4 +3,3 @@ 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 22a94b3f4..7424e723e 100644
--- a/cmd/helm/testdata/output/schema.txt
+++ b/cmd/helm/testdata/output/schema.txt
@@ -3,4 +3,3 @@ 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 e992ce91e..f460b343d 100644
--- a/cmd/helm/testdata/output/status-with-notes.txt
+++ b/cmd/helm/testdata/output/status-with-notes.txt
@@ -3,6 +3,5 @@ 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 58c67e103..79ea4e442 100644
--- a/cmd/helm/testdata/output/status-with-test-suite.txt
+++ b/cmd/helm/testdata/output/status-with-test-suite.txt
@@ -7,7 +7,9 @@ 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 a326c3db0..9d89842db 100644
--- a/cmd/helm/testdata/output/status.txt
+++ b/cmd/helm/testdata/output/status.txt
@@ -3,4 +3,3 @@ 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 22a94b3f4..7424e723e 100644
--- a/cmd/helm/testdata/output/subchart-schema-cli.txt
+++ b/cmd/helm/testdata/output/subchart-schema-cli.txt
@@ -3,4 +3,3 @@ 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 5d8d3a4ea..e49fb7dc4 100644
--- a/cmd/helm/testdata/output/upgrade-with-install-timeout.txt
+++ b/cmd/helm/testdata/output/upgrade-with-install-timeout.txt
@@ -4,4 +4,3 @@ 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 af61212bd..bf8b8c403 100644
--- a/cmd/helm/testdata/output/upgrade-with-install.txt
+++ b/cmd/helm/testdata/output/upgrade-with-install.txt
@@ -4,4 +4,3 @@ 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 01f1c0ac8..dfb8fec5c 100644
--- a/cmd/helm/testdata/output/upgrade-with-reset-values.txt
+++ b/cmd/helm/testdata/output/upgrade-with-reset-values.txt
@@ -4,4 +4,3 @@ 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 fdd1d2db7..34fbe3d07 100644
--- a/cmd/helm/testdata/output/upgrade-with-reset-values2.txt
+++ b/cmd/helm/testdata/output/upgrade-with-reset-values2.txt
@@ -4,4 +4,3 @@ 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 be3a42368..7b4181f09 100644
--- a/cmd/helm/testdata/output/upgrade-with-timeout.txt
+++ b/cmd/helm/testdata/output/upgrade-with-timeout.txt
@@ -4,4 +4,3 @@ 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 500d07a11..f47ac718d 100644
--- a/cmd/helm/testdata/output/upgrade-with-wait.txt
+++ b/cmd/helm/testdata/output/upgrade-with-wait.txt
@@ -4,4 +4,3 @@ 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 bea42db54..0f070cce5 100644
--- a/cmd/helm/testdata/output/upgrade.txt
+++ b/cmd/helm/testdata/output/upgrade.txt
@@ -4,4 +4,3 @@ 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 fbd36bef8..0f594512b 100644
--- a/pkg/action/release_testing.go
+++ b/pkg/action/release_testing.go
@@ -17,6 +17,9 @@ limitations under the License.
 package action
 
 import (
+	"bytes"
+	"fmt"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -28,8 +31,10 @@ 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.
@@ -40,25 +45,43 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting {
 }
 
 // Run executes 'helm test' against the given release.
-func (r *ReleaseTesting) Run(name string) (*release.Release, error) {
+func (r *ReleaseTesting) Run(name string) error {
 	if err := r.cfg.KubeClient.IsReachable(); err != nil {
-		return nil, err
+		return err
 	}
 
 	if err := validateReleaseName(name); err != nil {
-		return nil, errors.Errorf("releaseTest: Release name is invalid: %s", name)
+		return 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 rel, err
+		return err
 	}
 
 	if err := r.cfg.execHook(rel, release.HookTest, r.Timeout); err != nil {
 		r.cfg.Releases.Update(rel)
-		return rel, err
+		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, r.cfg.Releases.Update(rel)
+	return r.cfg.Releases.Update(rel)
 }
diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go
index 377e89dcb..8941d0368 100644
--- a/pkg/chartutil/save_test.go
+++ b/pkg/chartutil/save_test.go
@@ -17,10 +17,12 @@ limitations under the License.
 package chartutil
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -46,7 +48,9 @@ func TestSave(t *testing.T) {
 				Files: []*chart.File{
 					{Name: "scheherazade/shahryar.txt", Data: []byte("1,001 Nights")},
 				},
+				Schema: []byte("{\n  \"title\": \"Values\"\n}"),
 			}
+			chartWithInvalidJSON := withSchema(*c, []byte("{"))
 
 			where, err := Save(c, dest)
 			if err != nil {
@@ -69,10 +73,32 @@ func TestSave(t *testing.T) {
 			if len(c2.Files) != 1 || c2.Files[0].Name != "scheherazade/shahryar.txt" {
 				t.Fatal("Files data did not match")
 			}
+
+			if !bytes.Equal(c.Schema, c2.Schema) {
+				indentation := 4
+				formattedExpected := Indent(indentation, string(c.Schema))
+				formattedActual := Indent(indentation, string(c2.Schema))
+				t.Fatalf("Schema data did not match.\nExpected:\n%s\nActual:\n%s", formattedExpected, formattedActual)
+			}
+			if _, err := Save(&chartWithInvalidJSON, dest); err == nil {
+				t.Fatalf("Invalid JSON was not caught while saving chart")
+			}
 		})
 	}
 }
 
+// Creates a copy with a different schema; does not modify anything.
+func withSchema(chart chart.Chart, schema []byte) chart.Chart {
+	chart.Schema = schema
+	return chart
+}
+
+func Indent(n int, text string) string {
+	startOfLine := regexp.MustCompile(`(?m)^`)
+	indentation := strings.Repeat(" ", n)
+	return startOfLine.ReplaceAllLiteralString(text, indentation)
+}
+
 func TestSaveDir(t *testing.T) {
 	tmp, err := ioutil.TempDir("", "helm-")
 	if err != nil {
diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go
index d6856dd01..58cf1c7f4 100644
--- a/pkg/cli/environment_test.go
+++ b/pkg/cli/environment_test.go
@@ -38,7 +38,7 @@ func TestEnvSettings(t *testing.T) {
 	}{
 		{
 			name: "defaults",
-			ns:   "default",
+			ns:   "",
 		},
 		{
 			name:  "with flags set",
@@ -78,8 +78,8 @@ func TestEnvSettings(t *testing.T) {
 			if settings.Debug != tt.debug {
 				t.Errorf("expected debug %t, got %t", tt.debug, settings.Debug)
 			}
-			if settings.Namespace() != tt.ns {
-				t.Errorf("expected namespace %q, got %q", tt.ns, settings.Namespace())
+			if settings.Namespace != tt.ns {
+				t.Errorf("expected namespace %q, got %q", tt.ns, settings.Namespace)
 			}
 			if settings.KubeContext != tt.kcontext {
 				t.Errorf("expected kube-context %q, got %q", tt.kcontext, settings.KubeContext)