Skip to content

Allow ECS tasks to continue when an Airflow task is killed - #74129

Open
jingi723 wants to merge 2 commits into
apache:mainfrom
jingi723:feature/ecs-stop-task-on-kill
Open

jingi723 wants to merge 2 commits into
apache:mainfrom
jingi723:feature/ecs-stop-task-on-kill

Conversation

@jingi723

@jingi723 jingi723 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

For workloads whose completion is managed independently of Airflow, stopping the waiting Airflow task should not necessarily stop the ECS task. Currently, Dag authors must override EcsRunTaskOperator.on_kill() to preserve that work, even with stop_task_on_failure=False: that existing option controls post-start waiter failures, not the kill callback.

Add stop_task_on_kill, defaulting to True. Setting it to False stops local log fetching and skips the ECS StopTask request without requiring an ECS client. Local log fetching is stopped before checking the policy or ECS client/ARN, so missing task details do not skip cleanup. By default, the operator still stops the ECS task when a client and ARN are available. stop_task_on_failure remains independent.

The option applies when the worker calls on_kill(), including an execution timeout while waiting synchronously. It does not change cancellation after deferral or guarantee callback execution after an abrupt worker failure. The guide explains responsibility for monitoring and cleaning up preserved tasks, and that reattach=True does not recover a completed task's result or guarantee deduplication.

Validation on upstream 7a8ce83f3851ec65dd2186c75f90e85c666a7cc0, Python 3.10.21, Linux amd64 Breeze:

  • ECS operator, hook and trigger tests: 115 passed on current sources; the same selection also passed on Airflow 2.11.0, the provider's declared minimum. The latter used source providers in an isolated container with incompatible provider metadata removed and development dependencies installed without constraints after a Ruff pin conflict, rather than the CI wheel-install matrix.
  • Regressions are in the existing test_ecs.py, using botocore Stubber for expected and forbidden requests. Keeping the new constructor option but restoring upstream on_kill() makes three cases fail; the final implementation passes them.
  • A separate local runtime probe passed four timeout/SIGTERM cases through the real Task SDK, operator, boto3 HTTP requests and log-fetcher thread, with ECS/CloudWatch responses served by a loopback substitute. It confirmed the stop/keep distinction and thread shutdown. The SIGTERM probe checks callback behavior, not the supervisor's final process termination or the complete cancellation outcome. This exploratory fixture is not added to the Task SDK suite.
  • Regular and manual prek checks, provider mypy, Amazon provider documentation build, selective-check and git diff --check passed. Manual checks omitted the three server/environment-provisioning hooks listed in the contributor instructions.

Before submission, rebased onto 9c14681a4a02deefc256dcb686f693ecb591ea42. The three reviewed files are unchanged; the related ECS suite passed again (115 tests), and selective-check passed on the final commit.

Review follow-up on upstream e0e3454998f626046038852121a0714cf9748ca9: move log-fetcher cleanup before the client/ARN guard. The two missing-client/missing-ARN regressions fail before this adjustment and pass after it; the related ECS suite passes all 117 tests. Regular and manual prek checks, including provider mypy, pass. The broader tests and documentation commands selected by breeze verify were attempted but did not complete within local memory limits; they are not claimed as passing.

Rebased on upstream d613e999deeee19e3358042eaf507ef2fd479fd2 after #73885 merged, preserving both fail_on_exit_code and stop_task_on_kill. The ECS operator, hook, and trigger test modules pass together (124 tests) in an existing Python 3.12.14 Linux/amd64 CI image. Regular and manual prek checks pass with CI-image-dependent hooks excluded (check-template-fields-valid, check-provider-yaml-valid, and manual mypy-providers); the full breeze verify matrix was not rerun locally. CI on this rebased head is pending.

No live AWS resources were used. The full dependent-provider, supported-version and platform CI matrices were not run locally. This is separate from the exit-code retry policy in #73885.


Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-6)

Generated-by: Codex (GPT-6) following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

Comment thread providers/amazon/src/airflow/providers/amazon/aws/operators/ecs.py Outdated
@jingi723
jingi723 force-pushed the feature/ecs-stop-task-on-kill branch from 7b9206d to 137ebd9 Compare October 5, 2026 14:32

@fat-catTW fat-catTW left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work

@jingi723

Copy link
Copy Markdown
Contributor Author

The review feedback is addressed, all review threads are resolved, and Tests (AMD), CodeQL, and the newsfragment check pass on the current head. Could a maintainer take a final look when available? Thanks!

Workloads managed independently of Airflow may need to finish after the
waiting task is terminated. Dag authors currently need an on_kill override
even when post-start failure cleanup is disabled.
@jingi723
jingi723 force-pushed the feature/ecs-stop-task-on-kill branch from 137ebd9 to a408a33 Compare October 23, 2026 16:17

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants