You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: make StoreExec and StoreDebugInstance cleanup configurable per CR - #261
CR cleanup was driven by a single operator-wide env var (SUCCESSFUL_CR_CLEANUP_GRACE_PERIOD, default 1h) shared by StoreExec and StoreDebugInstance, and on the exec side it only ever applied to executions that finished successfully. That has three consequences:
Every store in the cluster gets the same retention. A customer who wants failed migrations kept around for debugging and successful ones gone immediately has no way to say so.
StoreExecs in error are never collected at all, so they accumulate in the namespace forever.
The only opt-out was global. Setting successfulCRCleanupGracePeriod: "0" to keep debug CRs around also disabled StoreExec cleanup, and vice versa.
The fallback in storeExecFinishedAt also hid the first problem: if no condition carried a timestamp it returned time.Now() (or the CR creation time), so the delete window was computed from an arbitrary point rather than from when the execution actually finished.
Change
Retention is configured per CR, not per operator.SUCCESSFUL_CR_CLEANUP_GRACE_PERIOD is removed from internal/config, from both reconcilers, from cmd/main.go, and successfulCRCleanupGracePeriod is removed from the Helm chart. There is no operator-wide replacement.
StoreExec
StoreExecSpec gains two fields:
cleanupPeriodSuccessfulExec (default 5m)
cleanupPeriodErrorExec (default 1h)
reconcileSuccessfulStoreExecCleanup becomes reconcileStoreExecCleanup and picks the period from the CR's own state via cleanupPeriodFor: done → successful period, error → error period, anything else → no cleanup. A period of 0 disables cleanup for that case, so the old opt-out still exists, now per execution. Cron StoreExecs and CRs already being deleted are skipped as before.
Failed executions are collected too.error is now a terminal state for cleanup purposes, with its own (longer) default so there is time to look at it before it disappears.
No more guessed finish time.storeExecFinishedAt returns (time.Time, bool) and only answers for a terminal state, using the newest condition that actually has a LastTransitionTime. If nothing usable is recorded, cleanup does not run instead of deleting against a made-up timestamp.
StoreDebugInstance
spec.duration now drives both expiry and cleanup, replacing the removed grace period. spec.duration: 0 disables both, matching the 0 semantics of the two StoreExec fields.
spec.duration also changes from string to metav1.Duration. The CRD schema is unchanged (type: string, default: 1h) and the accepted grammar is identical, because metav1.Duration.UnmarshalJSON uses the same time.ParseDuration the four call sites called explicitly before. Those four manual parses, and the // validate duration block in Reconcile, are gone.
Duration validation
All three duration fields carry a CEL rule so the API server rejects malformed values at admission:
x-kubernetes-validations:
- message: must be a valid duration, e.g. 30s, 5m or 1hrule: self.matches('^(0|([0-9]+([.][0-9]+)?(ns|us|ms|s|m|h))+)$')
This is not cosmetic. metav1.Duration fails at JSON decode time, while the API server only validates type: string — so a single object with duration: "nonsense" is accepted into etcd and then breaks the typed client's LIST/WATCH for the whole type. Measured in a k3d cluster (see below): one such object took the entire StoreDebugInstance controller offline. +kubebuilder:validation:Pattern cannot be used here, controller-gen rejects it on non-string Go types.
Behaviour changes
before
after
StoreExec done retention
1h, operator-wide
spec.cleanupPeriodSuccessfulExec, default 5m
StoreExec error retention
never collected
spec.cleanupPeriodErrorExec, default 1h
StoreDebugInstance deletion
creation + duration + grace
creation + duration
StoreDebugInstance duration: 0
expires immediately, CR deleted after grace
never expires, CR never collected
invalid duration string
silently treated as 0
rejected by the API server
Two consequences worth calling out explicitly:
A completed debug CR is visible in done for about one requeue interval (10s) instead of the previous grace period.duration is the lifetime the caller asked for, and the CR no longer outliving it is the intended reading.
The combination "let the debug workload expire but retain its completed CR" is gone. It was only reachable through successfulCRCleanupGracePeriod: "0", which also disabled StoreExec cleanup. Removing that single global switch is the point of this PR.
CronJob templates lose TTL, retaining completed Jobs and pods indefinitely
internal/job/command.go:143
Removing the TTL from getJobSpec also removes it from CommandCronJob's JobTemplate, because that constructor uses this same helper. Cron-generated Jobs are owned by the CronJob rather than following the one-shot StoreExec lifecycle, and cron StoreExecs are explicitly excluded from the new cleanup path; suspended or no-longer-scheduled cron executions can therefore retain their completed Job and pods indefinitely instead of the previous 24 hours. Keep the TTL on the CronJob template while omitting it only for CommandJob.
Picking up the "previously missed" finding from the latest overview — CronJob templates lose TTL — it is correct and fixed in 3129184.
getJobSpec is shared by CommandJob and CommandCronJob, so dropping TTLSecondsAfterFinished took it off the cron job template too. Those jobs belong to the CronJob rather than to a one-shot StoreExec, and cron StoreExecs are excluded from the cleanup path, so nothing would have collected them. CommandCronJob now sets the TTL back on its own template; CommandJob keeps none.
One qualifier on the description: growth is not unbounded, since the CronJob history limits (defaults 3 successful / 1 failed) still apply. The real regression is a suspended or unscheduled cron — with no further runs to push them out, it would have kept those last jobs and their pods for good instead of 24h.
Covered by two tests now: the one-shot job carries no TTL, the cron template carries 86400.
Tim Lange (drzombey)
changed the title
fix: tie the command job lifetime to its StoreExec
fix: start a command job exactly once per StoreExec
Sep 29, 2026
Tim Lange (drzombey)
changed the title
fix: start a command job exactly once per StoreExec
feat: make StoreExec cleanup configurable per CR
Oct 5, 2026
StoreExec cleanup was driven by a single operator-wide grace period and
only applied to successful executions, so every store shared one retention
and failed executions were never collected.
StoreExecSpec gains cleanupPeriodSuccessfulExec (default 5m) and
cleanupPeriodErrorExec (default 1h). reconcileStoreExecCleanup picks the
period from the CR's own state; a period of zero disables cleanup for that
state, which keeps the previous opt-out per execution.
Both fields are pointers: omitempty does not drop a zero metav1.Duration,
so a value field would send an explicit "0s" from typed Go clients and
bypass the CRD defaults. An unset field falls back to the same default in
code, which also covers an older CRD that dropped the field.
storeExecFinishedAt no longer guesses. It answers only for a terminal state
and only from a condition that carries a LastTransitionTime, instead of
falling back to the creation time or time.Now().
CleanupGracePeriod is dropped from StoreExecReconciler; StoreDebugInstance
keeps using it.
Tim Lange (drzombey)
changed the title
feat: make StoreExec cleanup configurable per CR
feat: make StoreExec and StoreDebugInstance cleanup configurable per CR
Oct 7, 2026
Zero duration incorrectly publishes an expired expiry label
internal/util/labels.go:58
A zero duration now means the debug instance never expires, but this still publishes its creation time as store.debug.validUntil, making the resource appear already expired to label consumers. Omit the expiry label when duration is zero so the generated Pod and Service labels reflect the new semantics.
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
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.
Problem
CR cleanup was driven by a single operator-wide env var (
SUCCESSFUL_CR_CLEANUP_GRACE_PERIOD, default 1h) shared byStoreExecandStoreDebugInstance, and on the exec side it only ever applied to executions that finished successfully. That has three consequences:errorare never collected at all, so they accumulate in the namespace forever.successfulCRCleanupGracePeriod: "0"to keep debug CRs around also disabled StoreExec cleanup, and vice versa.The fallback in
storeExecFinishedAtalso hid the first problem: if no condition carried a timestamp it returnedtime.Now()(or the CR creation time), so the delete window was computed from an arbitrary point rather than from when the execution actually finished.Change
Retention is configured per CR, not per operator.
SUCCESSFUL_CR_CLEANUP_GRACE_PERIODis removed frominternal/config, from both reconcilers, fromcmd/main.go, andsuccessfulCRCleanupGracePeriodis removed from the Helm chart. There is no operator-wide replacement.StoreExec
StoreExecSpecgains two fields:cleanupPeriodSuccessfulExec(default5m)cleanupPeriodErrorExec(default1h)reconcileSuccessfulStoreExecCleanupbecomesreconcileStoreExecCleanupand picks the period from the CR's own state viacleanupPeriodFor:done→ successful period,error→ error period, anything else → no cleanup. A period of0disables cleanup for that case, so the old opt-out still exists, now per execution. Cron StoreExecs and CRs already being deleted are skipped as before.Failed executions are collected too.
erroris now a terminal state for cleanup purposes, with its own (longer) default so there is time to look at it before it disappears.No more guessed finish time.
storeExecFinishedAtreturns(time.Time, bool)and only answers for a terminal state, using the newest condition that actually has aLastTransitionTime. If nothing usable is recorded, cleanup does not run instead of deleting against a made-up timestamp.StoreDebugInstance
spec.durationnow drives both expiry and cleanup, replacing the removed grace period.spec.duration: 0disables both, matching the0semantics of the two StoreExec fields.spec.durationalso changes fromstringtometav1.Duration. The CRD schema is unchanged (type: string,default: 1h) and the accepted grammar is identical, becausemetav1.Duration.UnmarshalJSONuses the sametime.ParseDurationthe four call sites called explicitly before. Those four manual parses, and the// validate durationblock inReconcile, are gone.Duration validation
All three duration fields carry a CEL rule so the API server rejects malformed values at admission:
This is not cosmetic.
metav1.Durationfails at JSON decode time, while the API server only validatestype: string— so a single object withduration: "nonsense"is accepted into etcd and then breaks the typed client's LIST/WATCH for the whole type. Measured in a k3d cluster (see below): one such object took the entire StoreDebugInstance controller offline.+kubebuilder:validation:Patterncannot be used here, controller-gen rejects it on non-stringGo types.Behaviour changes
doneretentionspec.cleanupPeriodSuccessfulExec, default5merrorretentionspec.cleanupPeriodErrorExec, default1hcreation + duration + gracecreation + durationduration: 00Two consequences worth calling out explicitly:
donefor about one requeue interval (10s) instead of the previous grace period.durationis the lifetime the caller asked for, and the CR no longer outliving it is the intended reading.successfulCRCleanupGracePeriod: "0", which also disabled StoreExec cleanup. Removing that single global switch is the point of this PR.