Delete task definitions after deregistering during cleanup#331
Closed
jonpspri wants to merge 1 commit intomattclay:mainfrom
Closed
Delete task definitions after deregistering during cleanup#331jonpspri wants to merge 1 commit intomattclay:mainfrom
jonpspri wants to merge 1 commit intomattclay:mainfrom
Conversation
Now that ecs:DeleteTaskDefinitions is available, fully remove task definitions instead of leaving them in INACTIVE state after deregistering.
There was a problem hiding this comment.
Pull request overview
This PR enhances ECS cleanup by adding IAM permission and extending the ECS terminator to fully delete task definition revisions (rather than leaving them INACTIVE) after deregistration.
Changes:
- Add
ecs:DeleteTaskDefinitionsto the PaaS IAM policy. - Update ECS terminator cleanup to call
delete_task_definitionsin batches of 10 after deregistering task definitions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| aws/terminator/paas.py | Adds batched deletion of task definitions after deregistration during ECS cluster cleanup. |
| aws/policy/paas.yaml | Grants ecs:DeleteTaskDefinitions needed for the new cleanup behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+290
to
+295
| # Deregister and delete task definitions | ||
| task_definitions = _paginate_task_definition_results() | ||
| for each in task_definitions: | ||
| self.client.deregister_task_definition(taskDefinition=each) | ||
| for i in range(0, len(task_definitions), 10): | ||
| self.client.delete_task_definitions(taskDefinitions=task_definitions[i:i + 10]) |
There was a problem hiding this comment.
list_task_definitions is account/region-wide (no cluster filter), so this will deregister and now delete every task definition in the account, not just ones used by the cluster being terminated. That can break other ECS clusters/services. Consider scoping the task definitions to those referenced by services/tasks in this cluster (e.g., describe_services / describe_tasks to collect taskDefinition ARNs), and only delete those; also consider listing/deleting only INACTIVE revisions rather than deregistering everything.
jonpspri
added a commit
to jonpspri/aws-terminator
that referenced
this pull request
Feb 12, 2026
Move task definition lifecycle management out of Ecs.terminate() into a new EcsTaskDefinition(DbTerminator) class. This fixes the blast radius issue where cleaning up any stale cluster would deregister all task definitions in the account. Task definitions are now independently tracked and aged via DynamoDB. Also adds ecs:DeleteTaskDefinitions to the paas policy and fully deletes (not just deregisters) stale task definitions. Subsumes PR mattclay#331.
Author
|
Subsumed by #330 — task definition cleanup has been extracted into its own EcsTaskDefinition terminator class, which includes the DeleteTaskDefinitions permission and API call from this PR. |
jonpspri
added a commit
to jonpspri/aws-terminator
that referenced
this pull request
Feb 17, 2026
Move task definition lifecycle management out of Ecs.terminate() into a new EcsTaskDefinition(DbTerminator) class. This fixes the blast radius issue where cleaning up any stale cluster would deregister all task definitions in the account. Task definitions are now independently tracked and aged via DynamoDB. Also adds ecs:DeleteTaskDefinitions to the paas policy and fully deletes (not just deregisters) stale task definitions. Subsumes PR mattclay#331.
jonpspri
added a commit
to jonpspri/aws-terminator
that referenced
this pull request
Feb 17, 2026
Add IAM permissions and terminator class to support ecs_service Service Connect integration tests: - Add servicediscovery permissions (CreateHttpNamespace, DeleteNamespace, GetOperation, ListNamespaces) to networking policy - Add ELB service-linked role creation permission to paas policy - Add ServiceDiscoveryHttpNamespace terminator class for cleanup of HTTP namespaces created during integration tests Expand Service Discovery support for all namespace types Generalize the terminator from HTTP-only to all namespace types (HTTP, DNS_PUBLIC, DNS_PRIVATE) and delete child services before deleting namespaces. Add corresponding IAM permissions. Extract ECS task definition cleanup into its own terminator class Move task definition lifecycle management out of Ecs.terminate() into a new EcsTaskDefinition(DbTerminator) class. This fixes the blast radius issue where cleaning up any stale cluster would deregister all task definitions in the account. Task definitions are now independently tracked and aged via DynamoDB. Also adds ecs:DeleteTaskDefinitions to the paas policy and fully deletes (not just deregisters) stale task definitions. Subsumes PR mattclay#331. Split ECS policy into separate paas-ecs policy file
jonpspri
added a commit
to jonpspri/aws-terminator
that referenced
this pull request
Feb 20, 2026
Add IAM permissions and terminator class to support ecs_service Service Connect integration tests: - Add servicediscovery permissions (CreateHttpNamespace, DeleteNamespace, GetOperation, ListNamespaces) to networking policy - Add ELB service-linked role creation permission to paas policy - Add ServiceDiscoveryHttpNamespace terminator class for cleanup of HTTP namespaces created during integration tests Expand Service Discovery support for all namespace types Generalize the terminator from HTTP-only to all namespace types (HTTP, DNS_PUBLIC, DNS_PRIVATE) and delete child services before deleting namespaces. Add corresponding IAM permissions. Extract ECS task definition cleanup into its own terminator class Move task definition lifecycle management out of Ecs.terminate() into a new EcsTaskDefinition(DbTerminator) class. This fixes the blast radius issue where cleaning up any stale cluster would deregister all task definitions in the account. Task definitions are now independently tracked and aged via DynamoDB. Also adds ecs:DeleteTaskDefinitions to the paas policy and fully deletes (not just deregisters) stale task definitions. Subsumes PR mattclay#331. Split ECS policy into separate paas-ecs policy file
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.
Summary
ecs:DeleteTaskDefinitionspermission to the paas policyEcsterminator to calldelete_task_definitionsafter deregistering, fully removing task definitions instead of leaving them inINACTIVEstateTest plan