fix(backups): attribute a MariaDB dump only when no other run can own it

Every backup of a MariaDB application writes its dump under one shared
prefix, so the probe took the dump whose file-name timestamp (the
backup pod's clock) and LastModified (the object store's clock) both
fall in the operator Backup's lifetime, with a minute of slack for
skew. That keeps a neighbour's dump out only while this run's own dump
is in the window. With the pod clock more than the slack behind, the
own dump is named before the window opens, and a previous run that
finished within the minute before this one was created becomes the
single match: a 123-byte dump was reported as 666 bytes. A pod clock
ahead or an object store clock behind does the same with the next run,
and an own dump that is gone by the time it is looked up, as it can be
for a restore long after the backup, leaves a neighbour as the single
match with no skew at all.

A dump is now attributed only when ownership is proven:

- it is the one dump that either clock places in the window. A dump
  that one clock places there and the other does not may be this run's
  own with a skewed clock, so it leaves any other match unproven, the
  way a second dump in the window already did, and is not taken itself;
- both of its clocks place it in the window;
- neither clock places it in the window of another operator Backup
  that writes under the same bucket and prefix and has completed. This
  rejects a neighbour's dump when this run's own is missing, which the
  clocks alone cannot tell.

Operator Backups still running are not counted as neighbours: their
window has no end, and one stuck short of Complete (a Job whose pod
never starts stays JobRunning for good) would otherwise keep every
later run of the application from being sized. A dump such a run
writes inside this run's window still competes with this run's own.

Narrower slack does not prove ownership: a neighbour that finished
within the narrower slack still matches, and a smaller skew pushes the
own dump out. The Job's start and completion times are stamped by the
control plane, like the operator Backup's own, so they narrow the
window by seconds without removing the dependence on the two clocks a
dump carries. What is left: a neighbour's dump is taken only when this
run's own is missing from the listing or misplaced on both clocks and
the neighbour's operator Backup no longer exists or has not completed.
Every other unproven case leaves the size unset, as two runs that
overlap or follow each other within the slack already did.

Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>

internal/backupcontroller/artifact_size.go

@@ -60,7 +60,8 @@ type s3Connection struct {
 // so that the restore path, which has a reconciler of its own, can read
 // the same objects.
 type objectStore struct {
-	// reader must reach Secrets; the manager's client reads them uncached.
+	// reader reads Secrets, which the manager's client fetches uncached,
+	// and lists operator Backups, which it serves from the cache.
 	reader client.Reader
 	// newLister overrides the S3 client; nil means a real minio client.
 	newLister func(conn s3Connection) (s3ObjectLister, error)

internal/backupcontroller/mariadb_artifact_size.go

@@ -8,6 +8,10 @@ import (
 	"strings"
 	"time"
 
+	apimeta "k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	"github.com/cozystack/cozystack/internal/backupcontroller/mariadbtypes"
 )
 
@@ -60,6 +64,10 @@ func findMariaDBDump(ctx context.Context, store objectStore, mdbBackup *mariadbt
 	if s3 == nil {
 		return mariadbDump{}, errors.New("the operator Backup does not store to S3")
 	}
+	others, err := finishedMariaDBRuns(ctx, store.reader, mdbBackup)
+	if err != nil {
+		return mariadbDump{}, fmt.Errorf("list operator Backups: %w", err)
+	}
 	conn, err := mariadbS3Connection(ctx, store, mdbBackup.Namespace, s3)
 	if err != nil {
 		return mariadbDump{}, fmt.Errorf("resolve S3 credentials: %w", err)
@@ -76,35 +84,104 @@ func findMariaDBDump(ctx context.Context, store objectStore, mdbBackup *mariadbt
 	if err != nil {
 		return mariadbDump{}, fmt.Errorf("list s3://%s/%s: %w", s3.Bucket, prefix, err)
 	}
-	return matchMariaDBDump(objects, prefix, mdbBackup.CreationTimestamp.Time, completedAt)
+	own := newMariaDBRun(mdbBackup, mdbBackup.CreationTimestamp.Time, completedAt)
+	return matchMariaDBDump(objects, prefix, own, others)
 }
 
-// matchMariaDBDump picks the dump of the run that lived from created to
-// completed. The prefix is shared by every backup of the application, so
-// the dump is identified by both the timestamp in its file name and its
-// LastModified falling inside that lifetime: the two come from different
-// clocks, so a skewed pod clock alone cannot pull a neighbouring run's dump
-// into the window.
-func matchMariaDBDump(objects []s3ObjectInfo, prefix string, created, completed time.Time) (mariadbDump, error) {
-	lower := created.Add(-mariadbSizeProbeClockSkew)
-	upper := completed.Add(mariadbSizeProbeClockSkew)
-	var matched []mariadbDump
+// mariadbRun is the span in which an operator Backup can have written its
+// dump: its lifetime, widened by mariadbSizeProbeClockSkew on each side.
+type mariadbRun struct {
+	name         string
+	lower, upper time.Time
+}
+
+func newMariaDBRun(b *mariadbtypes.Backup, created, completed time.Time) mariadbRun {
+	return mariadbRun{
+		name:  b.Namespace + "/" + b.Name,
+		lower: created.Add(-mariadbSizeProbeClockSkew),
+		upper: completed.Add(mariadbSizeProbeClockSkew),
+	}
+}
+
+func (r mariadbRun) placesOnBothClocks(d mariadbDump) bool {
+	return inWindow(d.Timestamp, r.lower, r.upper) && inWindow(d.LastModified, r.lower, r.upper)
+}
+
+func (r mariadbRun) placesOnEitherClock(d mariadbDump) bool {
+	return inWindow(d.Timestamp, r.lower, r.upper) || inWindow(d.LastModified, r.lower, r.upper)
+}
+
+// matchMariaDBDump picks the dump of run own among objects, which share a
+// prefix with every other run of the application. A dump carries two
+// clocks, the backup pod's in its file name and the object store's in its
+// LastModified, and either can be skewed. A dump is taken only when it is
+// the one dump either clock places in own's window, both clocks place it
+// there, and neither places it in the window of another finished run: a
+// dump in the window on one clock only may be this run's own, misplaced by
+// a skewed clock, which leaves any other candidate unproven. A neighbour's
+// dump is still taken if this run's own is missing from the listing, or
+// misplaced on both clocks, and the neighbour's operator Backup is gone or
+// has not completed.
+func matchMariaDBDump(objects []s3ObjectInfo, prefix string, own mariadbRun, others []mariadbRun) (mariadbDump, error) {
+	var near []mariadbDump
 	for _, obj := range objects {
 		name := strings.TrimPrefix(obj.Key, prefix)
 		ts, ok := parseMariaDBLogicalBackupFile(name)
-		if !ok || !inWindow(ts, lower, upper) || !inWindow(obj.LastModified, lower, upper) {
+		if !ok {
 			continue
 		}
-		matched = append(matched, mariadbDump{Key: obj.Key, Name: name, Timestamp: ts, Size: obj.Size, LastModified: obj.LastModified})
+		d := mariadbDump{Key: obj.Key, Name: name, Timestamp: ts, Size: obj.Size, LastModified: obj.LastModified}
+		if own.placesOnEitherClock(d) {
+			near = append(near, d)
+		}
 	}
-	switch len(matched) {
-	case 0:
+	if len(near) == 0 {
 		return mariadbDump{}, errMariaDBDumpNotFound
-	case 1:
-		return matched[0], nil
-	default:
-		return mariadbDump{}, fmt.Errorf("%w: %d dumps fall in its lifetime", errMariaDBDumpAmbiguous, len(matched))
 	}
+	if len(near) > 1 {
+		return mariadbDump{}, fmt.Errorf("%w: %d dumps fall in its lifetime on at least one clock", errMariaDBDumpAmbiguous, len(near))
+	}
+	d := near[0]
+	if !own.placesOnBothClocks(d) {
+		return mariadbDump{}, fmt.Errorf("%w: %s falls in its lifetime on one clock only", errMariaDBDumpAmbiguous, d.Name)
+	}
+	for _, other := range others {
+		if other.placesOnEitherClock(d) {
+			return mariadbDump{}, fmt.Errorf("%w: %s also falls in the lifetime of operator Backup %s", errMariaDBDumpAmbiguous, d.Name, other.name)
+		}
+	}
+	return d, nil
+}
+
+// finishedMariaDBRuns returns the runs of the other operator Backups that
+// write under the same bucket and prefix as b and have completed, whether
+// their Job succeeded or failed. One still running is left out: its window
+// has no end yet, and an operator Backup stuck short of Complete, as one
+// whose Job pod never starts is, would otherwise contest every later run of
+// the application for good. The endpoint is not compared, since one has
+// several spellings, and a false neighbour only leaves a size unset.
+func finishedMariaDBRuns(ctx context.Context, reader client.Reader, b *mariadbtypes.Backup) ([]mariadbRun, error) {
+	list := &mariadbtypes.BackupList{}
+	if err := reader.List(ctx, list); err != nil {
+		return nil, err
+	}
+	var runs []mariadbRun
+	for i := range list.Items {
+		other := &list.Items[i]
+		if other.Namespace == b.Namespace && other.Name == b.Name {
+			continue
+		}
+		s3 := other.Spec.Storage.S3
+		if s3 == nil || s3.Bucket != b.Spec.Storage.S3.Bucket || mariadbOperatorS3Prefix(s3.Prefix) != mariadbOperatorS3Prefix(b.Spec.Storage.S3.Prefix) {
+			continue
+		}
+		cond := apimeta.FindStatusCondition(other.Status.Conditions, mariadbtypes.ConditionTypeComplete)
+		if cond == nil || cond.Status != metav1.ConditionTrue {
+			continue
+		}
+		runs = append(runs, newMariaDBRun(other, other.CreationTimestamp.Time, cond.LastTransitionTime.Time))
+	}
+	return runs, nil
 }
 
 // mariadbS3Connection resolves the Secrets and TLS settings of the operator

internal/backupcontroller/mariadb_artifact_size_test.go

@@ -239,8 +239,9 @@ func TestMatchMariaDBDump(t *testing.T) {
 	completed := created.Add(3 * time.Minute)
 	const prefix = "tenant/db/"
 	own := s3ObjectInfo{Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 4242, LastModified: created.Add(2 * time.Minute)}
+	run := newMariaDBRun(&mariadbtypes.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "op-bk"}}, created, completed)
 
-	got, err := matchMariaDBDump([]s3ObjectInfo{own}, prefix, created, completed)
+	got, err := matchMariaDBDump([]s3ObjectInfo{own}, prefix, run, nil)
 	if err != nil {
 		t.Fatalf("matchMariaDBDump: %v", err)
 	}
@@ -255,11 +256,11 @@ func TestMatchMariaDBDump(t *testing.T) {
 		t.Errorf("matchMariaDBDump = %+v, want %+v", got, want)
 	}
 
-	if _, err := matchMariaDBDump(nil, prefix, created, completed); !errors.Is(err, errMariaDBDumpNotFound) {
+	if _, err := matchMariaDBDump(nil, prefix, run, nil); !errors.Is(err, errMariaDBDumpNotFound) {
 		t.Errorf("no dump: err = %v, want errMariaDBDumpNotFound", err)
 	}
 	second := s3ObjectInfo{Key: dumpKey(prefix, created.Add(2*time.Minute), "gzip"), Size: 1, LastModified: created.Add(2 * time.Minute)}
-	if _, err := matchMariaDBDump([]s3ObjectInfo{own, second}, prefix, created, completed); !errors.Is(err, errMariaDBDumpAmbiguous) {
+	if _, err := matchMariaDBDump([]s3ObjectInfo{own, second}, prefix, run, nil); !errors.Is(err, errMariaDBDumpAmbiguous) {
 		t.Errorf("two dumps: err = %v, want errMariaDBDumpAmbiguous", err)
 	}
 }
@@ -303,6 +304,154 @@ func TestFindMariaDBDump(t *testing.T) {
 	}
 }
 
+// TestFindMariaDBDump_Ownership drives runs of one application that share
+// a prefix. This run lives from t0 to t0+40s, so its window with the
+// one-minute slack is [t0-1m, t0+1m40s]; its own dump is 123 bytes and a
+// neighbour's is 666 or 777. A neighbour's dump must never be returned.
+func TestFindMariaDBDump_Ownership(t *testing.T) {
+	t0 := time.Date(2026, 10, 21, 10, 0, 0, 0, time.UTC)
+	s3 := newRenderedMariaDBTemplate().Storage.S3
+	s3.Prefix = "tenant/db"
+	const prefix = "tenant/db/"
+	run := func(name string, created, completed time.Time) *mariadbtypes.Backup {
+		b := operatorBackupStoringTo(t, s3, created)
+		b.Name = name
+		b.Status.Conditions = []metav1.Condition{mariadbBackupSucceeded(metav1.NewTime(completed))}
+		return b
+	}
+	dump := func(named, uploaded time.Time, size int64) s3ObjectInfo {
+		return s3ObjectInfo{Key: dumpKey(prefix, named, "gzip"), Size: size, LastModified: uploaded}
+	}
+	own := run("op-bk", t0, t0.Add(40*time.Second))
+	find := func(t *testing.T, objects []s3ObjectInfo, operatorBackups ...client.Object) (mariadbDump, error) {
+		t.Helper()
+		store := objectStore{
+			reader: newMariaDBStrategyTestClient(t, append(operatorBackups, own, s3CredsSecret("tenant", "creds"))...),
+			newLister: func(s3Connection) (s3ObjectLister, error) {
+				return &fakeS3Lister{objects: objects}, nil
+			},
+		}
+		return findMariaDBDump(context.Background(), store, own, t0.Add(40*time.Second))
+	}
+	wantAmbiguous := func(t *testing.T, got mariadbDump, err error) {
+		t.Helper()
+		if !errors.Is(err, errMariaDBDumpAmbiguous) {
+			t.Errorf("findMariaDBDump = %+v, %v; want errMariaDBDumpAmbiguous", got, err)
+		}
+	}
+	wantOwn := func(t *testing.T, got mariadbDump, err error) {
+		t.Helper()
+		if err != nil || got.Size != 123 {
+			t.Errorf("findMariaDBDump = %+v, %v; want this run's 123-byte dump", got, err)
+		}
+	}
+
+	t.Run("a pod clock behind does not hand this run the previous run's dump", func(t *testing.T) {
+		// The previous run finished 20s before this one was created and its
+		// operator Backup is gone. This run's pod clock is 5m behind, so its
+		// own dump is named outside the window and uploaded inside it.
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(-30*time.Second), t0.Add(-20*time.Second), 666),
+			dump(t0.Add(5*time.Second-5*time.Minute), t0.Add(30*time.Second), 123),
+		})
+		wantAmbiguous(t, got, err)
+	})
+
+	t.Run("a pod clock ahead does not hand this run the next run's dump", func(t *testing.T) {
+		// The next run started 5s after this one completed and has uploaded
+		// its dump by the time this one is probed; it has not completed yet.
+		next := operatorBackupStoringTo(t, s3, t0.Add(45*time.Second))
+		next.Name = "op-next"
+		next.Status.Conditions = []metav1.Condition{{
+			Type: mariadbtypes.ConditionTypeComplete, Status: metav1.ConditionFalse,
+			Reason: mariadbtypes.ConditionReasonJobRunning, Message: "Running", LastTransitionTime: metav1.NewTime(t0.Add(45 * time.Second)),
+		}}
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(5*time.Second+5*time.Minute), t0.Add(30*time.Second), 123),
+			dump(t0.Add(50*time.Second), t0.Add(80*time.Second), 777),
+		}, next)
+		wantAmbiguous(t, got, err)
+	})
+
+	t.Run("an object store clock behind does not hand this run the next run's dump", func(t *testing.T) {
+		// The store clock is 2m behind: this run's upload is stamped before
+		// the window opens, the next run's is pulled back into it.
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(5*time.Second), t0.Add(30*time.Second-2*time.Minute), 123),
+			dump(t0.Add(50*time.Second), t0.Add(3*time.Minute-2*time.Minute), 777),
+		})
+		wantAmbiguous(t, got, err)
+	})
+
+	t.Run("a dump another finished run wrote is not taken when this run's own is gone", func(t *testing.T) {
+		// This run's dump was pruned; the next run's falls in this run's
+		// window on both clocks, and in its own.
+		next := run("op-next", t0.Add(45*time.Second), t0.Add(85*time.Second))
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(50*time.Second), t0.Add(75*time.Second), 777),
+		}, next)
+		wantAmbiguous(t, got, err)
+	})
+
+	t.Run("a dump the window holds on one clock only is not taken", func(t *testing.T) {
+		// This run's dump is gone; the previous run's operator Backup is too,
+		// and its dump, started 2m before this run, finished uploading 20s
+		// before it.
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(-2*time.Minute), t0.Add(-20*time.Second), 666),
+		})
+		if err == nil {
+			t.Errorf("findMariaDBDump = %+v; want no dump attributed", got)
+		}
+	})
+
+	t.Run("a run still in progress does not contest this run's dump", func(t *testing.T) {
+		// A concurrent run created 30s before this one has not uploaded yet.
+		running := operatorBackupStoringTo(t, s3, t0.Add(-30*time.Second))
+		running.Name = "op-running"
+		running.Status.Conditions = []metav1.Condition{{
+			Type: mariadbtypes.ConditionTypeComplete, Status: metav1.ConditionFalse,
+			Reason: mariadbtypes.ConditionReasonJobRunning, Message: "Running", LastTransitionTime: metav1.NewTime(t0.Add(-30 * time.Second)),
+		}}
+		got, err := find(t, []s3ObjectInfo{dump(t0.Add(5*time.Second), t0.Add(30*time.Second), 123)}, running)
+		wantOwn(t, got, err)
+	})
+
+	t.Run("runs minutes apart each keep their own dump", func(t *testing.T) {
+		previous := run("op-previous", t0.Add(-4*time.Minute), t0.Add(-3*time.Minute))
+		next := run("op-next", t0.Add(3*time.Minute), t0.Add(4*time.Minute))
+		got, err := find(t, []s3ObjectInfo{
+			dump(t0.Add(-230*time.Second), t0.Add(-190*time.Second), 666),
+			dump(t0.Add(5*time.Second), t0.Add(30*time.Second), 123),
+			dump(t0.Add(185*time.Second), t0.Add(220*time.Second), 777),
+		}, previous, next)
+		wantOwn(t, got, err)
+	})
+
+	t.Run("a run that writes elsewhere does not contest this run's dump", func(t *testing.T) {
+		other := newRenderedMariaDBTemplate().Storage.S3
+		other.Prefix = "tenant/other"
+		elsewhere := operatorBackupStoringTo(t, other, t0)
+		elsewhere.Name = "op-elsewhere"
+		elsewhere.Status.Conditions = []metav1.Condition{mariadbBackupSucceeded(metav1.NewTime(t0.Add(40 * time.Second)))}
+		got, err := find(t, []s3ObjectInfo{dump(t0.Add(5*time.Second), t0.Add(30*time.Second), 123)}, elsewhere)
+		wantOwn(t, got, err)
+	})
+
+	t.Run("a run stuck short of Complete does not contest later runs", func(t *testing.T) {
+		// A backup Job whose pod never starts leaves its operator Backup
+		// JobRunning for good.
+		stuck := operatorBackupStoringTo(t, s3, t0.Add(-3*time.Hour))
+		stuck.Name = "op-stuck"
+		stuck.Status.Conditions = []metav1.Condition{{
+			Type: mariadbtypes.ConditionTypeComplete, Status: metav1.ConditionFalse,
+			Reason: mariadbtypes.ConditionReasonJobRunning, Message: "Running", LastTransitionTime: metav1.NewTime(t0.Add(-3 * time.Hour)),
+		}}
+		got, err := find(t, []s3ObjectInfo{dump(t0.Add(5*time.Second), t0.Add(30*time.Second), 123)}, stuck)
+		wantOwn(t, got, err)
+	})
+}
+
 // TestMariaDBArtifactSizeBytes_MinioClient drives the real minio client
 // against a stub ListObjectsV2 endpoint, so the Secret wiring, prefix and
 // response parsing are exercised together.