test: fix flaky image timestamp check on coarse clocks#13588
Conversation
TestImagesCreateUpdateDelete asserts that an image's updatedat is strictly after its createdat. Both timestamps are stamped via time.Now().UTC(), which strips the monotonic reading, so the comparison falls back to the wall clock. On platforms with coarse timer resolution (e.g. Windows, which advances system time at the ~15.6ms tick), the Create and Update calls can land in the same tick and produce identical timestamps, making the strict After() check fail intermittently. Wait for the wall clock to advance past the creation timestamp before updating so the assertion stays meaningful without depending on clock resolution. On fine-resolution clocks the loop runs zero iterations. Signed-off-by: Austin Vazquez <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR fixes a flaky timestamp assertion in TestImagesCreateUpdateDelete by ensuring the wall clock advances between the image Create and Update operations, avoiding intermittent failures on coarse-resolution clocks (notably Windows).
Changes:
- Adds a wait loop before calling
store.Updateto ensure the nexttime.Now()wall-clock value is strictly after the creation timestamp. - Documents the root cause (loss of monotonic clock reading via
time.Now().UTC()and coarse wall-clock ticks).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kunalworldwide
left a comment
kunalworldwide
left a comment
There was a problem hiding this comment.
Clean fix for a real flake — on Windows (or any platform with coarse timer resolution), two back-to-back time.Now().UTC() calls can return the same value, so UpdatedAt equals CreatedAt and the strict ordering assertion fails.
The spin-wait approach is fine for a test — time.Sleep(time.Millisecond) is cheap and the loop exits as soon as the clock ticks forward. The comment explains the root cause well.
One minor thought: if you wanted to avoid the spin entirely, you could relax the assertion from After to !Before (i.e., allow equal timestamps). But that would weaken the test's intent, so I think the wait is the better choice here — it preserves the strict ordering guarantee.
LGTM.
github-project-automation
Bot
moved this from Needs Reviewers
to Review In Progress
in Pull Request Review
github-project-automation
Bot
moved this from Review In Progress
to Done
in Pull Request Review
|
/cherry-pick release/2.3 |
|
@fuweid: new pull request created: #13643 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
fuweid
added
cherry-picked/2.3.x
TestImagesCreateUpdateDelete asserts that an image's updatedat is strictly after its createdat. Both timestamps are stamped via time.Now().UTC(), which strips the monotonic reading, so the comparison falls back to the wall clock. On platforms with coarse timer resolution (e.g. Windows, which advances system time at the ~15.6ms tick), the Create and Update calls can land in the same tick and produce identical timestamps, making the strict After() check fail intermittently.
Wait for the wall clock to advance past the creation timestamp before updating so the assertion stays meaningful without depending on clock resolution. On fine-resolution clocks the loop runs zero iterations.
Testing
Before
After
Additional context
Seen in https://github.com/containerd/containerd/actions/runs/27430461506/job/81079589148