✨ TTL-based garbage collection for finished runs (AgentRun + AgentWorkflowRun) - #202
✨ TTL-based garbage collection for finished runs (AgentRun + AgentWorkflowRun)#202ibolton336 wants to merge 3 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terminal AgentRuns and AgentWorkflowRuns accumulate indefinitely today — a busy namespace fills with Succeeded/Failed runs and their owned Sandboxes, pods, and per-run config. There is no built-in expiry. Add Job-style TTL garbage collection, mirroring spec.ttlSecondsAfterFinished: - New optional spec.ttlSecondsAfterFinished on AgentRun and AgentWorkflowRun. When set, the controller deletes the run this many seconds after it reaches a terminal phase (0 = delete immediately); when unset, the run is kept unless a controller default applies. - New --agentrun-ttl flag sets a cluster-wide default lifetime applied when a run's spec does not; a run's own spec always overrides it. - The terminal short-circuit in each reconciler now schedules GC: resolve the effective TTL, anchor on CompletionTime (stamping it for terminal runs that never recorded one, e.g. validation failures), requeue for the remaining time, then delete once elapsed. Deletion cascades to owned children (Sandbox/pod/Secret; child AgentRuns for a workflow run) via existing owner references. - Extract an isTerminalPhase helper and reuse it. Covered by envtest cases: a short-TTL run is garbage-collected; a no-TTL run is kept. Closes konveyor#198 Signed-off-by: ibolton336 <ibolton@redhat.com>
The ttl-gc envtest specs asserted the transient terminal state (Phase=Failed + CompletionTime) in a first Eventually before checking for deletion. With TTL=1s the run is garbage-collected before that intermediate state can be observed on a slower CI runner, so the Get returned NotFound and the first Eventually timed out. Drop the fragile intermediate assertion in both the AgentRun and AgentWorkflowRun specs and keep only the deletion check. Eventual deletion can only happen once the run has gone terminal and been anchored, so the GC assertion still exercises the full path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: ibolton336 <ibolton@redhat.com>
104fd7e to
bae4107
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: ibolton336 <ibolton@redhat.com>
djzager
left a comment
djzager
left a comment
There was a problem hiding this comment.
Approving — solid, well-documented, and covered by envtest. Two low-severity, non-blocking notes below.
| (Succeeded or Failed), the controller deletes it that many seconds after | ||
| its completion time, mirroring Kubernetes Job TTL. Owned Sandboxes and | ||
| other child objects are removed by the existing owner-reference cascade. | ||
| Unset or negative disables TTL, so existing runs are never reaped |
There was a problem hiding this comment.
The changelog says "negative disables TTL", but the field is marked +kubebuilder:validation:Minimum=0, so the API rejects negative values — that path is unreachable. Suggest dropping "or negative" so the changelog matches the validation (unset disables; zero deletes immediately).
| flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") | ||
| flag.BoolVar(&enableHTTP2, "enable-http2", false, | ||
| "If set, HTTP/2 will be enabled for the metrics and webhook servers") | ||
| flag.DurationVar(&agentRunTTL, "agentrun-ttl", 0, |
There was a problem hiding this comment.
Worth confirming/documenting: enabling --agentrun-ttl applies to already-finished runs too. Their CompletionTime is in the past, so anything older than the TTL gets deleted on the first reconcile after the flag is turned on — a one-time bulk GC on rollout. If that's intended (I think it is), a note in the flag help or PR body would save an operator a surprise.
2 participants
What
Adds Job-style TTL garbage collection for finished
AgentRuns andAgentWorkflowRuns, so terminal runs no longer accumulate forever in a namespace (clutteringkubectl, the console, and etcd, and leaving owned Sandboxes/pods behind).Closes #198.
Changes
spec.ttlSecondsAfterFinished *int32onAgentRunandAgentWorkflowRun, mirroringJob.spec.ttlSecondsAfterFinished(0 = delete as soon as the run finishes; unset = keep unless a controller default applies).--agentrun-ttlsets a cluster-wide default lifetime applied when a run's spec does not set one; a run's ownspec.ttlSecondsAfterFinishedalways overrides it.0(default) disables the default, preserving today's keep-forever behaviour.reconcileTTL): resolve the effective TTL, anchor onCompletionTime(stamping it for terminal runs that never recorded one, e.g. a validation failure before a Sandbox existed), requeue for the remaining time, thenDeleteonce elapsed. Deletion cascades to owned children (Sandbox/pod/Secret for a run; childAgentRuns for a workflow run) via the existing owner references.isTerminalPhasehelper and reused it (replacing three inline copies).Testing
envtest cases (real apiserver + running manager):
Full
internal/controllersuite passes;go build ./...,go vet, andgofmtclean.Notes / follow-ups
Part of the stale/broken run cleanup thread (#201).