From ccac0e17e9cccdde4492c8331316e79f17863565 Mon Sep 17 00:00:00 2001 From: Alexey Artamonov Date: Wed, 4 Nov 2026 14:29:44 +0300 Subject: [PATCH 1/5] fix(mariadb): fail backup and restore jobs whose operator Job failed mariadb-operator reports a failed backing Job as Complete=True with reason JobFailed, and Complete=True alone was read as success. A backup whose dump or upload failed therefore ended Succeeded and left a Ready Backup pointing at an archive that does not exist, and a scheduled Plan kept producing such Backups; a failed restore ended Succeeded too. Success now requires reason JobComplete, and JobFailed fails the job whatever the status. Signed-off-by: Alexey Artamonov Assisted-by: LLM --- .../mariadbstrategy_controller.go | 43 ++--- .../mariadbstrategy_controller_test.go | 181 ++++++++++++++++++ .../backupcontroller/mariadbtypes/types.go | 12 +- 3 files changed, 207 insertions(+), 29 deletions(-) diff --git a/internal/backupcontroller/mariadbstrategy_controller.go b/internal/backupcontroller/mariadbstrategy_controller.go index a5bd77094..48abd3644 100644 --- a/internal/backupcontroller/mariadbstrategy_controller.go +++ b/internal/backupcontroller/mariadbstrategy_controller.go @@ -204,7 +204,10 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 cond := apimeta.FindStatusCondition(mdbBackup.Status.Conditions, mariadbtypes.ConditionTypeComplete) switch { - case cond != nil && cond.Status == metav1.ConditionTrue: + case cond != nil && cond.Reason == mariadbtypes.ConditionReasonJobFailed: + return r.markBackupJobFailed(ctx, j, mariadbJobFailedMessage("Backup", mdbBackup.Namespace, mdbBackup.Name, cond)) + + case cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == mariadbtypes.ConditionReasonJobComplete: // Backup completed successfully. Materialise the Cozystack artifact // and finalise the BackupJob. Idempotent: if the artifact already // exists (a previous reconcile created it and then raced on the @@ -231,22 +234,12 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 } return ctrl.Result{}, nil - case cond != nil && cond.Status == metav1.ConditionFalse && cond.Reason == mariadbtypes.ConditionReasonJobFailed: - // Terminal failure. The operator marks Complete=False/JobFailed only - // after the underlying Job exhausts its backoff budget, so we can - // fail the BackupJob immediately rather than waiting for the - // driver-side deadline. - message := cond.Message - if message == "" { - message = "k8s.mariadb.com Backup terminal failure" - } - return r.markBackupJobFailed(ctx, j, message) - default: // Still running (Complete=False/JobRunning, Complete=False/JobNotComplete, - // JobSuspended, or no Complete condition yet). Apply a wall-clock - // deadline so a permanently-stuck Backup eventually fails the - // BackupJob instead of pinning it Running forever. + // JobSuspended, a reason this driver does not know, or no Complete + // condition yet). Apply a wall-clock deadline so a permanently-stuck + // Backup eventually fails the BackupJob instead of pinning it Running + // forever. if mariadbBackupDeadlineExceeded(j.Status.StartedAt) { detail := "no Complete condition observed" if cond != nil { @@ -259,6 +252,14 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 } } +// mariadbJobFailedMessage names the operator object whose Job failed. The +// operator's own condition message is only "Failed"; the cause is in the +// logs of that object's Job. +func mariadbJobFailedMessage(kind, namespace, name string, cond *metav1.Condition) string { + return fmt.Sprintf("k8s.mariadb.com/%s %s/%s failed (Complete=%s/%s); the cause is in the logs of its Job", + kind, namespace, name, cond.Status, cond.Reason) +} + // mariadbBackupDeadlineExceeded reports whether enough wall-clock time has // elapsed since the BackupJob started that we should give up on a stuck // operator Backup. Returns false when StartedAt is nil so the very first @@ -570,7 +571,10 @@ func (r *RestoreJobReconciler) reconcileMariaDBRestore(ctx context.Context, rest cond := apimeta.FindStatusCondition(mdbRestore.Status.Conditions, mariadbtypes.ConditionTypeComplete) switch { - case cond != nil && cond.Status == metav1.ConditionTrue: + case cond != nil && cond.Reason == mariadbtypes.ConditionReasonJobFailed: + return r.markRestoreJobFailed(ctx, restoreJob, mariadbJobFailedMessage("Restore", mdbRestore.Namespace, mdbRestore.Name, cond)) + + case cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == mariadbtypes.ConditionReasonJobComplete: now := metav1.Now() restoreJob.Status.CompletedAt = &now restoreJob.Status.Phase = backupsv1alpha1.RestoreJobPhaseSucceeded @@ -585,13 +589,6 @@ func (r *RestoreJobReconciler) reconcileMariaDBRestore(ctx context.Context, rest } return ctrl.Result{}, nil - case cond != nil && cond.Status == metav1.ConditionFalse && cond.Reason == mariadbtypes.ConditionReasonJobFailed: - message := cond.Message - if message == "" { - message = fmt.Sprintf("k8s.mariadb.com/Restore %s/%s terminal failure", mdbRestore.Namespace, mdbRestore.Name) - } - return r.markRestoreJobFailed(ctx, restoreJob, message) - default: deadline := options.effectiveRestoreDeadline() if restoreJob.Status.StartedAt != nil && time.Since(restoreJob.Status.StartedAt.Time) > deadline { diff --git a/internal/backupcontroller/mariadbstrategy_controller_test.go b/internal/backupcontroller/mariadbstrategy_controller_test.go index 0e4f63b5e..307301223 100644 --- a/internal/backupcontroller/mariadbstrategy_controller_test.go +++ b/internal/backupcontroller/mariadbstrategy_controller_test.go @@ -867,6 +867,187 @@ func TestReconcileMariaDBRestore_KindCheckedBeforeSrcBackupGet(t *testing.T) { } } +// The operator reports a failed backing Job as Complete=True with reason +// JobFailed (SetCompleteWithJob in mariadb-operator 25.10.2), so the +// Complete status alone does not tell success from failure. +var mariadbCompleteOutcomeCases = []struct { + name string + status metav1.ConditionStatus + reason string + wantFailed bool + wantDone bool +}{ + {name: "Complete=True/JobComplete succeeds", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobComplete, wantDone: true}, + {name: "Complete=True/JobFailed fails", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, wantFailed: true}, + {name: "Complete=False/JobFailed fails", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobFailed, wantFailed: true}, + {name: "Complete=True with an unknown reason keeps waiting", status: metav1.ConditionTrue, reason: "SomethingElse"}, + {name: "Complete=False/JobComplete keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobComplete}, + {name: "Complete=False/JobRunning keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobRunning}, +} + +func mariadbCompleteCondition(status metav1.ConditionStatus, reason string) []metav1.Condition { + return []metav1.Condition{{ + Type: mariadbtypes.ConditionTypeComplete, + Status: status, + Reason: reason, + Message: "Failed", + LastTransitionTime: metav1.Now(), + }} +} + +func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { + strategyAPIGroup := strategyv1alpha1.GroupVersion.Group + resolved := &ResolvedBackupConfig{ + StrategyRef: corev1.TypedLocalObjectReference{ + APIGroup: &strategyAPIGroup, + Kind: strategyv1alpha1.MariaDBStrategyKind, + Name: "mdb-strategy", + }, + } + + for _, tc := range mariadbCompleteOutcomeCases { + t.Run(tc.name, func(t *testing.T) { + now := metav1.Now() + seed := newMariaDBBackupJob("bj", "tenant") + seed.Status.StartedAt = &now + strategy := &strategyv1alpha1.MariaDB{ + ObjectMeta: metav1.ObjectMeta{Name: "mdb-strategy"}, + Spec: strategyv1alpha1.MariaDBSpec{Template: *newRenderedMariaDBTemplate()}, + } + operatorMariaDB := &mariadbtypes.MariaDB{ + ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: mariadbNameForApp(seed.Spec.ApplicationRef.Name)}, + } + operatorBackup := &mariadbtypes.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant", + Name: "bj-op", + Labels: map[string]string{ + backupsv1alpha1.OwningJobNameLabel: seed.Name, + backupsv1alpha1.OwningJobNamespaceLabel: seed.Namespace, + }, + }, + Status: mariadbtypes.BackupStatus{Conditions: mariadbCompleteCondition(tc.status, tc.reason)}, + } + c := newMariaDBStrategyTestClient(t, seed, newMariaDBApp(seed.Spec.ApplicationRef.Name, "tenant"), + strategy, operatorMariaDB, operatorBackup) + r := &BackupJobReconciler{Client: c, Scheme: c.Scheme(), Recorder: record.NewFakeRecorder(10)} + + bj := &backupsv1alpha1.BackupJob{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(seed), bj); err != nil { + t.Fatalf("seed Get: %v", err) + } + res, err := r.reconcileMariaDB(context.Background(), bj, resolved) + if err != nil { + t.Fatalf("reconcileMariaDB: %v", err) + } + + artifacts := &backupsv1alpha1.BackupList{} + if err := c.List(context.Background(), artifacts, client.InNamespace("tenant")); err != nil { + t.Fatalf("list Backups: %v", err) + } + switch { + case tc.wantFailed: + if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseFailed { + t.Errorf("expected Phase=Failed, got %q", bj.Status.Phase) + } + if !strings.Contains(bj.Status.Message, "bj-op") { + t.Errorf("failure message must name the operator Backup, got %q", bj.Status.Message) + } + if bj.Status.BackupRef != nil || len(artifacts.Items) != 0 { + t.Errorf("a failed run must not produce a Backup: ref=%v artifacts=%d", bj.Status.BackupRef, len(artifacts.Items)) + } + case tc.wantDone: + if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseSucceeded { + t.Errorf("expected Phase=Succeeded, got %q (message %q)", bj.Status.Phase, bj.Status.Message) + } + if bj.Status.BackupRef == nil || len(artifacts.Items) != 1 { + t.Errorf("a successful run must produce exactly one Backup: ref=%v artifacts=%d", bj.Status.BackupRef, len(artifacts.Items)) + } + default: + if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseRunning { + t.Errorf("expected Phase=Running, got %q", bj.Status.Phase) + } + if res.RequeueAfter == 0 { + t.Errorf("expected a requeue while the run is in progress, got %+v", res) + } + if len(artifacts.Items) != 0 { + t.Errorf("an unfinished run must not produce a Backup, got %d", len(artifacts.Items)) + } + } + }) + } +} + +func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { + apps := mariadbapp.GroupName + + for _, tc := range mariadbCompleteOutcomeCases { + t.Run(tc.name, func(t *testing.T) { + now := metav1.Now() + cozyBackup := &backupsv1alpha1.Backup{ + ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "src-cozy-bk"}, + Spec: backupsv1alpha1.BackupSpec{ + ApplicationRef: corev1.TypedLocalObjectReference{Kind: "MariaDB", Name: "src", APIGroup: &apps}, + DriverMetadata: map[string]string{mariadbBackupNameKey: "src-op-bk"}, + }, + } + seed := &backupsv1alpha1.RestoreJob{ + ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "rj"}, + Spec: backupsv1alpha1.RestoreJobSpec{ + BackupRef: corev1.LocalObjectReference{Name: cozyBackup.Name}, + TargetApplicationRef: &corev1.TypedLocalObjectReference{Kind: "MariaDB", Name: "target", APIGroup: &apps}, + }, + Status: backupsv1alpha1.RestoreJobStatus{StartedAt: &now, Phase: backupsv1alpha1.RestoreJobPhaseRunning}, + } + operatorRestore := &mariadbtypes.Restore{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tenant", + Name: "rj-op", + Labels: map[string]string{ + backupsv1alpha1.OwningJobNameLabel: seed.Name, + backupsv1alpha1.OwningJobNamespaceLabel: seed.Namespace, + }, + }, + Status: mariadbtypes.RestoreStatus{Conditions: mariadbCompleteCondition(tc.status, tc.reason)}, + } + c := newMariaDBStrategyTestClient(t, cozyBackup, seed, operatorRestore, + &mariadbtypes.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "src-op-bk"}}, + &mariadbtypes.MariaDB{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: mariadbNameForApp("target")}}) + r := &RestoreJobReconciler{Client: c, Recorder: record.NewFakeRecorder(10)} + + rj := &backupsv1alpha1.RestoreJob{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(seed), rj); err != nil { + t.Fatalf("seed Get: %v", err) + } + res, err := r.reconcileMariaDBRestore(context.Background(), rj, cozyBackup) + if err != nil { + t.Fatalf("reconcileMariaDBRestore: %v", err) + } + + switch { + case tc.wantFailed: + if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseFailed { + t.Errorf("expected Phase=Failed, got %q", rj.Status.Phase) + } + if !strings.Contains(rj.Status.Message, "rj-op") { + t.Errorf("failure message must name the operator Restore, got %q", rj.Status.Message) + } + case tc.wantDone: + if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseSucceeded { + t.Errorf("expected Phase=Succeeded, got %q (message %q)", rj.Status.Phase, rj.Status.Message) + } + default: + if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseRunning { + t.Errorf("expected Phase=Running, got %q", rj.Status.Phase) + } + if res.RequeueAfter == 0 { + t.Errorf("expected a requeue while the run is in progress, got %+v", res) + } + } + }) + } +} + // --------------------------------------------------------------------------- // Test helpers // --------------------------------------------------------------------------- diff --git a/internal/backupcontroller/mariadbtypes/types.go b/internal/backupcontroller/mariadbtypes/types.go index 7bc5b03a1..d9c09f30a 100644 --- a/internal/backupcontroller/mariadbtypes/types.go +++ b/internal/backupcontroller/mariadbtypes/types.go @@ -27,14 +27,14 @@ const ( GroupName = "k8s.mariadb.com" Version = "v1alpha1" - // Operator-side condition type set on Backup and Restore once the run - // terminates. Status=True means success; Status=False with Reason - // explains the failure or in-progress state. + // Operator-side condition type set on Backup and Restore. Status=True + // means the backing Job terminated, successfully or not; only the + // Reason tells the two apart (SetCompleteWithJob in mariadb-operator). ConditionTypeComplete = "Complete" // Reason values the upstream operator emits on the Complete condition. // JobComplete -> Status=True (run succeeded). - // JobFailed -> Status=False (terminal failure of the backing Job). + // JobFailed -> Status=True (terminal failure of the backing Job). // JobRunning -> Status=False (still in progress). // JobSuspended -> Status=False (admin-suspended via spec). ConditionReasonJobComplete = "JobComplete" @@ -183,7 +183,7 @@ type S3TLS struct { } // BackupStatus mirrors Backup.status. The operator expresses success -// via condition type "Complete" with status=True. +// via condition type "Complete" with reason JobComplete. type BackupStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } @@ -217,7 +217,7 @@ type RestoreSpec struct { } // RestoreStatus mirrors Restore.status. Same condition convention as -// Backup: type=Complete, Status=True on success. +// Backup: type=Complete, reason JobComplete on success. type RestoreStatus struct { Conditions []metav1.Condition `json:"conditions,omitempty"` } From 7e1fc7173c628fdcbf5a1f1c810e0eb4edf92d28 Mon Sep 17 00:00:00 2001 From: Alexey Artamonov Date: Thu, 5 Nov 2026 21:03:14 +0300 Subject: [PATCH 2/5] test(mariadb): cover the absent Complete condition and expired deadlines Every first reconcile after the operator Backup or Restore is created sees no Complete condition, and the guards that keep that case from dereferencing nil could be dropped with the outcome tables staying green. The tables now also cover the deadline failure for a run that never reports a known reason, and read the job status back from the API, so a lost status write fails them instead of passing on the in-memory object. Assisted-by: LLM Signed-off-by: Alexey Artamonov --- .../mariadbstrategy_controller_test.go | 122 ++++++++++++------ 1 file changed, 79 insertions(+), 43 deletions(-) diff --git a/internal/backupcontroller/mariadbstrategy_controller_test.go b/internal/backupcontroller/mariadbstrategy_controller_test.go index 307301223..0c706574c 100644 --- a/internal/backupcontroller/mariadbstrategy_controller_test.go +++ b/internal/backupcontroller/mariadbstrategy_controller_test.go @@ -871,30 +871,64 @@ func TestReconcileMariaDBRestore_KindCheckedBeforeSrcBackupGet(t *testing.T) { // JobFailed (SetCompleteWithJob in mariadb-operator 25.10.2), so the // Complete status alone does not tell success from failure. var mariadbCompleteOutcomeCases = []struct { - name string - status metav1.ConditionStatus - reason string - wantFailed bool - wantDone bool + name string + noCondition bool + status metav1.ConditionStatus + reason string + message string + expired bool + wantFailed bool + wantDone bool + wantMessage []string }{ - {name: "Complete=True/JobComplete succeeds", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobComplete, wantDone: true}, - {name: "Complete=True/JobFailed fails", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, wantFailed: true}, - {name: "Complete=False/JobFailed fails", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobFailed, wantFailed: true}, - {name: "Complete=True with an unknown reason keeps waiting", status: metav1.ConditionTrue, reason: "SomethingElse"}, - {name: "Complete=False/JobComplete keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobComplete}, - {name: "Complete=False/JobRunning keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobRunning}, + {name: "Complete=True/JobComplete succeeds", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobComplete, message: "Success", wantDone: true}, + {name: "Complete=True/JobFailed fails", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", wantFailed: true, + wantMessage: []string{"tenant/op"}}, + {name: "Complete=False/JobFailed fails", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", wantFailed: true, + wantMessage: []string{"tenant/op"}}, + {name: "Complete=True with an unknown reason keeps waiting", status: metav1.ConditionTrue, reason: "SomethingElse", message: "Something"}, + {name: "Complete=False/JobComplete keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobComplete, message: "Success"}, + {name: "Complete=False/JobRunning keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobRunning, message: "Running"}, + {name: "no Complete condition keeps waiting", noCondition: true}, + {name: "no Complete condition past the deadline fails", noCondition: true, expired: true, wantFailed: true, + wantMessage: []string{"no Complete condition observed"}}, + {name: "Complete=True with an unknown reason past the deadline fails", status: metav1.ConditionTrue, reason: "SomethingElse", message: "Something", expired: true, wantFailed: true, + wantMessage: []string{"Complete=True/SomethingElse"}}, } -func mariadbCompleteCondition(status metav1.ConditionStatus, reason string) []metav1.Condition { +func mariadbCompleteConditions(noCondition bool, status metav1.ConditionStatus, reason, message string) []metav1.Condition { + if noCondition { + return nil + } return []metav1.Condition{{ Type: mariadbtypes.ConditionTypeComplete, Status: status, Reason: reason, - Message: "Failed", + Message: message, LastTransitionTime: metav1.Now(), }} } +// mariadbOutcomeStartedAt returns a StartedAt that is fresh, or older than +// both the backup and the default restore deadline. +func mariadbOutcomeStartedAt(expired bool) *metav1.Time { + if !expired { + now := metav1.Now() + return &now + } + past := metav1.NewTime(time.Now().Add(-2 * max(mariadbDefaultBackupDeadline, mariadbDefaultRestoreDeadline))) + return &past +} + +func assertMariaDBFailureMessage(t *testing.T, message string, want []string) { + t.Helper() + for _, w := range want { + if !strings.Contains(message, w) { + t.Errorf("failure message must contain %q, got %q", w, message) + } + } +} + func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { strategyAPIGroup := strategyv1alpha1.GroupVersion.Group resolved := &ResolvedBackupConfig{ @@ -907,9 +941,8 @@ func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { for _, tc := range mariadbCompleteOutcomeCases { t.Run(tc.name, func(t *testing.T) { - now := metav1.Now() seed := newMariaDBBackupJob("bj", "tenant") - seed.Status.StartedAt = &now + seed.Status.StartedAt = mariadbOutcomeStartedAt(tc.expired) strategy := &strategyv1alpha1.MariaDB{ ObjectMeta: metav1.ObjectMeta{Name: "mdb-strategy"}, Spec: strategyv1alpha1.MariaDBSpec{Template: *newRenderedMariaDBTemplate()}, @@ -920,13 +953,13 @@ func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { operatorBackup := &mariadbtypes.Backup{ ObjectMeta: metav1.ObjectMeta{ Namespace: "tenant", - Name: "bj-op", + Name: "op", Labels: map[string]string{ backupsv1alpha1.OwningJobNameLabel: seed.Name, backupsv1alpha1.OwningJobNamespaceLabel: seed.Namespace, }, }, - Status: mariadbtypes.BackupStatus{Conditions: mariadbCompleteCondition(tc.status, tc.reason)}, + Status: mariadbtypes.BackupStatus{Conditions: mariadbCompleteConditions(tc.noCondition, tc.status, tc.reason, tc.message)}, } c := newMariaDBStrategyTestClient(t, seed, newMariaDBApp(seed.Spec.ApplicationRef.Name, "tenant"), strategy, operatorMariaDB, operatorBackup) @@ -941,31 +974,33 @@ func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { t.Fatalf("reconcileMariaDB: %v", err) } + stored := &backupsv1alpha1.BackupJob{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(seed), stored); err != nil { + t.Fatalf("persisted Get: %v", err) + } artifacts := &backupsv1alpha1.BackupList{} if err := c.List(context.Background(), artifacts, client.InNamespace("tenant")); err != nil { t.Fatalf("list Backups: %v", err) } switch { case tc.wantFailed: - if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseFailed { - t.Errorf("expected Phase=Failed, got %q", bj.Status.Phase) + if stored.Status.Phase != backupsv1alpha1.BackupJobPhaseFailed { + t.Errorf("expected Phase=Failed, got %q", stored.Status.Phase) } - if !strings.Contains(bj.Status.Message, "bj-op") { - t.Errorf("failure message must name the operator Backup, got %q", bj.Status.Message) - } - if bj.Status.BackupRef != nil || len(artifacts.Items) != 0 { - t.Errorf("a failed run must not produce a Backup: ref=%v artifacts=%d", bj.Status.BackupRef, len(artifacts.Items)) + assertMariaDBFailureMessage(t, stored.Status.Message, tc.wantMessage) + if stored.Status.BackupRef != nil || len(artifacts.Items) != 0 { + t.Errorf("a failed run must not produce a Backup: ref=%v artifacts=%d", stored.Status.BackupRef, len(artifacts.Items)) } case tc.wantDone: - if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseSucceeded { - t.Errorf("expected Phase=Succeeded, got %q (message %q)", bj.Status.Phase, bj.Status.Message) + if stored.Status.Phase != backupsv1alpha1.BackupJobPhaseSucceeded { + t.Errorf("expected Phase=Succeeded, got %q (message %q)", stored.Status.Phase, stored.Status.Message) } - if bj.Status.BackupRef == nil || len(artifacts.Items) != 1 { - t.Errorf("a successful run must produce exactly one Backup: ref=%v artifacts=%d", bj.Status.BackupRef, len(artifacts.Items)) + if stored.Status.BackupRef == nil || len(artifacts.Items) != 1 { + t.Errorf("a successful run must produce exactly one Backup: ref=%v artifacts=%d", stored.Status.BackupRef, len(artifacts.Items)) } default: - if bj.Status.Phase != backupsv1alpha1.BackupJobPhaseRunning { - t.Errorf("expected Phase=Running, got %q", bj.Status.Phase) + if stored.Status.Phase != backupsv1alpha1.BackupJobPhaseRunning { + t.Errorf("expected Phase=Running, got %q", stored.Status.Phase) } if res.RequeueAfter == 0 { t.Errorf("expected a requeue while the run is in progress, got %+v", res) @@ -983,7 +1018,6 @@ func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { for _, tc := range mariadbCompleteOutcomeCases { t.Run(tc.name, func(t *testing.T) { - now := metav1.Now() cozyBackup := &backupsv1alpha1.Backup{ ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "src-cozy-bk"}, Spec: backupsv1alpha1.BackupSpec{ @@ -997,18 +1031,18 @@ func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { BackupRef: corev1.LocalObjectReference{Name: cozyBackup.Name}, TargetApplicationRef: &corev1.TypedLocalObjectReference{Kind: "MariaDB", Name: "target", APIGroup: &apps}, }, - Status: backupsv1alpha1.RestoreJobStatus{StartedAt: &now, Phase: backupsv1alpha1.RestoreJobPhaseRunning}, + Status: backupsv1alpha1.RestoreJobStatus{StartedAt: mariadbOutcomeStartedAt(tc.expired), Phase: backupsv1alpha1.RestoreJobPhaseRunning}, } operatorRestore := &mariadbtypes.Restore{ ObjectMeta: metav1.ObjectMeta{ Namespace: "tenant", - Name: "rj-op", + Name: "op", Labels: map[string]string{ backupsv1alpha1.OwningJobNameLabel: seed.Name, backupsv1alpha1.OwningJobNamespaceLabel: seed.Namespace, }, }, - Status: mariadbtypes.RestoreStatus{Conditions: mariadbCompleteCondition(tc.status, tc.reason)}, + Status: mariadbtypes.RestoreStatus{Conditions: mariadbCompleteConditions(tc.noCondition, tc.status, tc.reason, tc.message)}, } c := newMariaDBStrategyTestClient(t, cozyBackup, seed, operatorRestore, &mariadbtypes.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "src-op-bk"}}, @@ -1024,21 +1058,23 @@ func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { t.Fatalf("reconcileMariaDBRestore: %v", err) } + stored := &backupsv1alpha1.RestoreJob{} + if err := c.Get(context.Background(), client.ObjectKeyFromObject(seed), stored); err != nil { + t.Fatalf("persisted Get: %v", err) + } switch { case tc.wantFailed: - if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseFailed { - t.Errorf("expected Phase=Failed, got %q", rj.Status.Phase) - } - if !strings.Contains(rj.Status.Message, "rj-op") { - t.Errorf("failure message must name the operator Restore, got %q", rj.Status.Message) + if stored.Status.Phase != backupsv1alpha1.RestoreJobPhaseFailed { + t.Errorf("expected Phase=Failed, got %q", stored.Status.Phase) } + assertMariaDBFailureMessage(t, stored.Status.Message, tc.wantMessage) case tc.wantDone: - if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseSucceeded { - t.Errorf("expected Phase=Succeeded, got %q (message %q)", rj.Status.Phase, rj.Status.Message) + if stored.Status.Phase != backupsv1alpha1.RestoreJobPhaseSucceeded { + t.Errorf("expected Phase=Succeeded, got %q (message %q)", stored.Status.Phase, stored.Status.Message) } default: - if rj.Status.Phase != backupsv1alpha1.RestoreJobPhaseRunning { - t.Errorf("expected Phase=Running, got %q", rj.Status.Phase) + if stored.Status.Phase != backupsv1alpha1.RestoreJobPhaseRunning { + t.Errorf("expected Phase=Running, got %q", stored.Status.Phase) } if res.RequeueAfter == 0 { t.Errorf("expected a requeue while the run is in progress, got %+v", res) From 9f51b9db041f2d8753d0d81f2c0ad447827a11d7 Mon Sep 17 00:00:00 2001 From: Alexey Artamonov Date: Thu, 5 Nov 2026 21:05:36 +0300 Subject: [PATCH 3/5] fix(mariadb): report the operator Job's own failure on JobFailed The operator's JobFailed condition message is only "Failed", and with the operator's default restartPolicy OnFailure the Job controller deletes the pod once the backoff limit is spent, before the operator sets JobFailed. Pointing the reader at the Job's logs therefore sent them to logs that no longer exist. The Job, which the operator names after the Backup or Restore, keeps its Failed condition with the reason and message, so the BackupJob or RestoreJob now carries those instead; a Job that cannot be read is named in the message rather than dropped. Assisted-by: LLM Signed-off-by: Alexey Artamonov --- .../mariadbstrategy_controller.go | 29 +++++-- .../mariadbstrategy_controller_test.go | 81 +++++++++++++++++-- 2 files changed, 94 insertions(+), 16 deletions(-) diff --git a/internal/backupcontroller/mariadbstrategy_controller.go b/internal/backupcontroller/mariadbstrategy_controller.go index 48abd3644..c724e2dab 100644 --- a/internal/backupcontroller/mariadbstrategy_controller.go +++ b/internal/backupcontroller/mariadbstrategy_controller.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -205,7 +206,7 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 switch { case cond != nil && cond.Reason == mariadbtypes.ConditionReasonJobFailed: - return r.markBackupJobFailed(ctx, j, mariadbJobFailedMessage("Backup", mdbBackup.Namespace, mdbBackup.Name, cond)) + return r.markBackupJobFailed(ctx, j, mariadbJobFailedMessage(ctx, r.Client, "Backup", mdbBackup.Namespace, mdbBackup.Name, cond)) case cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == mariadbtypes.ConditionReasonJobComplete: // Backup completed successfully. Materialise the Cozystack artifact @@ -252,12 +253,24 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 } } -// mariadbJobFailedMessage names the operator object whose Job failed. The -// operator's own condition message is only "Failed"; the cause is in the -// logs of that object's Job. -func mariadbJobFailedMessage(kind, namespace, name string, cond *metav1.Condition) string { - return fmt.Sprintf("k8s.mariadb.com/%s %s/%s failed (Complete=%s/%s); the cause is in the logs of its Job", - kind, namespace, name, cond.Status, cond.Reason) +// mariadbJobFailedMessage names the operator object whose Job failed and +// carries that Job's own Failed condition, because the operator's condition +// message is only "Failed". The operator names the Job after the object. +func mariadbJobFailedMessage(ctx context.Context, c client.Reader, kind, namespace, name string, cond *metav1.Condition) string { + message := fmt.Sprintf("k8s.mariadb.com/%s %s/%s failed (Complete=%s/%s)", kind, namespace, name, cond.Status, cond.Reason) + job := &batchv1.Job{} + if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, job); err != nil { + if apierrors.IsNotFound(err) { + return fmt.Sprintf("%s; Job %s/%s not found", message, namespace, name) + } + return fmt.Sprintf("%s; reading Job %s/%s: %v", message, namespace, name, err) + } + for _, jc := range job.Status.Conditions { + if jc.Type == batchv1.JobFailed && jc.Status == corev1.ConditionTrue { + return fmt.Sprintf("%s; Job %s/%s: %s: %s", message, namespace, name, jc.Reason, jc.Message) + } + } + return fmt.Sprintf("%s; Job %s/%s reports no Failed condition", message, namespace, name) } // mariadbBackupDeadlineExceeded reports whether enough wall-clock time has @@ -572,7 +585,7 @@ func (r *RestoreJobReconciler) reconcileMariaDBRestore(ctx context.Context, rest cond := apimeta.FindStatusCondition(mdbRestore.Status.Conditions, mariadbtypes.ConditionTypeComplete) switch { case cond != nil && cond.Reason == mariadbtypes.ConditionReasonJobFailed: - return r.markRestoreJobFailed(ctx, restoreJob, mariadbJobFailedMessage("Restore", mdbRestore.Namespace, mdbRestore.Name, cond)) + return r.markRestoreJobFailed(ctx, restoreJob, mariadbJobFailedMessage(ctx, r.Client, "Restore", mdbRestore.Namespace, mdbRestore.Name, cond)) case cond != nil && cond.Status == metav1.ConditionTrue && cond.Reason == mariadbtypes.ConditionReasonJobComplete: now := metav1.Now() diff --git a/internal/backupcontroller/mariadbstrategy_controller_test.go b/internal/backupcontroller/mariadbstrategy_controller_test.go index 0c706574c..2ace7329e 100644 --- a/internal/backupcontroller/mariadbstrategy_controller_test.go +++ b/internal/backupcontroller/mariadbstrategy_controller_test.go @@ -4,12 +4,15 @@ package backupcontroller import ( "context" "encoding/json" + "errors" "strings" "testing" "time" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -17,6 +20,7 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" strategyv1alpha1 "github.com/cozystack/cozystack/api/backups/strategy/v1alpha1" backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1" @@ -877,15 +881,18 @@ var mariadbCompleteOutcomeCases = []struct { reason string message string expired bool + failedJob bool wantFailed bool wantDone bool wantMessage []string }{ {name: "Complete=True/JobComplete succeeds", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobComplete, message: "Success", wantDone: true}, - {name: "Complete=True/JobFailed fails", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", wantFailed: true, - wantMessage: []string{"tenant/op"}}, - {name: "Complete=False/JobFailed fails", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", wantFailed: true, - wantMessage: []string{"tenant/op"}}, + {name: "Complete=True/JobFailed fails with the Job's failure", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", failedJob: true, wantFailed: true, + wantMessage: []string{"tenant/op", "BackoffLimitExceeded", "Job has reached the specified backoff limit"}}, + {name: "Complete=False/JobFailed fails with the Job's failure", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", failedJob: true, wantFailed: true, + wantMessage: []string{"tenant/op", "BackoffLimitExceeded", "Job has reached the specified backoff limit"}}, + {name: "Complete=True/JobFailed with its Job gone fails", status: metav1.ConditionTrue, reason: mariadbtypes.ConditionReasonJobFailed, message: "Failed", wantFailed: true, + wantMessage: []string{"tenant/op", "Job tenant/op not found"}}, {name: "Complete=True with an unknown reason keeps waiting", status: metav1.ConditionTrue, reason: "SomethingElse", message: "Something"}, {name: "Complete=False/JobComplete keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobComplete, message: "Success"}, {name: "Complete=False/JobRunning keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobRunning, message: "Running"}, @@ -920,8 +927,29 @@ func mariadbOutcomeStartedAt(expired bool) *metav1.Time { return &past } +// mariadbOutcomeObjects adds the operator's Job, named after the operator +// Backup or Restore, failed the way the Job controller fails it once the +// backoff limit is spent. +func mariadbOutcomeObjects(failedJob bool, objs ...client.Object) []client.Object { + if !failedJob { + return objs + } + return append(objs, &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "op"}, + Status: batchv1.JobStatus{Conditions: []batchv1.JobCondition{{ + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + Reason: batchv1.JobReasonBackoffLimitExceeded, + Message: "Job has reached the specified backoff limit", + }}}, + }) +} + func assertMariaDBFailureMessage(t *testing.T, message string, want []string) { t.Helper() + if strings.Contains(message, "logs") { + t.Errorf("failure message must not send the reader to Job logs, which are gone with the deleted pod: %q", message) + } for _, w := range want { if !strings.Contains(message, w) { t.Errorf("failure message must contain %q, got %q", w, message) @@ -961,8 +989,8 @@ func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { }, Status: mariadbtypes.BackupStatus{Conditions: mariadbCompleteConditions(tc.noCondition, tc.status, tc.reason, tc.message)}, } - c := newMariaDBStrategyTestClient(t, seed, newMariaDBApp(seed.Spec.ApplicationRef.Name, "tenant"), - strategy, operatorMariaDB, operatorBackup) + c := newMariaDBStrategyTestClient(t, mariadbOutcomeObjects(tc.failedJob, seed, newMariaDBApp(seed.Spec.ApplicationRef.Name, "tenant"), + strategy, operatorMariaDB, operatorBackup)...) r := &BackupJobReconciler{Client: c, Scheme: c.Scheme(), Recorder: record.NewFakeRecorder(10)} bj := &backupsv1alpha1.BackupJob{} @@ -1013,6 +1041,43 @@ func TestReconcileMariaDBBackup_CompleteConditionOutcome(t *testing.T) { } } +func TestMariaDBJobFailedMessage_JobUnreadable(t *testing.T) { + cond := &metav1.Condition{Type: mariadbtypes.ConditionTypeComplete, Status: metav1.ConditionTrue, Reason: mariadbtypes.ConditionReasonJobFailed} + readErr := apierrors.NewForbidden(batchv1.Resource("jobs"), "op", errors.New("rbac")) + c := interceptor.NewClient(newMariaDBStrategyTestClient(t).(client.WithWatch), interceptor.Funcs{ + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*batchv1.Job); ok { + return readErr + } + return cl.Get(ctx, key, obj, opts...) + }, + }) + + got := mariadbJobFailedMessage(context.Background(), c, "Backup", "tenant", "op", cond) + for _, want := range []string{"k8s.mariadb.com/Backup tenant/op failed (Complete=True/JobFailed)", "reading Job tenant/op", readErr.Error()} { + if !strings.Contains(got, want) { + t.Errorf("message must contain %q, got %q", want, got) + } + } +} + +func TestMariaDBJobFailedMessage_JobWithoutFailedCondition(t *testing.T) { + cond := &metav1.Condition{Type: mariadbtypes.ConditionTypeComplete, Status: metav1.ConditionTrue, Reason: mariadbtypes.ConditionReasonJobFailed} + c := newMariaDBStrategyTestClient(t, &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "op"}, + Status: batchv1.JobStatus{Conditions: []batchv1.JobCondition{ + {Type: batchv1.JobFailureTarget, Status: corev1.ConditionTrue, Reason: batchv1.JobReasonPodFailurePolicy}, + {Type: batchv1.JobFailed, Status: corev1.ConditionFalse, Reason: batchv1.JobReasonBackoffLimitExceeded}, + }}, + }) + + got := mariadbJobFailedMessage(context.Background(), c, "Restore", "tenant", "op", cond) + if !strings.Contains(got, "Job tenant/op reports no Failed condition") || + strings.Contains(got, batchv1.JobReasonBackoffLimitExceeded) || strings.Contains(got, batchv1.JobReasonPodFailurePolicy) { + t.Errorf("a Failed condition that is not True must not be reported as the cause, got %q", got) + } +} + func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { apps := mariadbapp.GroupName @@ -1044,9 +1109,9 @@ func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { }, Status: mariadbtypes.RestoreStatus{Conditions: mariadbCompleteConditions(tc.noCondition, tc.status, tc.reason, tc.message)}, } - c := newMariaDBStrategyTestClient(t, cozyBackup, seed, operatorRestore, + c := newMariaDBStrategyTestClient(t, mariadbOutcomeObjects(tc.failedJob, cozyBackup, seed, operatorRestore, &mariadbtypes.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "src-op-bk"}}, - &mariadbtypes.MariaDB{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: mariadbNameForApp("target")}}) + &mariadbtypes.MariaDB{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: mariadbNameForApp("target")}})...) r := &RestoreJobReconciler{Client: c, Recorder: record.NewFakeRecorder(10)} rj := &backupsv1alpha1.RestoreJob{} From 69cea9ccbdd3799be27e465db6479d3313e4dcda Mon Sep 17 00:00:00 2001 From: Alexey Artamonov Date: Thu, 5 Nov 2026 21:06:27 +0300 Subject: [PATCH 4/5] fix(mariadb): keep the operator's message in deadline failures The operator sets Complete=False with reason Failed and its own cause in the message ("Error creating Job", "Backup not complete") when a run fails before or outside its Job. Such a run waits out the deadline, and the deadline failure kept only the status and reason, so the BackupJob or RestoreJob ended without the one line that says why. Assisted-by: LLM Signed-off-by: Alexey Artamonov --- .../mariadbstrategy_controller.go | 26 ++++++++++++------- .../mariadbstrategy_controller_test.go | 17 ++++++++++++ 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/internal/backupcontroller/mariadbstrategy_controller.go b/internal/backupcontroller/mariadbstrategy_controller.go index c724e2dab..f17d6fdaf 100644 --- a/internal/backupcontroller/mariadbstrategy_controller.go +++ b/internal/backupcontroller/mariadbstrategy_controller.go @@ -242,12 +242,8 @@ func (r *BackupJobReconciler) reconcileMariaDB(ctx context.Context, j *backupsv1 // Backup eventually fails the BackupJob instead of pinning it Running // forever. if mariadbBackupDeadlineExceeded(j.Status.StartedAt) { - detail := "no Complete condition observed" - if cond != nil { - detail = fmt.Sprintf("Complete=%s/%s", cond.Status, cond.Reason) - } return r.markBackupJobFailed(ctx, j, fmt.Sprintf( - "k8s.mariadb.com Backup did not complete within %s (%s)", mariadbDefaultBackupDeadline, detail)) + "k8s.mariadb.com Backup did not complete within %s (%s)", mariadbDefaultBackupDeadline, mariadbCompleteDetail(cond))) } return ctrl.Result{RequeueAfter: mariadbPollInterval}, nil } @@ -273,6 +269,20 @@ func mariadbJobFailedMessage(ctx context.Context, c client.Reader, kind, namespa return fmt.Sprintf("%s; Job %s/%s reports no Failed condition", message, namespace, name) } +// mariadbCompleteDetail describes the last Complete condition for a deadline +// failure. The message matters for reason Failed, which the operator sets +// with its own cause ("Error creating Job", "Backup not complete") rather +// than from a Job. +func mariadbCompleteDetail(cond *metav1.Condition) string { + if cond == nil { + return "no Complete condition observed" + } + if cond.Message == "" { + return fmt.Sprintf("Complete=%s/%s", cond.Status, cond.Reason) + } + return fmt.Sprintf("Complete=%s/%s: %s", cond.Status, cond.Reason, cond.Message) +} + // mariadbBackupDeadlineExceeded reports whether enough wall-clock time has // elapsed since the BackupJob started that we should give up on a stuck // operator Backup. Returns false when StartedAt is nil so the very first @@ -605,13 +615,9 @@ func (r *RestoreJobReconciler) reconcileMariaDBRestore(ctx context.Context, rest default: deadline := options.effectiveRestoreDeadline() if restoreJob.Status.StartedAt != nil && time.Since(restoreJob.Status.StartedAt.Time) > deadline { - detail := "no Complete condition observed" - if cond != nil { - detail = fmt.Sprintf("Complete=%s/%s", cond.Status, cond.Reason) - } return r.markRestoreJobFailed(ctx, restoreJob, fmt.Sprintf( "k8s.mariadb.com/Restore did not complete within %s (%s; override via spec.options.restoreTimeoutSeconds)", - deadline, detail)) + deadline, mariadbCompleteDetail(cond))) } return ctrl.Result{RequeueAfter: mariadbPollInterval}, nil } diff --git a/internal/backupcontroller/mariadbstrategy_controller_test.go b/internal/backupcontroller/mariadbstrategy_controller_test.go index 2ace7329e..a23351584 100644 --- a/internal/backupcontroller/mariadbstrategy_controller_test.go +++ b/internal/backupcontroller/mariadbstrategy_controller_test.go @@ -901,6 +901,8 @@ var mariadbCompleteOutcomeCases = []struct { wantMessage: []string{"no Complete condition observed"}}, {name: "Complete=True with an unknown reason past the deadline fails", status: metav1.ConditionTrue, reason: "SomethingElse", message: "Something", expired: true, wantFailed: true, wantMessage: []string{"Complete=True/SomethingElse"}}, + {name: "Complete=False/Failed past the deadline carries the operator's message", status: metav1.ConditionFalse, reason: "Failed", message: "Error creating Job", expired: true, wantFailed: true, + wantMessage: []string{"Complete=False/Failed: Error creating Job"}}, } func mariadbCompleteConditions(noCondition bool, status metav1.ConditionStatus, reason, message string) []metav1.Condition { @@ -1078,6 +1080,21 @@ func TestMariaDBJobFailedMessage_JobWithoutFailedCondition(t *testing.T) { } } +func TestMariaDBCompleteDetail(t *testing.T) { + for _, tc := range []struct { + cond *metav1.Condition + want string + }{ + {nil, "no Complete condition observed"}, + {&metav1.Condition{Status: metav1.ConditionFalse, Reason: "Failed", Message: "Backup not complete"}, "Complete=False/Failed: Backup not complete"}, + {&metav1.Condition{Status: metav1.ConditionFalse, Reason: "Failed"}, "Complete=False/Failed"}, + } { + if got := mariadbCompleteDetail(tc.cond); got != tc.want { + t.Errorf("mariadbCompleteDetail(%+v) = %q, want %q", tc.cond, got, tc.want) + } + } +} + func TestReconcileMariaDBRestore_CompleteConditionOutcome(t *testing.T) { apps := mariadbapp.GroupName From 5f0da5a09aab36000e4e1004065ce73e83e3f142 Mon Sep 17 00:00:00 2001 From: Alexey Artamonov Date: Thu, 5 Nov 2026 22:11:54 +0300 Subject: [PATCH 5/5] docs(mariadb): list the operator's Failed reason on the Complete condition The reason list read as everything the operator writes on Complete, yet mariadb-operator also sets Complete=False with reason Failed when a run fails before or outside its Job. A reader taking the list as exhaustive would treat Failed as an unknown reason, or make it terminal, while the operator retries those failures. A table row pins that the driver keeps waiting on it. Assisted-by: LLM Signed-off-by: Alexey Artamonov --- internal/backupcontroller/mariadbstrategy_controller_test.go | 1 + internal/backupcontroller/mariadbtypes/types.go | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/internal/backupcontroller/mariadbstrategy_controller_test.go b/internal/backupcontroller/mariadbstrategy_controller_test.go index a23351584..f03b67d94 100644 --- a/internal/backupcontroller/mariadbstrategy_controller_test.go +++ b/internal/backupcontroller/mariadbstrategy_controller_test.go @@ -896,6 +896,7 @@ var mariadbCompleteOutcomeCases = []struct { {name: "Complete=True with an unknown reason keeps waiting", status: metav1.ConditionTrue, reason: "SomethingElse", message: "Something"}, {name: "Complete=False/JobComplete keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobComplete, message: "Success"}, {name: "Complete=False/JobRunning keeps waiting", status: metav1.ConditionFalse, reason: mariadbtypes.ConditionReasonJobRunning, message: "Running"}, + {name: "Complete=False/Failed keeps waiting", status: metav1.ConditionFalse, reason: "Failed", message: "Error creating Job"}, {name: "no Complete condition keeps waiting", noCondition: true}, {name: "no Complete condition past the deadline fails", noCondition: true, expired: true, wantFailed: true, wantMessage: []string{"no Complete condition observed"}}, diff --git a/internal/backupcontroller/mariadbtypes/types.go b/internal/backupcontroller/mariadbtypes/types.go index d9c09f30a..61fc71019 100644 --- a/internal/backupcontroller/mariadbtypes/types.go +++ b/internal/backupcontroller/mariadbtypes/types.go @@ -37,6 +37,10 @@ const ( // JobFailed -> Status=True (terminal failure of the backing Job). // JobRunning -> Status=False (still in progress). // JobSuspended -> Status=False (admin-suspended via spec). + // The operator also writes reason Failed with Status=False and its own + // message ("Error creating Job", "Backup not complete") when a run fails + // outside its Job. It retries those, so the driver keeps waiting on + // Failed until its deadline rather than failing the run at once. ConditionReasonJobComplete = "JobComplete" ConditionReasonJobFailed = "JobFailed" ConditionReasonJobRunning = "JobRunning"