Repository navigation
fix(ci): orphan sweep spares other homes, dep gate blocks on failed deps (D-1117, D-1119) - #476
Merged
Merged
Conversation
… or unplaced deps block waiters (D-1117, D-1119) The container sweep lists every labelled container on the daemon and treated an unknown coordinator as stale, so a run with another NSELF_CI_HOME could remove that home's live container. Only coordinators the store knows are now swept. depOK let jobs run after a failed allow_failure dependency (contrary to plan/dag.go) and after a prerequisite that was never placed, admitted or run. Both now block the waiter.
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
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.
D-1117 (fixed)
Recovertreated a coordinator id missing from the store as stale, and the container sweep lists every labelled container on the daemon. A run under anotherNSELF_CI_HOMEcouldrm -fthat home's live container.exec.KnownChecker(optional extension ofStaleChecker):sweepRuntimeskips containers whose coordinator the checker does not know.run.staleChecker.Knownanswers from the store.Staleis unchanged, so own-home process markers of a lost coordinator are still swept.TestRecoverSweepSkipsContainersOfUnknownCoordinators(fake docker). Fails on main (rm -f foreign1), passes now.D-1119 (fixed)
depOKaccepted a failed allow_failure dependency (contrary toplan/dag.go) and any skipped prerequisite not marked dep.failed/fail_fast, including ones that never ran.dep.failed,fail_fast.cancelled,unplaceable.*,admission.*oradvisory.*. Scoped-out skips (selection.*,when.*,pipeline.*) still let waiters run. This reason split is my reading of the review text ("unplaceable skipped prerequisites"); say if you want it narrower or wider.TestDepOK(table) andTestFailedAllowFailureJobBlocksItsWaiters(end to end). Both fail on main.D-0187 (not changed, blocked)
Accepted residual: the legacy engine is removed at the v1.6 removal release together with D-0119 (
scheduled_in: v1.6.0 removal release). Deleting or reshapinginternal/legacyis release scope, not a local fix.Checks
gofmt -l,go vet -mod=vendor ./internal/run ./internal/exec: cleango test -mod=vendor ./... -count=1infree/ci: two failures, identical on clean origin/main (internal/legacyTestSecretsEnvLiveKeyGateTrackedValueOnly,cmd/agentTestServeStdioExitsNonZeroWithE663OnAProdStackMarker); all else ok