Repository navigation
test(e2e): hold the pod in Pending so the Sandbox Pending phase is observable - #1154
NopeCharon wants to merge 1 commit into
Conversation
…servable The "should create sandbox and transition to running phase" spec polls every 500ms for Sandbox.Status.Phase == Pending. The sandbox controller moves a Sandbox from Pending to Running as soon as its Pod phase becomes Running (EnsureSandboxRunning in common_control.go), independent of the Pod's readiness. On a warm Kind node the whole Pending window is only scheduling plus IP allocation, which has been observed at ~497ms, so the first poll can already see Running and the spec times out. The readiness probe with InitialDelaySeconds: 5 that was meant to keep the Pending phase observable only delays the Pod Ready condition, not the Pod phase, so it never extended the window. Replace it with a short init container that sleeps for 5 seconds. A Pod stays in its Pending phase while init containers run, so the Sandbox remains Pending for at least 5 seconds (about ten poll intervals). The init container reuses the image of the main container, so no extra image pull is introduced. The other Pending assertions in this file belong to the ResourceQuota spec, where the Pod is never created and Pending is stable, so they do not race. Fixes openkruise#1110 Fixes openkruise#1094 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @NopeCharon! It looks like this is your first PR to openkruise/agents 🎉 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1154 +/- ##
=======================================
Coverage 84.47% 84.47%
=======================================
Files 280 280
Lines 25109 25109
=======================================
Hits 21212 21212
Misses 3082 3082
Partials 815 815
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Ⅰ. Describe what this PR does
Fixes the intermittent failure of the E2E spec
Sandbox / creation and pending phase / should create sandbox and transition to running phase. The spec fails withExpected <Running> to equal <Pending>after the 30s timeout.The sandbox controller moves a Sandbox from
PendingtoRunningas soon as the Pod phase becomesRunning(EnsureSandboxRunninginpkg/controller/sandbox/core/common_control.go). It doesn't wait for readiness. The spec currently sets a readiness probe withInitialDelaySeconds: 5to keepPendingobservable, but that probe only delays the PodReadycondition, not the Pod phase. On a warm Kind node the SandboxPendingwindow is just scheduling plus IP allocation. It was ~497ms in the run linked from #1110, which is shorter than the 500msEventuallypoll interval, so the first GET can already returnRunning.This PR replaces the readiness probe with a short init container (
sh -c "sleep 5", same image as the main container, so there's no extra pull). A Pod stays in phasePendingwhile its init containers run, so the Sandbox staysPendingfor at least 5 seconds, about ten poll intervals. Nothing changes after that: the spec still waits forRunningand checks the revision.The other
Should(Equal(SandboxPending))assertions in this file (mentioned in #1094) belong to the ResourceQuota spec. There the Pod is never created, soPendingis stable and those assertions don't race.Ⅱ. Does this pull request fix one issue?
fixes #1110
fixes #1094
Ⅲ. Describe how to verify it
go vet ./test/e2e/passes.E2E-1.32workflow. Thecreation and pending phasespec now observesPendingeven on a node wherenginx:stable-alpine3.20is already cached, because the Pod is held in itsPendingphase for the 5 seconds the init container runs.Ⅳ. Special notes for reviews
This is a test-only change. Controller behavior is unchanged.