dilim/helm
1
0
Fork 0
mirror of https://github.com/helm/helm.git synced 2026-08-04 23:20:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #31579 from benoittgt/delete-hook-after-log-retrieval

Browse Source 
fix: Delay test hook deletion until after logs are retrieved with `helm test release --logs`
This commit is contained in:
Evans Mungai
2026-01-09 15:02:37 +00:00
committed by GitHub
parent c3a0d3b860 45c5f3aaca
commit 05f25a3834
3 changed files with 72 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
pkg/action/hooks.go
View File

@@ -33,6 +33,27 @@ import (
// execHook executes all of the hooks for the given hook event.
func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent, waitStrategy kube.WaitStrategy, timeout time.Duration, serverSideApply bool) error {
shutdown, err := cfg.execHookWithDelayedShutdown(rl, hook, waitStrategy, timeout, serverSideApply)
if shutdown == nil {
return err
}
if err != nil {
if err := shutdown(); err != nil {
return err
}
return err
}
return shutdown()
}
type ExecuteShutdownFunc = func() error
func shutdownNoOp() error {
return nil
}
// execHookWithDelayedShutdown executes all of the hooks for the given hook event and returns a shutdownHook function to trigger deletions after doing other things like e.g. retrieving logs.
func (cfg *Configuration) execHookWithDelayedShutdown(rl *release.Release, hook release.HookEvent, waitStrategy kube.WaitStrategy, timeout time.Duration, serverSideApply bool) (ExecuteShutdownFunc, error) {
executingHooks := []*release.Hook{}
for _, h := range rl.Hooks {
@@ -51,12 +72,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent,
cfg.hookSetDeletePolicy(h)
if err := cfg.deleteHookByPolicy(h, release.HookBeforeHookCreation, waitStrategy, timeout); err != nil {
return err
return shutdownNoOp, err
}
resources, err := cfg.KubeClient.Build(bytes.NewBufferString(h.Manifest), true)
if err != nil {
return fmt.Errorf("unable to build kubernetes object for %s hook %s: %w", hook, h.Path, err)
return shutdownNoOp, fmt.Errorf("unable to build kubernetes object for %s hook %s: %w", hook, h.Path, err)
}
// Record the time at which the hook was applied to the cluster
@@ -77,12 +98,12 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent,
kube.ClientCreateOptionServerSideApply(serverSideApply, false)); err != nil {
h.LastRun.CompletedAt = time.Now()
h.LastRun.Phase = release.HookPhaseFailed
return fmt.Errorf("warning: Hook %s %s failed: %w", hook, h.Path, err)
return shutdownNoOp, fmt.Errorf("warning: Hook %s %s failed: %w", hook, h.Path, err)
}
waiter, err := cfg.KubeClient.GetWaiter(waitStrategy)
if err != nil {
return fmt.Errorf("unable to get waiter: %w", err)
return shutdownNoOp, fmt.Errorf("unable to get waiter: %w", err)
}
// Watch hook resources until they have completed
err = waiter.WatchUntilReady(resources, timeout)
@@ -98,36 +119,38 @@ func (cfg *Configuration) execHook(rl *release.Release, hook release.HookEvent,
}
// 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 errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed, waitStrategy, timeout); errDeleting != nil {
// We log the error here as we want to propagate the hook failure upwards to the release object.
log.Printf("error deleting the hook resource on hook failure: %v", errDeleting)
}
return func() error {
if errDeleting := cfg.deleteHookByPolicy(h, release.HookFailed, waitStrategy, timeout); errDeleting != nil {
// We log the error here as we want to propagate the hook failure upwards to the release object.
log.Printf("error deleting the hook resource on hook failure: %v", errDeleting)
}
// If a hook is failed, check the annotation of the previous successful hooks to determine whether the hooks
// should be deleted under succeeded condition.
if err := cfg.deleteHooksByPolicy(executingHooks[0:i], release.HookSucceeded, waitStrategy, timeout); err != nil {
// If a hook is failed, check the annotation of the previous successful hooks to determine whether the hooks
// should be deleted under succeeded condition.
if err := cfg.deleteHooksByPolicy(executingHooks[0:i], release.HookSucceeded, waitStrategy, timeout); err != nil {
return err
}
return err
}
return err
}, err
}
h.LastRun.Phase = release.HookPhaseSucceeded
}
// If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted
// or output should be logged under succeeded condition. If so, then clear the corresponding resource object in each hook
for i := len(executingHooks) - 1; i >= 0; i-- {
h := executingHooks[i]
if err := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnSucceeded); err != nil {
// We log here as we still want to attempt hook resource deletion even if output logging fails.
log.Printf("error outputting logs for hook failure: %v", err)
return func() error {
// If all hooks are successful, check the annotation of each hook to determine whether the hook should be deleted
// or output should be logged under succeeded condition. If so, then clear the corresponding resource object in each hook
for i := len(executingHooks) - 1; i >= 0; i-- {
h := executingHooks[i]
if err := cfg.outputLogsByPolicy(h, rl.Namespace, release.HookOutputOnSucceeded); err != nil {
// We log here as we still want to attempt hook resource deletion even if output logging fails.
log.Printf("error outputting logs for hook failure: %v", err)
}
if err := cfg.deleteHookByPolicy(h, release.HookSucceeded, waitStrategy, timeout); err != nil {
return err
}
}
if err := cfg.deleteHookByPolicy(h, release.HookSucceeded, waitStrategy, timeout); err != nil {
return err
}
}
return nil
return nil
}, nil
}
// hookByWeight is a sorter for hooks

20
pkg/action/release_testing.go
View File

@@ -57,24 +57,24 @@ func NewReleaseTesting(cfg *Configuration) *ReleaseTesting {
}
// Run executes 'helm test' against the given release.
func (r *ReleaseTesting) Run(name string) (ri.Releaser, error) {
func (r *ReleaseTesting) Run(name string) (ri.Releaser, ExecuteShutdownFunc, error) {
if err := r.cfg.KubeClient.IsReachable(); err != nil {
return nil, err
return nil, shutdownNoOp, err
}
if err := chartutil.ValidateReleaseName(name); err != nil {
return nil, fmt.Errorf("releaseTest: Release name is invalid: %s", name)
return nil, shutdownNoOp, fmt.Errorf("releaseTest: Release name is invalid: %s", name)
}
// finds the non-deleted release with the given name
reli, err := r.cfg.Releases.Last(name)
if err != nil {
return reli, err
return reli, shutdownNoOp, err
}
rel, err := releaserToV1Release(reli)
if err != nil {
return rel, err
return reli, shutdownNoOp, err
}
skippedHooks := []*release.Hook{}
@@ -102,14 +102,16 @@ func (r *ReleaseTesting) Run(name string) (ri.Releaser, error) {
}
serverSideApply := rel.ApplyMethod == string(release.ApplyMethodServerSideApply)
if err := r.cfg.execHook(rel, release.HookTest, kube.StatusWatcherStrategy, r.Timeout, serverSideApply); err != nil {
shutdown, err := r.cfg.execHookWithDelayedShutdown(rel, release.HookTest, kube.StatusWatcherStrategy, r.Timeout, serverSideApply)
if err != nil {
rel.Hooks = append(skippedHooks, rel.Hooks...)
r.cfg.Releases.Update(rel)
return rel, err
r.cfg.Releases.Update(reli)
return reli, shutdown, err
}
rel.Hooks = append(skippedHooks, rel.Hooks...)
return rel, r.cfg.Releases.Update(rel)
return reli, shutdown, r.cfg.Releases.Update(reli)
}
// GetPodLogs will write the logs for all test pods in the given release into

13
pkg/cmd/release_testing.go
View File

@@ -55,7 +55,7 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command
}
return compListReleases(toComplete, args, cfg)
},
RunE: func(_ *cobra.Command, args []string) error {
RunE: func(_ *cobra.Command, args []string) (returnError error) {
client.Namespace = settings.Namespace()
notName := regexp.MustCompile(`^!\s?name=`)
for _, f := range filter {
@@ -65,7 +65,16 @@ func newReleaseTestCmd(cfg *action.Configuration, out io.Writer) *cobra.Command
client.Filters[action.ExcludeNameFilter] = append(client.Filters[action.ExcludeNameFilter], notName.ReplaceAllLiteralString(f, ""))
}
}
reli, runErr := client.Run(args[0])
reli, shutdown, runErr := client.Run(args[0])
defer func() {
if shutdownErr := shutdown(); shutdownErr != nil {
if returnError == nil {
returnError = shutdownErr
}
}
}()
// We only return an error if we weren't even able to get the
// release, otherwise we keep going so we can print status and logs
// if requested