fix(restore_finalizer): bound WaitRestoreExecHook poll with resourceT… (#10280)

* fix: reuse DefaultResourceTimeout from server config for hook wait

Signed-off-by: Nitish Malang <71919457+nitishmalang@users.noreply.github.com>

* Add changelog for PR 10280

Signed-off-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>

---------

Signed-off-by: Nitish Malang <71919457+nitishmalang@users.noreply.github.com>
Signed-off-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>
Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com>

changelogs/unreleased/10280-nitishmalang

@@ -0,0 +1 @@
+Bound WaitRestoreExecHook polling with resourceTimeout to avoid an infinite wait when restore exec hooks never complete.

pkg/cmd/server/config/config.go

@@ -28,6 +28,11 @@ const (
 	defaultPodVolumeOperationTimeout  = 240 * time.Minute
 	defaultResourceTerminatingTimeout = 10 * time.Minute
 
+	// DefaultResourceTimeout is the default for --resource-timeout. It matches
+	// defaultResourceTerminatingTimeout so controller fallbacks stay aligned with
+	// server defaults (see pkg/cmd/server/config/config.go).
+	DefaultResourceTimeout = defaultResourceTerminatingTimeout
+
 	// server's client default qps and burst
 	defaultClientQPS      float32 = 100.0
 	defaultClientBurst    int     = 100
@@ -41,7 +46,7 @@ const (
 	defaultCSISnapshotTimeout   = 10 * time.Minute
 	defaultItemOperationTimeout = 4 * time.Hour
 
-	resourceTimeout = 10 * time.Minute
+	resourceTimeout = defaultResourceTerminatingTimeout
 
 	defaultMaxConcurrentK8SConnections = 30
 	defaultDisableInformerCache        = false

pkg/controller/restore_finalizer_controller.go

@@ -39,6 +39,7 @@ import (
 	"github.com/vmware-tanzu/velero/internal/hook"
 	"github.com/vmware-tanzu/velero/internal/volume"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	serverconfig "github.com/vmware-tanzu/velero/pkg/cmd/server/config"
 	"github.com/vmware-tanzu/velero/pkg/constant"
 	"github.com/vmware-tanzu/velero/pkg/itemoperation"
 	"github.com/vmware-tanzu/velero/pkg/metrics"
@@ -577,8 +578,18 @@ func (ctx *finalizerContext) WaitRestoreExecHook() (errs results.Result) {
 	log := ctx.logger.WithField("restore", ctx.restore.Name)
 	log.Info("Waiting for restore exec hooks starts")
 
-	// wait for restore exec hooks to finish
-	err := wait.PollUntilContextCancel(context.Background(), 1*time.Second, true, func(context.Context) (bool, error) {
+	// Bound the wait by resourceTimeout (the same budget Velero already
+	// applies to other finalizer phases). Previously this poll had no
+	// deadline, so a hook that was registered via Add() but never
+	// recorded as executed left the restore stuck in Finalizing forever
+	// and blocked every other restore on the cluster.
+	timeout := ctx.resourceTimeout
+	if timeout <= 0 {
+		timeout = serverconfig.DefaultResourceTimeout
+	}
+	pollCtx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	err := wait.PollUntilContextCancel(pollCtx, 1*time.Second, true, func(context.Context) (bool, error) {
 		log.Debug("Checking the progress of hooks execution")
 		if ctx.multiHookTracker.IsComplete(ctx.restore.Name) {
 			return true, nil

pkg/controller/restore_finalizer_controller_test.go

@@ -482,6 +482,10 @@ func TestWaitRestoreExecHook(t *testing.T) {
 	hookFailed, hookErr := true, fmt.Errorf("hook failed")
 	hookTracker3.Add(restoreName3, podNs, podName, container, source, hookName, hook.PhasePre, 0)
 
+	hookTracker4 := hook.NewMultiHookTracker()
+	restoreName4 := "restore4"
+	hookTracker4.Add(restoreName4, "ns", "pod", "con1", "s1", "h1", hook.PhasePre, 0)
+
 	tests := []struct {
 		name                   string
 		hookTracker            *hook.MultiHookTracker
@@ -497,6 +501,8 @@ func TestWaitRestoreExecHook(t *testing.T) {
 		hookName               string
 		hookFailed             bool
 		hookErr                error
+		resourceTimeout        time.Duration
+		expectTimeoutErr       bool
 	}{
 		{
 			name:                   "no restore exec hooks",
@@ -530,6 +536,16 @@ func TestWaitRestoreExecHook(t *testing.T) {
 			hookFailed:             hookFailed,
 			hookErr:                hookErr,
 		},
+		{
+			name:                   "hook never recorded should timeout instead of hanging",
+			hookTracker:            hookTracker4,
+			restore:                builder.ForRestore(velerov1api.DefaultNamespace, restoreName4).Result(),
+			expectedHooksAttempted: 0,
+			expectedHooksFailed:    0,
+			expectedHookErrs:       1,
+			resourceTimeout:        3 * time.Second,
+			expectTimeoutErr:       true,
+		},
 	}
 
 	for _, tc := range tests {
@@ -542,6 +558,7 @@ func TestWaitRestoreExecHook(t *testing.T) {
 			crClient:         fakeClient,
 			restore:          tc.restore,
 			multiHookTracker: tc.hookTracker,
+			resourceTimeout:  tc.resourceTimeout,
 		}
 		require.NoError(t, ctx.crClient.Create(t.Context(), tc.restore))
 
@@ -553,6 +570,10 @@ func TestWaitRestoreExecHook(t *testing.T) {
 		}
 
 		errs := ctx.WaitRestoreExecHook()
+		if tc.expectTimeoutErr {
+			assert.NotEmpty(t, errs.Namespaces, "expected timeout error but got none")
+			continue
+		}
 		assert.Len(t, errs.Namespaces, tc.expectedHookErrs)
 
 		updated := &velerov1api.Restore{}