[V2] Add TaskAction garbage collector for terminal CRDs by pingsutw · Pull Request #6994 · flyteorg/flyte

pingsutw · 2026-03-09T18:32:18Z

Tracking issue

Why are the changes needed?

Terminal TaskAction CRDs (Succeeded/Failed) remain in the cluster indefinitely after completion. This wastes etcd storage and slows down list operations. We need a garbage collector that periodically deletes terminal TaskActions after a configurable TTL.

What changes were proposed in this pull request?

Adds a label-based TTL garbage collector for terminal TaskActions, modeled after the propeller FlyteWorkflow GC (flytepropeller/pkg/controller/garbage_collector.go):

executor/pkg/config/config.go — Added GCConfig struct with Interval and MaxTTL fields.
executor/pkg/controller/taskaction_controller.go — Added ensureTerminalLabels() method that stamps terminal TaskActions with flyte.org/termination-status=terminated and flyte.org/completed-time=<UTC hour> labels. Called from both the terminal short-circuit path and after status updates that result in terminal state. Idempotent — no-op if labels already set.
executor/pkg/controller/garbage_collector.go (NEW) — GarbageCollector implementing manager.Runnable. Runs on a ticker, lists TaskActions with terminated label, filters by completed-time using lexicographic string comparison, deletes expired ones.
executor/setup.go — Conditionally adds GC to the controller manager when cfg.GC.Interval.Duration > 0
executor/pkg/config/config_flags.go / config_flags_test.go — Regenerated via go generate

How was this patch tested?

executor/pkg/controller/garbage_collector_test.go (NEW) — Ginkgo/envtest tests for: expired deletion, recent retention, non-terminated retention, empty list handling, and ensureTerminalLabels idempotency
executor/pkg/controller/taskaction_controller_test.go — Extended with test verifying GC labels are set when reconciling a terminal TaskAction
go build ./executor/... compiles
go test ./executor/pkg/config/... passes

Labels

added: New TaskAction garbage collector feature

Check all the applicable boxes

I updated the documentation accordingly.
All new and existing tests passed.
All commits are signed-off.

main
- Flyte 2 WIP #6583
  - [V2] Add TaskAction garbage collector for terminal CRDs #6994 👈

Copilot

Pull request overview

This PR introduces a TTL-based garbage collector for terminal TaskAction CRDs to prevent indefinite accumulation in the cluster, reducing etcd/storage bloat and improving list performance.

Changes:

Stamp terminal TaskActions with GC discovery labels (termination-status and completed-time) during reconcile.
Add a controller-runtime manager.Runnable garbage collector that periodically deletes expired terminal TaskActions.
Add GC config (interval + TTL) and wire the runnable into executor manager startup.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
manager/cmd/main.go	Initializes `SetupContext.Scope` for unified mode metrics.
gen/rust/Cargo.lock	Removes the Rust `Cargo.lock` from generated crate directory.
executor/setup.go	Conditionally registers the new GC runnable with the controller manager.
executor/pkg/controller/taskaction_controller.go	Adds terminal labeling logic (`ensureTerminalLabels`) and label constants.
executor/pkg/controller/garbage_collector.go	New GC runnable that lists terminated `TaskAction`s and deletes expired ones.
executor/pkg/controller/garbage_collector_test.go	New envtest coverage for GC behavior and `ensureTerminalLabels` idempotency.
executor/pkg/controller/taskaction_controller_test.go	Adds a reconcile test verifying GC labels are applied to terminal resources.
executor/pkg/config/config.go	Adds `GCConfig` and default GC settings.
executor/pkg/config/config_flags.go	Adds CLI flags for `gc.interval` and `gc.maxTTL` (generated).
executor/pkg/config/config_flags_test.go	Adds generated flag wiring tests for GC config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T19:04:37Z

executor/setup.go

+		if cfg.GC.Interval.Duration > 0 {
+			gc := controller.NewGarbageCollector(mgr.GetClient(), cfg.GC.Interval.Duration, cfg.GC.MaxTTL.Duration)
+			if err := mgr.Add(gc); err != nil {
+				return fmt.Errorf("executor: failed to add garbage collector: %w", err)
+			}
+		}


GC is enabled based solely on cfg.GC.Interval > 0. If cfg.GC.MaxTTL is set to 0 or a negative duration, collect() will treat the cutoff as now/future and may delete essentially all terminated TaskActions. Consider validating MaxTTL > 0 (and returning an error or disabling GC) before adding the runnable.

I think we can support immediate clean-up when set MaxTTL to <= 0? Should we?

yup, can be a follow-up

Copilot · 2026-03-09T19:04:38Z

executor/pkg/controller/taskaction_controller.go

+	labels := taskAction.GetLabels()
+	if labels != nil && labels[LabelTerminationStatus] == LabelValueTerminated {
+		return nil // already labeled
+	}


ensureTerminalLabels returns early when termination-status=terminated is present, but it doesn't verify that completed-time is set. If a TaskAction has the terminated label but is missing/empty completed-time (manual edits, partial migrations, etc.), it will never become GC-eligible. Consider checking both labels and patching any missing/empty one(s).

^ I think this is valid?

Copilot · 2026-03-09T19:04:38Z

executor/pkg/controller/taskaction_controller.go

+	labels[LabelTerminationStatus] = LabelValueTerminated
+	labels[LabelCompletedTime] = time.Now().UTC().Format(labelHourTimeFormat)
+	taskAction.SetLabels(labels)


LabelCompletedTime is described as “when the TaskAction became terminal”, but it’s set using time.Now() rather than the terminal condition’s LastTransitionTime. This means restarted controllers / pre-existing terminal TaskActions will get a much newer completed-time and be retained longer than the configured TTL. Consider deriving the label from the succeeded/failed condition transition time when available (falling back to time.Now() if missing).

executor/pkg/controller/garbage_collector.go

executor/pkg/controller/garbage_collector_test.go

executor/pkg/config/config.go

executor/pkg/controller/garbage_collector.go

Copilot · 2026-03-09T19:04:40Z

manager/cmd/main.go

+	// Initialize metrics scope
+	sc.Scope = promutils.NewScope("flyte")
+


sc.Scope is initialized here, but the storage DataStore is still constructed with promutils.NewTestScope(), so storage metrics won't be emitted under the configured scope in unified mode. Consider passing sc.Scope (or a child scope) into storage.NewDataStore for consistency with other services that use sc.Scope.

^ Should we fix this?

machichima · 2026-03-10T09:22:00Z

executor/pkg/controller/garbage_collector.go

+			client.Limit(gcPageSize),
+		}
+		if continueToken != "" {
+			listOpts = append(listOpts, client.Continue(continueToken))


Could we add the check for whether LabelCompletedTime exists into the listOpts?

Terminal TaskAction CRDs (Succeeded/Failed) remain in the cluster indefinitely, wasting etcd storage and slowing list operations. This adds a garbage collector that periodically deletes terminal TaskActions after a configurable TTL, modeled after the propeller FlyteWorkflow GC. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Kevin Su <pingsutw@apache.org>

Signed-off-by: Kevin Su <pingsutw@apache.org>

…e fields, regenerate pflags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Kevin Su <pingsutw@apache.org>

machichima

LGTM!

pingsutw self-assigned this Mar 9, 2026

github-actions bot mentioned this pull request Mar 9, 2026

Flyte 2 WIP #6583

Draft

3 tasks

pingsutw mentioned this pull request Mar 9, 2026

[V2] Add TaskAction garbage collector for terminal CRDs #6995

Closed

pingsutw requested review from Copilot and machichima March 9, 2026 18:58

Copilot started reviewing on behalf of pingsutw March 9, 2026 18:58 View session

pingsutw added the flyte2 label Mar 9, 2026

Copilot AI reviewed Mar 9, 2026

View reviewed changes

pingsutw linked an issue Mar 10, 2026 that may be closed by this pull request

[V2] Add TaskAction garbage collector for terminal CRDs #6995

Closed

machichima reviewed Mar 10, 2026

View reviewed changes

pingsutw mentioned this pull request Mar 10, 2026

Support immediate GC cleanup when MaxTTL <= 0 #7008

Open

pingsutw and others added 7 commits March 10, 2026 20:16

pagination

15fda70

Signed-off-by: Kevin Su <pingsutw@apache.org>

address comment

6227ec8

Signed-off-by: Kevin Su <pingsutw@apache.org>

nit

19af9c8

Signed-off-by: Kevin Su <pingsutw@apache.org>

min

67529fe

Signed-off-by: Kevin Su <pingsutw@apache.org>

test

1af5f3f

Signed-off-by: Kevin Su <pingsutw@apache.org>

fix: resolve rebase conflicts — remove duplicate TaskType/TaskTemplat…

5f86964

…e fields, regenerate pflags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Kevin Su <pingsutw@apache.org>

pingsutw force-pushed the gc branch from ab7fe23 to 5f86964 Compare March 11, 2026 03:20

machichima approved these changes Mar 11, 2026

View reviewed changes

pingsutw merged commit df8959c into v2 Mar 11, 2026
14 checks passed

pingsutw deleted the gc branch March 11, 2026 03:27

mvanhorn mentioned this pull request Mar 19, 2026

feat(executor): support immediate GC cleanup when MaxTTL <= 0 #7052

Open

		// Initialize metrics scope
		sc.Scope = promutils.NewScope("flyte")

Conversation

pingsutw commented Mar 9, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

Labels

Check all the applicable boxes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

machichima left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pingsutw commented Mar 9, 2026 •

edited by github-actions bot

Loading