feat: add spec.backoffLimit; default restore jobs to a single attempt

sionsmith · claude · sionsmith · commit 89fed9a1301a · 2026-06-12T07:22:33.000+01:00
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>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file.
 
 ## 0.2.10 - 2026-06-12
 
+### Added
+
+- Add `spec.backoffLimit` to `KafkaRestore` and `KafkaBackup` to control pod retries on generated Jobs (including scheduled CronJob runs). Fixes [#31](https://github.com/osodevops/strimzi-backup-operator/issues/31).
+
+### Changed
+
+- Restore Jobs now default to `backoffLimit: 0` (a single attempt, previously 3): restores append to or purge target topics, so retrying a partially completed attempt can duplicate data. Set `spec.backoffLimit` explicitly to opt into retries. Backup Jobs keep the previous default of 3.
+
 ### Fixed
 
 - Delete Jobs, CronJobs, and ConfigMaps with explicit Background propagation when a `KafkaBackup`/`KafkaRestore` is deleted. The batch/v1 Job API's legacy default deletion propagation is `Orphan`, which stripped the Job ownerReference from its pods and left Completed pods behind. Fixes [#30](https://github.com/osodevops/strimzi-backup-operator/issues/30).
diff --git a/deploy/crds/kafkabackups.yaml b/deploy/crds/kafkabackups.yaml
@@ -96,6 +96,12 @@ spec:
                 required:
                 - type
                 type: object
+              backoffLimit:
+                description: Number of pod retries before a backup Job is marked failed (`spec.backoffLimit` on generated Jobs, including scheduled CronJob runs). Defaults to 3.
+                format: int32
+                minimum: 0.0
+                nullable: true
+                type: integer
               backup:
                 description: Backup options (compression, encryption, parallelism)
                 nullable: true
diff --git a/deploy/crds/kafkarestores.yaml b/deploy/crds/kafkarestores.yaml
@@ -93,6 +93,12 @@ spec:
                 required:
                 - type
                 type: object
+              backoffLimit:
+                description: 'Number of pod retries before the restore Job is marked failed (`spec.backoffLimit` on the generated Job). Defaults to 0 so a restore runs exactly once: restores append to or purge target topics, so a retry of a partially completed attempt can duplicate data and must be opted into deliberately.'
+                format: int32
+                minimum: 0.0
+                nullable: true
+                type: integer
               backupRef:
                 description: Reference to the source backup
                 properties:
diff --git a/deploy/helm/strimzi-backup-operator/crds/kafkabackups.yaml b/deploy/helm/strimzi-backup-operator/crds/kafkabackups.yaml
@@ -96,6 +96,12 @@ spec:
                 required:
                 - type
                 type: object
+              backoffLimit:
+                description: Number of pod retries before a backup Job is marked failed (`spec.backoffLimit` on generated Jobs, including scheduled CronJob runs). Defaults to 3.
+                format: int32
+                minimum: 0.0
+                nullable: true
+                type: integer
               backup:
                 description: Backup options (compression, encryption, parallelism)
                 nullable: true
diff --git a/deploy/helm/strimzi-backup-operator/crds/kafkarestores.yaml b/deploy/helm/strimzi-backup-operator/crds/kafkarestores.yaml
@@ -93,6 +93,12 @@ spec:
                 required:
                 - type
                 type: object
+              backoffLimit:
+                description: 'Number of pod retries before the restore Job is marked failed (`spec.backoffLimit` on the generated Job). Defaults to 0 so a restore runs exactly once: restores append to or purge target topics, so a retry of a partially completed attempt can duplicate data and must be opted into deliberately.'
+                format: int32
+                minimum: 0.0
+                nullable: true
+                type: integer
               backupRef:
                 description: Reference to the source backup
                 properties:
diff --git a/src/adapters/backup_config.rs b/src/adapters/backup_config.rs
@@ -507,6 +507,7 @@ mod tests {
             resources: None,
             template: None,
             image: None,
+            backoff_limit: None,
             consumer_groups: None,
         };
         let mut backup = KafkaBackup::new("test-backup", spec);
diff --git a/src/crd/kafka_backup.rs b/src/crd/kafka_backup.rs
@@ -89,6 +89,13 @@ pub struct KafkaBackupSpec {
     /// Container image for the backup job (default: osodevops/kafka-backup:v0.15.6)
     #[serde(skip_serializing_if = "Option::is_none")]
     pub image: Option<String>,
+
+    /// Number of pod retries before a backup Job is marked failed
+    /// (`spec.backoffLimit` on generated Jobs, including scheduled CronJob
+    /// runs). Defaults to 3.
+    #[schemars(range(min = 0))]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub backoff_limit: Option<i32>,
 }
 
 /// Backup-specific options
diff --git a/src/crd/kafka_restore.rs b/src/crd/kafka_restore.rs
@@ -85,6 +85,15 @@ pub struct KafkaRestoreSpec {
     /// Container image for the restore job (default: osodevops/kafka-backup:v0.15.6)
     #[serde(skip_serializing_if = "Option::is_none")]
     pub image: Option<String>,
+
+    /// Number of pod retries before the restore Job is marked failed
+    /// (`spec.backoffLimit` on the generated Job). Defaults to 0 so a restore
+    /// runs exactly once: restores append to or purge target topics, so a
+    /// retry of a partially completed attempt can duplicate data and must be
+    /// opted into deliberately.
+    #[schemars(range(min = 0))]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub backoff_limit: Option<i32>,
 }
 
 /// Reference to a KafkaBackup CR and optional specific backup snapshot
diff --git a/src/jobs/backup_job.rs b/src/jobs/backup_job.rs
@@ -99,7 +99,7 @@ pub fn build_backup_job(
             ..Default::default()
         },
         spec: Some(JobSpec {
-            backoff_limit: Some(3),
+            backoff_limit: Some(backup.spec.backoff_limit.unwrap_or(3)),
             template: PodTemplateSpec {
                 metadata: Some(ObjectMeta {
                     labels: Some(build_labels(&cr_name, &cluster.name, "backup")),
@@ -163,6 +163,7 @@ mod tests {
             resources: None,
             template: None,
             image: None,
+            backoff_limit: None,
         };
 
         let mut backup = KafkaBackup::new("test-backup", spec);
diff --git a/src/jobs/cronjob.rs b/src/jobs/cronjob.rs
@@ -108,7 +108,7 @@ pub fn build_backup_cronjob(
                     ..Default::default()
                 }),
                 spec: Some(JobSpec {
-                    backoff_limit: Some(3),
+                    backoff_limit: Some(backup.spec.backoff_limit.unwrap_or(3)),
                     template: PodTemplateSpec {
                         metadata: Some(ObjectMeta {
                             labels: Some(labels),
diff --git a/src/jobs/restore_job.rs b/src/jobs/restore_job.rs
@@ -103,7 +103,9 @@ pub fn build_restore_job(
             ..Default::default()
         },
         spec: Some(JobSpec {
-            backoff_limit: Some(3),
+            // Restores append/purge data: default to a single attempt so a
+            // partially completed restore is never re-applied implicitly.
+            backoff_limit: Some(restore.spec.backoff_limit.unwrap_or(0)),
             template: PodTemplateSpec {
                 metadata: Some(ObjectMeta {
                     labels: Some(build_labels(&cr_name, &cluster.name, "restore")),
diff --git a/tests/integration/backup_test.rs b/tests/integration/backup_test.rs
@@ -78,6 +78,7 @@ fn sample_backup() -> KafkaBackup {
         resources: None,
         template: None,
         image: None,
+        backoff_limit: None,
     };
     let mut backup = KafkaBackup::new("daily-backup", spec);
     backup.metadata.namespace = Some("kafka".to_string());
@@ -464,3 +465,61 @@ fn test_backup_job_falls_back_to_operator_service_account() {
         Some("strimzi-backup-operator")
     );
 }
+
+/// Backups are safe to retry; the historical default of 3 is kept.
+#[test]
+fn test_backup_job_default_backoff_limit_is_three() {
+    let backup = sample_backup();
+    let job = build_backup_job(
+        &backup,
+        "daily-backup-20260213-020000",
+        "daily-backup-config",
+        &sample_cluster(),
+        &ResolvedAuth::None,
+        None,
+    )
+    .unwrap();
+
+    assert_eq!(job.spec.as_ref().unwrap().backoff_limit, Some(3));
+}
+
+#[test]
+fn test_backup_job_custom_backoff_limit() {
+    let mut backup = sample_backup();
+    backup.spec.backoff_limit = Some(1);
+    let job = build_backup_job(
+        &backup,
+        "daily-backup-20260213-020000",
+        "daily-backup-config",
+        &sample_cluster(),
+        &ResolvedAuth::None,
+        None,
+    )
+    .unwrap();
+
+    assert_eq!(job.spec.as_ref().unwrap().backoff_limit, Some(1));
+}
+
+#[test]
+fn test_backup_cronjob_custom_backoff_limit() {
+    let mut backup = sample_backup();
+    backup.spec.backoff_limit = Some(0);
+    let cronjob = build_backup_cronjob(
+        &backup,
+        "daily-backup-config",
+        &sample_cluster(),
+        &ResolvedAuth::None,
+        None,
+    )
+    .unwrap();
+
+    let job_spec = cronjob
+        .spec
+        .as_ref()
+        .unwrap()
+        .job_template
+        .spec
+        .as_ref()
+        .unwrap();
+    assert_eq!(job_spec.backoff_limit, Some(0));
+}
diff --git a/tests/integration/reconcile_backup_test.rs b/tests/integration/reconcile_backup_test.rs
@@ -53,6 +53,7 @@ fn scheduled_backup(suspend: bool) -> KafkaBackup {
         resources: None,
         template: None,
         image: None,
+        backoff_limit: None,
     };
     let mut backup = KafkaBackup::new("daily-backup", spec);
     backup.metadata.namespace = Some("kafka".to_string());
diff --git a/tests/integration/restore_test.rs b/tests/integration/restore_test.rs
@@ -49,6 +49,7 @@ fn sample_backup() -> KafkaBackup {
         resources: None,
         template: None,
         image: None,
+        backoff_limit: None,
     };
     let mut backup = KafkaBackup::new("daily-backup", spec);
     backup.metadata.namespace = Some("kafka".to_string());
@@ -120,6 +121,7 @@ fn sample_restore() -> KafkaRestore {
         resources: None,
         template: None,
         image: None,
+        backoff_limit: None,
     };
     let mut restore = KafkaRestore::new("pitr-restore", spec);
     restore.metadata.namespace = Some("kafka".to_string());
@@ -392,3 +394,41 @@ fn test_restore_job_applies_template_service_account() {
         Some("restore-jobs")
     );
 }
+
+/// Issue #31: restores append to or purge target topics, so a retried Job pod
+/// re-applies a partially completed restore. The default must be exactly one
+/// attempt (backoffLimit 0); retries are opt-in via spec.backoffLimit.
+#[test]
+fn test_restore_job_default_backoff_limit_is_zero() {
+    let restore = sample_restore();
+    let job = build_restore_job(
+        &restore,
+        "pitr-restore-20260213-093000",
+        "pitr-restore-config",
+        &sample_cluster(),
+        &ResolvedAuth::None,
+        &sample_backup(),
+        None,
+    )
+    .unwrap();
+
+    assert_eq!(job.spec.as_ref().unwrap().backoff_limit, Some(0));
+}
+
+#[test]
+fn test_restore_job_custom_backoff_limit() {
+    let mut restore = sample_restore();
+    restore.spec.backoff_limit = Some(2);
+    let job = build_restore_job(
+        &restore,
+        "pitr-restore-20260213-093000",
+        "pitr-restore-config",
+        &sample_cluster(),
+        &ResolvedAuth::None,
+        &sample_backup(),
+        None,
+    )
+    .unwrap();
+
+    assert_eq!(job.spec.as_ref().unwrap().backoff_limit, Some(2));
+}
