fix: Background-propagation cleanup (#30) + configurable backoffLimit (#31)#33
Merged
Conversation
A finished Job has status.active=0, which is_job_running() read as "no job running", so every 5-minute requeue created a fresh restore Job — and the completion check ran only after the creation step, so the duplicate was already spawned by the time the status flipped to RestoreComplete. Terminally failed Jobs were re-created forever, and the one-shot KafkaBackup path had the identical defect. - Add jobs::job_state::classify_jobs(): aggregate the full set of Jobs for a CR into Succeeded/InProgress/Failed/NoJobs using Job conditions (Complete/Failed), so pod retries within backoffLimit stay InProgress and a pending Job with no status yet blocks duplicate creation. - Gate Job creation on that classification: restores never create a second Job; one-shot backups only re-run via the manual trigger annotation, which still never stacks onto an active Job. - Watch owned Jobs (.owns) in both controllers so status reflects completion/failure within seconds instead of the next requeue. - Report a terminal RestoreFailed condition for failed restore Jobs instead of silently re-creating them. - Make status patches idempotent (Job-derived timestamps, skip when the condition/lastBackup already reflects the outcome) so the new watch cannot churn lastTransitionTime in a reconcile loop. Fixes #29 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deleting a KafkaRestore (or KafkaBackup) removed its Jobs but left the Jobs' pods behind: the finalizer cleanup deleted Jobs with DeleteParams::default(), and the batch/v1 Job API's legacy server-side default deletion propagation is Orphan, so the garbage collector stripped the Job ownerReference from the pods instead of deleting them (kubectl masks this by always sending Background propagation). Cleanup now deletes Jobs, CronJobs, and ConfigMaps through a shared cleanup_delete_params() helper that requests Background propagation, so dependents are garbage collected with their parents. Fixes #30 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Jobs generated for KafkaRestore and KafkaBackup hardcoded backoffLimit: 3, so a failing restore made up to 4 pod attempts. Restores append to or purge target topics, which makes implicit retries of a partially completed attempt a data-duplication hazard — each run should be intentional. - Add spec.backoffLimit (minimum 0) to both CRDs, plumbed into the restore Job, one-shot backup Job, and scheduled CronJob template. - Restore Jobs now default to backoffLimit 0 (exactly one attempt); retries are opt-in. Backup Jobs keep the previous default of 3. Fixes #31 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sionsmith
changed the title
This was referenced Jun 12, 2026
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
1 participant
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.
Fixes #30 and #31. Stacked on #32 — merge that first; this PR then reduces to the two commits here (releases as 0.2.10).
#30 — orphaned pods after CR deletion
Root cause: finalizer cleanup deleted Jobs with
DeleteParams::default()— nopropagationPolicy. The batch/v1 Job API is a legacy API whose server-side default deletion propagation isOrphan, so the garbage collector strips the Job ownerReference from the pods instead of deleting them — exactly the reporter's "ownerReference exists then vanishes".kubectlmasks this by always sending Background; bare API clients hit the legacy default.Fix: all cleanup deletes (restore Jobs/ConfigMap, backup Jobs/CronJob/ConfigMap) go through a shared
cleanup_delete_params()helper requesting Background propagation.Verification: reproduced on kind with the released operator (pod ownerReference present before deletion, stripped after; Completed pod left behind). With the fix: deleting a KafkaRestore and a one-shot KafkaBackup removed Jobs and pods.
#31 — configurable backoffLimit, single-attempt restores by default
Reproduction: generated restore Job had hardcoded
backoffLimit: 3; one failing KafkaRestore produced 4 pod attempts. Restores append/purge target topics, so implicitly retrying a partially completed attempt can duplicate data.Fix: new
spec.backoffLimit(minimum 0) on bothKafkaRestoreandKafkaBackup, plumbed into the restore Job, one-shot backup Job, and scheduled CronJob template.Behavior change (deliberate): restore Jobs now default to
backoffLimit: 0— exactly one attempt per restore, retries opt-in — matching the reporter's intent ("I personally want to be intentional for each run"). Backup Jobs keep the previous default of 3 (backups are safe to retry).Verification: TDD (4 red tests against hardcoded behavior, then green; 37 integration tests total). E2E on kind: default restore → Job
backoffLimit=0, exactly 1 pod attempt, terminalRestoreFailed;spec.backoffLimit: 2→ exactly 3 attempts; no Job re-creation (#32 interplay) and no orphaned pods on deletion (#30 interplay). CRDs regenerated and helm chart copies synced.🤖 Generated with Claude Code