mirror of
https://github.com/vmware-tanzu/velero.git
synced 2026-07-20 15:03:04 +00:00
fix: make TestWaitExecHandleHooks deterministic for 2-container hook ordering
Fixes a flaky test in TestWaitExecHandleHooks: "should return no error with 2 spec hooks in 2 different containers, 1st container starts running after 10ms, 2nd container after 20ms, both succeed" Observed failure (CI run https://github.com/velero-io/velero/actions/runs/28119573625/job/83267758421): mock: Unexpected Method Call ExecutePodCommand called with resourceVersion:"3" (both containers running), but mock was registered expecting resourceVersion:"2" (container1 running, container2 still waiting). Root cause: the two source.Modify calls were 10ms apart. The informer's DeltaFIFO queue can coalesce rapid updates, delivering only the latest pod state (resourceVersion:3) to the handler before the hook for container1 fires. The comment "each of these states will be seen by the UpdateFunc handler" was incorrect — intermediate states can be silently skipped under load. Fix: add waitForSignal chan struct{} to the change struct and onCalled func() to the expectedExecution struct. The goroutine now blocks after sending the first change until the mock signals that the hook has fired (via close), then sends the second change. This guarantees the handler observes the intermediate pod state (resourceVersion:2) when executing container1's hook. Signed-off-by: Lubron Zhan <lubronzhan@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1 @@
|
||||
Fix flaky TestWaitExecHandleHooks test for 2-container hook ordering by synchronizing pod state changes with hook execution using channels
|
||||
@@ -53,13 +53,25 @@ func TestWaitExecHandleHooks(t *testing.T) {
|
||||
// delta to wait since last change applied or pod added
|
||||
wait time.Duration
|
||||
updated *corev1api.Pod
|
||||
// waitForSignal, if set, blocks the goroutine after applying this change
|
||||
// until the channel is closed. Use this to ensure the handler processes
|
||||
// an intermediate pod state before the next change is applied.
|
||||
waitForSignal chan struct{}
|
||||
}
|
||||
type expectedExecution struct {
|
||||
hook *velerov1api.ExecHook
|
||||
name string
|
||||
error error
|
||||
pod *corev1api.Pod
|
||||
// onCalled, if set, is invoked by the mock when ExecutePodCommand is called.
|
||||
// Use this together with change.waitForSignal to synchronize state transitions.
|
||||
onCalled func()
|
||||
}
|
||||
// hookFired is used by the test case that has two containers with hooks in
|
||||
// different containers. It ensures the second pod state change is only sent
|
||||
// after the first hook has fired, preventing the informer from coalescing
|
||||
// both updates and skipping the intermediate state.
|
||||
hookFired := make(chan struct{})
|
||||
tests := []struct {
|
||||
name string
|
||||
// Used as argument to HandleHooks and first state added to ListerWatcher
|
||||
@@ -622,6 +634,8 @@ func TestWaitExecHandleHooks(t *testing.T) {
|
||||
},
|
||||
}).
|
||||
Result(),
|
||||
// Signal after this hook fires so the goroutine can apply the next change.
|
||||
onCalled: func() { close(hookFired) },
|
||||
},
|
||||
{
|
||||
name: "my-hook-1",
|
||||
@@ -678,6 +692,10 @@ func TestWaitExecHandleHooks(t *testing.T) {
|
||||
},
|
||||
}).
|
||||
Result(),
|
||||
// Block until the hook for container1 has fired before sending the
|
||||
// next change. Without this, the informer may coalesce both updates
|
||||
// and deliver only resourceVersion:3, skipping the intermediate state.
|
||||
waitForSignal: hookFired,
|
||||
},
|
||||
// 2nd modification: container2 starts running, resourceVersion 3
|
||||
{
|
||||
@@ -838,11 +856,15 @@ func TestWaitExecHandleHooks(t *testing.T) {
|
||||
go func() {
|
||||
// This is the state of the pod that will be seen by the AddFunc handler.
|
||||
source.Add(test.initialPod)
|
||||
// Changes holds the versions of the pod over time. Each of these states
|
||||
// will be seen by the UpdateFunc handler.
|
||||
// Changes holds the versions of the pod over time. The informer may
|
||||
// coalesce rapid updates, so use waitForSignal when a test requires the
|
||||
// handler to observe a specific intermediate state before the next change.
|
||||
for _, change := range test.changes {
|
||||
time.Sleep(change.wait)
|
||||
source.Modify(change.updated)
|
||||
if change.waitForSignal != nil {
|
||||
<-change.waitForSignal
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
@@ -857,7 +879,11 @@ func TestWaitExecHandleHooks(t *testing.T) {
|
||||
for _, e := range test.expectedExecutions {
|
||||
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(e.pod)
|
||||
require.NoError(t, err)
|
||||
podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
|
||||
call := podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
|
||||
if e.onCalled != nil {
|
||||
onCalled := e.onCalled
|
||||
call.Run(func(mock.Arguments) { onCalled() })
|
||||
}
|
||||
}
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
Reference in New Issue
Block a user