From 57370296ab3f309f205512d4f661a898684ec5c2 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Mon, 27 Oct 2025 18:34:04 +0800 Subject: [PATCH] Use hookIndex for recording multiple restore exec hooks. Signed-off-by: Xun Jiang --- changelogs/unreleased/9366-blackpiglet | 1 + internal/hook/wait_exec_hook_handler.go | 8 +- internal/hook/wait_exec_hook_handler_test.go | 124 +++++++++++++++++++ 3 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/9366-blackpiglet diff --git a/changelogs/unreleased/9366-blackpiglet b/changelogs/unreleased/9366-blackpiglet new file mode 100644 index 000000000..644ef6a48 --- /dev/null +++ b/changelogs/unreleased/9366-blackpiglet @@ -0,0 +1 @@ +Use hookIndex for recording multiple restore exec hooks. diff --git a/internal/hook/wait_exec_hook_handler.go b/internal/hook/wait_exec_hook_handler.go index ce947e85a..92a905c8c 100644 --- a/internal/hook/wait_exec_hook_handler.go +++ b/internal/hook/wait_exec_hook_handler.go @@ -169,7 +169,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks( hookLog.Error(err) errors = append(errors, err) - errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), i, true, err) + errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), hook.hookIndex, true, err) if errTracker != nil { hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker") } @@ -195,7 +195,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks( hookFailed = true } - errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), i, hookFailed, hookErr) + errTracker := multiHookTracker.Record(restoreName, newPod.Namespace, newPod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), hook.hookIndex, hookFailed, hookErr) if errTracker != nil { hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker") } @@ -239,7 +239,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks( // containers to become ready. // Each unexecuted hook is logged as an error and this error will be returned from this function. for _, hooks := range byContainer { - for i, hook := range hooks { + for _, hook := range hooks { if hook.executed { continue } @@ -252,7 +252,7 @@ func (e *DefaultWaitExecHookHandler) HandleHooks( }, ) - errTracker := multiHookTracker.Record(restoreName, pod.Namespace, pod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), i, true, err) + errTracker := multiHookTracker.Record(restoreName, pod.Namespace, pod.Name, hook.Hook.Container, hook.HookSource, hook.HookName, HookPhase(""), hook.hookIndex, true, err) if errTracker != nil { hookLog.WithError(errTracker).Warn("Error recording the hook in hook tracker") } diff --git a/internal/hook/wait_exec_hook_handler_test.go b/internal/hook/wait_exec_hook_handler_test.go index 7f549ba32..fb102b16f 100644 --- a/internal/hook/wait_exec_hook_handler_test.go +++ b/internal/hook/wait_exec_hook_handler_test.go @@ -706,6 +706,130 @@ func TestWaitExecHandleHooks(t *testing.T) { }, }, }, + { + name: "Multiple hooks with non-sequential indices (bug #9359)", + initialPod: builder.ForPod("default", "my-pod"). + Containers(&corev1api.Container{ + Name: "container1", + }). + ContainerStatuses(&corev1api.ContainerStatus{ + Name: "container1", + State: corev1api.ContainerState{ + Running: &corev1api.ContainerStateRunning{}, + }, + }). + Result(), + groupResource: "pods", + byContainer: map[string][]PodExecRestoreHook{ + "container1": { + { + HookName: "first-hook", + HookSource: HookSourceAnnotation, + Hook: velerov1api.ExecRestoreHook{ + Container: "container1", + Command: []string{"/usr/bin/foo"}, + OnError: velerov1api.HookErrorModeContinue, + ExecTimeout: metav1.Duration{Duration: time.Second}, + WaitTimeout: metav1.Duration{Duration: time.Minute}, + }, + hookIndex: 0, + }, + { + HookName: "second-hook", + HookSource: HookSourceAnnotation, + Hook: velerov1api.ExecRestoreHook{ + Container: "container1", + Command: []string{"/usr/bin/bar"}, + OnError: velerov1api.HookErrorModeContinue, + ExecTimeout: metav1.Duration{Duration: time.Second}, + WaitTimeout: metav1.Duration{Duration: time.Minute}, + }, + hookIndex: 2, + }, + { + HookName: "third-hook", + HookSource: HookSourceAnnotation, + Hook: velerov1api.ExecRestoreHook{ + Container: "container1", + Command: []string{"/usr/bin/third"}, + OnError: velerov1api.HookErrorModeContinue, + ExecTimeout: metav1.Duration{Duration: time.Second}, + WaitTimeout: metav1.Duration{Duration: time.Minute}, + }, + hookIndex: 4, + }, + }, + }, + expectedExecutions: []expectedExecution{ + { + name: "first-hook", + hook: &velerov1api.ExecHook{ + Container: "container1", + Command: []string{"/usr/bin/foo"}, + OnError: velerov1api.HookErrorModeContinue, + Timeout: metav1.Duration{Duration: time.Second}, + }, + error: nil, + pod: builder.ForPod("default", "my-pod"). + ObjectMeta(builder.WithResourceVersion("1")). + Containers(&corev1api.Container{ + Name: "container1", + }). + ContainerStatuses(&corev1api.ContainerStatus{ + Name: "container1", + State: corev1api.ContainerState{ + Running: &corev1api.ContainerStateRunning{}, + }, + }). + Result(), + }, + { + name: "second-hook", + hook: &velerov1api.ExecHook{ + Container: "container1", + Command: []string{"/usr/bin/bar"}, + OnError: velerov1api.HookErrorModeContinue, + Timeout: metav1.Duration{Duration: time.Second}, + }, + error: nil, + pod: builder.ForPod("default", "my-pod"). + ObjectMeta(builder.WithResourceVersion("1")). + Containers(&corev1api.Container{ + Name: "container1", + }). + ContainerStatuses(&corev1api.ContainerStatus{ + Name: "container1", + State: corev1api.ContainerState{ + Running: &corev1api.ContainerStateRunning{}, + }, + }). + Result(), + }, + { + name: "third-hook", + hook: &velerov1api.ExecHook{ + Container: "container1", + Command: []string{"/usr/bin/third"}, + OnError: velerov1api.HookErrorModeContinue, + Timeout: metav1.Duration{Duration: time.Second}, + }, + error: nil, + pod: builder.ForPod("default", "my-pod"). + ObjectMeta(builder.WithResourceVersion("1")). + Containers(&corev1api.Container{ + Name: "container1", + }). + ContainerStatuses(&corev1api.ContainerStatus{ + Name: "container1", + State: corev1api.ContainerState{ + Running: &corev1api.ContainerStateRunning{}, + }, + }). + Result(), + }, + }, + expectedErrors: nil, + }, } for _, test := range tests {