diff --git a/internal/backupcontroller/artifact_size.go b/internal/backupcontroller/artifact_size.go index 7a5fec8f8..517fe07a6 100644 --- a/internal/backupcontroller/artifact_size.go +++ b/internal/backupcontroller/artifact_size.go @@ -26,8 +26,9 @@ import ( const artifactSizeProbeTimeout = 10 * time.Second type s3ObjectInfo struct { - Key string - Size int64 + Key string + Size int64 + LastModified time.Time } type s3ObjectLister interface { @@ -110,7 +111,7 @@ func collectObjects(ctx context.Context, objects <-chan minio.ObjectInfo) ([]s3O if obj.Err != nil { return nil, obj.Err } - out = append(out, s3ObjectInfo{Key: obj.Key, Size: obj.Size}) + out = append(out, s3ObjectInfo{Key: obj.Key, Size: obj.Size, LastModified: obj.LastModified}) } if err := ctx.Err(); err != nil { return nil, err @@ -120,10 +121,12 @@ func collectObjects(ctx context.Context, objects <-chan minio.ObjectInfo) ([]s3O // existingBackup returns the Backup a previous reconcile already created // for the job, so a retry skips the size probe whose result it would -// discard. Returns (nil, nil) when there is none. +// discard. Returns (nil, nil) when there is none. It reads past the cache: +// a retry typically follows a Create made moments earlier, which the cache +// may not show yet. func (r *BackupJobReconciler) existingBackup(ctx context.Context, namespace, name string) (*backupsv1alpha1.Backup, error) { existing := &backupsv1alpha1.Backup{} - if err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, existing); err != nil { + if err := r.apiReader().Get(ctx, types.NamespacedName{Namespace: namespace, Name: name}, existing); err != nil { if apierrors.IsNotFound(err) { return nil, nil } diff --git a/internal/backupcontroller/artifact_size_test.go b/internal/backupcontroller/artifact_size_test.go index 5b636a44b..f24079f8b 100644 --- a/internal/backupcontroller/artifact_size_test.go +++ b/internal/backupcontroller/artifact_size_test.go @@ -11,6 +11,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1" ) type fakeS3Lister struct { @@ -89,3 +91,18 @@ func TestNewMinioObjectLister_RejectsEndpointPath(t *testing.T) { t.Errorf("host:port endpoint rejected: %v", err) } } + +// TestExistingBackup_ReadsPastTheCache pins that the retry check sees a +// Backup the cache does not show yet: the previous reconcile created it +// moments earlier. +func TestExistingBackup_ReadsPastTheCache(t *testing.T) { + backup := &backupsv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "bj"}} + r := &BackupJobReconciler{ + Client: newMariaDBStrategyTestClient(t), + APIReader: newMariaDBStrategyTestClient(t, backup), + } + got, err := r.existingBackup(context.Background(), "tenant", "bj") + if err != nil || got == nil { + t.Fatalf("existingBackup = %v, %v; want the Backup the API server holds", got, err) + } +} diff --git a/internal/backupcontroller/cnpg_artifact_size_test.go b/internal/backupcontroller/cnpg_artifact_size_test.go index de3960261..13b647bdd 100644 --- a/internal/backupcontroller/cnpg_artifact_size_test.go +++ b/internal/backupcontroller/cnpg_artifact_size_test.go @@ -83,6 +83,19 @@ func TestCNPGArtifactSizeBytes(t *testing.T) { } }) + t.Run("an empty endpointURL means AWS", func(t *testing.T) { + store := newSizedBarmanObjectStore() + store.EndpointURL = "" + l := &fakeS3Lister{objects: []s3ObjectInfo{{Key: prefix + "data.tar", Size: 5}}} + r := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")) + if got := r.cnpgArtifactSizeBytes(context.Background(), "tenant", store, "srv", "20261021T100000"); got != 5 { + t.Errorf("size = %d, want 5", got) + } + if l.conn.Endpoint != "s3.amazonaws.com" || !l.conn.Secure { + t.Errorf("connection = %+v, want s3.amazonaws.com over TLS", l.conn) + } + }) + t.Run("honours custom credential keys and the endpoint CA", func(t *testing.T) { store := newSizedBarmanObjectStore() store.S3Credentials.AccessKeyIDKey = "id" diff --git a/internal/backupcontroller/cnpgstrategy_controller.go b/internal/backupcontroller/cnpgstrategy_controller.go index 50e9dd8ab..5dabfa66c 100644 --- a/internal/backupcontroller/cnpgstrategy_controller.go +++ b/internal/backupcontroller/cnpgstrategy_controller.go @@ -876,7 +876,7 @@ func (r *BackupJobReconciler) createCNPGBackupArtifact( return nil, err } existing := &backupsv1alpha1.Backup{} - if getErr := r.Get(ctx, types.NamespacedName{Namespace: backup.Namespace, Name: backup.Name}, existing); getErr != nil { + if getErr := r.apiReader().Get(ctx, types.NamespacedName{Namespace: backup.Namespace, Name: backup.Name}, existing); getErr != nil { return nil, getErr } return existing, nil diff --git a/internal/backupcontroller/mariadb_artifact_size.go b/internal/backupcontroller/mariadb_artifact_size.go index 86d31694a..273eb8105 100644 --- a/internal/backupcontroller/mariadb_artifact_size.go +++ b/internal/backupcontroller/mariadb_artifact_size.go @@ -11,15 +11,17 @@ import ( ) // Slack around the [created, completed] window of the operator Backup when -// matching the timestamp mariadb-operator embeds in the dump file name: that -// timestamp comes from the backup Job pod's clock, not the apiserver's, and -// is truncated to seconds. +// matching a dump: the timestamp in its file name comes from the backup Job +// pod's clock and is truncated to seconds, and its LastModified comes from +// the object store's clock, neither of which is the apiserver's. const mariadbSizeProbeClockSkew = time.Minute // mariadbArtifactSizeBytes measures the dump mariadb-operator uploaded for // mdbBackup. The strategy prefix is shared by every backup of the -// application, so the dump is identified by the timestamp in its file name -// falling inside the operator Backup's lifetime. Returns 0 - leaving +// application, so the dump is identified by both the timestamp in its file +// name and its LastModified falling inside the operator Backup's lifetime: +// the two come from different clocks, so a skewed pod clock alone cannot +// pull a neighbouring run's dump into the window. Returns 0 - leaving // Artifact.SizeBytes absent - when the object store cannot be read or the // window does not match exactly one dump: an unknown size must not be // reported as a measured one. @@ -59,7 +61,7 @@ func (r *BackupJobReconciler) mariadbArtifactSizeBytes( var matched []s3ObjectInfo for _, obj := range objects { ts, ok := parseMariaDBLogicalBackupFile(strings.TrimPrefix(obj.Key, prefix)) - if !ok || ts.Before(lower) || ts.After(upper) { + if !ok || !inWindow(ts, lower, upper) || !inWindow(obj.LastModified, lower, upper) { continue } matched = append(matched, obj) @@ -115,15 +117,30 @@ func mariadbOperatorS3Prefix(prefix string) string { return prefix } +func inWindow(t, lower, upper time.Time) bool { + return !t.Before(lower) && !t.After(upper) +} + +// Compression tokens mariadb-operator puts in a dump file name: the +// algorithm before .sql in v25.10 (backup..gzip.sql), the extension +// after it in later releases (backup..sql.gz). +var ( + mariadbDumpAlgorithms = map[string]bool{"none": true, "bzip2": true, "gzip": true, "zstd": true} + mariadbDumpExtensions = map[string]bool{"bz2": true, "gz": true, "zst": true} +) + // parseMariaDBLogicalBackupFile accepts the logical-backup file names -// mariadb-operator writes, backup..sql and -// backup...sql, and returns the embedded timestamp. +// mariadb-operator writes and returns the embedded timestamp. func parseMariaDBLogicalBackupFile(name string) (time.Time, bool) { parts := strings.Split(name, ".") - if len(parts) != 3 && len(parts) != 4 { + if parts[0] != "backup" { return time.Time{}, false } - if parts[0] != "backup" || parts[len(parts)-1] != "sql" { + switch { + case len(parts) == 3 && parts[2] == "sql": + case len(parts) == 4 && parts[2] == "sql" && mariadbDumpExtensions[parts[3]]: + case len(parts) == 4 && parts[3] == "sql" && mariadbDumpAlgorithms[parts[2]]: + default: return time.Time{}, false } ts, err := time.Parse(time.RFC3339, parts[1]) diff --git a/internal/backupcontroller/mariadb_artifact_size_test.go b/internal/backupcontroller/mariadb_artifact_size_test.go index d068d9898..6f9df56e7 100644 --- a/internal/backupcontroller/mariadb_artifact_size_test.go +++ b/internal/backupcontroller/mariadb_artifact_size_test.go @@ -33,6 +33,11 @@ func TestParseMariaDBLogicalBackupFile(t *testing.T) { "backup.2026-10-21T10:00:00Z.gzip.sql": true, "backup.2026-10-21T10:00:00+04:00.sql": true, "backup.2026-10-21T10:00:00Z.sql.tmp": false, + "backup.2026-10-21T10:00:00Z.sql.gz": true, + "backup.2026-10-21T10:00:00Z.sql.zst": true, + "backup.2026-10-21T10:00:00Z.zstd.sql": true, + "backup.2026-10-21T10:00:00Z.xz.sql": false, + "backup.2026-10-21T10:00:00Z.sql.gz.tmp": false, "physicalbackup-20261021100000.xb": false, "backup.not-a-date.sql": false, "sub/backup.2026-10-21T10:00:00Z.sql": false, @@ -69,8 +74,8 @@ func TestMariaDBArtifactSizeBytes(t *testing.T) { t.Run("single dump inside the backup window", func(t *testing.T) { l := &fakeS3Lister{objects: []s3ObjectInfo{ - {Key: dumpKey(prefix, created.Add(-24*time.Hour), "gzip"), Size: 111}, - {Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 4242}, + {Key: dumpKey(prefix, created.Add(-24*time.Hour), "gzip"), Size: 111, LastModified: created.Add(-24 * time.Hour)}, + {Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 4242, LastModified: created.Add(time.Minute)}, {Key: prefix + "0-backup-target.txt", Size: 7}, }} got := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")).mariadbArtifactSizeBytes(context.Background(), "tenant", s3, mdbBackup, completed) @@ -96,7 +101,7 @@ func TestMariaDBArtifactSizeBytes(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "ca"}, Data: map[string][]byte{"ca.crt": []byte("PEM")}, } - l := &fakeS3Lister{objects: []s3ObjectInfo{{Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 3}}} + l := &fakeS3Lister{objects: []s3ObjectInfo{{Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 3, LastModified: created.Add(time.Minute)}}} r := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds"), ca) if got := r.mariadbArtifactSizeBytes(context.Background(), "tenant", tlsS3, mdbBackup, completed); got != 3 { t.Errorf("size = %d, want 3", got) @@ -114,7 +119,7 @@ func TestMariaDBArtifactSizeBytes(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "sts"}, Data: map[string][]byte{"token": []byte("TOKEN")}, } - l := &fakeS3Lister{objects: []s3ObjectInfo{{Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 4}}} + l := &fakeS3Lister{objects: []s3ObjectInfo{{Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 4, LastModified: created.Add(time.Minute)}}} r := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds"), sts) if got := r.mariadbArtifactSizeBytes(context.Background(), "tenant", stsS3, mdbBackup, completed); got != 4 { t.Errorf("size = %d, want 4", got) @@ -126,17 +131,30 @@ func TestMariaDBArtifactSizeBytes(t *testing.T) { t.Run("pod clock skew within tolerance", func(t *testing.T) { l := &fakeS3Lister{objects: []s3ObjectInfo{ - {Key: dumpKey(prefix, created.Add(-30*time.Second), ""), Size: 9}, + {Key: dumpKey(prefix, created.Add(-30*time.Second), ""), Size: 9, LastModified: created.Add(time.Minute)}, }} if got := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")).mariadbArtifactSizeBytes(context.Background(), "tenant", s3, mdbBackup, completed); got != 9 { t.Errorf("size = %d, want 9", got) } }) + t.Run("a skewed pod clock does not attribute a neighbouring run's dump", func(t *testing.T) { + // The pod clock runs 5m behind. This run's dump is named outside the + // window; the previous run's dump, uploaded before this run started, + // is named inside it. + l := &fakeS3Lister{objects: []s3ObjectInfo{ + {Key: dumpKey(prefix, created.Add(-4*time.Minute), "gzip"), Size: 7, LastModified: created.Add(time.Minute)}, + {Key: dumpKey(prefix, created.Add(30*time.Second), "gzip"), Size: 666, LastModified: created.Add(-4 * time.Minute)}, + }} + if got := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")).mariadbArtifactSizeBytes(context.Background(), "tenant", s3, mdbBackup, completed); got != 0 { + t.Errorf("size = %d, want 0: no dump is inside the window on both clocks", got) + } + }) + t.Run("two dumps in the window leave size unset", func(t *testing.T) { l := &fakeS3Lister{objects: []s3ObjectInfo{ - {Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 1}, - {Key: dumpKey(prefix, created.Add(2*time.Minute), "gzip"), Size: 2}, + {Key: dumpKey(prefix, created.Add(time.Minute), "gzip"), Size: 1, LastModified: created.Add(time.Minute)}, + {Key: dumpKey(prefix, created.Add(2*time.Minute), "gzip"), Size: 2, LastModified: created.Add(2 * time.Minute)}, }} if got := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")).mariadbArtifactSizeBytes(context.Background(), "tenant", s3, mdbBackup, completed); got != 0 { t.Errorf("size = %d, want 0 (ambiguous)", got) @@ -145,7 +163,7 @@ func TestMariaDBArtifactSizeBytes(t *testing.T) { t.Run("no dump in the window leaves size unset", func(t *testing.T) { l := &fakeS3Lister{objects: []s3ObjectInfo{ - {Key: dumpKey(prefix, completed.Add(time.Hour), "gzip"), Size: 1}, + {Key: dumpKey(prefix, completed.Add(time.Hour), "gzip"), Size: 1, LastModified: completed.Add(time.Hour)}, }} if got := reconcilerWithLister(t, l, s3CredsSecret("tenant", "creds")).mariadbArtifactSizeBytes(context.Background(), "tenant", s3, mdbBackup, completed); got != 0 { t.Errorf("size = %d, want 0", got) @@ -234,7 +252,7 @@ func TestCreateMariaDBBackupArtifact_SizeBytes(t *testing.T) { }, } r := reconcilerWithLister(t, &fakeS3Lister{objects: []s3ObjectInfo{ - {Key: dumpKey("src/", completed.Add(-time.Minute), "gzip"), Size: 2048}, + {Key: dumpKey("src/", completed.Add(-time.Minute), "gzip"), Size: 2048, LastModified: completed.Add(-time.Minute)}, }}, job, s3CredsSecret("tenant", "creds")) rendered := newRenderedMariaDBTemplate() rendered.Storage.S3.Prefix = "src/" diff --git a/internal/backupcontroller/mariadbstrategy_controller.go b/internal/backupcontroller/mariadbstrategy_controller.go index bdffc7e7c..f6abed32d 100644 --- a/internal/backupcontroller/mariadbstrategy_controller.go +++ b/internal/backupcontroller/mariadbstrategy_controller.go @@ -421,7 +421,7 @@ func (r *BackupJobReconciler) createMariaDBBackupArtifact( return nil, err } existing := &backupsv1alpha1.Backup{} - if getErr := r.Get(ctx, types.NamespacedName{Namespace: backup.Namespace, Name: backup.Name}, existing); getErr != nil { + if getErr := r.apiReader().Get(ctx, types.NamespacedName{Namespace: backup.Namespace, Name: backup.Name}, existing); getErr != nil { return nil, getErr } return existing, nil