diff --git a/cmd/helm/release_testing_run.go b/cmd/helm/release_testing_run.go index 9608ba374..30b42d10a 100644 --- a/cmd/helm/release_testing_run.go +++ b/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 { diff --git a/docs/examples/nginx/templates/service-test.yaml b/docs/examples/nginx/templates/tests/service-test.yaml similarity index 92% rename from docs/examples/nginx/templates/service-test.yaml rename to docs/examples/nginx/templates/tests/service-test.yaml index ffb37e9f4..d79d4a521 100644 --- a/docs/examples/nginx/templates/service-test.yaml +++ b/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 diff --git a/pkg/action/install.go b/pkg/action/install.go index 1a463e64e..7371dd012 100644 --- a/pkg/action/install.go +++ b/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 to / diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 6aeb8b5b1..168f6ae1e 100644 --- a/pkg/action/release_testing.go +++ b/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 { diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index e3a1c1309..7c9002971 100644 --- a/pkg/action/uninstall.go +++ b/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 diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7e79d3971..34dea2b96 100644 --- a/pkg/action/upgrade.go +++ b/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 { diff --git a/pkg/release/release.go b/pkg/release/release.go index 80813dd7b..5bf0a28f5 100644 --- a/pkg/release/release.go +++ b/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. diff --git a/pkg/release/test.go b/pkg/release/test.go new file mode 100644 index 000000000..180795dc2 --- /dev/null +++ b/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"` +} diff --git a/pkg/releasetesting/environment.go b/pkg/releasetesting/environment.go index 7bff936b8..a02927bf1 100644 --- a/pkg/releasetesting/environment.go +++ b/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()) } diff --git a/pkg/releasetesting/test_suite.go b/pkg/releasetesting/test_suite.go index 62c6fb157..1a2a7a09b 100644 --- a/pkg/releasetesting/test_suite.go +++ b/pkg/releasetesting/test_suite.go @@ -24,17 +24,16 @@ 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" ) // TestSuite what tests are run, results, and metadata type TestSuite struct { - StartedAt time.Time - CompletedAt time.Time - TestManifests []string - Results []*release.TestRun + StartedAt time.Time + CompletedAt time.Time + Tests []*release.Test + Results []*release.TestRun } type test struct { @@ -48,8 +47,8 @@ type test struct { // extracted from the release func NewTestSuite(rel *release.Release) *TestSuite { return &TestSuite{ - TestManifests: extractTestManifestsFromHooks(rel.Hooks), - Results: []*release.TestRun{}, + 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, }, diff --git a/pkg/releasetesting/test_suite_test.go b/pkg/releasetesting/test_suite_test.go index 9256df467..c6f08b48e 100644 --- a/pkg/releasetesting/test_suite_test.go +++ b/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,41 +209,18 @@ 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, - Results: testResults, + Tests: tests, + Results: testResults, } return ts } diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index 41e501c9f..943d6ac36 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/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{ diff --git a/pkg/releaseutil/manifest_sorter_test.go b/pkg/releaseutil/manifest_sorter_test.go index 580b83137..010f81036 100644 --- a/pkg/releaseutil/manifest_sorter_test.go +++ b/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)) }