From 72127c391cee9ab8f8cfa14306ab2cf9b1445a14 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Sat, 20 Jul 2019 12:05:02 -0400 Subject: [PATCH 1/8] feat(test): define tests as Jobs and allow arbitrary supporting resources This updates commands install, upgrade, delete, and test to share the same implementation for hook execution. BREAKING CHANGES: - The `test-failure` hook annotation is removed. Signed-off-by: Jacob LeGrone --- cmd/helm/release_testing_run.go | 33 +-- pkg/action/action_test.go | 4 +- pkg/action/hooks.go | 106 ++++++++++ pkg/action/install.go | 88 +------- pkg/action/release_testing.go | 45 +--- pkg/action/rollback.go | 63 +----- pkg/action/uninstall.go | 53 +---- pkg/action/upgrade.go | 53 +---- pkg/hooks/hooks.go | 67 ------ pkg/kube/client.go | 27 --- pkg/kube/fake/fake.go | 26 +-- pkg/kube/fake/printer.go | 6 - pkg/kube/interface.go | 6 - pkg/release/hook.go | 39 ++-- pkg/releasetesting/environment.go | 120 ----------- pkg/releasetesting/environment_test.go | 71 ------- pkg/releasetesting/test_suite.go | 187 ----------------- pkg/releasetesting/test_suite_test.go | 259 ------------------------ pkg/releaseutil/manifest_sorter.go | 27 ++- pkg/releaseutil/manifest_sorter_test.go | 4 +- pkg/releaseutil/manifest_test.go | 4 +- 21 files changed, 179 insertions(+), 1109 deletions(-) create mode 100644 pkg/action/hooks.go delete mode 100644 pkg/hooks/hooks.go delete mode 100644 pkg/releasetesting/environment.go delete mode 100644 pkg/releasetesting/environment_test.go delete mode 100644 pkg/releasetesting/test_suite.go delete mode 100644 pkg/releasetesting/test_suite_test.go diff --git a/cmd/helm/release_testing_run.go b/cmd/helm/release_testing_run.go index 9608ba374..d300a65e6 100644 --- a/cmd/helm/release_testing_run.go +++ b/cmd/helm/release_testing_run.go @@ -16,16 +16,13 @@ limitations under the License. package main import ( - "fmt" "io" "time" - "github.com/pkg/errors" "github.com/spf13/cobra" "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" - "helm.sh/helm/pkg/release" ) const releaseTestRunHelp = ` @@ -44,27 +41,7 @@ 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{} - - for { - select { - case err := <-errc: - if err != nil && testErr.failed > 0 { - return testErr.Error() - } - return err - case res, ok := <-c: - if !ok { - break - } - - if res.Status == release.TestRunFailure { - testErr.failed++ - } - fmt.Fprintf(out, res.Msg+"\n") - } - } + return client.Run(args[0]) }, } @@ -74,11 +51,3 @@ func newReleaseTestRunCmd(cfg *action.Configuration, out io.Writer) *cobra.Comma return cmd } - -type testErr struct { - failed int -} - -func (err *testErr) Error() error { - return errors.Errorf("%v test(s) failed", err.failed) -} diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index 83346ea58..33fd98c28 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -89,7 +89,7 @@ var manifestWithTestHook = `kind: Pod metadata: name: finding-nemo, annotations: - "helm.sh/hook": test-success + "helm.sh/hook": test spec: containers: - name: nemo-test @@ -231,7 +231,7 @@ func namedReleaseStub(name string, status release.Status) *release.Release { Path: "finding-nemo", Manifest: manifestWithTestHook, Events: []release.HookEvent{ - release.HookReleaseTestSuccess, + release.HookTest, }, }, }, diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go new file mode 100644 index 000000000..dd768d112 --- /dev/null +++ b/pkg/action/hooks.go @@ -0,0 +1,106 @@ +/* +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 action + +import ( + "bytes" + "sort" + "time" + + "github.com/pkg/errors" + + "helm.sh/helm/pkg/release" +) + +// execHook executes all of the hooks for the given hook event. +func (cfg *Configuration) execHook(hs []*release.Hook, hook release.HookEvent, timeout time.Duration) error { + executingHooks := []*release.Hook{} + + for _, h := range hs { + for _, e := range h.Events { + if e == hook { + executingHooks = append(executingHooks, h) + } + } + } + + sort.Sort(hookByWeight(executingHooks)) + + for _, h := range executingHooks { + if err := deleteHookByPolicy(cfg, h, release.HookBeforeHookCreation); err != nil { + return err + } + + b := bytes.NewBufferString(h.Manifest) + if err := cfg.KubeClient.Create(b); err != nil { + return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) + } + b.Reset() + b.WriteString(h.Manifest) + + if err := cfg.KubeClient.WatchUntilReady(b, timeout); err != nil { + // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted + // under failed condition. If so, then clear the corresponding resource object in the hook + if err := deleteHookByPolicy(cfg, h, release.HookFailed); err != nil { + return err + } + return err + } + } + + // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted + // under succeeded condition. If so, then clear the corresponding resource object in each hook + for _, h := range executingHooks { + if err := deleteHookByPolicy(cfg, h, release.HookSucceeded); err != nil { + return err + } + h.LastRun = time.Now() + } + + return nil +} + +// hookByWeight is a sorter for hooks +type hookByWeight []*release.Hook + +func (x hookByWeight) Len() int { return len(x) } +func (x hookByWeight) Swap(i, j int) { x[i], x[j] = x[j], x[i] } +func (x hookByWeight) Less(i, j int) bool { + if x[i].Weight == x[j].Weight { + return x[i].Name < x[j].Name + } + return x[i].Weight < x[j].Weight +} + +// deleteHookByPolicy deletes a hook if the hook policy instructs it to +func deleteHookByPolicy(cfg *Configuration, h *release.Hook, policy release.HookDeletePolicy) error { + if hookHasDeletePolicy(h, policy) { + b := bytes.NewBufferString(h.Manifest) + return cfg.KubeClient.Delete(b) + } + return nil +} + +// hookHasDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices +// supported by helm. If so, mark the hook as one should be deleted. +func hookHasDeletePolicy(h *release.Hook, policy release.HookDeletePolicy) bool { + for _, v := range h.DeletePolicies { + if policy == v { + return true + } + } + return false +} diff --git a/pkg/action/install.go b/pkg/action/install.go index 52eff5262..3969cb26d 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -25,7 +25,6 @@ import ( "os" "path" "path/filepath" - "sort" "strings" "text/template" "time" @@ -40,7 +39,6 @@ import ( "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/engine" "helm.sh/helm/pkg/getter" - "helm.sh/helm/pkg/hooks" kubefake "helm.sh/helm/pkg/kube/fake" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/releaseutil" @@ -198,7 +196,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { // pre-install hooks if !i.DisableHooks { - if err := i.execHook(rel.Hooks, hooks.PreInstall); err != nil { + if err := i.execHook(rel.Hooks, release.HookPreInstall); err != nil { return i.failRelease(rel, fmt.Errorf("failed pre-install: %s", err)) } } @@ -220,7 +218,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { } if !i.DisableHooks { - if err := i.execHook(rel.Hooks, hooks.PostInstall); err != nil { + if err := i.execHook(rel.Hooks, release.HookPostInstall); err != nil { return i.failRelease(rel, fmt.Errorf("failed post-install: %s", err)) } } @@ -466,86 +464,8 @@ func (i *Install) validateManifest(manifest io.Reader) error { } // execHook executes all of the hooks for the given hook event. -func (i *Install) execHook(hs []*release.Hook, hook string) error { - executingHooks := []*release.Hook{} - - for _, h := range hs { - for _, e := range h.Events { - if string(e) == hook { - executingHooks = append(executingHooks, h) - } - } - } - - sort.Sort(hookByWeight(executingHooks)) - - for _, h := range executingHooks { - if err := deleteHookByPolicy(i.cfg, h, hooks.BeforeHookCreation); err != nil { - return err - } - - b := bytes.NewBufferString(h.Manifest) - if err := i.cfg.KubeClient.Create(b); err != nil { - return errors.Wrapf(err, "warning: Release %s %s %s failed", i.ReleaseName, hook, h.Path) - } - b.Reset() - b.WriteString(h.Manifest) - - if err := i.cfg.KubeClient.WatchUntilReady(b, i.Timeout); err != nil { - // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted - // under failed condition. If so, then clear the corresponding resource object in the hook - if err := deleteHookByPolicy(i.cfg, h, hooks.HookFailed); err != nil { - return err - } - return err - } - } - - // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted - // under succeeded condition. If so, then clear the corresponding resource object in each hook - for _, h := range executingHooks { - if err := deleteHookByPolicy(i.cfg, h, hooks.HookSucceeded); err != nil { - return err - } - h.LastRun = time.Now() - } - - return nil -} - -// deletePolices represents a mapping between the key in the annotation for label deleting policy and its real meaning -// FIXME: Can we refactor this out? -var deletePolices = map[string]release.HookDeletePolicy{ - hooks.HookSucceeded: release.HookSucceeded, - hooks.HookFailed: release.HookFailed, - hooks.BeforeHookCreation: release.HookBeforeHookCreation, -} - -// hookHasDeletePolicy determines whether the defined hook deletion policy matches the hook deletion polices -// supported by helm. If so, mark the hook as one should be deleted. -func hookHasDeletePolicy(h *release.Hook, policy string) bool { - dp, ok := deletePolices[policy] - if !ok { - return false - } - for _, v := range h.DeletePolicies { - if dp == v { - return true - } - } - return false -} - -// hookByWeight is a sorter for hooks -type hookByWeight []*release.Hook - -func (x hookByWeight) Len() int { return len(x) } -func (x hookByWeight) Swap(i, j int) { x[i], x[j] = x[j], x[i] } -func (x hookByWeight) Less(i, j int) bool { - if x[i].Weight == x[j].Weight { - return x[i].Name < x[j].Name - } - return x[i].Weight < x[j].Weight +func (i *Install) execHook(hs []*release.Hook, hook release.HookEvent) error { + return i.cfg.execHook(hs, hook, i.Timeout) } // NameAndChart returns the name and chart that should be used. diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index 6aeb8b5b1..eb7e1ccc7 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -22,7 +22,6 @@ import ( "github.com/pkg/errors" "helm.sh/helm/pkg/release" - reltesting "helm.sh/helm/pkg/releasetesting" ) // ReleaseTesting is the action for testing a release. @@ -43,52 +42,16 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting { } // Run executes 'helm test' against the given release. -func (r *ReleaseTesting) Run(name string) (<-chan *release.TestReleaseResponse, <-chan error) { - errc := make(chan error, 1) +func (r *ReleaseTesting) Run(name string) error { if err := validateReleaseName(name); err != nil { - errc <- errors.Errorf("releaseTest: Release name is invalid: %s", name) - return nil, errc + 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 { - errc <- err - return nil, errc + return err } - ch := make(chan *release.TestReleaseResponse, 1) - testEnv := &reltesting.Environment{ - Namespace: rel.Namespace, - KubeClient: r.cfg.KubeClient, - Timeout: r.Timeout, - Messages: ch, - } - r.cfg.Log("running tests for release %s", rel.Name) - tSuite := reltesting.NewTestSuite(rel) - - go func() { - defer close(errc) - defer close(ch) - - if err := tSuite.Run(testEnv); err != nil { - errc <- errors.Wrapf(err, "error running test suite for %s", rel.Name) - return - } - - rel.Info.LastTestSuiteRun = &release.TestSuite{ - StartedAt: tSuite.StartedAt, - CompletedAt: tSuite.CompletedAt, - Results: tSuite.Results, - } - - if r.Cleanup { - testEnv.DeleteTestPods(tSuite.TestManifests) - } - - if err := r.cfg.Releases.Update(rel); err != nil { - r.cfg.Log("test: Failed to store updated release: %s", err) - } - }() - return ch, errc + return r.cfg.execHook(rel.Hooks, release.HookTest, r.Timeout) } diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 2db6ed7a9..c709ff20c 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -19,12 +19,10 @@ package action import ( "bytes" "fmt" - "sort" "time" "github.com/pkg/errors" - "helm.sh/helm/pkg/hooks" "helm.sh/helm/pkg/release" ) @@ -140,7 +138,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas // pre-rollback hooks if !r.DisableHooks { - if err := r.execHook(targetRelease.Hooks, hooks.PreRollback); err != nil { + if err := r.execHook(targetRelease.Hooks, release.HookPreRollback); err != nil { return targetRelease, err } } else { @@ -173,7 +171,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas // post-rollback hooks if !r.DisableHooks { - if err := r.execHook(targetRelease.Hooks, hooks.PostRollback); err != nil { + if err := r.execHook(targetRelease.Hooks, release.HookPostRollback); err != nil { return targetRelease, err } } @@ -195,59 +193,6 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas } // execHook executes all of the hooks for the given hook event. -func (r *Rollback) execHook(hs []*release.Hook, hook string) error { - timeout := r.Timeout - executingHooks := []*release.Hook{} - - for _, h := range hs { - for _, e := range h.Events { - if string(e) == hook { - executingHooks = append(executingHooks, h) - } - } - } - - sort.Sort(hookByWeight(executingHooks)) - - for _, h := range executingHooks { - if err := deleteHookByPolicy(r.cfg, h, hooks.BeforeHookCreation); err != nil { - return err - } - - b := bytes.NewBufferString(h.Manifest) - if err := r.cfg.KubeClient.Create(b); err != nil { - return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) - } - b.Reset() - b.WriteString(h.Manifest) - - if err := r.cfg.KubeClient.WatchUntilReady(b, timeout); err != nil { - // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted - // under failed condition. If so, then clear the corresponding resource object in the hook - if err := deleteHookByPolicy(r.cfg, h, hooks.HookFailed); err != nil { - return err - } - return err - } - } - - // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted - // under succeeded condition. If so, then clear the corresponding resource object in each hook - for _, h := range executingHooks { - if err := deleteHookByPolicy(r.cfg, h, hooks.HookSucceeded); err != nil { - return err - } - h.LastRun = time.Now() - } - - return nil -} - -// deleteHookByPolicy deletes a hook if the hook policy instructs it to -func deleteHookByPolicy(cfg *Configuration, h *release.Hook, policy string) error { - if hookHasDeletePolicy(h, policy) { - b := bytes.NewBufferString(h.Manifest) - return cfg.KubeClient.Delete(b) - } - return nil +func (r *Rollback) execHook(hs []*release.Hook, hook release.HookEvent) error { + return r.cfg.execHook(hs, hook, r.Timeout) } diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index cbe3f49dc..4090e8241 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -18,13 +18,11 @@ package action import ( "bytes" - "sort" "strings" "time" "github.com/pkg/errors" - "helm.sh/helm/pkg/hooks" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/releaseutil" @@ -94,7 +92,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) res := &release.UninstallReleaseResponse{Release: rel} if !u.DisableHooks { - if err := u.execHook(rel.Hooks, hooks.PreDelete); err != nil { + if err := u.execHook(rel.Hooks, release.HookPreDelete); err != nil { return res, err } } else { @@ -111,7 +109,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) res.Info = kept if !u.DisableHooks { - if err := u.execHook(rel.Hooks, hooks.PostDelete); err != nil { + if err := u.execHook(rel.Hooks, release.HookPostDelete); err != nil { errs = append(errs, err) } } @@ -162,51 +160,8 @@ func joinErrors(errs []error) string { } // execHook executes all of the hooks for the given hook event. -func (u *Uninstall) execHook(hs []*release.Hook, hook string) error { - executingHooks := []*release.Hook{} - - for _, h := range hs { - for _, e := range h.Events { - if string(e) == hook { - executingHooks = append(executingHooks, h) - } - } - } - - sort.Sort(hookByWeight(executingHooks)) - - for _, h := range executingHooks { - if err := deleteHookByPolicy(u.cfg, h, hooks.BeforeHookCreation); err != nil { - return err - } - - b := bytes.NewBufferString(h.Manifest) - if err := u.cfg.KubeClient.Create(b); err != nil { - return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) - } - b.Reset() - b.WriteString(h.Manifest) - - if err := u.cfg.KubeClient.WatchUntilReady(b, u.Timeout); err != nil { - // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted - // under failed condition. If so, then clear the corresponding resource object in the hook - if err := deleteHookByPolicy(u.cfg, h, hooks.HookFailed); err != nil { - return err - } - return err - } - } - - // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted - // under succeeded condition. If so, then clear the corresponding resource object in each hook - for _, h := range executingHooks { - if err := deleteHookByPolicy(u.cfg, h, hooks.HookSucceeded); err != nil { - return err - } - h.LastRun = time.Now() - } - - return nil +func (u *Uninstall) execHook(hs []*release.Hook, hook release.HookEvent) error { + return u.cfg.execHook(hs, hook, u.Timeout) } // deleteRelease deletes the release and returns manifests that were kept in the deletion process diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 43f05beac..6de69a842 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -19,14 +19,12 @@ package action import ( "bytes" "fmt" - "sort" "time" "github.com/pkg/errors" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chartutil" - "helm.sh/helm/pkg/hooks" "helm.sh/helm/pkg/kube" "helm.sh/helm/pkg/release" "helm.sh/helm/pkg/releaseutil" @@ -201,7 +199,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea // pre-upgrade hooks if !u.DisableHooks { - if err := u.execHook(upgradedRelease.Hooks, hooks.PreUpgrade); err != nil { + if err := u.execHook(upgradedRelease.Hooks, release.HookPreUpgrade); err != nil { return u.failRelease(upgradedRelease, fmt.Errorf("pre-upgrade hooks failed: %s", err)) } } else { @@ -222,7 +220,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea // post-upgrade hooks if !u.DisableHooks { - if err := u.execHook(upgradedRelease.Hooks, hooks.PostUpgrade); err != nil { + if err := u.execHook(upgradedRelease.Hooks, release.HookPostUpgrade); err != nil { return u.failRelease(upgradedRelease, fmt.Errorf("post-upgrade hooks failed: %s", err)) } } @@ -335,49 +333,6 @@ func validateManifest(c kube.Interface, manifest []byte) error { } // execHook executes all of the hooks for the given hook event. -func (u *Upgrade) execHook(hs []*release.Hook, hook string) error { - timeout := u.Timeout - executingHooks := []*release.Hook{} - - for _, h := range hs { - for _, e := range h.Events { - if string(e) == hook { - executingHooks = append(executingHooks, h) - } - } - } - - sort.Sort(hookByWeight(executingHooks)) - for _, h := range executingHooks { - if err := deleteHookByPolicy(u.cfg, h, hooks.BeforeHookCreation); err != nil { - return err - } - - b := bytes.NewBufferString(h.Manifest) - if err := u.cfg.KubeClient.Create(b); err != nil { - return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) - } - b.Reset() - b.WriteString(h.Manifest) - - if err := u.cfg.KubeClient.WatchUntilReady(b, timeout); err != nil { - // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted - // under failed condition. If so, then clear the corresponding resource object in the hook - if err := deleteHookByPolicy(u.cfg, h, hooks.HookFailed); err != nil { - return err - } - return err - } - } - - // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted - // under succeeded condition. If so, then clear the corresponding resource object in each hook - for _, h := range executingHooks { - if err := deleteHookByPolicy(u.cfg, h, hooks.HookSucceeded); err != nil { - return err - } - h.LastRun = time.Now() - } - - return nil +func (u *Upgrade) execHook(hs []*release.Hook, hook release.HookEvent) error { + return u.cfg.execHook(hs, hook, u.Timeout) } diff --git a/pkg/hooks/hooks.go b/pkg/hooks/hooks.go deleted file mode 100644 index 6b6c6fdcc..000000000 --- a/pkg/hooks/hooks.go +++ /dev/null @@ -1,67 +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 hooks - -import ( - "helm.sh/helm/pkg/release" -) - -// HookAnno is the label name for a hook -const HookAnno = "helm.sh/hook" - -// HookWeightAnno is the label name for a hook weight -const HookWeightAnno = "helm.sh/hook-weight" - -// HookDeleteAnno is the label name for the delete policy for a hook -const HookDeleteAnno = "helm.sh/hook-delete-policy" - -// Types of hooks -const ( - PreInstall = "pre-install" - PostInstall = "post-install" - PreDelete = "pre-delete" - PostDelete = "post-delete" - PreUpgrade = "pre-upgrade" - PostUpgrade = "post-upgrade" - PreRollback = "pre-rollback" - PostRollback = "post-rollback" - ReleaseTestSuccess = "test-success" - ReleaseTestFailure = "test-failure" -) - -// Type of policy for deleting the hook -const ( - HookSucceeded = "hook-succeeded" - HookFailed = "hook-failed" - BeforeHookCreation = "before-hook-creation" -) - -// FilterTestHooks filters the list of hooks are returns only testing hooks. -func FilterTestHooks(hooks []*release.Hook) []*release.Hook { - testHooks := []*release.Hook{} - - for _, h := range hooks { - for _, e := range h.Events { - if e == release.HookReleaseTestSuccess || e == release.HookReleaseTestFailure { - testHooks = append(testHooks, h) - continue - } - } - } - - return testHooks -} diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 2af142787..4aa817af3 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -19,7 +19,6 @@ package kube // import "helm.sh/helm/pkg/kube" import ( "context" "encoding/json" - "fmt" "io" "log" "strings" @@ -487,29 +486,3 @@ func scrubValidationError(err error) error { } return err } - -// WaitAndGetCompletedPodPhase waits up to a timeout until a pod enters a completed phase -// and returns said phase (PodSucceeded or PodFailed qualify). -func (c *Client) WaitAndGetCompletedPodPhase(name string, timeout time.Duration) (v1.PodPhase, error) { - client, _ := c.KubernetesClientSet() - to := int64(timeout) - watcher, err := client.CoreV1().Pods(c.namespace()).Watch(metav1.ListOptions{ - FieldSelector: fmt.Sprintf("metadata.name=%s", name), - TimeoutSeconds: &to, - }) - - for event := range watcher.ResultChan() { - p, ok := event.Object.(*v1.Pod) - if !ok { - return v1.PodUnknown, fmt.Errorf("%s not a pod", name) - } - switch p.Status.Phase { - case v1.PodFailed: - return v1.PodFailed, nil - case v1.PodSucceeded: - return v1.PodSucceeded, nil - } - } - - return v1.PodUnknown, err -} diff --git a/pkg/kube/fake/fake.go b/pkg/kube/fake/fake.go index d22f0acf8..9d3a62069 100644 --- a/pkg/kube/fake/fake.go +++ b/pkg/kube/fake/fake.go @@ -21,7 +21,6 @@ import ( "io" "time" - v1 "k8s.io/api/core/v1" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/pkg/kube" @@ -32,15 +31,14 @@ import ( // delegates all its calls to `PrintingKubeClient` type FailingKubeClient struct { PrintingKubeClient - CreateError error - WaitError error - GetError error - DeleteError error - WatchUntilReadyError error - UpdateError error - BuildError error - BuildUnstructuredError error - WaitAndGetCompletedPodPhaseError error + DeleteError error + WatchUntilReadyError error + UpdateError error + BuildError error + BuildUnstructuredError error + CreateError error + WaitError error + GetError error } // Create returns the configured error if set or prints @@ -106,11 +104,3 @@ func (f *FailingKubeClient) BuildUnstructured(r io.Reader) (kube.Result, error) } return f.PrintingKubeClient.Build(r) } - -// WaitAndGetCompletedPodPhase returns the configured error if set or prints -func (f *FailingKubeClient) WaitAndGetCompletedPodPhase(s string, d time.Duration) (v1.PodPhase, error) { - if f.WaitAndGetCompletedPodPhaseError != nil { - return v1.PodSucceeded, f.WaitAndGetCompletedPodPhaseError - } - return f.PrintingKubeClient.WaitAndGetCompletedPodPhase(s, d) -} diff --git a/pkg/kube/fake/printer.go b/pkg/kube/fake/printer.go index b8d927e8d..cc21358f5 100644 --- a/pkg/kube/fake/printer.go +++ b/pkg/kube/fake/printer.go @@ -20,7 +20,6 @@ import ( "io" "time" - v1 "k8s.io/api/core/v1" "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/pkg/kube" @@ -77,8 +76,3 @@ func (p *PrintingKubeClient) Build(_ io.Reader) (kube.Result, error) { func (p *PrintingKubeClient) BuildUnstructured(_ io.Reader) (kube.Result, error) { return p.Build(nil) } - -// WaitAndGetCompletedPodPhase implements KubeClient WaitAndGetCompletedPodPhase. -func (p *PrintingKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Duration) (v1.PodPhase, error) { - return v1.PodSucceeded, nil -} diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 9256f5e1c..72d7a0ea9 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -19,8 +19,6 @@ package kube import ( "io" "time" - - v1 "k8s.io/api/core/v1" ) // KubernetesClient represents a client capable of communicating with the Kubernetes API. @@ -57,10 +55,6 @@ type Interface interface { Build(reader io.Reader) (Result, error) BuildUnstructured(reader io.Reader) (Result, error) - - // WaitAndGetCompletedPodPhase waits up to a timeout until a pod enters a completed phase - // and returns said phase (PodSucceeded or PodFailed qualify). - WaitAndGetCompletedPodPhase(name string, timeout time.Duration) (v1.PodPhase, error) } var _ Interface = (*Client)(nil) diff --git a/pkg/release/hook.go b/pkg/release/hook.go index d4cb73d54..a36412555 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -1,10 +1,11 @@ /* 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, @@ -15,23 +16,24 @@ limitations under the License. package release -import "time" +import ( + "time" +) // HookEvent specifies the hook event type HookEvent string // Hook event types const ( - HookPreInstall HookEvent = "pre-install" - HookPostInstall HookEvent = "post-install" - HookPreDelete HookEvent = "pre-delete" - HookPostDelete HookEvent = "post-delete" - HookPreUpgrade HookEvent = "pre-upgrade" - HookPostUpgrade HookEvent = "post-upgrade" - HookPreRollback HookEvent = "pre-rollback" - HookPostRollback HookEvent = "post-rollback" - HookReleaseTestSuccess HookEvent = "release-test-success" - HookReleaseTestFailure HookEvent = "release-test-failure" + HookPreInstall HookEvent = "pre-install" + HookPostInstall HookEvent = "post-install" + HookPreDelete HookEvent = "pre-delete" + HookPostDelete HookEvent = "post-delete" + HookPreUpgrade HookEvent = "pre-upgrade" + HookPostUpgrade HookEvent = "post-upgrade" + HookPreRollback HookEvent = "pre-rollback" + HookPostRollback HookEvent = "post-rollback" + HookTest HookEvent = "test" ) func (x HookEvent) String() string { return string(x) } @@ -41,13 +43,22 @@ type HookDeletePolicy string // Hook delete policy types const ( - HookSucceeded HookDeletePolicy = "succeeded" - HookFailed HookDeletePolicy = "failed" + HookSucceeded HookDeletePolicy = "hook-succeeded" + HookFailed HookDeletePolicy = "hook-failed" HookBeforeHookCreation HookDeletePolicy = "before-hook-creation" ) func (x HookDeletePolicy) String() string { return string(x) } +// HookAnnotation is the label name for a hook +const HookAnnotation = "helm.sh/hook" + +// HookWeightAnnotation is the label name for a hook weight +const HookWeightAnnotation = "helm.sh/hook-weight" + +// HookDeleteAnnotation is the label name for the delete policy for a hook +const HookDeleteAnnotation = "helm.sh/hook-delete-policy" + // Hook defines a hook object. type Hook struct { Name string `json:"name,omitempty"` diff --git a/pkg/releasetesting/environment.go b/pkg/releasetesting/environment.go deleted file mode 100644 index 7bff936b8..000000000 --- a/pkg/releasetesting/environment.go +++ /dev/null @@ -1,120 +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 releasetesting - -import ( - "bytes" - "fmt" - "log" - "time" - - v1 "k8s.io/api/core/v1" - - "helm.sh/helm/pkg/kube" - "helm.sh/helm/pkg/release" -) - -// Environment encapsulates information about where test suite executes and returns results -type Environment struct { - Namespace string - KubeClient kube.Interface - Messages chan *release.TestReleaseResponse - Timeout time.Duration -} - -func (env *Environment) createTestPod(test *test) error { - b := bytes.NewBufferString(test.manifest) - if err := env.KubeClient.Create(b); err != nil { - test.result.Info = err.Error() - test.result.Status = release.TestRunFailure - return err - } - - return nil -} - -func (env *Environment) getTestPodStatus(test *test) (v1.PodPhase, error) { - status, err := env.KubeClient.WaitAndGetCompletedPodPhase(test.name, env.Timeout) - if err != nil { - log.Printf("Error getting status for pod %s: %s", test.result.Name, err) - test.result.Info = err.Error() - test.result.Status = release.TestRunUnknown - return status, err - } - - return status, err -} - -func (env *Environment) streamResult(r *release.TestRun) error { - switch r.Status { - case release.TestRunSuccess: - if err := env.streamSuccess(r.Name); err != nil { - return err - } - case release.TestRunFailure: - if err := env.streamFailed(r.Name); err != nil { - return err - } - - default: - if err := env.streamUnknown(r.Name, r.Info); err != nil { - return err - } - } - return nil -} - -func (env *Environment) streamRunning(name string) error { - msg := "RUNNING: " + name - return env.streamMessage(msg, release.TestRunRunning) -} - -func (env *Environment) streamError(info string) error { - msg := "ERROR: " + info - return env.streamMessage(msg, release.TestRunFailure) -} - -func (env *Environment) streamFailed(name string) error { - msg := fmt.Sprintf("FAILED: %s, run `kubectl logs %s --namespace %s` for more info", name, name, env.Namespace) - return env.streamMessage(msg, release.TestRunFailure) -} - -func (env *Environment) streamSuccess(name string) error { - msg := fmt.Sprintf("PASSED: %s", name) - return env.streamMessage(msg, release.TestRunSuccess) -} - -func (env *Environment) streamUnknown(name, info string) error { - msg := fmt.Sprintf("UNKNOWN: %s: %s", name, info) - return env.streamMessage(msg, release.TestRunUnknown) -} - -func (env *Environment) streamMessage(msg string, status release.TestRunStatus) error { - resp := &release.TestReleaseResponse{Msg: msg, Status: status} - env.Messages <- resp - return nil -} - -// DeleteTestPods deletes resources given in testManifests -func (env *Environment) DeleteTestPods(testManifests []string) { - for _, testManifest := range testManifests { - err := env.KubeClient.Delete(bytes.NewBufferString(testManifest)) - if err != nil { - env.streamError(err.Error()) - } - } -} diff --git a/pkg/releasetesting/environment_test.go b/pkg/releasetesting/environment_test.go deleted file mode 100644 index fbff19d3b..000000000 --- a/pkg/releasetesting/environment_test.go +++ /dev/null @@ -1,71 +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 releasetesting - -import ( - "testing" - - "github.com/pkg/errors" - - "helm.sh/helm/pkg/release" -) - -func TestCreateTestPodSuccess(t *testing.T) { - env := testEnvFixture() - test := testFixture() - - if err := env.createTestPod(test); err != nil { - t.Errorf("Expected no error, got an error: %s", err) - } -} - -func TestCreateTestPodFailure(t *testing.T) { - env := testEnvFixture() - env.KubeClient = &mockKubeClient{ - err: errors.New("We ran out of budget and couldn't create finding-nemo"), - } - test := testFixture() - - if err := env.createTestPod(test); err == nil { - t.Errorf("Expected error, got no error") - } - if test.result.Info == "" { - t.Errorf("Expected error to be saved in test result info but found empty string") - } - if test.result.Status != release.TestRunFailure { - t.Errorf("Expected test result status to be failure but got: %v", test.result.Status) - } -} - -func TestStreamMessage(t *testing.T) { - env := testEnvFixture() - defer close(env.Messages) - - expectedMessage := "testing streamMessage" - expectedStatus := release.TestRunSuccess - if err := env.streamMessage(expectedMessage, expectedStatus); err != nil { - t.Errorf("Expected no errors, got: %s", err) - } - - got := <-env.Messages - if got.Msg != expectedMessage { - t.Errorf("Expected message: %s, got: %s", expectedMessage, got.Msg) - } - if got.Status != expectedStatus { - t.Errorf("Expected status: %v, got: %v", expectedStatus, got.Status) - } -} diff --git a/pkg/releasetesting/test_suite.go b/pkg/releasetesting/test_suite.go deleted file mode 100644 index 05500881c..000000000 --- a/pkg/releasetesting/test_suite.go +++ /dev/null @@ -1,187 +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 releasetesting - -import ( - "strings" - "time" - - "github.com/pkg/errors" - v1 "k8s.io/api/core/v1" - "sigs.k8s.io/yaml" - - "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 -} - -type test struct { - name string - manifest string - expectedSuccess bool - result *release.TestRun -} - -// NewTestSuite takes a release object and returns a TestSuite object with test definitions -// extracted from the release -func NewTestSuite(rel *release.Release) *TestSuite { - return &TestSuite{ - TestManifests: extractTestManifestsFromHooks(rel.Hooks), - Results: []*release.TestRun{}, - } -} - -// Run executes tests in a test suite and stores a result within a given environment -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 - env.streamMessage("No Tests Found", release.TestRunUnknown) - } - - for _, testManifest := range ts.TestManifests { - test, err := newTest(testManifest) - if err != nil { - return err - } - - test.result.StartedAt = time.Now() - if err := env.streamRunning(test.name); err != nil { - return err - } - test.result.Status = release.TestRunRunning - - resourceCreated := true - if err := env.createTestPod(test); err != nil { - resourceCreated = false - if streamErr := env.streamError(test.result.Info); streamErr != nil { - return err - } - } - - resourceCleanExit := true - status := v1.PodUnknown - if resourceCreated { - status, err = env.getTestPodStatus(test) - if err != nil { - resourceCleanExit = false - if streamErr := env.streamError(test.result.Info); streamErr != nil { - return streamErr - } - } - } - - if resourceCreated && resourceCleanExit { - if err := test.assignTestResult(status); err != nil { - return err - } - - if err := env.streamResult(test.result); err != nil { - return err - } - } - - test.result.CompletedAt = time.Now() - ts.Results = append(ts.Results, test.result) - } - - ts.CompletedAt = time.Now() - return nil -} - -func (t *test) assignTestResult(podStatus v1.PodPhase) error { - switch podStatus { - case v1.PodSucceeded: - if t.expectedSuccess { - t.result.Status = release.TestRunSuccess - } else { - t.result.Status = release.TestRunFailure - } - case v1.PodFailed: - if !t.expectedSuccess { - t.result.Status = release.TestRunSuccess - } else { - t.result.Status = release.TestRunFailure - } - default: - t.result.Status = release.TestRunUnknown - } - - 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) { - var sh util.SimpleHead - err := yaml.Unmarshal([]byte(testManifest), &sh) - if err != nil { - return nil, err - } - - if sh.Kind != "Pod" { - 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, - result: &release.TestRun{ - Name: name, - }, - }, nil -} diff --git a/pkg/releasetesting/test_suite_test.go b/pkg/releasetesting/test_suite_test.go deleted file mode 100644 index 9256df467..000000000 --- a/pkg/releasetesting/test_suite_test.go +++ /dev/null @@ -1,259 +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 releasetesting - -import ( - "io" - "testing" - "time" - - v1 "k8s.io/api/core/v1" - - "helm.sh/helm/pkg/kube" - "helm.sh/helm/pkg/release" -) - -const manifestWithTestSuccessHook = ` -apiVersion: v1 -kind: Pod -metadata: - name: finding-nemo, - annotations: - "helm.sh/hook": test-success -spec: - containers: - - name: nemo-test - image: fake-image - cmd: fake-command -` - -const manifestWithTestFailureHook = ` -apiVersion: v1 -kind: Pod -metadata: - name: gold-rush, - annotations: - "helm.sh/hook": test-failure -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) - env := testEnvFixture() - - go func() { - defer close(env.Messages) - if err := ts.Run(env); err != nil { - t.Error(err) - } - }() - - for i := 0; i <= 4; i++ { - <-env.Messages - } - if _, ok := <-env.Messages; ok { - t.Errorf("Expected 4 messages streamed") - } - - if ts.StartedAt.IsZero() { - t.Errorf("Expected StartedAt to not be nil. Got: %v", ts.StartedAt) - } - if ts.CompletedAt.IsZero() { - t.Errorf("Expected CompletedAt to not be nil. Got: %v", ts.CompletedAt) - } - if len(ts.Results) != 2 { - t.Errorf("Expected 2 test result. Got %v", len(ts.Results)) - } - - result := ts.Results[0] - if result.StartedAt.IsZero() { - t.Errorf("Expected test StartedAt to not be nil. Got: %v", result.StartedAt) - } - if result.CompletedAt.IsZero() { - t.Errorf("Expected test CompletedAt to not be nil. Got: %v", result.CompletedAt) - } - if result.Name != "finding-nemo" { - t.Errorf("Expected test name to be finding-nemo. Got: %v", result.Name) - } - if result.Status != release.TestRunSuccess { - t.Errorf("Expected test result to be successful, got: %v", result.Status) - } - result2 := ts.Results[1] - if result2.StartedAt.IsZero() { - t.Errorf("Expected test StartedAt to not be nil. Got: %v", result2.StartedAt) - } - if result2.CompletedAt.IsZero() { - t.Errorf("Expected test CompletedAt to not be nil. Got: %v", result2.CompletedAt) - } - if result2.Name != "gold-rush" { - t.Errorf("Expected test name to be gold-rush, Got: %v", result2.Name) - } - if result2.Status != release.TestRunFailure { - t.Errorf("Expected test result to be successful, got: %v", result2.Status) - } -} - -func TestRunEmptyTestSuite(t *testing.T) { - ts := testSuiteFixture([]string{}) - env := testEnvFixture() - - go func() { - defer close(env.Messages) - if err := ts.Run(env); err != nil { - t.Error(err) - } - }() - - msg := <-env.Messages - if msg.Msg != "No Tests Found" { - t.Errorf("Expected message 'No Tests Found', Got: %v", msg.Msg) - } - - for range env.Messages { - } - - if ts.StartedAt.IsZero() { - t.Errorf("Expected StartedAt to not be nil. Got: %v", ts.StartedAt) - } - if ts.CompletedAt.IsZero() { - t.Errorf("Expected CompletedAt to not be nil. Got: %v", ts.CompletedAt) - } - if len(ts.Results) != 0 { - t.Errorf("Expected 0 test result. Got %v", len(ts.Results)) - } -} - -func TestRunSuccessWithTestFailureHook(t *testing.T) { - ts := testSuiteFixture([]string{manifestWithTestFailureHook}) - env := testEnvFixture() - env.KubeClient = &mockKubeClient{podFail: true} - - go func() { - defer close(env.Messages) - if err := ts.Run(env); err != nil { - t.Error(err) - } - }() - - for i := 0; i <= 4; i++ { - <-env.Messages - } - if _, ok := <-env.Messages; ok { - t.Errorf("Expected 4 messages streamed") - } - - if ts.StartedAt.IsZero() { - t.Errorf("Expected StartedAt to not be nil. Got: %v", ts.StartedAt) - } - if ts.CompletedAt.IsZero() { - t.Errorf("Expected CompletedAt to not be nil. Got: %v", ts.CompletedAt) - } - if len(ts.Results) != 1 { - t.Errorf("Expected 1 test result. Got %v", len(ts.Results)) - } - - result := ts.Results[0] - if result.StartedAt.IsZero() { - t.Errorf("Expected test StartedAt to not be nil. Got: %v", result.StartedAt) - } - if result.CompletedAt.IsZero() { - t.Errorf("Expected test CompletedAt to not be nil. Got: %v", result.CompletedAt) - } - if result.Name != "gold-rush" { - t.Errorf("Expected test name to be gold-rush, Got: %v", result.Name) - } - if result.Status != release.TestRunSuccess { - t.Errorf("Expected test result to be successful, got: %v", result.Status) - } -} - -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, - result: &release.TestRun{}, - } -} - -func testSuiteFixture(testManifests []string) *TestSuite { - testResults := []*release.TestRun{} - ts := &TestSuite{ - TestManifests: testManifests, - Results: testResults, - } - return ts -} - -func testEnvFixture() *Environment { - return &Environment{ - Namespace: "default", - KubeClient: &mockKubeClient{}, - Timeout: 1, - Messages: make(chan *release.TestReleaseResponse, 1), - } -} - -type mockKubeClient struct { - kube.Interface - podFail bool - err error -} - -func (c *mockKubeClient) WaitAndGetCompletedPodPhase(_ string, _ time.Duration) (v1.PodPhase, error) { - if c.podFail { - return v1.PodFailed, nil - } - return v1.PodSucceeded, nil -} -func (c *mockKubeClient) Create(_ io.Reader) error { return c.err } -func (c *mockKubeClient) Delete(_ io.Reader) error { return nil } diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index c4e5e255a..eb6016094 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -26,7 +26,6 @@ import ( "sigs.k8s.io/yaml" "helm.sh/helm/pkg/chartutil" - "helm.sh/helm/pkg/hooks" "helm.sh/helm/pkg/release" ) @@ -53,16 +52,16 @@ type result struct { // TODO: Refactor this out. It's here because naming conventions were not followed through. // So fix the Test hook names and then remove this. var events = map[string]release.HookEvent{ - hooks.PreInstall: release.HookPreInstall, - hooks.PostInstall: release.HookPostInstall, - hooks.PreDelete: release.HookPreDelete, - hooks.PostDelete: release.HookPostDelete, - hooks.PreUpgrade: release.HookPreUpgrade, - hooks.PostUpgrade: release.HookPostUpgrade, - hooks.PreRollback: release.HookPreRollback, - hooks.PostRollback: release.HookPostRollback, - hooks.ReleaseTestSuccess: release.HookReleaseTestSuccess, - hooks.ReleaseTestFailure: release.HookReleaseTestFailure, + release.HookPreInstall.String(): release.HookPreInstall, + release.HookPostInstall.String(): release.HookPostInstall, + release.HookPreDelete.String(): release.HookPreDelete, + release.HookPostDelete.String(): release.HookPostDelete, + release.HookPreUpgrade.String(): release.HookPreUpgrade, + release.HookPostUpgrade.String(): release.HookPostUpgrade, + release.HookPreRollback.String(): release.HookPreRollback, + release.HookPostRollback.String(): release.HookPostRollback, + release.HookTest.String(): release.HookTest, + "test-success": release.HookTest, } // SortManifests takes a map of filename/YAML contents, splits the file @@ -142,7 +141,7 @@ func (file *manifestFile) sort(result *result) error { continue } - hookTypes, ok := entry.Metadata.Annotations[hooks.HookAnno] + hookTypes, ok := entry.Metadata.Annotations[release.HookAnnotation] if !ok { result.generic = append(result.generic, Manifest{ Name: file.path, @@ -182,7 +181,7 @@ func (file *manifestFile) sort(result *result) error { result.hooks = append(result.hooks, h) - operateAnnotationValues(entry, hooks.HookDeleteAnno, func(value string) { + operateAnnotationValues(entry, release.HookDeleteAnnotation, func(value string) { h.DeletePolicies = append(h.DeletePolicies, release.HookDeletePolicy(value)) }) } @@ -201,7 +200,7 @@ func hasAnyAnnotation(entry SimpleHead) bool { // // If no weight is found, the assigned weight is 0 func calculateHookWeight(entry SimpleHead) int { - hws := entry.Metadata.Annotations[hooks.HookWeightAnno] + hws := entry.Metadata.Annotations[release.HookWeightAnnotation] hw, err := strconv.Atoi(hws) if err != nil { hw = 0 diff --git a/pkg/releaseutil/manifest_sorter_test.go b/pkg/releaseutil/manifest_sorter_test.go index 4fa22a192..a82a1941e 100644 --- a/pkg/releaseutil/manifest_sorter_test.go +++ b/pkg/releaseutil/manifest_sorter_test.go @@ -116,7 +116,7 @@ metadata: name: []string{"eighth", "example-test"}, path: "eight", kind: []string{"ConfigMap", "Pod"}, - hooks: map[string][]release.HookEvent{"eighth": nil, "example-test": {release.HookReleaseTestSuccess}}, + hooks: map[string][]release.HookEvent{"eighth": nil, "example-test": {release.HookTest}}, manifest: `kind: ConfigMap apiVersion: v1 metadata: @@ -129,7 +129,7 @@ kind: Pod metadata: name: example-test annotations: - "helm.sh/hook": test-success + "helm.sh/hook": test `, }, } diff --git a/pkg/releaseutil/manifest_test.go b/pkg/releaseutil/manifest_test.go index e8a8a7262..b667f796f 100644 --- a/pkg/releaseutil/manifest_test.go +++ b/pkg/releaseutil/manifest_test.go @@ -29,7 +29,7 @@ kind: Pod metadata: name: finding-nemo, annotations: - "helm.sh/hook": test-success + "helm.sh/hook": test spec: containers: - name: nemo-test @@ -42,7 +42,7 @@ kind: Pod metadata: name: finding-nemo, annotations: - "helm.sh/hook": test-success + "helm.sh/hook": test spec: containers: - name: nemo-test From 97fe285adad8e343c7d2d7c44fcd07d9d633970c Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Sat, 20 Jul 2019 13:14:01 -0400 Subject: [PATCH 2/8] feat(client): wait for Pods during hook execution Signed-off-by: Jacob LeGrone --- pkg/kube/client.go | 31 ++++++++++++++++++++++++++++++- pkg/kube/interface.go | 5 +++-- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 4aa817af3..2162f083f 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -255,6 +255,8 @@ func (c *Client) watchTimeout(t time.Duration) func(*resource.Info) error { // // - Jobs: A job is marked "Ready" when it has successfully completed. This is // ascertained by watching the Status fields in a job's output. +// - Pods: A pod is marked "Ready" when it has successfully completed. This is +// ascertained by watching the status.phase field in a pod's output. // // Handling for other kinds will be added as necessary. func (c *Client) WatchUntilReady(reader io.Reader, timeout time.Duration) error { @@ -435,8 +437,11 @@ func (c *Client) watchUntilReady(timeout time.Duration, info *resource.Info) err // the status go into a good state. For other types, like ReplicaSet // we don't really do anything to support these as hooks. c.Log("Add/Modify event for %s: %v", info.Name, e.Type) - if kind == "Job" { + switch kind { + case "Job": return c.waitForJob(e, info.Name) + case "Pod": + return c.waitForPodSuccess(e, info.Name) } return true, nil case watch.Deleted: @@ -474,6 +479,30 @@ func (c *Client) waitForJob(e watch.Event, name string) (bool, error) { return false, nil } +// waitForPodSuccess is a helper that waits for a pod to complete. +// +// This operates on an event returned from a watcher. +func (c *Client) waitForPodSuccess(e watch.Event, name string) (bool, error) { + o, ok := e.Object.(*v1.Pod) + if !ok { + return true, errors.Errorf("expected %s to be a *v1.Pod, got %T", name, e.Object) + } + + switch o.Status.Phase { + case v1.PodSucceeded: + fmt.Printf("Pod %s succeeded\n", o.Name) + return true, nil + case v1.PodFailed: + return true, errors.Errorf("pod %s failed", o.Name) + case v1.PodPending: + fmt.Printf("Pod %s pending\n", o.Name) + case v1.PodRunning: + fmt.Printf("Pod %s running\n", o.Name) + } + + return false, nil +} + // scrubValidationError removes kubectl info from the message. func scrubValidationError(err error) error { if err == nil { diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 72d7a0ea9..53da50b8b 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -21,7 +21,7 @@ import ( "time" ) -// KubernetesClient represents a client capable of communicating with the Kubernetes API. +// Interface represents a client capable of communicating with the Kubernetes API. // // A KubernetesClient must be concurrency safe. type Interface interface { @@ -41,7 +41,8 @@ type Interface interface { // Watch the resource in reader until it is "ready". // - // For Jobs, "ready" means the job ran to completion (excited without error). + // For Jobs, "ready" means the Job ran to completion (exited without error). + // For Pods, "ready" means the Pod phase is marked "succeeded". // For all other kinds, it means the kind was created or modified without // error. WatchUntilReady(reader io.Reader, timeout time.Duration) error From caa4240a30bd9783d1ac7172028be7e9f7c498e8 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 25 Jul 2019 14:45:03 -0400 Subject: [PATCH 3/8] refactor(release): track test executions via Hook type Signed-off-by: Jacob LeGrone --- cmd/helm/install_test.go | 2 +- cmd/helm/status_test.go | 34 +++++++------- .../testdata/output/status-with-notes.txt | 2 +- .../testdata/output/status-with-resource.txt | 2 +- .../output/status-with-test-suite.txt | 15 ++++--- cmd/helm/testdata/output/status.json | 2 +- cmd/helm/testdata/output/status.txt | 2 +- cmd/helm/testdata/output/status.yaml | 1 + pkg/action/hooks.go | 15 +++++-- pkg/action/install.go | 11 ++--- pkg/action/install_test.go | 6 +-- pkg/action/printer.go | 44 +++++++++---------- pkg/action/release_testing.go | 2 +- pkg/action/rollback.go | 9 +--- pkg/action/uninstall.go | 9 +--- pkg/action/upgrade.go | 9 +--- pkg/kube/client.go | 1 + pkg/release/hook.go | 12 ++++- pkg/release/info.go | 2 - pkg/release/mock.go | 21 +++------ pkg/release/test_suite.go | 28 ------------ 21 files changed, 97 insertions(+), 132 deletions(-) delete mode 100644 pkg/release/test_suite.go diff --git a/cmd/helm/install_test.go b/cmd/helm/install_test.go index 20571867f..9ab25417b 100644 --- a/cmd/helm/install_test.go +++ b/cmd/helm/install_test.go @@ -25,7 +25,7 @@ func TestInstall(t *testing.T) { // Install, base case { name: "basic install", - cmd: "install aeneas testdata/testcharts/empty", + cmd: "install aeneas testdata/testcharts/empty --namespace default", golden: "output/install.txt", }, diff --git a/cmd/helm/status_test.go b/cmd/helm/status_test.go index 54117bdc7..d9a686dab 100644 --- a/cmd/helm/status_test.go +++ b/cmd/helm/status_test.go @@ -25,12 +25,14 @@ import ( ) func TestStatusCmd(t *testing.T) { - releasesMockWithStatus := func(info *release.Info) []*release.Release { + releasesMockWithStatus := func(info *release.Info, hooks ...*release.Hook) []*release.Release { info.LastDeployed = time.Unix(1452902400, 0).UTC() return []*release.Release{{ - Name: "flummoxed-chickadee", - Info: info, - Chart: &chart.Chart{}, + Name: "flummoxed-chickadee", + Namespace: "default", + Info: info, + Chart: &chart.Chart{}, + Hooks: hooks, }} } @@ -77,19 +79,19 @@ func TestStatusCmd(t *testing.T) { name: "get status of a deployed release with test suite", cmd: "status flummoxed-chickadee", golden: "output/status-with-test-suite.txt", - rels: releasesMockWithStatus(&release.Info{ - Status: release.StatusDeployed, - LastTestSuiteRun: &release.TestSuite{ - Results: []*release.TestRun{{ - Name: "test run 1", - Status: release.TestRunSuccess, - Info: "extra info", - }, { - Name: "test run 2", - Status: release.TestRunFailure, - }}, + rels: releasesMockWithStatus( + &release.Info{ + Status: release.StatusDeployed, }, - }), + &release.Hook{ + Name: "foo", + Events: []release.HookEvent{release.HookTest}, + }, + &release.Hook{ + Name: "bar", + Events: []release.HookEvent{release.HookTest}, + }, + ), }} runTestCmd(t, tests) } diff --git a/cmd/helm/testdata/output/status-with-notes.txt b/cmd/helm/testdata/output/status-with-notes.txt index 22a34368b..111c5f98a 100644 --- a/cmd/helm/testdata/output/status-with-notes.txt +++ b/cmd/helm/testdata/output/status-with-notes.txt @@ -1,6 +1,6 @@ NAME: flummoxed-chickadee LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC -NAMESPACE: +NAMESPACE: default STATUS: deployed NOTES: diff --git a/cmd/helm/testdata/output/status-with-resource.txt b/cmd/helm/testdata/output/status-with-resource.txt index b6aa27c66..5f6f4e663 100644 --- a/cmd/helm/testdata/output/status-with-resource.txt +++ b/cmd/helm/testdata/output/status-with-resource.txt @@ -1,6 +1,6 @@ NAME: flummoxed-chickadee LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC -NAMESPACE: +NAMESPACE: default STATUS: deployed RESOURCES: diff --git a/cmd/helm/testdata/output/status-with-test-suite.txt b/cmd/helm/testdata/output/status-with-test-suite.txt index 057cab434..cc4be3460 100644 --- a/cmd/helm/testdata/output/status-with-test-suite.txt +++ b/cmd/helm/testdata/output/status-with-test-suite.txt @@ -1,12 +1,15 @@ NAME: flummoxed-chickadee LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC -NAMESPACE: +NAMESPACE: default STATUS: deployed -TEST SUITE: -Last Started: 0001-01-01 00:00:00 +0000 UTC +TEST SUITE: foo +Last Started: 0001-01-01 00:00:00 +0000 UTC Last Completed: 0001-01-01 00:00:00 +0000 UTC +Successful: false + +TEST SUITE: bar +Last Started: 0001-01-01 00:00:00 +0000 UTC +Last Completed: 0001-01-01 00:00:00 +0000 UTC +Successful: false -TEST STATUS INFO STARTED COMPLETED -test run 1 success extra info 0001-01-01 00:00:00 +0000 UTC 0001-01-01 00:00:00 +0000 UTC -test run 2 failure 0001-01-01 00:00:00 +0000 UTC 0001-01-01 00:00:00 +0000 UTC diff --git a/cmd/helm/testdata/output/status.json b/cmd/helm/testdata/output/status.json index aee9340d3..b57687c5c 100644 --- a/cmd/helm/testdata/output/status.json +++ b/cmd/helm/testdata/output/status.json @@ -1 +1 @@ -{"name":"flummoxed-chickadee","info":{"first_deployed":"0001-01-01T00:00:00Z","last_deployed":"2016-01-16T00:00:00Z","deleted":"0001-01-01T00:00:00Z","status":"deployed","notes":"release notes"}} \ No newline at end of file +{"name":"flummoxed-chickadee","info":{"first_deployed":"0001-01-01T00:00:00Z","last_deployed":"2016-01-16T00:00:00Z","deleted":"0001-01-01T00:00:00Z","status":"deployed","notes":"release notes"},"namespace":"default"} \ No newline at end of file diff --git a/cmd/helm/testdata/output/status.txt b/cmd/helm/testdata/output/status.txt index cbd76fba2..a7fc22c72 100644 --- a/cmd/helm/testdata/output/status.txt +++ b/cmd/helm/testdata/output/status.txt @@ -1,5 +1,5 @@ NAME: flummoxed-chickadee LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC -NAMESPACE: +NAMESPACE: default STATUS: deployed diff --git a/cmd/helm/testdata/output/status.yaml b/cmd/helm/testdata/output/status.yaml index 969bfb26f..a7bc12276 100644 --- a/cmd/helm/testdata/output/status.yaml +++ b/cmd/helm/testdata/output/status.yaml @@ -7,3 +7,4 @@ info: resource B status: deployed name: flummoxed-chickadee +namespace: default diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index dd768d112..c896c415a 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -26,10 +26,10 @@ import ( ) // execHook executes all of the hooks for the given hook event. -func (cfg *Configuration) execHook(hs []*release.Hook, hook release.HookEvent, timeout time.Duration) error { +func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, timeout time.Duration) error { executingHooks := []*release.Hook{} - for _, h := range hs { + for _, h := range rl.Hooks { for _, e := range h.Events { if e == hook { executingHooks = append(executingHooks, h) @@ -51,7 +51,15 @@ func (cfg *Configuration) execHook(hs []*release.Hook, hook release.HookEvent, t b.Reset() b.WriteString(h.Manifest) - if err := cfg.KubeClient.WatchUntilReady(b, timeout); err != nil { + // Get the time at which the hook was applied to the cluster + start := time.Now() + err := cfg.KubeClient.WatchUntilReady(b, timeout) + h.LastRun = release.HookExecution{ + StartedAt: start, + CompletedAt: time.Now(), + Successful: err == nil, + } + if err != nil { // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted // under failed condition. If so, then clear the corresponding resource object in the hook if err := deleteHookByPolicy(cfg, h, release.HookFailed); err != nil { @@ -67,7 +75,6 @@ func (cfg *Configuration) execHook(hs []*release.Hook, hook release.HookEvent, t if err := deleteHookByPolicy(cfg, h, release.HookSucceeded); err != nil { return err } - h.LastRun = time.Now() } return nil diff --git a/pkg/action/install.go b/pkg/action/install.go index 3969cb26d..d02f0929f 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -178,7 +178,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { return rel, nil } - // If Replace is true, we need to supersede the last release. + // If Replace is true, we need to supercede the last release. if i.Replace { if err := i.replaceRelease(rel); err != nil { return nil, err @@ -196,7 +196,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { // pre-install hooks if !i.DisableHooks { - if err := i.execHook(rel.Hooks, release.HookPreInstall); err != nil { + if err := i.cfg.execHook(rel, release.HookPreInstall, i.Timeout); err != nil { return i.failRelease(rel, fmt.Errorf("failed pre-install: %s", err)) } } @@ -218,7 +218,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { } if !i.DisableHooks { - if err := i.execHook(rel.Hooks, release.HookPostInstall); err != nil { + if err := i.cfg.execHook(rel, release.HookPostInstall, i.Timeout); err != nil { return i.failRelease(rel, fmt.Errorf("failed post-install: %s", err)) } } @@ -463,11 +463,6 @@ func (i *Install) validateManifest(manifest io.Reader) error { return err } -// execHook executes all of the hooks for the given hook event. -func (i *Install) execHook(hs []*release.Hook, hook release.HookEvent) error { - return i.cfg.execHook(hs, hook, i.Timeout) -} - // NameAndChart returns the name and chart that should be used. // // This will read the flags and handle name generation if necessary. diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index af8b28ee5..78c90a62e 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -178,7 +178,7 @@ func TestInstallRelease_DryRun(t *testing.T) { _, err = instAction.cfg.Releases.Get(res.Name, res.Version) is.Error(err) is.Len(res.Hooks, 1) - is.True(res.Hooks[0].LastRun.IsZero(), "expect hook to not be marked as run") + is.True(res.Hooks[0].LastRun.CompletedAt.IsZero(), "expect hook to not be marked as run") is.Equal(res.Info.Description, "Dry run complete") } @@ -195,7 +195,7 @@ func TestInstallRelease_NoHooks(t *testing.T) { t.Fatalf("Failed install: %s", err) } - is.True(res.Hooks[0].LastRun.IsZero(), "hooks should not run with no-hooks") + is.True(res.Hooks[0].LastRun.CompletedAt.IsZero(), "hooks should not run with no-hooks") } func TestInstallRelease_FailedHooks(t *testing.T) { @@ -210,7 +210,7 @@ func TestInstallRelease_FailedHooks(t *testing.T) { res, err := instAction.Run(buildChart()) is.Error(err) is.Contains(res.Info.Description, "failed post-install") - is.Equal(res.Info.Status, release.StatusFailed) + is.Equal(release.StatusFailed, res.Info.Status) } func TestInstallRelease_ReplaceRelease(t *testing.T) { diff --git a/pkg/action/printer.go b/pkg/action/printer.go index 3e23448a9..006c3c24c 100644 --- a/pkg/action/printer.go +++ b/pkg/action/printer.go @@ -23,9 +23,6 @@ import ( "strings" "text/tabwriter" - "github.com/gosuri/uitable" - "github.com/gosuri/uitable/util/strutil" - "helm.sh/helm/pkg/release" ) @@ -48,12 +45,17 @@ func PrintRelease(out io.Writer, rel *release.Release) { fmt.Fprintf(w, "RESOURCES:\n%s\n", re.ReplaceAllString(rel.Info.Resources, "\t")) w.Flush() } - if rel.Info.LastTestSuiteRun != nil { - lastRun := rel.Info.LastTestSuiteRun - fmt.Fprintf(out, "TEST SUITE:\n%s\n%s\n\n%s\n", - fmt.Sprintf("Last Started: %s", lastRun.StartedAt), - fmt.Sprintf("Last Completed: %s", lastRun.CompletedAt), - formatTestResults(lastRun.Results)) + + executions := executionsByHookEvent(rel) + if tests, ok := executions[release.HookTest]; ok { + for _, h := range tests { + fmt.Fprintf(out, "TEST SUITE: %s\n%s\n%s\n%s\n\n", + h.Name, + fmt.Sprintf("Last Started: %s", h.LastRun.StartedAt), + fmt.Sprintf("Last Completed: %s", h.LastRun.CompletedAt), + fmt.Sprintf("Successful: %t", h.LastRun.Successful), + ) + } } if strings.EqualFold(rel.Info.Description, "Dry run complete") { @@ -65,18 +67,16 @@ func PrintRelease(out io.Writer, rel *release.Release) { } } -func formatTestResults(results []*release.TestRun) string { - tbl := uitable.New() - tbl.MaxColWidth = 50 - tbl.AddRow("TEST", "STATUS", "INFO", "STARTED", "COMPLETED") - for i := 0; i < len(results); i++ { - r := results[i] - n := r.Name - s := strutil.PadRight(r.Status.String(), 10, ' ') - i := r.Info - ts := r.StartedAt - tc := r.CompletedAt - tbl.AddRow(n, s, i, ts, tc) +func executionsByHookEvent(rel *release.Release) map[release.HookEvent][]*release.Hook { + result := make(map[release.HookEvent][]*release.Hook) + for _, h := range rel.Hooks { + for _, e := range h.Events { + executions, ok := result[e] + if !ok { + executions = []*release.Hook{} + } + result[e] = append(executions, h) + } } - return tbl.String() + return result } diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index eb7e1ccc7..bcfe94fe0 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -53,5 +53,5 @@ func (r *ReleaseTesting) Run(name string) error { return err } - return r.cfg.execHook(rel.Hooks, release.HookTest, r.Timeout) + return r.cfg.execHook(rel, release.HookTest, r.Timeout) } diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index c709ff20c..850b70d2a 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -138,7 +138,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas // pre-rollback hooks if !r.DisableHooks { - if err := r.execHook(targetRelease.Hooks, release.HookPreRollback); err != nil { + if err := r.cfg.execHook(targetRelease, release.HookPreRollback, r.Timeout); err != nil { return targetRelease, err } } else { @@ -171,7 +171,7 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas // post-rollback hooks if !r.DisableHooks { - if err := r.execHook(targetRelease.Hooks, release.HookPostRollback); err != nil { + if err := r.cfg.execHook(targetRelease, release.HookPostRollback, r.Timeout); err != nil { return targetRelease, err } } @@ -191,8 +191,3 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return targetRelease, nil } - -// execHook executes all of the hooks for the given hook event. -func (r *Rollback) execHook(hs []*release.Hook, hook release.HookEvent) error { - return r.cfg.execHook(hs, hook, r.Timeout) -} diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 4090e8241..a15fc188d 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -92,7 +92,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) res := &release.UninstallReleaseResponse{Release: rel} if !u.DisableHooks { - if err := u.execHook(rel.Hooks, release.HookPreDelete); err != nil { + if err := u.cfg.execHook(rel, release.HookPreDelete, u.Timeout); err != nil { return res, err } } else { @@ -109,7 +109,7 @@ func (u *Uninstall) Run(name string) (*release.UninstallReleaseResponse, error) res.Info = kept if !u.DisableHooks { - if err := u.execHook(rel.Hooks, release.HookPostDelete); err != nil { + if err := u.cfg.execHook(rel, release.HookPostDelete, u.Timeout); err != nil { errs = append(errs, err) } } @@ -159,11 +159,6 @@ func joinErrors(errs []error) string { return strings.Join(es, "; ") } -// execHook executes all of the hooks for the given hook event. -func (u *Uninstall) execHook(hs []*release.Hook, hook release.HookEvent) error { - return u.cfg.execHook(hs, hook, u.Timeout) -} - // deleteRelease deletes the release and returns manifests that were kept in the deletion process func (u *Uninstall) deleteRelease(rel *release.Release) (kept string, errs []error) { caps, err := u.cfg.getCapabilities() diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 6de69a842..aff9f40ac 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -199,7 +199,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea // pre-upgrade hooks if !u.DisableHooks { - if err := u.execHook(upgradedRelease.Hooks, release.HookPreUpgrade); err != nil { + if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.Timeout); err != nil { return u.failRelease(upgradedRelease, fmt.Errorf("pre-upgrade hooks failed: %s", err)) } } else { @@ -220,7 +220,7 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea // post-upgrade hooks if !u.DisableHooks { - if err := u.execHook(upgradedRelease.Hooks, release.HookPostUpgrade); err != nil { + if err := u.cfg.execHook(upgradedRelease, release.HookPostUpgrade, u.Timeout); err != nil { return u.failRelease(upgradedRelease, fmt.Errorf("post-upgrade hooks failed: %s", err)) } } @@ -331,8 +331,3 @@ func validateManifest(c kube.Interface, manifest []byte) error { _, err := c.Build(bytes.NewReader(manifest)) return err } - -// execHook executes all of the hooks for the given hook event. -func (u *Upgrade) execHook(hs []*release.Hook, hook release.HookEvent) error { - return u.cfg.execHook(hs, hook, u.Timeout) -} diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 2162f083f..e633a0dc0 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -19,6 +19,7 @@ package kube // import "helm.sh/helm/pkg/kube" import ( "context" "encoding/json" + "fmt" "io" "log" "strings" diff --git a/pkg/release/hook.go b/pkg/release/hook.go index a36412555..f29da4a72 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -71,9 +71,19 @@ type Hook struct { // Events are the events that this hook fires on. Events []HookEvent `json:"events,omitempty"` // LastRun indicates the date/time this was last run. - LastRun time.Time `json:"last_run,omitempty"` + LastRun HookExecution `json:"last_run,omitempty"` // Weight indicates the sort order for execution among similar Hook type Weight int `json:"weight,omitempty"` // DeletePolicies are the policies that indicate when to delete the hook DeletePolicies []HookDeletePolicy `json:"delete_policies,omitempty"` } + +// A HookExecution records the result for the last execution of a hook for a given release. +type HookExecution struct { + // StartedAt indicates the date/time this hook was started + StartedAt time.Time `json:"started_at,omitempty"` + // CompletedAt indicates the date/time this hook was completed + CompletedAt time.Time `json:"completed_at,omitempty"` + // Successful indicates whether the hook completed successfully + Successful bool `json:"successful"` +} diff --git a/pkg/release/info.go b/pkg/release/info.go index 97191615d..03922360b 100644 --- a/pkg/release/info.go +++ b/pkg/release/info.go @@ -33,6 +33,4 @@ type Info struct { Resources string `json:"resources,omitempty"` // Contains the rendered templates/NOTES.txt if available Notes string `json:"notes,omitempty"` - // LastTestSuiteRun provides results on the last test run on a release - LastTestSuiteRun *TestSuite `json:"last_test_suite_run,omitempty"` } diff --git a/pkg/release/mock.go b/pkg/release/mock.go index 3e8a8e361..32644a3cd 100644 --- a/pkg/release/mock.go +++ b/pkg/release/mock.go @@ -40,12 +40,11 @@ metadata: // MockReleaseOptions allows for user-configurable options on mock release objects. type MockReleaseOptions struct { - Name string - Version int - Chart *chart.Chart - Status Status - Namespace string - TestSuiteResults []*TestRun + Name string + Version int + Chart *chart.Chart + Status Status + Namespace string } // Mock creates a mock release object based on options set by MockReleaseOptions. This function should typically not be used outside of testing. @@ -93,14 +92,6 @@ func Mock(opts *MockReleaseOptions) *Release { Description: "Release mock", } - if len(opts.TestSuiteResults) > 0 { - info.LastTestSuiteRun = &TestSuite{ - StartedAt: date, - CompletedAt: date, - Results: opts.TestSuiteResults, - } - } - return &Release{ Name: name, Info: info, @@ -114,7 +105,7 @@ func Mock(opts *MockReleaseOptions) *Release { Kind: "Job", Path: "pre-install-hook.yaml", Manifest: MockHookTemplate, - LastRun: date, + LastRun: HookExecution{}, Events: []HookEvent{HookPreInstall}, }, }, diff --git a/pkg/release/test_suite.go b/pkg/release/test_suite.go deleted file mode 100644 index f50f83763..000000000 --- a/pkg/release/test_suite.go +++ /dev/null @@ -1,28 +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 release - -import "time" - -// TestSuite comprises of the last run of the pre-defined test suite of a release version -type TestSuite struct { - // StartedAt indicates the date/time this test suite was kicked off - StartedAt time.Time `json:"started_at,omitempty"` - // CompletedAt indicates the date/time this test suite was completed - CompletedAt time.Time `json:"completed_at,omitempty"` - // Results are the results of each segment of the test - Results []*TestRun `json:"results,omitempty"` -} From 7f532b4917d1fe34b1560c8876954742d5991bbe Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Thu, 25 Jul 2019 15:23:45 -0400 Subject: [PATCH 4/8] doc(hooks): note helm 2 test annotation support requirement Signed-off-by: Jacob LeGrone --- pkg/releaseutil/manifest_sorter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/releaseutil/manifest_sorter.go b/pkg/releaseutil/manifest_sorter.go index eb6016094..3e91189e4 100644 --- a/pkg/releaseutil/manifest_sorter.go +++ b/pkg/releaseutil/manifest_sorter.go @@ -61,7 +61,8 @@ var events = map[string]release.HookEvent{ release.HookPreRollback.String(): release.HookPreRollback, release.HookPostRollback.String(): release.HookPostRollback, release.HookTest.String(): release.HookTest, - "test-success": release.HookTest, + // Support test-success for backward compatibility with Helm 2 tests + "test-success": release.HookTest, } // SortManifests takes a map of filename/YAML contents, splits the file From 49987975a8c79c7a197af17c38c21094288c40db Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Fri, 2 Aug 2019 00:04:59 -0400 Subject: [PATCH 5/8] fix(test): wait for pods and record status Signed-off-by: Jacob LeGrone --- cmd/helm/status_test.go | 10 ++++++ .../output/status-with-test-suite.txt | 11 ++----- pkg/action/printer.go | 4 +++ pkg/action/release_testing.go | 7 ++++- pkg/kube/client.go | 31 ++++++++++++++++++- pkg/kube/interface.go | 5 +-- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/cmd/helm/status_test.go b/cmd/helm/status_test.go index d9a686dab..8aca8aefb 100644 --- a/cmd/helm/status_test.go +++ b/cmd/helm/status_test.go @@ -90,8 +90,18 @@ func TestStatusCmd(t *testing.T) { &release.Hook{ Name: "bar", Events: []release.HookEvent{release.HookTest}, + LastRun: release.HookExecution{ + StartedAt: mustParseTime("2006-01-02T15:04:05Z"), + CompletedAt: mustParseTime("2006-01-02T15:04:07Z"), + Successful: true, + }, }, ), }} runTestCmd(t, tests) } + +func mustParseTime(t string) time.Time { + res, _ := time.Parse(time.RFC3339, t) + return res +} diff --git a/cmd/helm/testdata/output/status-with-test-suite.txt b/cmd/helm/testdata/output/status-with-test-suite.txt index cc4be3460..6790ea5ea 100644 --- a/cmd/helm/testdata/output/status-with-test-suite.txt +++ b/cmd/helm/testdata/output/status-with-test-suite.txt @@ -3,13 +3,8 @@ LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC NAMESPACE: default STATUS: deployed -TEST SUITE: foo -Last Started: 0001-01-01 00:00:00 +0000 UTC -Last Completed: 0001-01-01 00:00:00 +0000 UTC -Successful: false - TEST SUITE: bar -Last Started: 0001-01-01 00:00:00 +0000 UTC -Last Completed: 0001-01-01 00:00:00 +0000 UTC -Successful: false +Last Started: 2006-01-02 15:04:05 +0000 UTC +Last Completed: 2006-01-02 15:04:07 +0000 UTC +Successful: true diff --git a/pkg/action/printer.go b/pkg/action/printer.go index 006c3c24c..6fe3c6385 100644 --- a/pkg/action/printer.go +++ b/pkg/action/printer.go @@ -49,6 +49,10 @@ func PrintRelease(out io.Writer, rel *release.Release) { executions := executionsByHookEvent(rel) 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\n", h.Name, fmt.Sprintf("Last Started: %s", h.LastRun.StartedAt), diff --git a/pkg/action/release_testing.go b/pkg/action/release_testing.go index bcfe94fe0..d416da6bb 100644 --- a/pkg/action/release_testing.go +++ b/pkg/action/release_testing.go @@ -53,5 +53,10 @@ func (r *ReleaseTesting) Run(name string) error { return err } - return r.cfg.execHook(rel, release.HookTest, r.Timeout) + if err := r.cfg.execHook(rel, release.HookTest, r.Timeout); err != nil { + r.cfg.Releases.Update(rel) + return err + } + + return r.cfg.Releases.Update(rel) } diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 6176ad0d0..93f090af2 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -235,6 +235,8 @@ func (c *Client) watchTimeout(t time.Duration) func(*resource.Info) error { // // - Jobs: A job is marked "Ready" when it has successfully completed. This is // ascertained by watching the Status fields in a job's output. +// - Pods: A pod is marked "Ready" when it has successfully completed. This is +// ascertained by watching the status.phase field in a pod's output. // // Handling for other kinds will be added as necessary. func (c *Client) WatchUntilReady(resources ResourceList, timeout time.Duration) error { @@ -383,8 +385,11 @@ func (c *Client) watchUntilReady(timeout time.Duration, info *resource.Info) err // the status go into a good state. For other types, like ReplicaSet // we don't really do anything to support these as hooks. c.Log("Add/Modify event for %s: %v", info.Name, e.Type) - if kind == "Job" { + switch kind { + case "Job": return c.waitForJob(obj, info.Name) + case "Pod": + return c.waitForPodSuccess(obj, info.Name) } return true, nil case watch.Deleted: @@ -422,6 +427,30 @@ func (c *Client) waitForJob(obj runtime.Object, name string) (bool, error) { return false, nil } +// waitForPodSuccess is a helper that waits for a pod to complete. +// +// This operates on an event returned from a watcher. +func (c *Client) waitForPodSuccess(obj runtime.Object, name string) (bool, error) { + o, ok := obj.(*v1.Pod) + if !ok { + return true, errors.Errorf("expected %s to be a *v1.Pod, got %T", name, obj) + } + + switch o.Status.Phase { + case v1.PodSucceeded: + fmt.Printf("Pod %s succeeded\n", o.Name) + return true, nil + case v1.PodFailed: + return true, errors.Errorf("pod %s failed", o.Name) + case v1.PodPending: + fmt.Printf("Pod %s pending\n", o.Name) + case v1.PodRunning: + fmt.Printf("Pod %s running\n", o.Name) + } + + return false, nil +} + // scrubValidationError removes kubectl info from the message. func scrubValidationError(err error) error { if err == nil { diff --git a/pkg/kube/interface.go b/pkg/kube/interface.go index 2069d8cdd..73dd42835 100644 --- a/pkg/kube/interface.go +++ b/pkg/kube/interface.go @@ -23,7 +23,7 @@ import ( v1 "k8s.io/api/core/v1" ) -// KubernetesClient represents a client capable of communicating with the Kubernetes API. +// Interface represents a client capable of communicating with the Kubernetes API. // // A KubernetesClient must be concurrency safe. type Interface interface { @@ -37,7 +37,8 @@ type Interface interface { // Watch the resource in reader until it is "ready". This method // - // For Jobs, "ready" means the job ran to completion (excited without error). + // For Jobs, "ready" means the Job ran to completion (exited without error). + // For Pods, "ready" means the Pod phase is marked "succeeded". // For all other kinds, it means the kind was created or modified without // error. WatchUntilReady(resources ResourceList, timeout time.Duration) error From 047dd5911ada4b37a4f8863a1cf029a03801bb6f Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Tue, 6 Aug 2019 14:06:05 -0400 Subject: [PATCH 6/8] Fix make test Signed-off-by: Jacob LeGrone --- pkg/action/upgrade.go | 1 - pkg/helmpath/lazypath_darwin_test.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 617cc02e7..fe2839da3 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -22,7 +22,6 @@ import ( "time" "github.com/pkg/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "helm.sh/helm/pkg/chart" diff --git a/pkg/helmpath/lazypath_darwin_test.go b/pkg/helmpath/lazypath_darwin_test.go index bf222ec4b..b34cf3b4d 100644 --- a/pkg/helmpath/lazypath_darwin_test.go +++ b/pkg/helmpath/lazypath_darwin_test.go @@ -21,6 +21,7 @@ import ( "testing" "helm.sh/helm/pkg/helmpath/xdg" + "k8s.io/client-go/util/homedir" ) From 4f6814afb51f25f6cc3ccf7d5fa81abc456f141a Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Wed, 7 Aug 2019 14:33:02 -0400 Subject: [PATCH 7/8] refactor(hooks): replace hook execution Successful bool with HookPhase Signed-off-by: Jacob LeGrone --- cmd/helm/status_test.go | 24 +++++++++-- .../output/status-with-test-suite.txt | 9 +++- pkg/action/hooks.go | 18 ++++---- pkg/action/printer.go | 2 +- pkg/release/hook.go | 21 ++++++++-- pkg/release/responses.go | 6 --- pkg/release/test_run.go | 41 ------------------- 7 files changed, 58 insertions(+), 63 deletions(-) delete mode 100644 pkg/release/test_run.go diff --git a/cmd/helm/status_test.go b/cmd/helm/status_test.go index 8aca8aefb..167c1ed38 100644 --- a/cmd/helm/status_test.go +++ b/cmd/helm/status_test.go @@ -84,16 +84,34 @@ func TestStatusCmd(t *testing.T) { Status: release.StatusDeployed, }, &release.Hook{ - Name: "foo", + Name: "never-run-test", Events: []release.HookEvent{release.HookTest}, }, &release.Hook{ - Name: "bar", + Name: "passing-test", Events: []release.HookEvent{release.HookTest}, LastRun: release.HookExecution{ StartedAt: mustParseTime("2006-01-02T15:04:05Z"), CompletedAt: mustParseTime("2006-01-02T15:04:07Z"), - Successful: true, + Phase: release.HookPhaseSucceeded, + }, + }, + &release.Hook{ + Name: "failing-test", + Events: []release.HookEvent{release.HookTest}, + LastRun: release.HookExecution{ + StartedAt: mustParseTime("2006-01-02T15:10:05Z"), + CompletedAt: mustParseTime("2006-01-02T15:10:07Z"), + Phase: release.HookPhaseFailed, + }, + }, + &release.Hook{ + Name: "passing-pre-install", + Events: []release.HookEvent{release.HookPreInstall}, + LastRun: release.HookExecution{ + StartedAt: mustParseTime("2006-01-02T15:00:05Z"), + CompletedAt: mustParseTime("2006-01-02T15:00:07Z"), + Phase: release.HookPhaseSucceeded, }, }, ), diff --git a/cmd/helm/testdata/output/status-with-test-suite.txt b/cmd/helm/testdata/output/status-with-test-suite.txt index 6790ea5ea..69c9cf425 100644 --- a/cmd/helm/testdata/output/status-with-test-suite.txt +++ b/cmd/helm/testdata/output/status-with-test-suite.txt @@ -3,8 +3,13 @@ LAST DEPLOYED: 2016-01-16 00:00:00 +0000 UTC NAMESPACE: default STATUS: deployed -TEST SUITE: bar +TEST SUITE: passing-test Last Started: 2006-01-02 15:04:05 +0000 UTC Last Completed: 2006-01-02 15:04:07 +0000 UTC -Successful: true +Phase: Succeeded + +TEST SUITE: failing-test +Last Started: 2006-01-02 15:10:05 +0000 UTC +Last Completed: 2006-01-02 15:10:07 +0000 UTC +Phase: Failed diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 84cedead3..8338b599a 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -53,24 +53,28 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, } // Get the time at which the hook was applied to the cluster - start := time.Now() - err = cfg.KubeClient.WatchUntilReady(resources, timeout) h.LastRun = release.HookExecution{ - StartedAt: start, - CompletedAt: time.Now(), - Successful: err == nil, + StartedAt: time.Now(), + Phase: release.HookPhaseUnknown, } + // Execute the hook + err = cfg.KubeClient.WatchUntilReady(resources, timeout) + // Note the time of success/failure + h.LastRun.CompletedAt = time.Now() + // Mark hook as succeeded or failed if err != nil { - // If a hook is failed, checkout the annotation of the hook to determine whether the hook should be deleted + h.LastRun.Phase = release.HookPhaseFailed + // If a hook is failed, check the annotation of the hook to determine whether the hook should be deleted // under failed condition. If so, then clear the corresponding resource object in the hook if err := cfg.deleteHookByPolicy(h, release.HookFailed); err != nil { return err } return err } + h.LastRun.Phase = release.HookPhaseSucceeded } - // If all hooks are succeeded, checkout the annotation of each hook to determine whether the hook should be deleted + // If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted // under succeeded condition. If so, then clear the corresponding resource object in each hook for _, h := range executingHooks { if err := cfg.deleteHookByPolicy(h, release.HookSucceeded); err != nil { diff --git a/pkg/action/printer.go b/pkg/action/printer.go index 6fe3c6385..128e9fb65 100644 --- a/pkg/action/printer.go +++ b/pkg/action/printer.go @@ -57,7 +57,7 @@ func PrintRelease(out io.Writer, rel *release.Release) { h.Name, fmt.Sprintf("Last Started: %s", h.LastRun.StartedAt), fmt.Sprintf("Last Completed: %s", h.LastRun.CompletedAt), - fmt.Sprintf("Successful: %t", h.LastRun.Successful), + fmt.Sprintf("Phase: %s", h.LastRun.Phase), ) } } diff --git a/pkg/release/hook.go b/pkg/release/hook.go index f29da4a72..5cec89e3a 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -82,8 +82,23 @@ type Hook struct { type HookExecution struct { // StartedAt indicates the date/time this hook was started StartedAt time.Time `json:"started_at,omitempty"` - // CompletedAt indicates the date/time this hook was completed + // CompletedAt indicates the date/time this hook was completed. CompletedAt time.Time `json:"completed_at,omitempty"` - // Successful indicates whether the hook completed successfully - Successful bool `json:"successful"` + // Phase indicates whether the hook completed successfully + Phase HookPhase `json:"phase"` } + +// A HookPhase indicates the state of a hook execution +type HookPhase string + +const ( + // HookPhaseUnknown indicates that a hook is in an unknown state + HookPhaseUnknown HookPhase = "Unknown" + // HookPhaseSucceeded indicates that hook execution succeeded + HookPhaseSucceeded HookPhase = "Succeeded" + // HookPhaseFailed indicates that hook execution failed + HookPhaseFailed HookPhase = "Failed" +) + +// Strng converts a hook phase to a printable string +func (x HookPhase) String() string { return string(x) } diff --git a/pkg/release/responses.go b/pkg/release/responses.go index 6eb9cbb5a..10b7f2054 100644 --- a/pkg/release/responses.go +++ b/pkg/release/responses.go @@ -32,9 +32,3 @@ type UninstallReleaseResponse struct { // Info is an uninstall message Info string `json:"info,omitempty"` } - -// TestReleaseResponse represents a message from executing a test -type TestReleaseResponse struct { - Msg string `json:"msg,omitempty"` - Status TestRunStatus `json:"status,omitempty"` -} diff --git a/pkg/release/test_run.go b/pkg/release/test_run.go deleted file mode 100644 index ff55301ab..000000000 --- a/pkg/release/test_run.go +++ /dev/null @@ -1,41 +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 release - -import "time" - -// TestRunStatus is the status of a test run -type TestRunStatus string - -// Indicates the results of a test run -const ( - TestRunUnknown TestRunStatus = "unknown" - TestRunSuccess TestRunStatus = "success" - TestRunFailure TestRunStatus = "failure" - TestRunRunning TestRunStatus = "running" -) - -// Strng converts a test run status to a printable string -func (x TestRunStatus) String() string { return string(x) } - -// TestRun describes the run of a test -type TestRun struct { - Name string `json:"name,omitempty"` - Status TestRunStatus `json:"status,omitempty"` - Info string `json:"info,omitempty"` - StartedAt time.Time `json:"started_at,omitempty"` - CompletedAt time.Time `json:"completed_at,omitempty"` -} From 2085228b506505035b5431731bc13eaff8d0cd57 Mon Sep 17 00:00:00 2001 From: Jacob LeGrone Date: Fri, 9 Aug 2019 15:05:46 -0400 Subject: [PATCH 8/8] feat(hooks): add Running phase Signed-off-by: Jacob LeGrone --- pkg/action/hooks.go | 23 +++++++++++++++++------ pkg/release/hook.go | 2 ++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pkg/action/hooks.go b/pkg/action/hooks.go index 8338b599a..3796b726a 100644 --- a/pkg/action/hooks.go +++ b/pkg/action/hooks.go @@ -48,16 +48,27 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, if err != nil { return errors.Wrapf(err, "unable to build kubernetes object for %s hook %s", hook, h.Path) } - if _, err := cfg.KubeClient.Create(resources); err != nil { - return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) - } - // Get the time at which the hook was applied to the cluster + // Record the time at which the hook was applied to the cluster h.LastRun = release.HookExecution{ StartedAt: time.Now(), - Phase: release.HookPhaseUnknown, + Phase: release.HookPhaseRunning, + } + cfg.recordRelease(rl) + + // As long as the implementation of WatchUntilReady does not panic, HookPhaseFailed or HookPhaseSucceeded + // should always be set by this function. If we fail to do that for any reason, then HookPhaseUnknown is + // the most appropriate value to surface. + h.LastRun.Phase = release.HookPhaseUnknown + + // Create hook resources + if _, err := cfg.KubeClient.Create(resources); err != nil { + h.LastRun.CompletedAt = time.Now() + h.LastRun.Phase = release.HookPhaseFailed + return errors.Wrapf(err, "warning: Hook %s %s failed", hook, h.Path) } - // Execute the hook + + // Watch hook resources until they have completed err = cfg.KubeClient.WatchUntilReady(resources, timeout) // Note the time of success/failure h.LastRun.CompletedAt = time.Now() diff --git a/pkg/release/hook.go b/pkg/release/hook.go index 5cec89e3a..96cc4f250 100644 --- a/pkg/release/hook.go +++ b/pkg/release/hook.go @@ -94,6 +94,8 @@ type HookPhase string const ( // HookPhaseUnknown indicates that a hook is in an unknown state HookPhaseUnknown HookPhase = "Unknown" + // HookPhaseRunning indicates that a hook is currently executing + HookPhaseRunning HookPhase = "Running" // HookPhaseSucceeded indicates that hook execution succeeded HookPhaseSucceeded HookPhase = "Succeeded" // HookPhaseFailed indicates that hook execution failed