Skip to content

Commit

Permalink
jobs,test: Fix TestTimer_ExitOnCloseFnCtx channel close panic
Browse files Browse the repository at this point in the history
This test enforces that a timer callback will not be called after the
context has been canceled. However, it was written such that if the test
fails a panic is thrown. This fixes both the panic-on-fail and resolves
the edge case causing the flake.

Fixes: #25177

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
  • Loading branch information
dylandreimerink authored and joestringer committed May 5, 2023
1 parent c4677ee commit daa85a0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
6 changes: 6 additions & 0 deletions pkg/hive/job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,12 @@ func (jt *jobTimer) start(ctx context.Context, wg *sync.WaitGroup, options optio
jt.shutdown.Shutdown(hive.ShutdownWithError(err))
}
}

// If we exited due to the ctx closing we do not guaranteed return.
// The select can pick the timer or trigger signals over ctx.Done due to fair scheduling, so this guarantees it.
if ctx.Err() != nil {
return
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/hive/job/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,10 @@ func TestTimer_ExitOnCloseFnCtx(t *testing.T) {
g.Add(
Timer("on-interval", func(ctx context.Context) error {
i++
close(started)
if started != nil {
close(started)
started = nil
}
<-ctx.Done()
return nil
}, 1*time.Millisecond),
Expand Down

0 comments on commit daa85a0

Please sign in to comment.