AIP-97 POC: classify ECS task failures as infra - #28
Open
1fanwang wants to merge 1 commit into
Open
Conversation
Signed-off-by: 1fanwang <1fannnw@gmail.com>
1fanwang
changed the base branch from
aip97-failure-details
to
1fanwang/aip97-failure-details
August 9, 2026 07:26
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.
Why
Review feedback on AIP-97 said the failure classification looks Kubernetes-only, and asked for a
contract on
BaseExecutorthat other executors can implement against instead of the untypedinfoslot on
change_state.ECS is the cheapest counter-example, because it is not missing the data. It throws it away.
describe_tasksreturnsTask.stopCode, a closed enum naming who stopped the task, butBotoTaskSchemaparses onlystoppedReason, and__handle_failed_workloadlogs that string andthen calls
self.fail(task_key)with nothing attached. A reclaimed spot task and a task that raisedreach the scheduler looking identical.
What changed
BaseExecutor.fail()takes optional typedfailure_kindandreason. That is the contract: thefailure hook every executor already calls, typed, additive, and it leaves
infoalone, sinceinfois polymorphic today and carries
external_executor_idon queued and running.ECS then parses
stopCodeand classifies through it.SpotInterruptionis infra. A host thatstopped or terminated under a running task is infra, matched on the same
stoppedReasontextEcsRunTaskOperatoralready treats as a distinct failure.EssentialContainerExitedis applicationand earns no refund.
The other five stay unclassified and behave as today.
TerminationNoticeandInfrastructureHealthdo read as infra, but AWS documents neither, so mapping them would be a guess.Also fixes two leaked
mock.patch(...).start()calls in the ECS tests. They never stopped, soAwsEcsExecutor.failstayed mocked for every test that ran after them.Refs
TaskStopCodeenum: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_Task.htmlSpotInterruption: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/spot-interruption-errors.htmlTesting Done
Committed coverage is 14 provider tests, run in CI. The chain below is evidence.
retries=0up_for_retry,max_tries 0->1failed,max_tries 0->0failed,max_tries 0->01-3: ECS executor to refund, live Postgres 16
Real
sync_running_workloads()with only the boto client replaced, realhandle_failure, reallistener, real database.
stoppedReasonis verbatim from AWS's Fargate Spot example event.db migratestamping to the existing head is the runnable proof this adds no schema. Scenario 1uses
retries=0, so the refund is what rescues it at all.4: red control, and the leaked-patch fix
Deleting only the one-line
stop_codeschema field, on otherwise identical source:stop_code=None failure_kind=Noneon a real spot interruption is the defect this fixes, in oneline. Restored, both pass.
The two new tests passed alone but failed in the full file, which surfaced the leaked patches:
Evidence script (inline, so it can be re-run)
Not committed: it imports the amazon provider from a core test path, which core CI does not install.