feat(*): support/run tests in templates/tests/

* save tests on release object
* removes dependency on test hooks
* test annotation key is now "helm.sh/test-expect-success" instead
of the test hook

Signed-off-by: Michelle Noorali <michellemolu@gmail.com>

cmd/helm/release_testing_run.go

@@ -44,8 +44,8 @@ func newReleaseTestRunCmd(cfg *action.Configuration, out io.Writer) *cobra.Comma
 		Long:  releaseTestRunHelp,
 		Args:  require.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			c, errc := client.Run(args[0])
 			testErr := &testErr{}
+			c, errc := client.Run(args[0])
 
 			for {
 				select {

docs/examples/nginx/templates/tests/service-test.yaml

@@ -8,7 +8,7 @@ metadata:
     helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version }}
     app.kubernetes.io/name: {{ template "nginx.name" . }}
   annotations:
-    "helm.sh/hook": test-success
+    "helm.sh/test-expect-success": "true"
 spec:
   containers:
     - name: curl

pkg/action/install.go

@@ -55,7 +55,7 @@ import (
 // See https://github.com/helm/helm/issues/1528
 const releaseNameMaxLen = 53
 
-// NOTESFILE_SUFFIX that we want to treat special. It goes through the templating engine
+// notesFileSuffix that we want to treat special. It goes through the templating engine
 // but it's not a yaml file (resource) hence can't have hooks, etc. And the user actually
 // wants to see this file after rendering in the status command. However, it must be a suffix
 // since there can be filepath in front of it.
@@ -135,7 +135,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) {
 
 	rel := i.createRelease(chrt, i.rawValues)
 	var manifestDoc *bytes.Buffer
-	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.OutputDir)
+	rel.Hooks, manifestDoc, rel.Info.Notes, rel.Tests, err = i.cfg.renderResources(chrt, valuesToRender, i.OutputDir)
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()
@@ -308,24 +308,25 @@ func (i *Install) replaceRelease(rel *release.Release) error {
 }
 
 // renderResources renders the templates in a chart
-func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, outputDir string) ([]*release.Hook, *bytes.Buffer, string, error) {
+func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, outputDir string) ([]*release.Hook, *bytes.Buffer, string, []*release.Test, error) {
 	hs := []*release.Hook{}
+	ts := []*release.Test{}
 	b := bytes.NewBuffer(nil)
 
 	caps, err := c.getCapabilities()
 	if err != nil {
-		return hs, b, "", err
+		return hs, b, "", ts, err
 	}
 
 	if ch.Metadata.KubeVersion != "" {
 		if !version.IsCompatibleRange(ch.Metadata.KubeVersion, caps.KubeVersion.String()) {
-			return hs, b, "", errors.Errorf("chart requires kubernetesVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.String())
+			return hs, b, "", ts, errors.Errorf("chart requires kubernetesVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.String())
 		}
 	}
 
 	files, err := engine.Render(ch, values)
 	if err != nil {
-		return hs, b, "", err
+		return hs, b, "", ts, err
 	}
 
 	// NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource,
@@ -345,10 +346,10 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 		}
 	}
 
-	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
-	// as partials are not used after renderer.Render. Empty manifests are also
-	// removed here.
-	hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder)
+	// Sort hooks, tests, resource manifests, and partials. Only hooks, tests, and
+	// resource manifests are returned, as partials are not used after renderer.Render.
+	// Empty manifests and test manifests are also removed here.
+	hs, manifests, tests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
 		// to Kubernetes.
@@ -361,7 +362,7 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 			}
 			fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content)
 		}
-		return hs, b, "", err
+		return hs, b, "", ts, err
 	}
 
 	// Aggregate all valid manifests into one big doc.
@@ -371,12 +372,12 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
 		} else {
 			err = writeToFile(outputDir, m.Name, m.Content)
 			if err != nil {
-				return hs, b, "", err
+				return hs, b, "", ts, err
 			}
 		}
 	}
 
-	return hs, b, notes, nil
+	return hs, b, notes, tests, nil
 }
 
 // write the <data> to <output-dir>/<name>

pkg/action/release_testing.go

@@ -83,7 +83,7 @@ func (r *ReleaseTesting) Run(name string) (<-chan *release.TestReleaseResponse,
 		}
 
 		if r.Cleanup {
-			testEnv.DeleteTestPods(tSuite.TestManifests)
+			testEnv.DeleteTestPods(tSuite.Tests)
 		}
 
 		if err := r.cfg.Releases.Update(rel); err != nil {

pkg/action/uninstall.go

@@ -208,7 +208,7 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kept string, errs []err
 	}
 
 	manifests := releaseutil.SplitManifests(rel.Manifest)
-	_, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder)
+	_, files, _, err := releaseutil.SortManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder)
 	if err != nil {
 		// We could instead just delete everything in no particular order.
 		// FIXME: One way to delete at this point would be to try a label-based

pkg/action/upgrade.go

@@ -159,7 +159,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele
 		return nil, nil, err
 	}
 
-	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "")
+	hooks, manifestDoc, notesTxt, tests, err := u.cfg.renderResources(chart, valuesToRender, "")
 	if err != nil {
 		return nil, nil, err
 	}
@@ -179,6 +179,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele
 		Version:  revision,
 		Manifest: manifestDoc.String(),
 		Hooks:    hooks,
+		Tests:    tests,
 	}
 
 	if len(notesTxt) > 0 {

pkg/release/release.go

@@ -37,6 +37,8 @@ type Release struct {
 	Version int `json:"version,omitempty"`
 	// Namespace is the kubernetes namespace of the release.
 	Namespace string `json:"namespace,omitempty"`
+	// Tests are all of the release tests declared for this release.
+	Tests []*Test `json:"tests,omitempty"`
 }
 
 // SetStatus is a helper for setting the status on a release.

pkg/release/test.go

@@ -0,0 +1,29 @@
+/*
+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 release
+
+// Test defines a test object.
+type Test struct {
+	Name string `json:"name,omitempty"`
+	// Kind is the Kubernetes kind.
+	Kind string `json:"kind,omitempty"`
+	// Path is the chart-relative path to the template.
+	Path string `json:"path,omitempty"`
+	// ExpectSuccess indicates whether test should be successful or fail
+	ExpectSuccess bool `json:"expect_success,omitempty"`
+	// Manifest is the manifest contents.
+	Manifest string `json:"manifest,omitempty"`
+}

pkg/releasetesting/environment.go

@@ -110,9 +110,9 @@ func (env *Environment) streamMessage(msg string, status release.TestRunStatus)
 }
 
 // DeleteTestPods deletes resources given in testManifests
-func (env *Environment) DeleteTestPods(testManifests []string) {
-	for _, testManifest := range testManifests {
-		err := env.KubeClient.Delete(bytes.NewBufferString(testManifest))
+func (env *Environment) DeleteTestPods(ts []*release.Test) {
+	for _, t := range ts {
+		err := env.KubeClient.Delete(bytes.NewBufferString(t.Manifest))
 		if err != nil {
 			env.streamError(err.Error())
 		}

pkg/releasetesting/test_suite.go

@@ -24,7 +24,6 @@ import (
 	"github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 
-	"helm.sh/helm/pkg/hooks"
 	"helm.sh/helm/pkg/release"
 	util "helm.sh/helm/pkg/releaseutil"
 )
@@ -33,7 +32,7 @@ import (
 type TestSuite struct {
 	StartedAt   time.Time
 	CompletedAt time.Time
-	TestManifests []string
+	Tests       []*release.Test
 	Results     []*release.TestRun
 }
 
@@ -48,7 +47,7 @@ type test struct {
 //  extracted from the release
 func NewTestSuite(rel *release.Release) *TestSuite {
 	return &TestSuite{
-		TestManifests: extractTestManifestsFromHooks(rel.Hooks),
+		Tests:   rel.Tests,
 		Results: []*release.TestRun{},
 	}
 }
@@ -57,13 +56,12 @@ func NewTestSuite(rel *release.Release) *TestSuite {
 func (ts *TestSuite) Run(env *Environment) error {
 	ts.StartedAt = time.Now()
 
-	if len(ts.TestManifests) == 0 {
-		// TODO: make this better, adding test run status on test suite is weird
+	if len(ts.Tests) == 0 {
 		env.streamMessage("No Tests Found", release.TestRunUnknown)
 	}
 
-	for _, testManifest := range ts.TestManifests {
-		test, err := newTest(testManifest)
+	for _, t := range ts.Tests {
+		test, err := newTest(t)
 		if err != nil {
 			return err
 		}
@@ -133,34 +131,9 @@ func (t *test) assignTestResult(podStatus v1.PodPhase) error {
 	return nil
 }
 
-func expectedSuccess(hookTypes []string) (bool, error) {
-	for _, hookType := range hookTypes {
-		hookType = strings.ToLower(strings.TrimSpace(hookType))
-		if hookType == hooks.ReleaseTestSuccess {
-			return true, nil
-		} else if hookType == hooks.ReleaseTestFailure {
-			return false, nil
-		}
-	}
-	return false, errors.Errorf("no %s or %s hook found", hooks.ReleaseTestSuccess, hooks.ReleaseTestFailure)
-}
-
-func extractTestManifestsFromHooks(h []*release.Hook) []string {
-	testHooks := hooks.FilterTestHooks(h)
-
-	tests := []string{}
-	for _, h := range testHooks {
-		individualTests := util.SplitManifests(h.Manifest)
-		for _, t := range individualTests {
-			tests = append(tests, t)
-		}
-	}
-	return tests
-}
-
-func newTest(testManifest string) (*test, error) {
+func newTest(ts *release.Test) (*test, error) {
 	var sh util.SimpleHead
-	err := yaml.Unmarshal([]byte(testManifest), &sh)
+	err := yaml.Unmarshal([]byte(ts.Manifest), &sh)
 	if err != nil {
 		return nil, err
 	}
@@ -169,17 +142,11 @@ func newTest(testManifest string) (*test, error) {
 		return nil, errors.Errorf("%s is not a pod", sh.Metadata.Name)
 	}
 
-	hookTypes := sh.Metadata.Annotations[hooks.HookAnno]
-	expected, err := expectedSuccess(strings.Split(hookTypes, ","))
-	if err != nil {
-		return nil, err
-	}
-
 	name := strings.TrimSuffix(sh.Metadata.Name, ",")
 	return &test{
 		name:            name,
-		manifest:        testManifest,
-		expectedSuccess: expected,
+		manifest:        ts.Manifest,
+		expectedSuccess: ts.ExpectSuccess,
 		result: &release.TestRun{
 			Name: name,
 		},

pkg/releasetesting/test_suite_test.go

@@ -27,13 +27,13 @@ import (
 	"helm.sh/helm/pkg/release"
 )
 
-const manifestWithTestSuccessHook = `
+const manifestTestSuccess = `
 apiVersion: v1
 kind: Pod
 metadata:
   name: finding-nemo,
   annotations:
-    "helm.sh/hook": test-success
+    "helm.sh/test-expect-success": "true"
 spec:
   containers:
   - name: nemo-test
@@ -41,32 +41,38 @@ spec:
     cmd: fake-command
 `
 
-const manifestWithTestFailureHook = `
+const manifestTestFailure = `
 apiVersion: v1
 kind: Pod
 metadata:
   name: gold-rush,
   annotations:
-    "helm.sh/hook": test-failure
+    "helm.sh/test-expect-success": "false"
 spec:
   containers:
   - name: gold-finding-test
     image: fake-gold-finding-image
     cmd: fake-gold-finding-command
 `
-const manifestWithInstallHooks = `apiVersion: v1
-kind: ConfigMap
-metadata:
-  name: test-cm
-  annotations:
-    "helm.sh/hook": post-install,pre-delete
-data:
-  name: value
-`
 
 func TestRun(t *testing.T) {
-	testManifests := []string{manifestWithTestSuccessHook, manifestWithTestFailureHook}
-	ts := testSuiteFixture(testManifests)
+	releaseTests := []*release.Test{
+		{
+			Name:          "finding-nemo",
+			Kind:          "Pod",
+			Path:          "somepath/here",
+			ExpectSuccess: true,
+			Manifest:      manifestTestSuccess,
+		},
+		{
+			Name:          "gold-rush",
+			Kind:          "Pod",
+			Path:          "anotherpath/here",
+			ExpectSuccess: false,
+			Manifest:      manifestTestFailure,
+		},
+	}
+	ts := testSuiteFixture(releaseTests)
 	env := testEnvFixture()
 
 	go func() {
@@ -122,7 +128,7 @@ func TestRun(t *testing.T) {
 }
 
 func TestRunEmptyTestSuite(t *testing.T) {
-	ts := testSuiteFixture([]string{})
+	ts := testSuiteFixture([]*release.Test{})
 	env := testEnvFixture()
 
 	go func() {
@@ -151,8 +157,16 @@ func TestRunEmptyTestSuite(t *testing.T) {
 	}
 }
 
-func TestRunSuccessWithTestFailureHook(t *testing.T) {
-	ts := testSuiteFixture([]string{manifestWithTestFailureHook})
+func TestRunSuccessWithTestFailure(t *testing.T) {
+	ts := testSuiteFixture(
+		[]*release.Test{
+			{
+				Name:          "gold-rus",
+				Kind:          "Pod",
+				Path:          "somepath/here",
+				ExpectSuccess: false,
+				Manifest:      manifestTestFailure,
+			}})
 	env := testEnvFixture()
 	env.KubeClient = &mockKubeClient{podFail: true}
 
@@ -195,40 +209,17 @@ func TestRunSuccessWithTestFailureHook(t *testing.T) {
 	}
 }
 
-func TestExtractTestManifestsFromHooks(t *testing.T) {
-	testManifests := extractTestManifestsFromHooks(hooksStub)
-
-	if len(testManifests) != 1 {
-		t.Errorf("Expected 1 test manifest, Got: %v", len(testManifests))
-	}
-}
-
-var hooksStub = []*release.Hook{
-	{
-		Manifest: manifestWithTestSuccessHook,
-		Events: []release.HookEvent{
-			release.HookReleaseTestSuccess,
-		},
-	},
-	{
-		Manifest: manifestWithInstallHooks,
-		Events: []release.HookEvent{
-			release.HookPostInstall,
-		},
-	},
-}
-
 func testFixture() *test {
 	return &test{
-		manifest: manifestWithTestSuccessHook,
+		manifest: manifestTestSuccess,
 		result:   &release.TestRun{},
 	}
 }
 
-func testSuiteFixture(testManifests []string) *TestSuite {
+func testSuiteFixture(tests []*release.Test) *TestSuite {
 	testResults := []*release.TestRun{}
 	ts := &TestSuite{
-		TestManifests: testManifests,
+		Tests:   tests,
 		Results: testResults,
 	}
 	return ts

pkg/releaseutil/manifest_sorter.go

@@ -48,6 +48,7 @@ type manifestFile struct {
 type result struct {
 	hooks   []*release.Hook
 	generic []Manifest
+	tests   []*release.Test
 }
 
 // TODO: Refactor this out. It's here because naming conventions were not followed through.
@@ -66,7 +67,8 @@ var events = map[string]release.HookEvent{
 }
 
 // SortManifests takes a map of filename/YAML contents, splits the file
-// by manifest entries, and sorts the entries into hook types.
+// by manifest entries, skips partials, skips empty manifests, sorts test
+// and hook entries.
 //
 // The resulting hooks struct will be populated with all of the generated hooks.
 // Any file that does not declare one of the hook types will be placed in the
@@ -74,16 +76,16 @@ var events = map[string]release.HookEvent{
 //
 // Files that do not parse into the expected format are simply placed into a map and
 // returned.
-func SortManifests(files map[string]string, apis chartutil.VersionSet, sort KindSortOrder) ([]*release.Hook, []Manifest, error) {
+func SortManifests(files map[string]string, apis chartutil.VersionSet, sort KindSortOrder) ([]*release.Hook, []Manifest, []*release.Test, error) {
 	result := &result{}
 
-	for filePath, c := range files {
-
+	for filepath, c := range files {
 		// Skip partials. We could return these as a separate map, but there doesn't
 		// seem to be any need for that at this time.
-		if strings.HasPrefix(path.Base(filePath), "_") {
+		if strings.HasPrefix(path.Base(filepath), "_") {
 			continue
 		}
+
 		// Skip empty files and log this.
 		if strings.TrimSpace(c) == "" {
 			continue
@@ -91,21 +93,21 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, sort Kind
 
 		manifestFile := &manifestFile{
 			entries: SplitManifests(c),
-			path:    filePath,
+			path:    filepath,
 			apis:    apis,
 		}
 
 		if err := manifestFile.sort(result); err != nil {
-			return result.hooks, result.generic, err
+			return result.hooks, result.generic, result.tests, err
 		}
 	}
 
-	return result.hooks, sortByKind(result.generic, sort), nil
+	return result.hooks, sortByKind(result.generic, sort), result.tests, nil
 }
 
 // sort takes a manifestFile object which may contain multiple resource definition
-// entries and sorts each entry by hook types, and saves the resulting hooks and
-// generic manifests (or non-hooks) to the result struct.
+// entries and sorts each entry by hook types, and saves the resulting hooks,
+// generic manifests (or non-hooks), and test manifests to the result struct.
 //
 // To determine hook type, it looks for a YAML structure like this:
 //
@@ -122,6 +124,14 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, sort Kind
 //  metadata:
 // 		annotations:
 // 			helm.sh/hook-delete-policy: hook-succeeded
+//
+// To determine is manifest is test type, it looks for a YAML structure like this:
+//
+//  kind: Pod
+//  apiVersion: v1
+// 	metadata:
+//		annotations:
+//			helm.sh/test-expect-success: true
 func (file *manifestFile) sort(result *result) error {
 	for _, m := range file.entries {
 		var entry SimpleHead
@@ -142,6 +152,25 @@ func (file *manifestFile) sort(result *result) error {
 			continue
 		}
 
+		expectation, ok := entry.Metadata.Annotations["helm.sh/test-expect-success"]
+
+		if ok {
+			expectBool, err := strconv.ParseBool(expectation)
+			if err != nil {
+				return err
+			}
+			t := &release.Test{
+				Name:          entry.Metadata.Name,
+				Kind:          entry.Kind,
+				Path:          file.path,
+				ExpectSuccess: expectBool,
+				Manifest:      m,
+			}
+			result.tests = append(result.tests, t)
+
+			continue
+		}
+
 		hookTypes, ok := entry.Metadata.Annotations[hooks.HookAnno]
 		if !ok {
 			result.generic = append(result.generic, Manifest{

pkg/releaseutil/manifest_sorter_test.go

@@ -104,7 +104,8 @@ metadata:
 			kind:     []string{"ReplicaSet"},
 			hooks:    map[string][]release.HookEvent{"sixth": nil},
 			manifest: `invalid manifest`, // This will fail if partial is not skipped.
-		}, {
+		},
+		{
 			// Regression test: files with no content should be skipped.
 			name:     []string{"seventh"},
 			path:     "seven",
@@ -130,6 +131,39 @@ metadata:
   name: example-test
   annotations:
     "helm.sh/hook": test-success
+`,
+		},
+		{
+			name:  []string{"ninth"},
+			path:  "tests/ninth",
+			kind:  []string{"Pod"},
+			hooks: nil,
+			manifest: `kind: Pod
+apiVersion: v1
+metadata:
+  name: ninth
+  annotations:
+    "helm.sh/test-expect-success": "true"
+`,
+		},
+		{
+			name:  []string{"tenth"},
+			path:  "tests/tenth",
+			kind:  []string{"Pod"},
+			hooks: nil,
+			manifest: `kind: Pod
+apiVersion: v1
+metadata:
+  name: tenth
+  annotations:
+    "helm.sh/test-expect-success": "true"
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: tenth-second-test
+  annotations:
+    "helm.sh/test-expect-success": "false"
 `,
 		},
 	}
@@ -139,16 +173,20 @@ metadata:
 		manifests[o.path] = o.manifest
 	}
 
-	hs, generic, err := SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder)
+	hs, generic, ts, err := SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder)
 	if err != nil {
 		t.Fatalf("Unexpected error: %s", err)
 	}
 
-	// This test will fail if 'six' or 'seven' was added.
+	// This test will fail if 'six' or 'seven' or 'nine' were added.
 	if len(generic) != 2 {
 		t.Errorf("Expected 2 generic manifests, got %d", len(generic))
 	}
 
+	if len(ts) != 3 {
+		t.Errorf("Expected 3 test manifests, got %d", len(ts))
+	}
+
 	if len(hs) != 4 {
 		t.Errorf("Expected 4 hooks, got %d", len(hs))
 	}