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 <aleksei.artamonov@aenix.io>
Assisted-by: LLM

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 {

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
 // ---------------------------------------------------------------------------

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"`
 }