e2e: stop TestGracefulWorkerTerminationSuspend racing the actor to CRASHED - #1019
Closed
Benjamin Elder (BenTheElder) wants to merge 1 commit into
Closed
Conversation
…ASHED The test deletes the worker pod, sleeps a fixed 2s, then suspends the actor, which is meant to land while the graceful shutdown is under way. On micro-VM it lands after the actor has already reached CRASHED, so the suspend is rejected and the test fails on every run since agent-substrate#893 enabled it there - on main as well as on unrelated PRs: termination_test.go: failed to suspend Actor: rpc error: code = FailedPrecondition desc = MarkSuspending prerequisite not met for Actor: demo/graceful-term-suspend-... (got: STATUS_CRASHED, want STATUS_RUNNING or STATUS_PAUSED) A duration is the wrong thing to wait on: how quickly the drain starts, and how long the actor survives it, differ by sandbox class. Wait for the state the test actually depends on instead - the control plane marking the worker DRAINING when it sees the pod's DeletionTimestamp - and suspend as soon as that is observable rather than 2s later. Measured against a kind cluster running this branch's main, micro-VM demo: before 0/3 pass, every failure the MarkSuspending rejection above after 14/15 pass; the one failure was elsewhere in the test (a resume readyz timeout, the pre-existing flake) and never the suspend The three termination tests also pass together, twice, with no skips. This keeps what the test is for. ateom's graceful shutdown deliberately releases its lock so 'a suspend arriving mid-drain' is still served, and that is what is being exercised; only the way the test finds that window changes.
Collaborator
Author
|
/hold so while we should avoid fixed timeouts, and tests are passing locally with this change where they failed before it, this doesn't make sense. it should stay crashed. |
Collaborator
Author
|
focusing on fixing the mechanism in ateom |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test deletes the worker pod, sleeps a fixed 2s, then suspends the actor, which is meant to land while the graceful shutdown is under way. On micro-VM it lands after the actor has already reached CRASHED, so the suspend is rejected and the test fails on every run since #893 enabled it there - on main as well as on unrelated PRs:
This PR updates the test to poll for STATE_DRAINING instead of a fixed sleep.
Fixes #1016