From 78a236081c9b428cc5bde9d9f08f8f3b664cd8b7 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Wed, 8 Jul 2026 13:03:04 -0400 Subject: [PATCH 1/9] test(longhaul): add ScheduledBackup + retention verifier Adds the data-protection component of the long-haul driver. A new test/longhaul/backup package bootstraps a ScheduledBackup against the canary cluster and runs a verifier concurrently with the operation scheduler (backup is deliberately not isolated, per the design). Each verification cycle checks: - scheduling liveness (status.lastScheduledTime advances) - child Backup completion and terminal failures - retention correctness (expiredAt == stoppedAt + retentionDays*24h) - retention garbage collection (no backup lingers past expiredAt) Retention or GC violations flip the run verdict to FAIL. New K8sClusterClient methods (EnsureScheduledBackup / GetScheduledBackup / ListScheduledChildBackups) back the verifier's Client interface, and new LONGHAUL_BACKUP_* env vars configure it. Also re-includes test/longhaul/backup/ in .gitignore (the generic Backup*/ VS rule swallowed it on case-insensitive filesystems). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- .gitignore | 3 + test/longhaul/README.md | 30 +++ test/longhaul/backup/metrics.go | 107 +++++++++++ test/longhaul/backup/metrics_test.go | 54 ++++++ test/longhaul/backup/suite_test.go | 16 ++ test/longhaul/backup/verifier.go | 265 ++++++++++++++++++++++++++ test/longhaul/backup/verifier_test.go | 222 +++++++++++++++++++++ test/longhaul/cmd/longhaul/main.go | 52 ++++- test/longhaul/config/config.go | 60 ++++++ test/longhaul/config/config_test.go | 31 +++ test/longhaul/monitor/k8sclient.go | 64 +++++++ test/longhaul/report/report.go | 21 ++ 12 files changed, 915 insertions(+), 10 deletions(-) create mode 100644 test/longhaul/backup/metrics.go create mode 100644 test/longhaul/backup/metrics_test.go create mode 100644 test/longhaul/backup/suite_test.go create mode 100644 test/longhaul/backup/verifier.go create mode 100644 test/longhaul/backup/verifier_test.go diff --git a/.gitignore b/.gitignore index 3db05b243..8e4de1d5f 100644 --- a/.gitignore +++ b/.gitignore @@ -278,6 +278,9 @@ Backup*/ !test/e2e/manifests/backup/ !test/e2e/tests/backup/ !test/e2e/pkg/e2eutils/backup/ +# The long-haul driver's data-protection component lives here and is +# likewise swallowed by the generic Backup*/ rule above. +!test/longhaul/backup/ UpgradeLog*.XML UpgradeLog*.htm ServiceFabricBackup/ diff --git a/test/longhaul/README.md b/test/longhaul/README.md index a7d311e94..e772b6536 100644 --- a/test/longhaul/README.md +++ b/test/longhaul/README.md @@ -132,8 +132,38 @@ All configuration is via environment variables. | `LONGHAUL_MIN_INSTANCES` | No | `1` | Minimum `spec.instancesPerNode` for scale-down operations (CRD lower bound: 1). | | `LONGHAUL_MAX_INSTANCES` | No | `3` | Maximum `spec.instancesPerNode` for scale-up operations (CRD upper bound: 3). | | `LONGHAUL_REPORT_INTERVAL` | No | `1h` | How often to write checkpoint reports to ConfigMap. | +| `LONGHAUL_BACKUP_ENABLED` | No | `true` | Enable the ScheduledBackup + retention verifier. | +| `LONGHAUL_BACKUP_SCHEDULE` | No | `0 */6 * * *` | Cron schedule for the canary `ScheduledBackup`. | +| `LONGHAUL_BACKUP_RETENTION_DAYS` | No | `1` | Retention window applied to child backups and validated against the operator-computed expiration. | +| `LONGHAUL_BACKUP_VERIFY_INTERVAL` | No | `5m` | How often the backup verifier polls scheduling / completion / retention. | | `LONGHAUL_RESET_DATA` | No | `false` | If `true`, drop the workload collection on startup. Off by default so a Deployment pod restart preserves durability history. | +### Data Protection (ScheduledBackup + retention) + +When `LONGHAUL_BACKUP_ENABLED` is true, the driver bootstraps a `ScheduledBackup` +named `-longhaul` and runs a verifier concurrently with the operation +scheduler (backup is deliberately **not** isolated from topology/chaos, per the +design). Each verification cycle checks: + +- **Scheduling liveness** — `status.lastScheduledTime` advances; a stalled + scheduler (past `status.nextScheduledTime` + grace) raises a warning. +- **Completion** — child `Backup` CRs reach the `completed` phase; terminal + failures (`failed` / `skipped`) are counted. +- **Retention correctness** — every completed backup's `status.expiredAt` + equals `stoppedAt + retentionDays*24h` (within a 2m tolerance). A mismatch is + a **FAIL** (the operator miscalculated retention). +- **Retention GC** — no backup lingers more than 10m past its `status.expiredAt`. + A lingering expired backup is a **FAIL** (retention garbage-collection leak). + +Because the minimum meaningful retention is 1 day, the GC check only fires on +multi-day runs — exactly the accumulation window long-haul exists to cover. + +> **RBAC.** The driver ServiceAccount needs `create`/`get`/`list` on +> `scheduledbackups.documentdb.io` and `list` on `backups.documentdb.io`. These +> verbs must be present in `deploy/rbac.yaml` (added in the CI/CD PR) for the +> backup verifier to function; without them it logs an error and the rest of the +> run continues. + ## CI Safety The long haul test binary is deployed as a Kubernetes Deployment on a dedicated AKS diff --git a/test/longhaul/backup/metrics.go b/test/longhaul/backup/metrics.go new file mode 100644 index 000000000..629f03dea --- /dev/null +++ b/test/longhaul/backup/metrics.go @@ -0,0 +1,107 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Package backup implements the data-protection component of the long haul +// driver. It provisions a ScheduledBackup against the canary cluster and +// continuously verifies that backups are produced on schedule, complete +// successfully, and that the operator honors the configured retention +// period (expiration is calculated correctly and expired backups are +// garbage-collected). +// +// The component runs concurrently with the operation scheduler — per the +// long-haul design, backup is deliberately NOT isolated so that +// backup-vs-topology serialization bugs surface here rather than in +// production. +package backup + +import ( + "sync/atomic" + "time" +) + +// Metrics tracks aggregate backup-verification counters using atomic +// operations so the reporter goroutine can snapshot them without locking. +// +// RetentionViolations and GCViolations are the retention oracle: any +// non-zero value indicates a real operator bug and flips the run verdict +// to FAIL. The remaining counters are observational and feed the report. +type Metrics struct { + // Scheduled counts backups observed to have been scheduled by the + // ScheduledBackup (advances of status.lastScheduledTime). + Scheduled atomic.Int64 + + // Completed is the number of child backups observed in the "completed" + // phase (deduplicated by name across verification cycles). + Completed atomic.Int64 + + // Failed is the number of child backups observed in a terminal failure + // phase (deduplicated by name). + Failed atomic.Int64 + + // RetentionViolations counts completed backups whose status.expiredAt + // does not match stoppedAt + retentionDays*24h within tolerance. + // Non-zero => FAIL (operator miscalculated retention). + RetentionViolations atomic.Int64 + + // GCViolations counts backups that remain present well past their + // status.expiredAt — i.e. the operator failed to garbage-collect an + // expired backup. Non-zero => FAIL (retention GC leak). + GCViolations atomic.Int64 + + // VerifyCycles is the number of completed verification passes. + VerifyCycles atomic.Int64 + + // LastChildCount is the number of child backups observed on the most + // recent verification cycle (a proxy for steady-state backup population). + LastChildCount atomic.Int64 + + // LastScheduledUnix is the Unix timestamp of the most recently observed + // status.lastScheduledTime; 0 until the first backup is scheduled. + LastScheduledUnix atomic.Int64 + + // StartTime is when this Metrics was constructed; resets on pod restart. + StartTime time.Time +} + +// NewMetrics creates a Metrics with the start time set to now. +func NewMetrics() *Metrics { + return &Metrics{StartTime: time.Now()} +} + +// MetricsSnapshot is a point-in-time copy of Metrics. +type MetricsSnapshot struct { + Scheduled int64 + Completed int64 + Failed int64 + RetentionViolations int64 + GCViolations int64 + VerifyCycles int64 + LastChildCount int64 + LastScheduled time.Time + Elapsed time.Duration +} + +// Snapshot captures the current metric values atomically. +func (m *Metrics) Snapshot() MetricsSnapshot { + var lastScheduled time.Time + if unix := m.LastScheduledUnix.Load(); unix > 0 { + lastScheduled = time.Unix(unix, 0) + } + return MetricsSnapshot{ + Scheduled: m.Scheduled.Load(), + Completed: m.Completed.Load(), + Failed: m.Failed.Load(), + RetentionViolations: m.RetentionViolations.Load(), + GCViolations: m.GCViolations.Load(), + VerifyCycles: m.VerifyCycles.Load(), + LastChildCount: m.LastChildCount.Load(), + LastScheduled: lastScheduled, + Elapsed: time.Since(m.StartTime), + } +} + +// HasRetentionFailure returns true if any retention or GC violation has +// been detected. A true result flips the overall run verdict to FAIL. +func (s MetricsSnapshot) HasRetentionFailure() bool { + return s.RetentionViolations > 0 || s.GCViolations > 0 +} diff --git a/test/longhaul/backup/metrics_test.go b/test/longhaul/backup/metrics_test.go new file mode 100644 index 000000000..bc6d5f8eb --- /dev/null +++ b/test/longhaul/backup/metrics_test.go @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package backup + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Metrics", func() { + It("snapshots counters atomically", func() { + m := NewMetrics() + m.Scheduled.Add(3) + m.Completed.Add(2) + m.Failed.Add(1) + m.RetentionViolations.Add(1) + m.GCViolations.Add(1) + m.VerifyCycles.Add(5) + m.LastChildCount.Store(7) + now := time.Now() + m.LastScheduledUnix.Store(now.Unix()) + + snap := m.Snapshot() + Expect(snap.Scheduled).To(Equal(int64(3))) + Expect(snap.Completed).To(Equal(int64(2))) + Expect(snap.Failed).To(Equal(int64(1))) + Expect(snap.RetentionViolations).To(Equal(int64(1))) + Expect(snap.GCViolations).To(Equal(int64(1))) + Expect(snap.VerifyCycles).To(Equal(int64(5))) + Expect(snap.LastChildCount).To(Equal(int64(7))) + Expect(snap.LastScheduled.Unix()).To(Equal(now.Unix())) + }) + + It("reports zero LastScheduled when never scheduled", func() { + snap := NewMetrics().Snapshot() + Expect(snap.LastScheduled.IsZero()).To(BeTrue()) + }) + + DescribeTable("HasRetentionFailure", + func(retention, gc int64, want bool) { + m := NewMetrics() + m.RetentionViolations.Add(retention) + m.GCViolations.Add(gc) + Expect(m.Snapshot().HasRetentionFailure()).To(Equal(want)) + }, + Entry("clean", int64(0), int64(0), false), + Entry("retention violated", int64(1), int64(0), true), + Entry("gc violated", int64(0), int64(1), true), + Entry("both", int64(2), int64(3), true), + ) +}) diff --git a/test/longhaul/backup/suite_test.go b/test/longhaul/backup/suite_test.go new file mode 100644 index 000000000..752094654 --- /dev/null +++ b/test/longhaul/backup/suite_test.go @@ -0,0 +1,16 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package backup + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestBackup(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Long Haul Backup Suite") +} diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go new file mode 100644 index 000000000..edd3a99d1 --- /dev/null +++ b/test/longhaul/backup/verifier.go @@ -0,0 +1,265 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package backup + +import ( + "context" + "fmt" + "time" + + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/longhaul/journal" +) + +const ( + // retentionTolerance is the allowed skew between the operator-computed + // status.expiredAt and the value we recompute from stoppedAt + + // retentionDays*24h. The operator derives both from the same stoppedAt, + // so any real drift is tiny; the tolerance only absorbs clock rounding. + retentionTolerance = 2 * time.Minute + + // gcGrace is how long past status.expiredAt a backup may still exist + // before we treat it as a garbage-collection failure. It absorbs the + // backup controller's requeue latency and apiserver propagation. + gcGrace = 10 * time.Minute + + // livenessGrace is how far past status.nextScheduledTime the scheduler + // may run before we warn that scheduling appears stalled. + livenessGrace = 5 * time.Minute +) + +// Client is the subset of the cluster API the backup verifier needs. +// monitor.K8sClusterClient satisfies it structurally. +type Client interface { + // EnsureScheduledBackup idempotently creates the ScheduledBackup CR. + EnsureScheduledBackup(ctx context.Context, name, schedule string, retentionDays int) error + // GetScheduledBackup fetches the ScheduledBackup CR by name. + GetScheduledBackup(ctx context.Context, name string) (*previewv1.ScheduledBackup, error) + // ListScheduledChildBackups lists the Backup CRs the ScheduledBackup produced. + ListScheduledChildBackups(ctx context.Context, scheduledBackupName string) ([]previewv1.Backup, error) +} + +// Config parameterizes the backup verifier. +type Config struct { + // ScheduledBackupName is the name of the ScheduledBackup CR to manage. + ScheduledBackupName string + // Schedule is the cron expression for the ScheduledBackup. + Schedule string + // RetentionDays is the retention window applied to every child backup. + RetentionDays int + // VerifyInterval is how often the verification loop runs. + VerifyInterval time.Duration +} + +// Verifier bootstraps a ScheduledBackup and continuously checks that the +// operator schedules, completes, and retires backups per policy. +type Verifier struct { + client Client + journal *journal.Journal + metrics *Metrics + cfg Config + + // dedup state — the verification loop re-observes the same backups every + // cycle, so we count each transition/violation exactly once by name. + seenCompleted map[string]struct{} + seenFailed map[string]struct{} + retentionFlagged map[string]struct{} + gcFlagged map[string]struct{} + lastScheduled time.Time + stallWarnedFor time.Time +} + +// NewVerifier creates a backup verifier. metrics must be non-nil. +func NewVerifier(client Client, j *journal.Journal, metrics *Metrics, cfg Config) *Verifier { + return &Verifier{ + client: client, + journal: j, + metrics: metrics, + cfg: cfg, + seenCompleted: make(map[string]struct{}), + seenFailed: make(map[string]struct{}), + retentionFlagged: make(map[string]struct{}), + gcFlagged: make(map[string]struct{}), + } +} + +// Bootstrap ensures the ScheduledBackup CR exists. It is safe to call on +// every startup; an existing ScheduledBackup is left untouched so the +// accumulated backup history survives driver restarts. +func (v *Verifier) Bootstrap(ctx context.Context) error { + if err := v.client.EnsureScheduledBackup(ctx, v.cfg.ScheduledBackupName, v.cfg.Schedule, v.cfg.RetentionDays); err != nil { + return fmt.Errorf("ensure ScheduledBackup: %w", err) + } + v.journal.Info("backup", fmt.Sprintf("ScheduledBackup %q ensured (schedule=%q retentionDays=%d)", + v.cfg.ScheduledBackupName, v.cfg.Schedule, v.cfg.RetentionDays)) + return nil +} + +// Run starts the verification loop. It blocks until ctx is cancelled. +func (v *Verifier) Run(ctx context.Context) { + v.journal.Info("backup", "backup verifier started") + defer v.journal.Info("backup", "backup verifier stopped") + + ticker := time.NewTicker(v.cfg.VerifyInterval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + v.checkOnce(ctx, time.Now()) + } + } +} + +// checkOnce runs a single verification pass. now is injected so unit tests +// can drive time deterministically. +func (v *Verifier) checkOnce(ctx context.Context, now time.Time) { + v.checkScheduling(ctx, now) + v.checkChildBackups(ctx, now) + v.metrics.VerifyCycles.Add(1) +} + +// checkScheduling inspects the ScheduledBackup status: it counts new +// scheduling events and warns if the scheduler appears stalled. +func (v *Verifier) checkScheduling(ctx context.Context, now time.Time) { + sb, err := v.client.GetScheduledBackup(ctx, v.cfg.ScheduledBackupName) + if err != nil { + v.journal.Warn("backup", fmt.Sprintf("cannot read ScheduledBackup (will retry): %v", err)) + return + } + + if sb.Status.LastScheduledTime != nil { + last := sb.Status.LastScheduledTime.Time + if last.After(v.lastScheduled) { + v.lastScheduled = last + v.metrics.Scheduled.Add(1) + v.metrics.LastScheduledUnix.Store(last.Unix()) + v.journal.Info("backup", fmt.Sprintf("backup scheduled at %s", last.Format(time.RFC3339))) + } + } + + // Liveness: if the next scheduled time is well in the past and no new + // backup has been scheduled since, surface a warning (once per overdue + // deadline). This is a Degraded signal, not a Fatal one — scheduling may + // be transiently delayed by apiserver load. + if next := sb.Status.NextScheduledTime; next != nil { + deadline := next.Time.Add(livenessGrace) + if now.After(deadline) && !v.lastScheduled.After(next.Time) && !v.stallWarnedFor.Equal(next.Time) { + v.stallWarnedFor = next.Time + v.journal.Warn("backup", fmt.Sprintf( + "scheduling appears stalled: nextScheduledTime %s passed %s ago with no new backup", + next.Time.Format(time.RFC3339), now.Sub(next.Time).Round(time.Second))) + } + } +} + +// checkChildBackups inspects every child Backup for completion, terminal +// failure, retention-calculation correctness, and garbage collection. +func (v *Verifier) checkChildBackups(ctx context.Context, now time.Time) { + children, err := v.client.ListScheduledChildBackups(ctx, v.cfg.ScheduledBackupName) + if err != nil { + v.journal.Warn("backup", fmt.Sprintf("cannot list child backups (will retry): %v", err)) + return + } + + v.metrics.LastChildCount.Store(int64(len(children))) + + for i := range children { + b := &children[i] + v.recordPhase(b) + v.checkRetention(b) + v.checkGarbageCollection(b, now) + } +} + +// recordPhase counts new completed / terminally-failed child backups, +// deduplicated by name. +func (v *Verifier) recordPhase(b *previewv1.Backup) { + switch { + case isCompleted(b): + if _, seen := v.seenCompleted[b.Name]; !seen { + v.seenCompleted[b.Name] = struct{}{} + v.metrics.Completed.Add(1) + v.journal.Info("backup", fmt.Sprintf("backup %q completed", b.Name)) + } + case isTerminalFailure(b): + if _, seen := v.seenFailed[b.Name]; !seen { + v.seenFailed[b.Name] = struct{}{} + v.metrics.Failed.Add(1) + v.journal.Warn("backup", fmt.Sprintf("backup %q reached terminal failure phase %q: %s", + b.Name, b.Status.Phase, b.Status.Message)) + } + } +} + +// checkRetention verifies that a completed backup's status.expiredAt equals +// stoppedAt + retentionDays*24h within tolerance. A mismatch is a Fatal +// operator bug and is flagged exactly once per backup. +func (v *Verifier) checkRetention(b *previewv1.Backup) { + if !isCompleted(b) || b.Status.StoppedAt == nil || b.Status.ExpiredAt == nil || b.Spec.RetentionDays == nil { + return + } + if _, done := v.retentionFlagged[b.Name]; done { + return + } + + expected := b.Status.StoppedAt.Time.Add(time.Duration(*b.Spec.RetentionDays) * 24 * time.Hour) + actual := b.Status.ExpiredAt.Time + skew := actual.Sub(expected) + if skew < 0 { + skew = -skew + } + if skew > retentionTolerance { + v.retentionFlagged[b.Name] = struct{}{} + v.metrics.RetentionViolations.Add(1) + v.journal.Error("backup", fmt.Sprintf( + "retention miscalculated for %q: retentionDays=%d stoppedAt=%s expiredAt=%s (expected %s, skew %s)", + b.Name, *b.Spec.RetentionDays, + b.Status.StoppedAt.Time.Format(time.RFC3339), + actual.Format(time.RFC3339), expected.Format(time.RFC3339), skew.Round(time.Second))) + } +} + +// checkGarbageCollection flags a backup that still exists well past its +// status.expiredAt — the operator failed to retire it. Flagged once per +// backup. +func (v *Verifier) checkGarbageCollection(b *previewv1.Backup, now time.Time) { + if b.Status.ExpiredAt == nil { + return + } + if _, done := v.gcFlagged[b.Name]; done { + return + } + if now.After(b.Status.ExpiredAt.Time.Add(gcGrace)) { + v.gcFlagged[b.Name] = struct{}{} + v.metrics.GCViolations.Add(1) + v.journal.Error("backup", fmt.Sprintf( + "retention GC failure: backup %q still present %s past expiredAt %s", + b.Name, now.Sub(b.Status.ExpiredAt.Time).Round(time.Second), + b.Status.ExpiredAt.Time.Format(time.RFC3339))) + } +} + +// isCompleted reports whether a Backup reached the "completed" phase. +func isCompleted(b *previewv1.Backup) bool { + return b != nil && b.Status.Phase == cnpgv1.BackupPhaseCompleted +} + +// isTerminalFailure reports whether a Backup reached a phase from which no +// reconcile will recover it. +func isTerminalFailure(b *previewv1.Backup) bool { + if b == nil { + return false + } + switch b.Status.Phase { + case cnpgv1.BackupPhaseFailed, previewv1.BackupPhaseSkipped: + return true + default: + return false + } +} diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go new file mode 100644 index 000000000..c1ea6c8af --- /dev/null +++ b/test/longhaul/backup/verifier_test.go @@ -0,0 +1,222 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package backup + +import ( + "context" + "errors" + "time" + + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + previewv1 "github.com/documentdb/documentdb-operator/api/preview" + "github.com/documentdb/documentdb-operator/test/longhaul/journal" +) + +// fakeBackupClient is a minimal Client stub for unit tests. +type fakeBackupClient struct { + ensureCalls []ensureArgs + ensureErr error + sb *previewv1.ScheduledBackup + sbErr error + children []previewv1.Backup + childErr error +} + +type ensureArgs struct { + name string + schedule string + retentionDays int +} + +func (f *fakeBackupClient) EnsureScheduledBackup(_ context.Context, name, schedule string, retentionDays int) error { + f.ensureCalls = append(f.ensureCalls, ensureArgs{name, schedule, retentionDays}) + return f.ensureErr +} + +func (f *fakeBackupClient) GetScheduledBackup(_ context.Context, _ string) (*previewv1.ScheduledBackup, error) { + return f.sb, f.sbErr +} + +func (f *fakeBackupClient) ListScheduledChildBackups(_ context.Context, _ string) ([]previewv1.Backup, error) { + return f.children, f.childErr +} + +// mkBackup builds a child Backup with the given phase, retention and +// stopped/expired times. Zero times are left nil. +func mkBackup(name string, phase cnpgv1.BackupPhase, retentionDays int, stopped, expired time.Time) previewv1.Backup { + rd := retentionDays + b := previewv1.Backup{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: previewv1.BackupSpec{RetentionDays: &rd}, + Status: previewv1.BackupStatus{Phase: phase}, + } + if !stopped.IsZero() { + b.Status.StoppedAt = &metav1.Time{Time: stopped} + } + if !expired.IsZero() { + b.Status.ExpiredAt = &metav1.Time{Time: expired} + } + return b +} + +func newTestVerifier(client Client) (*Verifier, *Metrics) { + m := NewMetrics() + v := NewVerifier(client, journal.New(), m, Config{ + ScheduledBackupName: "cluster-longhaul", + Schedule: "0 */6 * * *", + RetentionDays: 1, + VerifyInterval: time.Minute, + }) + return v, m +} + +var _ = Describe("Verifier.Bootstrap", func() { + It("ensures the ScheduledBackup with configured parameters", func() { + fc := &fakeBackupClient{} + v, _ := newTestVerifier(fc) + Expect(v.Bootstrap(context.Background())).To(Succeed()) + Expect(fc.ensureCalls).To(HaveLen(1)) + Expect(fc.ensureCalls[0]).To(Equal(ensureArgs{"cluster-longhaul", "0 */6 * * *", 1})) + }) + + It("wraps ensure errors", func() { + fc := &fakeBackupClient{ensureErr: errors.New("boom")} + v, _ := newTestVerifier(fc) + Expect(v.Bootstrap(context.Background())).To(MatchError(ContainSubstring("boom"))) + }) +}) + +var _ = Describe("Verifier.checkOnce", func() { + now := time.Date(2026, 7, 8, 12, 0, 0, 0, time.UTC) + + It("counts scheduling advances without double counting", func() { + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{ + Status: previewv1.ScheduledBackupStatus{ + LastScheduledTime: &metav1.Time{Time: now.Add(-time.Hour)}, + }, + }, + } + v, m := newTestVerifier(fc) + + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().Scheduled).To(Equal(int64(1))) + + // Same lastScheduledTime → no new count. + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().Scheduled).To(Equal(int64(1))) + + // Advance → counted. + fc.sb.Status.LastScheduledTime = &metav1.Time{Time: now.Add(-30 * time.Minute)} + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().Scheduled).To(Equal(int64(2))) + }) + + It("counts completed and failed child backups once each", func() { + stopped := now.Add(-30 * time.Minute) + expired := stopped.Add(24 * time.Hour) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{ + mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired), + mkBackup("b2", previewv1.BackupPhaseSkipped, 1, time.Time{}, time.Time{}), + mkBackup("b3", cnpgv1.BackupPhaseFailed, 1, time.Time{}, time.Time{}), + }, + } + v, m := newTestVerifier(fc) + + v.checkOnce(context.Background(), now) + v.checkOnce(context.Background(), now) // idempotent + + snap := m.Snapshot() + Expect(snap.Completed).To(Equal(int64(1))) + Expect(snap.Failed).To(Equal(int64(2))) // skipped + failed are both terminal + Expect(snap.LastChildCount).To(Equal(int64(3))) + Expect(snap.RetentionViolations).To(BeZero()) + Expect(snap.GCViolations).To(BeZero()) + }) + + It("passes retention when expiredAt matches stoppedAt + retention", func() { + stopped := now.Add(-time.Hour) + expired := stopped.Add(2 * 24 * time.Hour) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 2, stopped, expired)}, + } + v, m := newTestVerifier(fc) + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().RetentionViolations).To(BeZero()) + }) + + It("flags a retention miscalculation once", func() { + stopped := now.Add(-time.Hour) + // Wrong: only 12h instead of 24h. + expired := stopped.Add(12 * time.Hour) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + } + v, m := newTestVerifier(fc) + + v.checkOnce(context.Background(), now) + v.checkOnce(context.Background(), now) // must not double-count + Expect(m.Snapshot().RetentionViolations).To(Equal(int64(1))) + }) + + It("tolerates sub-tolerance skew", func() { + stopped := now.Add(-time.Hour) + expired := stopped.Add(24 * time.Hour).Add(30 * time.Second) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + } + v, m := newTestVerifier(fc) + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().RetentionViolations).To(BeZero()) + }) + + It("flags a garbage-collection failure once", func() { + stopped := now.Add(-48 * time.Hour) + // Expired 30m ago (well past gcGrace) but still present. + expired := now.Add(-30 * time.Minute) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + } + v, m := newTestVerifier(fc) + + v.checkOnce(context.Background(), now) + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().GCViolations).To(Equal(int64(1))) + }) + + It("does not flag GC within the grace window", func() { + stopped := now.Add(-48 * time.Hour) + expired := now.Add(-1 * time.Minute) // within gcGrace + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + } + v, m := newTestVerifier(fc) + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().GCViolations).To(BeZero()) + }) + + It("survives transient read errors without panicking", func() { + fc := &fakeBackupClient{ + sbErr: errors.New("apiserver throttled"), + childErr: errors.New("apiserver throttled"), + } + v, m := newTestVerifier(fc) + v.checkOnce(context.Background(), now) + snap := m.Snapshot() + Expect(snap.VerifyCycles).To(Equal(int64(1))) + Expect(snap.HasRetentionFailure()).To(BeFalse()) + }) +}) diff --git a/test/longhaul/cmd/longhaul/main.go b/test/longhaul/cmd/longhaul/main.go index c7f375bd8..6541378b5 100644 --- a/test/longhaul/cmd/longhaul/main.go +++ b/test/longhaul/cmd/longhaul/main.go @@ -16,6 +16,7 @@ import ( "os" "time" + "github.com/documentdb/documentdb-operator/test/longhaul/backup" "github.com/documentdb/documentdb-operator/test/longhaul/config" "github.com/documentdb/documentdb-operator/test/longhaul/journal" "github.com/documentdb/documentdb-operator/test/longhaul/monitor" @@ -146,12 +147,34 @@ func run(cfg config.Config) int { scheduler := operations.NewScheduler(ops, healthMon, j, cfg.OpCooldown) go scheduler.Run(ctx) + // Start data-protection verifier (ScheduledBackup + retention). Runs + // concurrently with the scheduler by design — backup is deliberately not + // isolated from topology/chaos operations. + backupMetrics := backup.NewMetrics() + if cfg.BackupEnabled { + verifier := backup.NewVerifier(clusterClient, j, backupMetrics, backup.Config{ + ScheduledBackupName: cfg.ClusterName + "-longhaul", + Schedule: cfg.BackupSchedule, + RetentionDays: cfg.BackupRetentionDays, + VerifyInterval: cfg.BackupVerifyInterval, + }) + if err := verifier.Bootstrap(ctx); err != nil { + // A bootstrap failure is not fatal to the whole run — the workload + // and other operations still provide signal. Surface it loudly. + j.Error("backup", fmt.Sprintf("ScheduledBackup bootstrap failed, backup verification disabled: %v", err)) + } else { + go verifier.Run(ctx) + } + } else { + j.Info("backup", "backup verification disabled via config") + } + // Start metrics sampling goroutine (feeds leak detector). go runMetricsSampling(ctx, clusterClient, leakDetector, j) // Start periodic checkpoint reporter. summaryFunc := func() report.Summary { - return buildSummary(metrics, leakDetector, scheduler, j) + return buildSummary(metrics, backupMetrics, leakDetector, scheduler, j) } reporter := report.NewCheckpointReporter(k8sClientset, cfg.Namespace, cfg.ReportInterval, summaryFunc) go reporter.Run(ctx) @@ -169,7 +192,7 @@ func run(cfg config.Config) int { // here (before os.Exit) so the authoritative verdict reaches the source // of truth that operators consult — the Run() goroutine cannot do this // reliably because os.Exit can kill it mid-Update. - summary := buildSummary(metrics, leakDetector, scheduler, j) + summary := buildSummary(metrics, backupMetrics, leakDetector, scheduler, j) markdown := report.GenerateMarkdown(summary) fmt.Println("\n" + markdown) reporter.EmitFinal() @@ -187,30 +210,39 @@ func run(cfg config.Config) int { } // buildSummary constructs a report.Summary from current state. -func buildSummary(metrics *workload.Metrics, leakDetector *monitor.LeakDetector, scheduler *operations.Scheduler, j *journal.Journal) report.Summary { +func buildSummary(metrics *workload.Metrics, backupMetrics *backup.Metrics, leakDetector *monitor.LeakDetector, scheduler *operations.Scheduler, j *journal.Journal) report.Summary { snap := metrics.Snapshot() + backupSnap := backupMetrics.Snapshot() leakAnalysis := leakDetector.Analyze() result := report.ResultPass failReason := "" - if snap.HasDataLoss() { - result = report.ResultFail - failReason = fmt.Sprintf("data loss: %d gaps, %d checksum errors", - snap.GapsDetected, snap.ChecksumErrors) - } - if j.HasPolicyViolation() { + appendReason := func(msg string) { result = report.ResultFail if failReason != "" { failReason += "; " } - failReason += "outage policy violated" + failReason += msg + } + + if snap.HasDataLoss() { + appendReason(fmt.Sprintf("data loss: %d gaps, %d checksum errors", + snap.GapsDetected, snap.ChecksumErrors)) + } + if j.HasPolicyViolation() { + appendReason("outage policy violated") + } + if backupSnap.HasRetentionFailure() { + appendReason(fmt.Sprintf("backup retention violated: %d retention, %d GC", + backupSnap.RetentionViolations, backupSnap.GCViolations)) } return report.Summary{ Result: result, Duration: snap.Elapsed, Metrics: snap, + Backup: backupSnap, LeakAnalysis: leakAnalysis, OpsExecuted: scheduler.OpsExecuted(), Windows: j.DisruptionWindows(), diff --git a/test/longhaul/config/config.go b/test/longhaul/config/config.go index 5665bcd77..557bb6684 100644 --- a/test/longhaul/config/config.go +++ b/test/longhaul/config/config.go @@ -32,6 +32,12 @@ const ( // Observability and reporting. EnvReportInterval = "LONGHAUL_REPORT_INTERVAL" + // Data protection (ScheduledBackup + retention verification). + EnvBackupEnabled = "LONGHAUL_BACKUP_ENABLED" + EnvBackupSchedule = "LONGHAUL_BACKUP_SCHEDULE" + EnvBackupRetentionDays = "LONGHAUL_BACKUP_RETENTION_DAYS" + EnvBackupVerifyInterval = "LONGHAUL_BACKUP_VERIFY_INTERVAL" + // Operational toggles. EnvResetData = "LONGHAUL_RESET_DATA" ) @@ -73,6 +79,20 @@ type Config struct { // ReportInterval is how often checkpoint reports are generated. ReportInterval time.Duration + // BackupEnabled controls whether the ScheduledBackup + retention + // verifier runs. Default true. + BackupEnabled bool + + // BackupSchedule is the cron expression for the canary ScheduledBackup. + BackupSchedule string + + // BackupRetentionDays is the retention window applied to child backups + // and validated against the operator-computed expiration. + BackupRetentionDays int + + // BackupVerifyInterval is how often the backup verifier polls state. + BackupVerifyInterval time.Duration + // ResetData controls whether the workload collection is dropped on startup. // Default false so that pod restarts preserve durability history; opt in // for fresh local/dev iterations. @@ -93,6 +113,11 @@ func DefaultConfig() Config { MinInstances: 1, MaxInstances: 3, ReportInterval: 1 * time.Hour, + + BackupEnabled: true, + BackupSchedule: "0 */6 * * *", + BackupRetentionDays: 1, + BackupVerifyInterval: 5 * time.Minute, } } @@ -177,6 +202,30 @@ func LoadFromEnv() (Config, error) { cfg.ReportInterval = d } + if v := strings.TrimSpace(strings.ToLower(os.Getenv(EnvBackupEnabled))); v != "" { + cfg.BackupEnabled = v == "true" || v == "1" || v == "yes" + } + + if v := os.Getenv(EnvBackupSchedule); v != "" { + cfg.BackupSchedule = v + } + + if v := os.Getenv(EnvBackupRetentionDays); v != "" { + n, err := strconv.Atoi(v) + if err != nil { + return cfg, fmt.Errorf("invalid %s=%q: %w", EnvBackupRetentionDays, v, err) + } + cfg.BackupRetentionDays = n + } + + if v := os.Getenv(EnvBackupVerifyInterval); v != "" { + d, err := time.ParseDuration(v) + if err != nil { + return cfg, fmt.Errorf("invalid %s=%q: %w", EnvBackupVerifyInterval, v, err) + } + cfg.BackupVerifyInterval = d + } + if v := strings.TrimSpace(strings.ToLower(os.Getenv(EnvResetData))); v != "" { cfg.ResetData = v == "true" || v == "1" || v == "yes" } @@ -213,6 +262,17 @@ func (c *Config) Validate() error { if c.MaxInstances < c.MinInstances { return fmt.Errorf("max instances (%d) must be >= min instances (%d)", c.MaxInstances, c.MinInstances) } + if c.BackupEnabled { + if c.BackupSchedule == "" { + return fmt.Errorf("backup schedule must not be empty when backups are enabled") + } + if c.BackupRetentionDays < 1 { + return fmt.Errorf("backup retention days must be at least 1, got %d", c.BackupRetentionDays) + } + if c.BackupVerifyInterval <= 0 { + return fmt.Errorf("backup verify interval must be positive, got %s", c.BackupVerifyInterval) + } + } return nil } diff --git a/test/longhaul/config/config_test.go b/test/longhaul/config/config_test.go index 506803f52..5341882fc 100644 --- a/test/longhaul/config/config_test.go +++ b/test/longhaul/config/config_test.go @@ -160,6 +160,37 @@ var _ = Describe("Config", func() { cfg.MaxInstances = 4 Expect(cfg.Validate()).To(MatchError(ContainSubstring("must not exceed 3"))) }) + + It("fails when backups enabled but schedule is empty", func() { + cfg := DefaultConfig() + cfg.ClusterName = "test" + cfg.BackupSchedule = "" + Expect(cfg.Validate()).To(MatchError(ContainSubstring("backup schedule"))) + }) + + It("fails when backup retention days is below 1", func() { + cfg := DefaultConfig() + cfg.ClusterName = "test" + cfg.BackupRetentionDays = 0 + Expect(cfg.Validate()).To(MatchError(ContainSubstring("backup retention days"))) + }) + + It("fails when backup verify interval is not positive", func() { + cfg := DefaultConfig() + cfg.ClusterName = "test" + cfg.BackupVerifyInterval = 0 + Expect(cfg.Validate()).To(MatchError(ContainSubstring("backup verify interval"))) + }) + + It("skips backup validation when backups disabled", func() { + cfg := DefaultConfig() + cfg.ClusterName = "test" + cfg.BackupEnabled = false + cfg.BackupSchedule = "" + cfg.BackupRetentionDays = 0 + cfg.BackupVerifyInterval = 0 + Expect(cfg.Validate()).To(Succeed()) + }) }) Describe("IsEnabled", func() { diff --git a/test/longhaul/monitor/k8sclient.go b/test/longhaul/monitor/k8sclient.go index b51ffdd82..6ee80c63d 100644 --- a/test/longhaul/monitor/k8sclient.go +++ b/test/longhaul/monitor/k8sclient.go @@ -11,6 +11,8 @@ import ( "sync/atomic" "time" + cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" @@ -267,3 +269,65 @@ func (k *K8sClusterClient) GetPodMetrics(ctx context.Context) ([]PodMetrics, err func (k *K8sClusterClient) MetricsAvailable() bool { return k.metricsAvail.Load() } + +// scheduledBackupLabel is the label the ScheduledBackup reconciler stamps +// on every child Backup CR it creates (see +// preview.ScheduledBackup.CreateBackup). Selecting on it isolates the +// backups produced by our canary ScheduledBackup from any ad-hoc Backup +// objects that might exist in the namespace. +const scheduledBackupLabel = "scheduledbackup" + +// EnsureScheduledBackup creates the ScheduledBackup CR for the target +// cluster if it does not already exist. It is idempotent: an existing +// ScheduledBackup with the same name is left untouched (a restart of the +// driver must not reset the accumulated backup history). +func (k *K8sClusterClient) EnsureScheduledBackup(ctx context.Context, name, schedule string, retentionDays int) error { + existing := &previewv1.ScheduledBackup{} + err := k.crClient.Get(ctx, types.NamespacedName{Namespace: k.namespace, Name: name}, existing) + if err == nil { + return nil + } + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to get ScheduledBackup %s/%s: %w", k.namespace, name, err) + } + + rd := retentionDays + sb := &previewv1.ScheduledBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: k.namespace, + }, + Spec: previewv1.ScheduledBackupSpec{ + Cluster: cnpgv1.LocalObjectReference{Name: k.clusterName}, + Schedule: schedule, + RetentionDays: &rd, + }, + } + if err := k.crClient.Create(ctx, sb); err != nil { + return fmt.Errorf("failed to create ScheduledBackup %s/%s: %w", k.namespace, name, err) + } + return nil +} + +// GetScheduledBackup fetches the ScheduledBackup CR by name in the target +// namespace. +func (k *K8sClusterClient) GetScheduledBackup(ctx context.Context, name string) (*previewv1.ScheduledBackup, error) { + sb := &previewv1.ScheduledBackup{} + if err := k.crClient.Get(ctx, types.NamespacedName{Namespace: k.namespace, Name: name}, sb); err != nil { + return nil, fmt.Errorf("failed to get ScheduledBackup %s/%s: %w", k.namespace, name, err) + } + return sb, nil +} + +// ListScheduledChildBackups returns every Backup CR that the named +// ScheduledBackup produced, identified by the "scheduledbackup" label. +func (k *K8sClusterClient) ListScheduledChildBackups(ctx context.Context, scheduledBackupName string) ([]previewv1.Backup, error) { + var list previewv1.BackupList + if err := k.crClient.List(ctx, &list, + ctrlclient.InNamespace(k.namespace), + ctrlclient.MatchingLabels{scheduledBackupLabel: scheduledBackupName}, + ); err != nil { + return nil, fmt.Errorf("failed to list child Backups for ScheduledBackup %s: %w", scheduledBackupName, err) + } + return list.Items, nil +} diff --git a/test/longhaul/report/report.go b/test/longhaul/report/report.go index e7316a7f7..1ada5fd23 100644 --- a/test/longhaul/report/report.go +++ b/test/longhaul/report/report.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/documentdb/documentdb-operator/test/longhaul/backup" "github.com/documentdb/documentdb-operator/test/longhaul/journal" "github.com/documentdb/documentdb-operator/test/longhaul/monitor" "github.com/documentdb/documentdb-operator/test/longhaul/workload" @@ -38,6 +39,11 @@ type Summary struct { // failed, verify passes, gaps, checksum errors). Metrics workload.MetricsSnapshot + // Backup is a snapshot of the data-protection counters (backups + // scheduled/completed/failed, retention and GC violations). Retention or + // GC violations flip Result to FAIL. + Backup backup.MetricsSnapshot + // LeakAnalysis is the operator-pod resource trend (memory/CPU slope over // the run); LeakAnalysis.HasLeak being true does NOT flip Result — it // only emits a warning annotation. @@ -90,6 +96,21 @@ func GenerateMarkdown(s Summary) string { fmt.Fprintf(&b, "| Checksum Errors | %d |\n", s.Metrics.ChecksumErrors) b.WriteString("\n") + // Data Protection (ScheduledBackup + retention) + b.WriteString("## Data Protection\n\n") + b.WriteString("| Metric | Value |\n") + b.WriteString("|--------|-------|\n") + fmt.Fprintf(&b, "| Backups Scheduled | %d |\n", s.Backup.Scheduled) + fmt.Fprintf(&b, "| Backups Completed | %d |\n", s.Backup.Completed) + fmt.Fprintf(&b, "| Backups Failed | %d |\n", s.Backup.Failed) + fmt.Fprintf(&b, "| Current Backup Count | %d |\n", s.Backup.LastChildCount) + fmt.Fprintf(&b, "| Retention Violations | %d |\n", s.Backup.RetentionViolations) + fmt.Fprintf(&b, "| Retention GC Violations | %d |\n", s.Backup.GCViolations) + if !s.Backup.LastScheduled.IsZero() { + fmt.Fprintf(&b, "| Last Scheduled | %s |\n", s.Backup.LastScheduled.Format(time.RFC3339)) + } + b.WriteString("\n") + // Disruption Windows if len(s.Windows) > 0 { b.WriteString("## Disruption Windows\n\n") From d73e6af43b21bac2434c135b499afdfa8a3008ee Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 13:23:41 -0400 Subject: [PATCH 2/9] test(longhaul): trim backup verifier to liveness + retention-leak oracle Drop the retention-arithmetic check (expiredAt == stoppedAt + retentionDays*24h). That is a pure function already covered by the operator unit tests (backup_funcs_test.go) and is evaluable on the very first backup, so it does not belong in a multi-day canary. Replace the two retention checks (correctness + GC) with a single black-box retention-leak oracle: a completed backup must not outlive its policy window (stoppedAt + cfg.RetentionDays*24h + grace). The deadline is derived from the driver's own config, not the operator's ExpiredAt field, so the check verifies the outcome (expired backups disappear, population stays bounded) without coupling to operator internals. Keeps: scheduling liveness + completion tracking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/README.md | 28 ++++--- test/longhaul/backup/metrics.go | 81 ++++++++++---------- test/longhaul/backup/metrics_test.go | 22 +++--- test/longhaul/backup/verifier.go | 102 ++++++++++---------------- test/longhaul/backup/verifier_test.go | 62 ++++++---------- test/longhaul/cmd/longhaul/main.go | 6 +- test/longhaul/report/report.go | 9 +-- 7 files changed, 134 insertions(+), 176 deletions(-) diff --git a/test/longhaul/README.md b/test/longhaul/README.md index e772b6536..c043a4f28 100644 --- a/test/longhaul/README.md +++ b/test/longhaul/README.md @@ -143,20 +143,26 @@ All configuration is via environment variables. When `LONGHAUL_BACKUP_ENABLED` is true, the driver bootstraps a `ScheduledBackup` named `-longhaul` and runs a verifier concurrently with the operation scheduler (backup is deliberately **not** isolated from topology/chaos, per the -design). Each verification cycle checks: +design). -- **Scheduling liveness** — `status.lastScheduledTime` advances; a stalled +The verifier only checks the properties a **multi-day** run can establish — +things unit and e2e tests cannot: + +- **Scheduling liveness** — `status.lastScheduledTime` keeps advancing; a stalled scheduler (past `status.nextScheduledTime` + grace) raises a warning. -- **Completion** — child `Backup` CRs reach the `completed` phase; terminal +- **Completion** — child `Backup` CRs keep reaching `completed`; terminal failures (`failed` / `skipped`) are counted. -- **Retention correctness** — every completed backup's `status.expiredAt` - equals `stoppedAt + retentionDays*24h` (within a 2m tolerance). A mismatch is - a **FAIL** (the operator miscalculated retention). -- **Retention GC** — no backup lingers more than 10m past its `status.expiredAt`. - A lingering expired backup is a **FAIL** (retention garbage-collection leak). - -Because the minimum meaningful retention is 1 day, the GC check only fires on -multi-day runs — exactly the accumulation window long-haul exists to cover. +- **Retention leak** — no completed backup outlives its retention window + (`stoppedAt + retentionDays*24h` + grace). A lingering backup is a **FAIL**: + it means expired backups aren't garbage-collected and the population (and its + PVCs / VolumeSnapshots) grows unbounded. + +It deliberately does **not** re-verify the operator's retention *arithmetic* +(`expiredAt == stoppedAt + retentionDays*24h`) — that is a pure function already +covered by the operator's unit tests and needs no accumulation. The oracle here +is black-box: expired backups disappear. Because the minimum meaningful +retention is 1 day, the leak check only fires on multi-day runs — exactly the +accumulation window long-haul exists to cover. > **RBAC.** The driver ServiceAccount needs `create`/`get`/`list` on > `scheduledbackups.documentdb.io` and `list` on `backups.documentdb.io`. These diff --git a/test/longhaul/backup/metrics.go b/test/longhaul/backup/metrics.go index 629f03dea..7c9afb052 100644 --- a/test/longhaul/backup/metrics.go +++ b/test/longhaul/backup/metrics.go @@ -3,10 +3,15 @@ // Package backup implements the data-protection component of the long haul // driver. It provisions a ScheduledBackup against the canary cluster and -// continuously verifies that backups are produced on schedule, complete -// successfully, and that the operator honors the configured retention -// period (expiration is calculated correctly and expired backups are -// garbage-collected). +// continuously verifies the properties that only a multi-day run can +// establish: that backups keep being produced on schedule and completing, +// and that expired backups are actually garbage-collected so the backup +// population stays bounded over time (no PVC / VolumeSnapshot accumulation). +// +// It deliberately does NOT re-verify the operator's retention *arithmetic* +// (expiredAt == stoppedAt + retentionDays*24h) — that is a pure function +// already covered by the operator's unit tests and needs no accumulation. +// The oracle here is black-box: expired backups disappear. // // The component runs concurrently with the operation scheduler — per the // long-haul design, backup is deliberately NOT isolated so that @@ -22,9 +27,10 @@ import ( // Metrics tracks aggregate backup-verification counters using atomic // operations so the reporter goroutine can snapshot them without locking. // -// RetentionViolations and GCViolations are the retention oracle: any -// non-zero value indicates a real operator bug and flips the run verdict -// to FAIL. The remaining counters are observational and feed the report. +// RetentionLeaks is the retention oracle: any non-zero value means a +// completed backup outlived its retention window (the operator failed to +// garbage-collect it) and flips the run verdict to FAIL. The remaining +// counters are observational and feed the report. type Metrics struct { // Scheduled counts backups observed to have been scheduled by the // ScheduledBackup (advances of status.lastScheduledTime). @@ -38,21 +44,20 @@ type Metrics struct { // phase (deduplicated by name). Failed atomic.Int64 - // RetentionViolations counts completed backups whose status.expiredAt - // does not match stoppedAt + retentionDays*24h within tolerance. - // Non-zero => FAIL (operator miscalculated retention). - RetentionViolations atomic.Int64 - - // GCViolations counts backups that remain present well past their - // status.expiredAt — i.e. the operator failed to garbage-collect an - // expired backup. Non-zero => FAIL (retention GC leak). - GCViolations atomic.Int64 + // RetentionLeaks counts completed backups that remain present well past + // their retention window (stoppedAt + retentionDays*24h) — i.e. the + // operator failed to garbage-collect an expired backup, so the backup + // population (and its PVCs/VolumeSnapshots) grows unbounded. + // Non-zero => FAIL. This is the accumulation bug long-haul exists to + // catch; it cannot fire until real retention time has elapsed. + RetentionLeaks atomic.Int64 // VerifyCycles is the number of completed verification passes. VerifyCycles atomic.Int64 // LastChildCount is the number of child backups observed on the most - // recent verification cycle (a proxy for steady-state backup population). + // recent verification cycle (the live backup population — expected to + // stabilize near retentionWindow/scheduleInterval at steady state). LastChildCount atomic.Int64 // LastScheduledUnix is the Unix timestamp of the most recently observed @@ -70,15 +75,14 @@ func NewMetrics() *Metrics { // MetricsSnapshot is a point-in-time copy of Metrics. type MetricsSnapshot struct { - Scheduled int64 - Completed int64 - Failed int64 - RetentionViolations int64 - GCViolations int64 - VerifyCycles int64 - LastChildCount int64 - LastScheduled time.Time - Elapsed time.Duration + Scheduled int64 + Completed int64 + Failed int64 + RetentionLeaks int64 + VerifyCycles int64 + LastChildCount int64 + LastScheduled time.Time + Elapsed time.Duration } // Snapshot captures the current metric values atomically. @@ -88,20 +92,19 @@ func (m *Metrics) Snapshot() MetricsSnapshot { lastScheduled = time.Unix(unix, 0) } return MetricsSnapshot{ - Scheduled: m.Scheduled.Load(), - Completed: m.Completed.Load(), - Failed: m.Failed.Load(), - RetentionViolations: m.RetentionViolations.Load(), - GCViolations: m.GCViolations.Load(), - VerifyCycles: m.VerifyCycles.Load(), - LastChildCount: m.LastChildCount.Load(), - LastScheduled: lastScheduled, - Elapsed: time.Since(m.StartTime), + Scheduled: m.Scheduled.Load(), + Completed: m.Completed.Load(), + Failed: m.Failed.Load(), + RetentionLeaks: m.RetentionLeaks.Load(), + VerifyCycles: m.VerifyCycles.Load(), + LastChildCount: m.LastChildCount.Load(), + LastScheduled: lastScheduled, + Elapsed: time.Since(m.StartTime), } } -// HasRetentionFailure returns true if any retention or GC violation has -// been detected. A true result flips the overall run verdict to FAIL. -func (s MetricsSnapshot) HasRetentionFailure() bool { - return s.RetentionViolations > 0 || s.GCViolations > 0 +// HasRetentionLeak returns true if any completed backup has outlived its +// retention window. A true result flips the overall run verdict to FAIL. +func (s MetricsSnapshot) HasRetentionLeak() bool { + return s.RetentionLeaks > 0 } diff --git a/test/longhaul/backup/metrics_test.go b/test/longhaul/backup/metrics_test.go index bc6d5f8eb..e91fb907b 100644 --- a/test/longhaul/backup/metrics_test.go +++ b/test/longhaul/backup/metrics_test.go @@ -16,8 +16,7 @@ var _ = Describe("Metrics", func() { m.Scheduled.Add(3) m.Completed.Add(2) m.Failed.Add(1) - m.RetentionViolations.Add(1) - m.GCViolations.Add(1) + m.RetentionLeaks.Add(1) m.VerifyCycles.Add(5) m.LastChildCount.Store(7) now := time.Now() @@ -27,8 +26,7 @@ var _ = Describe("Metrics", func() { Expect(snap.Scheduled).To(Equal(int64(3))) Expect(snap.Completed).To(Equal(int64(2))) Expect(snap.Failed).To(Equal(int64(1))) - Expect(snap.RetentionViolations).To(Equal(int64(1))) - Expect(snap.GCViolations).To(Equal(int64(1))) + Expect(snap.RetentionLeaks).To(Equal(int64(1))) Expect(snap.VerifyCycles).To(Equal(int64(5))) Expect(snap.LastChildCount).To(Equal(int64(7))) Expect(snap.LastScheduled.Unix()).To(Equal(now.Unix())) @@ -39,16 +37,14 @@ var _ = Describe("Metrics", func() { Expect(snap.LastScheduled.IsZero()).To(BeTrue()) }) - DescribeTable("HasRetentionFailure", - func(retention, gc int64, want bool) { + DescribeTable("HasRetentionLeak", + func(leaks int64, want bool) { m := NewMetrics() - m.RetentionViolations.Add(retention) - m.GCViolations.Add(gc) - Expect(m.Snapshot().HasRetentionFailure()).To(Equal(want)) + m.RetentionLeaks.Add(leaks) + Expect(m.Snapshot().HasRetentionLeak()).To(Equal(want)) }, - Entry("clean", int64(0), int64(0), false), - Entry("retention violated", int64(1), int64(0), true), - Entry("gc violated", int64(0), int64(1), true), - Entry("both", int64(2), int64(3), true), + Entry("clean", int64(0), false), + Entry("one leak", int64(1), true), + Entry("several leaks", int64(3), true), ) }) diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index edd3a99d1..44e487320 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -15,15 +15,10 @@ import ( ) const ( - // retentionTolerance is the allowed skew between the operator-computed - // status.expiredAt and the value we recompute from stoppedAt + - // retentionDays*24h. The operator derives both from the same stoppedAt, - // so any real drift is tiny; the tolerance only absorbs clock rounding. - retentionTolerance = 2 * time.Minute - - // gcGrace is how long past status.expiredAt a backup may still exist - // before we treat it as a garbage-collection failure. It absorbs the - // backup controller's requeue latency and apiserver propagation. + // gcGrace is how long past a backup's retention deadline + // (stoppedAt + retentionDays*24h) it may still exist before we treat it + // as a garbage-collection leak. It absorbs the backup controller's + // requeue latency and apiserver propagation. gcGrace = 10 * time.Minute // livenessGrace is how far past status.nextScheduledTime the scheduler @@ -64,25 +59,23 @@ type Verifier struct { // dedup state — the verification loop re-observes the same backups every // cycle, so we count each transition/violation exactly once by name. - seenCompleted map[string]struct{} - seenFailed map[string]struct{} - retentionFlagged map[string]struct{} - gcFlagged map[string]struct{} - lastScheduled time.Time - stallWarnedFor time.Time + seenCompleted map[string]struct{} + seenFailed map[string]struct{} + leakFlagged map[string]struct{} + lastScheduled time.Time + stallWarnedFor time.Time } // NewVerifier creates a backup verifier. metrics must be non-nil. func NewVerifier(client Client, j *journal.Journal, metrics *Metrics, cfg Config) *Verifier { return &Verifier{ - client: client, - journal: j, - metrics: metrics, - cfg: cfg, - seenCompleted: make(map[string]struct{}), - seenFailed: make(map[string]struct{}), - retentionFlagged: make(map[string]struct{}), - gcFlagged: make(map[string]struct{}), + client: client, + journal: j, + metrics: metrics, + cfg: cfg, + seenCompleted: make(map[string]struct{}), + seenFailed: make(map[string]struct{}), + leakFlagged: make(map[string]struct{}), } } @@ -172,8 +165,7 @@ func (v *Verifier) checkChildBackups(ctx context.Context, now time.Time) { for i := range children { b := &children[i] v.recordPhase(b) - v.checkRetention(b) - v.checkGarbageCollection(b, now) + v.checkRetentionLeak(b, now) } } @@ -197,51 +189,31 @@ func (v *Verifier) recordPhase(b *previewv1.Backup) { } } -// checkRetention verifies that a completed backup's status.expiredAt equals -// stoppedAt + retentionDays*24h within tolerance. A mismatch is a Fatal -// operator bug and is flagged exactly once per backup. -func (v *Verifier) checkRetention(b *previewv1.Backup) { - if !isCompleted(b) || b.Status.StoppedAt == nil || b.Status.ExpiredAt == nil || b.Spec.RetentionDays == nil { - return - } - if _, done := v.retentionFlagged[b.Name]; done { - return - } - - expected := b.Status.StoppedAt.Time.Add(time.Duration(*b.Spec.RetentionDays) * 24 * time.Hour) - actual := b.Status.ExpiredAt.Time - skew := actual.Sub(expected) - if skew < 0 { - skew = -skew - } - if skew > retentionTolerance { - v.retentionFlagged[b.Name] = struct{}{} - v.metrics.RetentionViolations.Add(1) - v.journal.Error("backup", fmt.Sprintf( - "retention miscalculated for %q: retentionDays=%d stoppedAt=%s expiredAt=%s (expected %s, skew %s)", - b.Name, *b.Spec.RetentionDays, - b.Status.StoppedAt.Time.Format(time.RFC3339), - actual.Format(time.RFC3339), expected.Format(time.RFC3339), skew.Round(time.Second))) - } -} - -// checkGarbageCollection flags a backup that still exists well past its -// status.expiredAt — the operator failed to retire it. Flagged once per -// backup. -func (v *Verifier) checkGarbageCollection(b *previewv1.Backup, now time.Time) { - if b.Status.ExpiredAt == nil { +// checkRetentionLeak flags a completed backup that has outlived its +// retention window. The deadline is derived independently from our own +// configured policy (stoppedAt + retentionDays*24h) rather than from the +// operator-populated status.expiredAt — this keeps the oracle black-box: +// it verifies the *outcome* (expired backups get deleted, so the +// population stays bounded) without re-deriving the operator's internal +// expiration formula, which the operator's own unit tests already cover. +// Flagged once per backup. +func (v *Verifier) checkRetentionLeak(b *previewv1.Backup, now time.Time) { + if !isCompleted(b) || b.Status.StoppedAt == nil { return } - if _, done := v.gcFlagged[b.Name]; done { + if _, done := v.leakFlagged[b.Name]; done { return } - if now.After(b.Status.ExpiredAt.Time.Add(gcGrace)) { - v.gcFlagged[b.Name] = struct{}{} - v.metrics.GCViolations.Add(1) + deadline := b.Status.StoppedAt.Time. + Add(time.Duration(v.cfg.RetentionDays) * 24 * time.Hour). + Add(gcGrace) + if now.After(deadline) { + v.leakFlagged[b.Name] = struct{}{} + v.metrics.RetentionLeaks.Add(1) v.journal.Error("backup", fmt.Sprintf( - "retention GC failure: backup %q still present %s past expiredAt %s", - b.Name, now.Sub(b.Status.ExpiredAt.Time).Round(time.Second), - b.Status.ExpiredAt.Time.Format(time.RFC3339))) + "retention leak: backup %q still present %s past its %d-day retention window (stoppedAt %s)", + b.Name, now.Sub(b.Status.StoppedAt.Time).Round(time.Second), + v.cfg.RetentionDays, b.Status.StoppedAt.Time.Format(time.RFC3339))) } } diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index c1ea6c8af..654a7005e 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -120,11 +120,10 @@ var _ = Describe("Verifier.checkOnce", func() { It("counts completed and failed child backups once each", func() { stopped := now.Add(-30 * time.Minute) - expired := stopped.Add(24 * time.Hour) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, children: []previewv1.Backup{ - mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired), + mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{}), mkBackup("b2", previewv1.BackupPhaseSkipped, 1, time.Time{}, time.Time{}), mkBackup("b3", cnpgv1.BackupPhaseFailed, 1, time.Time{}, time.Time{}), }, @@ -138,74 +137,57 @@ var _ = Describe("Verifier.checkOnce", func() { Expect(snap.Completed).To(Equal(int64(1))) Expect(snap.Failed).To(Equal(int64(2))) // skipped + failed are both terminal Expect(snap.LastChildCount).To(Equal(int64(3))) - Expect(snap.RetentionViolations).To(BeZero()) - Expect(snap.GCViolations).To(BeZero()) + Expect(snap.RetentionLeaks).To(BeZero()) }) - It("passes retention when expiredAt matches stoppedAt + retention", func() { + It("does not flag a fresh completed backup within its retention window", func() { + // retentionDays=1; stopped 1h ago → well within the 1-day window. stopped := now.Add(-time.Hour) - expired := stopped.Add(2 * 24 * time.Hour) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 2, stopped, expired)}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) - Expect(m.Snapshot().RetentionViolations).To(BeZero()) + Expect(m.Snapshot().RetentionLeaks).To(BeZero()) }) - It("flags a retention miscalculation once", func() { - stopped := now.Add(-time.Hour) - // Wrong: only 12h instead of 24h. - expired := stopped.Add(12 * time.Hour) + It("flags a retention leak when a backup outlives its window, once", func() { + // retentionDays=1 (verifier config); stopped 48h ago and still present + // → well past the 1-day window + grace. + stopped := now.Add(-48 * time.Hour) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) v.checkOnce(context.Background(), now) // must not double-count - Expect(m.Snapshot().RetentionViolations).To(Equal(int64(1))) - }) - - It("tolerates sub-tolerance skew", func() { - stopped := now.Add(-time.Hour) - expired := stopped.Add(24 * time.Hour).Add(30 * time.Second) - fc := &fakeBackupClient{ - sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, - } - v, m := newTestVerifier(fc) - v.checkOnce(context.Background(), now) - Expect(m.Snapshot().RetentionViolations).To(BeZero()) + Expect(m.Snapshot().RetentionLeaks).To(Equal(int64(1))) }) - It("flags a garbage-collection failure once", func() { - stopped := now.Add(-48 * time.Hour) - // Expired 30m ago (well past gcGrace) but still present. - expired := now.Add(-30 * time.Minute) + It("does not flag a leak within the GC grace window", func() { + // stopped just over 1 day ago but inside the 10m GC grace. + stopped := now.Add(-24 * time.Hour).Add(-1 * time.Minute) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, } v, m := newTestVerifier(fc) - - v.checkOnce(context.Background(), now) v.checkOnce(context.Background(), now) - Expect(m.Snapshot().GCViolations).To(Equal(int64(1))) + Expect(m.Snapshot().RetentionLeaks).To(BeZero()) }) - It("does not flag GC within the grace window", func() { - stopped := now.Add(-48 * time.Hour) - expired := now.Add(-1 * time.Minute) // within gcGrace + It("ignores incomplete backups for leak detection", func() { + // A never-completed backup has no stoppedAt → not a leak candidate. fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, expired)}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseRunning, 1, time.Time{}, time.Time{})}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) - Expect(m.Snapshot().GCViolations).To(BeZero()) + Expect(m.Snapshot().RetentionLeaks).To(BeZero()) }) It("survives transient read errors without panicking", func() { @@ -217,6 +199,6 @@ var _ = Describe("Verifier.checkOnce", func() { v.checkOnce(context.Background(), now) snap := m.Snapshot() Expect(snap.VerifyCycles).To(Equal(int64(1))) - Expect(snap.HasRetentionFailure()).To(BeFalse()) + Expect(snap.HasRetentionLeak()).To(BeFalse()) }) }) diff --git a/test/longhaul/cmd/longhaul/main.go b/test/longhaul/cmd/longhaul/main.go index 6541378b5..af4de54f4 100644 --- a/test/longhaul/cmd/longhaul/main.go +++ b/test/longhaul/cmd/longhaul/main.go @@ -233,9 +233,9 @@ func buildSummary(metrics *workload.Metrics, backupMetrics *backup.Metrics, leak if j.HasPolicyViolation() { appendReason("outage policy violated") } - if backupSnap.HasRetentionFailure() { - appendReason(fmt.Sprintf("backup retention violated: %d retention, %d GC", - backupSnap.RetentionViolations, backupSnap.GCViolations)) + if backupSnap.HasRetentionLeak() { + appendReason(fmt.Sprintf("backup retention leak: %d expired backups not collected", + backupSnap.RetentionLeaks)) } return report.Summary{ diff --git a/test/longhaul/report/report.go b/test/longhaul/report/report.go index 1ada5fd23..fc2af67b8 100644 --- a/test/longhaul/report/report.go +++ b/test/longhaul/report/report.go @@ -40,8 +40,8 @@ type Summary struct { Metrics workload.MetricsSnapshot // Backup is a snapshot of the data-protection counters (backups - // scheduled/completed/failed, retention and GC violations). Retention or - // GC violations flip Result to FAIL. + // scheduled/completed/failed, live population, retention leaks). A + // retention leak flips Result to FAIL. Backup backup.MetricsSnapshot // LeakAnalysis is the operator-pod resource trend (memory/CPU slope over @@ -103,9 +103,8 @@ func GenerateMarkdown(s Summary) string { fmt.Fprintf(&b, "| Backups Scheduled | %d |\n", s.Backup.Scheduled) fmt.Fprintf(&b, "| Backups Completed | %d |\n", s.Backup.Completed) fmt.Fprintf(&b, "| Backups Failed | %d |\n", s.Backup.Failed) - fmt.Fprintf(&b, "| Current Backup Count | %d |\n", s.Backup.LastChildCount) - fmt.Fprintf(&b, "| Retention Violations | %d |\n", s.Backup.RetentionViolations) - fmt.Fprintf(&b, "| Retention GC Violations | %d |\n", s.Backup.GCViolations) + fmt.Fprintf(&b, "| Live Backup Count | %d |\n", s.Backup.LastChildCount) + fmt.Fprintf(&b, "| Retention Leaks | %d |\n", s.Backup.RetentionLeaks) if !s.Backup.LastScheduled.IsZero() { fmt.Fprintf(&b, "| Last Scheduled | %s |\n", s.Backup.LastScheduled.Format(time.RFC3339)) } From 22e4be56399c1eef2f71e1b38e758eaa95483775 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 16:27:41 -0400 Subject: [PATCH 3/9] test(longhaul): prune redundant comments and dead test params Remove comments that merely restate the code (interface method and Config field doc lines), and fix two comments left stale by the trim: checkChildBackups no longer verifies retention arithmetic, and BackupRetentionDays is no longer validated against operator-computed expiration. De-duplicate the black-box rationale (now stated once in the package doc) from checkRetentionLeak and the RetentionLeaks field. Drop the unused retentionDays/expired parameters from the mkBackup test helper: the leak oracle derives its deadline from the verifier config, so Spec.RetentionDays and Status.ExpiredAt are never read. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/backup/metrics.go | 8 ++------ test/longhaul/backup/verifier.go | 26 ++++++++------------------ test/longhaul/backup/verifier_test.go | 27 ++++++++++++--------------- test/longhaul/config/config.go | 2 +- 4 files changed, 23 insertions(+), 40 deletions(-) diff --git a/test/longhaul/backup/metrics.go b/test/longhaul/backup/metrics.go index 7c9afb052..846a25745 100644 --- a/test/longhaul/backup/metrics.go +++ b/test/longhaul/backup/metrics.go @@ -44,12 +44,8 @@ type Metrics struct { // phase (deduplicated by name). Failed atomic.Int64 - // RetentionLeaks counts completed backups that remain present well past - // their retention window (stoppedAt + retentionDays*24h) — i.e. the - // operator failed to garbage-collect an expired backup, so the backup - // population (and its PVCs/VolumeSnapshots) grows unbounded. - // Non-zero => FAIL. This is the accumulation bug long-haul exists to - // catch; it cannot fire until real retention time has elapsed. + // RetentionLeaks counts completed backups still present past their + // retention window (stoppedAt + retentionDays*24h). Non-zero => FAIL. RetentionLeaks atomic.Int64 // VerifyCycles is the number of completed verification passes. diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index 44e487320..2844027d5 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -29,24 +29,17 @@ const ( // Client is the subset of the cluster API the backup verifier needs. // monitor.K8sClusterClient satisfies it structurally. type Client interface { - // EnsureScheduledBackup idempotently creates the ScheduledBackup CR. EnsureScheduledBackup(ctx context.Context, name, schedule string, retentionDays int) error - // GetScheduledBackup fetches the ScheduledBackup CR by name. GetScheduledBackup(ctx context.Context, name string) (*previewv1.ScheduledBackup, error) - // ListScheduledChildBackups lists the Backup CRs the ScheduledBackup produced. ListScheduledChildBackups(ctx context.Context, scheduledBackupName string) ([]previewv1.Backup, error) } // Config parameterizes the backup verifier. type Config struct { - // ScheduledBackupName is the name of the ScheduledBackup CR to manage. ScheduledBackupName string - // Schedule is the cron expression for the ScheduledBackup. - Schedule string - // RetentionDays is the retention window applied to every child backup. - RetentionDays int - // VerifyInterval is how often the verification loop runs. - VerifyInterval time.Duration + Schedule string + RetentionDays int + VerifyInterval time.Duration } // Verifier bootstraps a ScheduledBackup and continuously checks that the @@ -152,7 +145,7 @@ func (v *Verifier) checkScheduling(ctx context.Context, now time.Time) { } // checkChildBackups inspects every child Backup for completion, terminal -// failure, retention-calculation correctness, and garbage collection. +// failure, and retention leaks. func (v *Verifier) checkChildBackups(ctx context.Context, now time.Time) { children, err := v.client.ListScheduledChildBackups(ctx, v.cfg.ScheduledBackupName) if err != nil { @@ -189,13 +182,10 @@ func (v *Verifier) recordPhase(b *previewv1.Backup) { } } -// checkRetentionLeak flags a completed backup that has outlived its -// retention window. The deadline is derived independently from our own -// configured policy (stoppedAt + retentionDays*24h) rather than from the -// operator-populated status.expiredAt — this keeps the oracle black-box: -// it verifies the *outcome* (expired backups get deleted, so the -// population stays bounded) without re-deriving the operator's internal -// expiration formula, which the operator's own unit tests already cover. +// checkRetentionLeak flags a completed backup still present past its +// retention window. The deadline is derived from our own configured policy +// (stoppedAt + retentionDays*24h + gcGrace), not the operator-populated +// status.expiredAt, so the oracle stays black-box (see package doc). // Flagged once per backup. func (v *Verifier) checkRetentionLeak(b *previewv1.Backup, now time.Time) { if !isCompleted(b) || b.Status.StoppedAt == nil { diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index 654a7005e..3fa307527 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -47,21 +47,18 @@ func (f *fakeBackupClient) ListScheduledChildBackups(_ context.Context, _ string return f.children, f.childErr } -// mkBackup builds a child Backup with the given phase, retention and -// stopped/expired times. Zero times are left nil. -func mkBackup(name string, phase cnpgv1.BackupPhase, retentionDays int, stopped, expired time.Time) previewv1.Backup { - rd := retentionDays +// mkBackup builds a child Backup with the given phase and stopped time. +// A zero stopped time is left nil. RetentionDays/ExpiredAt are omitted +// because the leak oracle derives its deadline from the verifier config, +// not from the backup's own fields. +func mkBackup(name string, phase cnpgv1.BackupPhase, stopped time.Time) previewv1.Backup { b := previewv1.Backup{ ObjectMeta: metav1.ObjectMeta{Name: name}, - Spec: previewv1.BackupSpec{RetentionDays: &rd}, Status: previewv1.BackupStatus{Phase: phase}, } if !stopped.IsZero() { b.Status.StoppedAt = &metav1.Time{Time: stopped} } - if !expired.IsZero() { - b.Status.ExpiredAt = &metav1.Time{Time: expired} - } return b } @@ -123,9 +120,9 @@ var _ = Describe("Verifier.checkOnce", func() { fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, children: []previewv1.Backup{ - mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{}), - mkBackup("b2", previewv1.BackupPhaseSkipped, 1, time.Time{}, time.Time{}), - mkBackup("b3", cnpgv1.BackupPhaseFailed, 1, time.Time{}, time.Time{}), + mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped), + mkBackup("b2", previewv1.BackupPhaseSkipped, time.Time{}), + mkBackup("b3", cnpgv1.BackupPhaseFailed, time.Time{}), }, } v, m := newTestVerifier(fc) @@ -145,7 +142,7 @@ var _ = Describe("Verifier.checkOnce", func() { stopped := now.Add(-time.Hour) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped)}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) @@ -158,7 +155,7 @@ var _ = Describe("Verifier.checkOnce", func() { stopped := now.Add(-48 * time.Hour) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped)}, } v, m := newTestVerifier(fc) @@ -172,7 +169,7 @@ var _ = Describe("Verifier.checkOnce", func() { stopped := now.Add(-24 * time.Hour).Add(-1 * time.Minute) fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, 1, stopped, time.Time{})}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped)}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) @@ -183,7 +180,7 @@ var _ = Describe("Verifier.checkOnce", func() { // A never-completed backup has no stoppedAt → not a leak candidate. fc := &fakeBackupClient{ sb: &previewv1.ScheduledBackup{}, - children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseRunning, 1, time.Time{}, time.Time{})}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseRunning, time.Time{})}, } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) diff --git a/test/longhaul/config/config.go b/test/longhaul/config/config.go index 557bb6684..60b012fdb 100644 --- a/test/longhaul/config/config.go +++ b/test/longhaul/config/config.go @@ -87,7 +87,7 @@ type Config struct { BackupSchedule string // BackupRetentionDays is the retention window applied to child backups - // and validated against the operator-computed expiration. + // and used to derive the retention-leak deadline. BackupRetentionDays int // BackupVerifyInterval is how often the backup verifier polls state. From 743cc2cdf8f8b0467cc8021e07d639a7e8436c0b Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 16:37:26 -0400 Subject: [PATCH 4/9] test(longhaul): drop unused backup metrics fields Remove three Metrics fields that had no report or verdict consumer: - VerifyCycles: incremented every pass but never surfaced in the report or verdict; only its own test read it. - StartTime / Elapsed: the backup snapshot's Elapsed was never read (the report's duration comes from the workload snapshot), and StartTime existed only to compute it. Remaining fields (Scheduled, Completed, Failed, RetentionLeaks, LastChildCount, LastScheduled) each back a report row and/or the FAIL verdict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/backup/metrics.go | 14 ++------------ test/longhaul/backup/metrics_test.go | 2 -- test/longhaul/backup/verifier.go | 1 - test/longhaul/backup/verifier_test.go | 4 +--- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/test/longhaul/backup/metrics.go b/test/longhaul/backup/metrics.go index 846a25745..46f67c333 100644 --- a/test/longhaul/backup/metrics.go +++ b/test/longhaul/backup/metrics.go @@ -48,9 +48,6 @@ type Metrics struct { // retention window (stoppedAt + retentionDays*24h). Non-zero => FAIL. RetentionLeaks atomic.Int64 - // VerifyCycles is the number of completed verification passes. - VerifyCycles atomic.Int64 - // LastChildCount is the number of child backups observed on the most // recent verification cycle (the live backup population — expected to // stabilize near retentionWindow/scheduleInterval at steady state). @@ -59,14 +56,11 @@ type Metrics struct { // LastScheduledUnix is the Unix timestamp of the most recently observed // status.lastScheduledTime; 0 until the first backup is scheduled. LastScheduledUnix atomic.Int64 - - // StartTime is when this Metrics was constructed; resets on pod restart. - StartTime time.Time } -// NewMetrics creates a Metrics with the start time set to now. +// NewMetrics creates an empty Metrics. func NewMetrics() *Metrics { - return &Metrics{StartTime: time.Now()} + return &Metrics{} } // MetricsSnapshot is a point-in-time copy of Metrics. @@ -75,10 +69,8 @@ type MetricsSnapshot struct { Completed int64 Failed int64 RetentionLeaks int64 - VerifyCycles int64 LastChildCount int64 LastScheduled time.Time - Elapsed time.Duration } // Snapshot captures the current metric values atomically. @@ -92,10 +84,8 @@ func (m *Metrics) Snapshot() MetricsSnapshot { Completed: m.Completed.Load(), Failed: m.Failed.Load(), RetentionLeaks: m.RetentionLeaks.Load(), - VerifyCycles: m.VerifyCycles.Load(), LastChildCount: m.LastChildCount.Load(), LastScheduled: lastScheduled, - Elapsed: time.Since(m.StartTime), } } diff --git a/test/longhaul/backup/metrics_test.go b/test/longhaul/backup/metrics_test.go index e91fb907b..08dad75ba 100644 --- a/test/longhaul/backup/metrics_test.go +++ b/test/longhaul/backup/metrics_test.go @@ -17,7 +17,6 @@ var _ = Describe("Metrics", func() { m.Completed.Add(2) m.Failed.Add(1) m.RetentionLeaks.Add(1) - m.VerifyCycles.Add(5) m.LastChildCount.Store(7) now := time.Now() m.LastScheduledUnix.Store(now.Unix()) @@ -27,7 +26,6 @@ var _ = Describe("Metrics", func() { Expect(snap.Completed).To(Equal(int64(2))) Expect(snap.Failed).To(Equal(int64(1))) Expect(snap.RetentionLeaks).To(Equal(int64(1))) - Expect(snap.VerifyCycles).To(Equal(int64(5))) Expect(snap.LastChildCount).To(Equal(int64(7))) Expect(snap.LastScheduled.Unix()).To(Equal(now.Unix())) }) diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index 2844027d5..fd1fa709b 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -107,7 +107,6 @@ func (v *Verifier) Run(ctx context.Context) { func (v *Verifier) checkOnce(ctx context.Context, now time.Time) { v.checkScheduling(ctx, now) v.checkChildBackups(ctx, now) - v.metrics.VerifyCycles.Add(1) } // checkScheduling inspects the ScheduledBackup status: it counts new diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index 3fa307527..586a0e7c8 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -194,8 +194,6 @@ var _ = Describe("Verifier.checkOnce", func() { } v, m := newTestVerifier(fc) v.checkOnce(context.Background(), now) - snap := m.Snapshot() - Expect(snap.VerifyCycles).To(Equal(int64(1))) - Expect(snap.HasRetentionLeak()).To(BeFalse()) + Expect(m.Snapshot().HasRetentionLeak()).To(BeFalse()) }) }) From e1c0532ab5c42d112d32946e68753f87e9baeb8b Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 16:54:28 -0400 Subject: [PATCH 5/9] test(longhaul): reconcile ScheduledBackup on param drift; per-backup retention oracle Previously EnsureScheduledBackup was create-if-not-exists: starting a run with a different schedule/retention silently kept the old CR, while the verifier judged backups against the new config. A retention mismatch could raise a false-positive leak FAIL. Fix both sides: - EnsureScheduledBackup now reconciles an existing CR in place when the requested schedule/retention differ (history preserved, never recreated), so a run's parameters actually take effect. - The retention-leak oracle derives each backup's deadline from that backup's own spec.retentionDays (stamped by the ScheduledBackup at creation), falling back to run config, so it stays correct regardless of later parameter changes or a pre-existing CR. Update Bootstrap/README docs and note the added 'update' RBAC verb on scheduledbackups. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/README.md | 20 ++++++++++------- test/longhaul/backup/verifier.go | 24 +++++++++++++------- test/longhaul/backup/verifier_test.go | 17 ++++++++++++++ test/longhaul/monitor/k8sclient.go | 32 +++++++++++++++++++++++---- 4 files changed, 73 insertions(+), 20 deletions(-) diff --git a/test/longhaul/README.md b/test/longhaul/README.md index c043a4f28..b12a11396 100644 --- a/test/longhaul/README.md +++ b/test/longhaul/README.md @@ -140,10 +140,12 @@ All configuration is via environment variables. ### Data Protection (ScheduledBackup + retention) -When `LONGHAUL_BACKUP_ENABLED` is true, the driver bootstraps a `ScheduledBackup` -named `-longhaul` and runs a verifier concurrently with the operation -scheduler (backup is deliberately **not** isolated from topology/chaos, per the -design). +When `LONGHAUL_BACKUP_ENABLED` is true, the driver ensures a `ScheduledBackup` +named `-longhaul` exists and matches the run's schedule/retention +(an existing CR is reconciled in place, never recreated, so backup history is +preserved across restarts and parameter changes) and runs a verifier +concurrently with the operation scheduler (backup is deliberately **not** +isolated from topology/chaos, per the design). The verifier only checks the properties a **multi-day** run can establish — things unit and e2e tests cannot: @@ -153,9 +155,11 @@ things unit and e2e tests cannot: - **Completion** — child `Backup` CRs keep reaching `completed`; terminal failures (`failed` / `skipped`) are counted. - **Retention leak** — no completed backup outlives its retention window - (`stoppedAt + retentionDays*24h` + grace). A lingering backup is a **FAIL**: - it means expired backups aren't garbage-collected and the population (and its - PVCs / VolumeSnapshots) grows unbounded. + (`stoppedAt + spec.retentionDays*24h` + grace). The window is taken from each + backup's **own** `spec.retentionDays` (stamped at creation), so the check + stays correct even if a later run uses a different retention. A lingering + backup is a **FAIL**: expired backups aren't garbage-collected and the + population (and its PVCs / VolumeSnapshots) grows unbounded. It deliberately does **not** re-verify the operator's retention *arithmetic* (`expiredAt == stoppedAt + retentionDays*24h`) — that is a pure function already @@ -164,7 +168,7 @@ is black-box: expired backups disappear. Because the minimum meaningful retention is 1 day, the leak check only fires on multi-day runs — exactly the accumulation window long-haul exists to cover. -> **RBAC.** The driver ServiceAccount needs `create`/`get`/`list` on +> **RBAC.** The driver ServiceAccount needs `create`/`get`/`list`/`update` on > `scheduledbackups.documentdb.io` and `list` on `backups.documentdb.io`. These > verbs must be present in `deploy/rbac.yaml` (added in the CI/CD PR) for the > backup verifier to function; without them it logs an error and the rest of the diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index fd1fa709b..e9bcec782 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -72,9 +72,10 @@ func NewVerifier(client Client, j *journal.Journal, metrics *Metrics, cfg Config } } -// Bootstrap ensures the ScheduledBackup CR exists. It is safe to call on -// every startup; an existing ScheduledBackup is left untouched so the -// accumulated backup history survives driver restarts. +// Bootstrap ensures the ScheduledBackup CR exists and matches this run's +// schedule/retention. It is safe to call on every startup: an existing CR is +// reconciled in place (never recreated), so the accumulated backup history +// survives driver restarts and parameter changes. func (v *Verifier) Bootstrap(ctx context.Context) error { if err := v.client.EnsureScheduledBackup(ctx, v.cfg.ScheduledBackupName, v.cfg.Schedule, v.cfg.RetentionDays); err != nil { return fmt.Errorf("ensure ScheduledBackup: %w", err) @@ -182,9 +183,12 @@ func (v *Verifier) recordPhase(b *previewv1.Backup) { } // checkRetentionLeak flags a completed backup still present past its -// retention window. The deadline is derived from our own configured policy -// (stoppedAt + retentionDays*24h + gcGrace), not the operator-populated -// status.expiredAt, so the oracle stays black-box (see package doc). +// retention window. The deadline is derived from the backup's own declared +// retention (spec.retentionDays, stamped by the ScheduledBackup) plus +// gcGrace — not the operator-populated status.expiredAt — so the oracle +// stays black-box (see package doc) and stays correct even when the run's +// configured retention differs from what an older backup was created under. +// Falls back to the run config if a backup carries no retention. // Flagged once per backup. func (v *Verifier) checkRetentionLeak(b *previewv1.Backup, now time.Time) { if !isCompleted(b) || b.Status.StoppedAt == nil { @@ -193,8 +197,12 @@ func (v *Verifier) checkRetentionLeak(b *previewv1.Backup, now time.Time) { if _, done := v.leakFlagged[b.Name]; done { return } + retentionDays := v.cfg.RetentionDays + if b.Spec.RetentionDays != nil { + retentionDays = *b.Spec.RetentionDays + } deadline := b.Status.StoppedAt.Time. - Add(time.Duration(v.cfg.RetentionDays) * 24 * time.Hour). + Add(time.Duration(retentionDays) * 24 * time.Hour). Add(gcGrace) if now.After(deadline) { v.leakFlagged[b.Name] = struct{}{} @@ -202,7 +210,7 @@ func (v *Verifier) checkRetentionLeak(b *previewv1.Backup, now time.Time) { v.journal.Error("backup", fmt.Sprintf( "retention leak: backup %q still present %s past its %d-day retention window (stoppedAt %s)", b.Name, now.Sub(b.Status.StoppedAt.Time).Round(time.Second), - v.cfg.RetentionDays, b.Status.StoppedAt.Time.Format(time.RFC3339))) + retentionDays, b.Status.StoppedAt.Time.Format(time.RFC3339))) } } diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index 586a0e7c8..485ee6559 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -187,6 +187,23 @@ var _ = Describe("Verifier.checkOnce", func() { Expect(m.Snapshot().RetentionLeaks).To(BeZero()) }) + It("uses the backup's own retention, not the run config, for the deadline", func() { + // Run config is retentionDays=1, but this backup was created under a + // 7-day policy (e.g. a run started earlier with different params). + // At 2 days old it is well within its own window → not a leak. + stopped := now.Add(-48 * time.Hour) + b := mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped) + rd := 7 + b.Spec.RetentionDays = &rd + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{b}, + } + v, m := newTestVerifier(fc) + v.checkOnce(context.Background(), now) + Expect(m.Snapshot().RetentionLeaks).To(BeZero()) + }) + It("survives transient read errors without panicking", func() { fc := &fakeBackupClient{ sbErr: errors.New("apiserver throttled"), diff --git a/test/longhaul/monitor/k8sclient.go b/test/longhaul/monitor/k8sclient.go index 6ee80c63d..f843f960b 100644 --- a/test/longhaul/monitor/k8sclient.go +++ b/test/longhaul/monitor/k8sclient.go @@ -278,14 +278,15 @@ func (k *K8sClusterClient) MetricsAvailable() bool { const scheduledBackupLabel = "scheduledbackup" // EnsureScheduledBackup creates the ScheduledBackup CR for the target -// cluster if it does not already exist. It is idempotent: an existing -// ScheduledBackup with the same name is left untouched (a restart of the -// driver must not reset the accumulated backup history). +// cluster, or reconciles an existing one so a run started with a different +// schedule/retention actually takes effect. Existing child backups and their +// accumulated history are preserved — the CR is updated in place, never +// recreated — so only backups created after the update pick up the new spec. func (k *K8sClusterClient) EnsureScheduledBackup(ctx context.Context, name, schedule string, retentionDays int) error { existing := &previewv1.ScheduledBackup{} err := k.crClient.Get(ctx, types.NamespacedName{Namespace: k.namespace, Name: name}, existing) if err == nil { - return nil + return k.reconcileScheduledBackup(ctx, existing, schedule, retentionDays) } if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get ScheduledBackup %s/%s: %w", k.namespace, name, err) @@ -309,6 +310,29 @@ func (k *K8sClusterClient) EnsureScheduledBackup(ctx context.Context, name, sche return nil } +// reconcileScheduledBackup updates an existing ScheduledBackup in place when +// its schedule or retention differs from what this run requested. +func (k *K8sClusterClient) reconcileScheduledBackup(ctx context.Context, existing *previewv1.ScheduledBackup, schedule string, retentionDays int) error { + curRetention := 0 + if existing.Spec.RetentionDays != nil { + curRetention = *existing.Spec.RetentionDays + } + if existing.Spec.Schedule == schedule && curRetention == retentionDays { + return nil + } + + log.Printf("[k8sclient] ScheduledBackup %s/%s spec drift; updating (schedule %q->%q, retentionDays %d->%d)", + k.namespace, existing.Name, existing.Spec.Schedule, schedule, curRetention, retentionDays) + + rd := retentionDays + existing.Spec.Schedule = schedule + existing.Spec.RetentionDays = &rd + if err := k.crClient.Update(ctx, existing); err != nil { + return fmt.Errorf("failed to update ScheduledBackup %s/%s: %w", k.namespace, existing.Name, err) + } + return nil +} + // GetScheduledBackup fetches the ScheduledBackup CR by name in the target // namespace. func (k *K8sClusterClient) GetScheduledBackup(ctx context.Context, name string) (*previewv1.ScheduledBackup, error) { From ce3a8ed5701cebfd0c3cc8e322301ad1c7d0baf9 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 16:56:39 -0400 Subject: [PATCH 6/9] test(longhaul): bound backup verifier dedup maps to live population The seenCompleted/seenFailed/leakFlagged maps were keyed by backup name and never pruned, so they grew monotonically for the life of the process. The rate is tiny (~4/day at the default schedule), but a driver built to catch accumulation bugs should not itself accumulate unbounded state. Prune each map to the set of backups present in the current list every cycle. Backup names are timestamp-unique and never reused, so a pruned entry cannot reappear and be double-counted; the cumulative counters are unaffected. This bounds the maps to the live backup population. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/backup/verifier.go | 22 ++++++++++++++++++++++ test/longhaul/backup/verifier_test.go | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index e9bcec782..284ee9010 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -52,6 +52,8 @@ type Verifier struct { // dedup state — the verification loop re-observes the same backups every // cycle, so we count each transition/violation exactly once by name. + // Pruned to the live backup set each cycle (see checkChildBackups) so + // these maps stay bounded over a multi-day run. seenCompleted map[string]struct{} seenFailed map[string]struct{} leakFlagged map[string]struct{} @@ -155,11 +157,31 @@ func (v *Verifier) checkChildBackups(ctx context.Context, now time.Time) { v.metrics.LastChildCount.Store(int64(len(children))) + present := make(map[string]struct{}, len(children)) for i := range children { b := &children[i] + present[b.Name] = struct{}{} v.recordPhase(b) v.checkRetentionLeak(b, now) } + + // Prune dedup state for backups the operator has garbage-collected, so + // the maps stay bounded by the live population over a multi-day run + // instead of growing for the life of the process. Backup names are + // timestamp-unique and never reused, so a pruned entry cannot reappear + // and be double-counted. + pruneAbsent(v.seenCompleted, present) + pruneAbsent(v.seenFailed, present) + pruneAbsent(v.leakFlagged, present) +} + +// pruneAbsent deletes keys from seen that are not in present. +func pruneAbsent(seen, present map[string]struct{}) { + for name := range seen { + if _, ok := present[name]; !ok { + delete(seen, name) + } + } } // recordPhase counts new completed / terminally-failed child backups, diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index 485ee6559..81c754abf 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -204,6 +204,27 @@ var _ = Describe("Verifier.checkOnce", func() { Expect(m.Snapshot().RetentionLeaks).To(BeZero()) }) + It("prunes dedup state for garbage-collected backups without double-counting", func() { + stopped := now.Add(-30 * time.Minute) + fc := &fakeBackupClient{ + sb: &previewv1.ScheduledBackup{}, + children: []previewv1.Backup{mkBackup("b1", cnpgv1.BackupPhaseCompleted, stopped)}, + } + v, m := newTestVerifier(fc) + + v.checkOnce(context.Background(), now) + Expect(v.seenCompleted).To(HaveLen(1)) + Expect(m.Snapshot().Completed).To(Equal(int64(1))) + + // b1 is garbage-collected and replaced by a new backup b2. + fc.children = []previewv1.Backup{mkBackup("b2", cnpgv1.BackupPhaseCompleted, stopped)} + v.checkOnce(context.Background(), now) + + // Map stays bounded to the live set; b1 pruned, b2 counted once. + Expect(v.seenCompleted).To(HaveLen(1)) + Expect(m.Snapshot().Completed).To(Equal(int64(2))) + }) + It("survives transient read errors without panicking", func() { fc := &fakeBackupClient{ sbErr: errors.New("apiserver throttled"), From 04d7cc6506516c256ba277a2868da326d7a36e9b Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 16:58:21 -0400 Subject: [PATCH 7/9] docs(longhaul): correct AKS prerequisite to k8s >= 1.35 The operator requires ImageVolume support (GA in Kubernetes 1.35) and crash-loops on older nodes, so the setup prerequisite of k8s >= 1.30 was incorrect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/deploy/setup.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/longhaul/deploy/setup.yaml b/test/longhaul/deploy/setup.yaml index e05115567..8f3f4fe81 100644 --- a/test/longhaul/deploy/setup.yaml +++ b/test/longhaul/deploy/setup.yaml @@ -7,7 +7,7 @@ # It creates the target namespace, credentials secret, and DocumentDB CR. # # Prerequisites: -# 1. AKS cluster with k8s >= 1.30 +# 1. AKS cluster with k8s >= 1.35 (operator requires ImageVolume, GA in 1.35) # 2. cert-manager installed: # helm repo add jetstack https://charts.jetstack.io && helm repo update # helm install cert-manager jetstack/cert-manager --namespace cert-manager \ From e6f1b2ce030fd1b346fdd6524e760e44aa3d9a97 Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 17:07:03 -0400 Subject: [PATCH 8/9] test(longhaul): stop counting skipped backups as failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BackupPhaseSkipped is set by the operator when a backup is intentionally not taken (e.g. the cluster is a non-primary/standby) — an expected no-op, not a failure of the backup machinery. Lumping it into isTerminalFailure inflated the Failed count and logged an intentional skip as a 'terminal failure phase'. Treat only BackupPhaseFailed as a genuine terminal failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/backup/verifier.go | 17 ++++++----------- test/longhaul/backup/verifier_test.go | 2 +- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index 284ee9010..202e9b77f 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -241,16 +241,11 @@ func isCompleted(b *previewv1.Backup) bool { return b != nil && b.Status.Phase == cnpgv1.BackupPhaseCompleted } -// isTerminalFailure reports whether a Backup reached a phase from which no -// reconcile will recover it. +// isTerminalFailure reports whether a Backup reached a genuine failure +// phase from which no reconcile will recover it. BackupPhaseSkipped is +// deliberately excluded: the operator emits it when a backup is +// intentionally not taken (e.g. the cluster is a non-primary/standby), which +// is an expected no-op, not a failure of the backup machinery. func isTerminalFailure(b *previewv1.Backup) bool { - if b == nil { - return false - } - switch b.Status.Phase { - case cnpgv1.BackupPhaseFailed, previewv1.BackupPhaseSkipped: - return true - default: - return false - } + return b != nil && b.Status.Phase == cnpgv1.BackupPhaseFailed } diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index 81c754abf..bfa129419 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -132,7 +132,7 @@ var _ = Describe("Verifier.checkOnce", func() { snap := m.Snapshot() Expect(snap.Completed).To(Equal(int64(1))) - Expect(snap.Failed).To(Equal(int64(2))) // skipped + failed are both terminal + Expect(snap.Failed).To(Equal(int64(1))) // skipped is not a failure Expect(snap.LastChildCount).To(Equal(int64(3))) Expect(snap.RetentionLeaks).To(BeZero()) }) From 676b00a9ad6711047acd948d12fee8ddacf0712a Mon Sep 17 00:00:00 2001 From: Wenting Wu Date: Thu, 9 Jul 2026 17:12:30 -0400 Subject: [PATCH 9/9] test(longhaul): make backup verify interval a package const The backup verifier's poll cadence is an internal implementation detail, not a test-shaping parameter. Collapse LONGHAUL_BACKUP_VERIFY_INTERVAL (env var, Config field, default, parsing, validation) into a verifyInterval const in the backup package, matching workload.verifier's existing pattern. Also drop the stale 'validated against operator-computed expiration' note from the retention-days README row. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Wenting Wu --- test/longhaul/README.md | 3 +-- test/longhaul/backup/verifier.go | 6 ++++-- test/longhaul/backup/verifier_test.go | 1 - test/longhaul/cmd/longhaul/main.go | 1 - test/longhaul/config/config.go | 28 ++++++--------------------- test/longhaul/config/config_test.go | 8 -------- 6 files changed, 11 insertions(+), 36 deletions(-) diff --git a/test/longhaul/README.md b/test/longhaul/README.md index b12a11396..b649fc36f 100644 --- a/test/longhaul/README.md +++ b/test/longhaul/README.md @@ -134,8 +134,7 @@ All configuration is via environment variables. | `LONGHAUL_REPORT_INTERVAL` | No | `1h` | How often to write checkpoint reports to ConfigMap. | | `LONGHAUL_BACKUP_ENABLED` | No | `true` | Enable the ScheduledBackup + retention verifier. | | `LONGHAUL_BACKUP_SCHEDULE` | No | `0 */6 * * *` | Cron schedule for the canary `ScheduledBackup`. | -| `LONGHAUL_BACKUP_RETENTION_DAYS` | No | `1` | Retention window applied to child backups and validated against the operator-computed expiration. | -| `LONGHAUL_BACKUP_VERIFY_INTERVAL` | No | `5m` | How often the backup verifier polls scheduling / completion / retention. | +| `LONGHAUL_BACKUP_RETENTION_DAYS` | No | `1` | Retention window applied to child backups; also used to derive the retention-leak deadline. | | `LONGHAUL_RESET_DATA` | No | `false` | If `true`, drop the workload collection on startup. Off by default so a Deployment pod restart preserves durability history. | ### Data Protection (ScheduledBackup + retention) diff --git a/test/longhaul/backup/verifier.go b/test/longhaul/backup/verifier.go index 202e9b77f..7d9933161 100644 --- a/test/longhaul/backup/verifier.go +++ b/test/longhaul/backup/verifier.go @@ -24,6 +24,9 @@ const ( // livenessGrace is how far past status.nextScheduledTime the scheduler // may run before we warn that scheduling appears stalled. livenessGrace = 5 * time.Minute + + // verifyInterval is how often the verification loop runs. + verifyInterval = 5 * time.Minute ) // Client is the subset of the cluster API the backup verifier needs. @@ -39,7 +42,6 @@ type Config struct { ScheduledBackupName string Schedule string RetentionDays int - VerifyInterval time.Duration } // Verifier bootstraps a ScheduledBackup and continuously checks that the @@ -92,7 +94,7 @@ func (v *Verifier) Run(ctx context.Context) { v.journal.Info("backup", "backup verifier started") defer v.journal.Info("backup", "backup verifier stopped") - ticker := time.NewTicker(v.cfg.VerifyInterval) + ticker := time.NewTicker(verifyInterval) defer ticker.Stop() for { diff --git a/test/longhaul/backup/verifier_test.go b/test/longhaul/backup/verifier_test.go index bfa129419..f58fdecfb 100644 --- a/test/longhaul/backup/verifier_test.go +++ b/test/longhaul/backup/verifier_test.go @@ -68,7 +68,6 @@ func newTestVerifier(client Client) (*Verifier, *Metrics) { ScheduledBackupName: "cluster-longhaul", Schedule: "0 */6 * * *", RetentionDays: 1, - VerifyInterval: time.Minute, }) return v, m } diff --git a/test/longhaul/cmd/longhaul/main.go b/test/longhaul/cmd/longhaul/main.go index af4de54f4..7879cb4a3 100644 --- a/test/longhaul/cmd/longhaul/main.go +++ b/test/longhaul/cmd/longhaul/main.go @@ -156,7 +156,6 @@ func run(cfg config.Config) int { ScheduledBackupName: cfg.ClusterName + "-longhaul", Schedule: cfg.BackupSchedule, RetentionDays: cfg.BackupRetentionDays, - VerifyInterval: cfg.BackupVerifyInterval, }) if err := verifier.Bootstrap(ctx); err != nil { // A bootstrap failure is not fatal to the whole run — the workload diff --git a/test/longhaul/config/config.go b/test/longhaul/config/config.go index 60b012fdb..6528b23cc 100644 --- a/test/longhaul/config/config.go +++ b/test/longhaul/config/config.go @@ -33,10 +33,9 @@ const ( EnvReportInterval = "LONGHAUL_REPORT_INTERVAL" // Data protection (ScheduledBackup + retention verification). - EnvBackupEnabled = "LONGHAUL_BACKUP_ENABLED" - EnvBackupSchedule = "LONGHAUL_BACKUP_SCHEDULE" - EnvBackupRetentionDays = "LONGHAUL_BACKUP_RETENTION_DAYS" - EnvBackupVerifyInterval = "LONGHAUL_BACKUP_VERIFY_INTERVAL" + EnvBackupEnabled = "LONGHAUL_BACKUP_ENABLED" + EnvBackupSchedule = "LONGHAUL_BACKUP_SCHEDULE" + EnvBackupRetentionDays = "LONGHAUL_BACKUP_RETENTION_DAYS" // Operational toggles. EnvResetData = "LONGHAUL_RESET_DATA" @@ -90,9 +89,6 @@ type Config struct { // and used to derive the retention-leak deadline. BackupRetentionDays int - // BackupVerifyInterval is how often the backup verifier polls state. - BackupVerifyInterval time.Duration - // ResetData controls whether the workload collection is dropped on startup. // Default false so that pod restarts preserve durability history; opt in // for fresh local/dev iterations. @@ -114,10 +110,9 @@ func DefaultConfig() Config { MaxInstances: 3, ReportInterval: 1 * time.Hour, - BackupEnabled: true, - BackupSchedule: "0 */6 * * *", - BackupRetentionDays: 1, - BackupVerifyInterval: 5 * time.Minute, + BackupEnabled: true, + BackupSchedule: "0 */6 * * *", + BackupRetentionDays: 1, } } @@ -218,14 +213,6 @@ func LoadFromEnv() (Config, error) { cfg.BackupRetentionDays = n } - if v := os.Getenv(EnvBackupVerifyInterval); v != "" { - d, err := time.ParseDuration(v) - if err != nil { - return cfg, fmt.Errorf("invalid %s=%q: %w", EnvBackupVerifyInterval, v, err) - } - cfg.BackupVerifyInterval = d - } - if v := strings.TrimSpace(strings.ToLower(os.Getenv(EnvResetData))); v != "" { cfg.ResetData = v == "true" || v == "1" || v == "yes" } @@ -269,9 +256,6 @@ func (c *Config) Validate() error { if c.BackupRetentionDays < 1 { return fmt.Errorf("backup retention days must be at least 1, got %d", c.BackupRetentionDays) } - if c.BackupVerifyInterval <= 0 { - return fmt.Errorf("backup verify interval must be positive, got %s", c.BackupVerifyInterval) - } } return nil } diff --git a/test/longhaul/config/config_test.go b/test/longhaul/config/config_test.go index 5341882fc..d035d8525 100644 --- a/test/longhaul/config/config_test.go +++ b/test/longhaul/config/config_test.go @@ -175,20 +175,12 @@ var _ = Describe("Config", func() { Expect(cfg.Validate()).To(MatchError(ContainSubstring("backup retention days"))) }) - It("fails when backup verify interval is not positive", func() { - cfg := DefaultConfig() - cfg.ClusterName = "test" - cfg.BackupVerifyInterval = 0 - Expect(cfg.Validate()).To(MatchError(ContainSubstring("backup verify interval"))) - }) - It("skips backup validation when backups disabled", func() { cfg := DefaultConfig() cfg.ClusterName = "test" cfg.BackupEnabled = false cfg.BackupSchedule = "" cfg.BackupRetentionDays = 0 - cfg.BackupVerifyInterval = 0 Expect(cfg.Validate()).To(Succeed()) }) })