From 8cf03998ddfc643830195128d0c72785ed24cfd0 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 14 Jul 2026 09:47:39 -0700 Subject: [PATCH] Reset backupLastSuccessfulTimestamp during periodic resync The periodic backup metrics resync in updateTotalBackupMetric only set backupLastSuccessfulTimestamp values but never removed stale entries. When a schedule was deleted and its backups removed, the gauge persisted until the Velero pod was restarted. Reset the gauge before re-setting current values so that deleted schedules are pruned automatically each resync cycle. Fixes #9239 Signed-off-by: Shubham Pampattiwar --- pkg/controller/backup_controller.go | 4 ++- pkg/controller/backup_controller_test.go | 32 ++++++++++++++++++++++++ pkg/metrics/metrics.go | 17 +++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 74b857fd2..7a58424eb 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -215,7 +215,9 @@ func (b *backupReconciler) updateTotalBackupMetric() { } // recompute backup_last_successful_timestamp metric for each - // schedule (including the empty schedule, i.e. ad-hoc backups) + // schedule (including the empty schedule, i.e. ad-hoc backups). + // Reset first to prune stale entries for deleted schedules. + b.metrics.ResetBackupLastSuccessfulTimestamp() for schedule, timestamp := range getLastSuccessBySchedule(backups.Items) { b.metrics.SetBackupLastSuccessfulTimestamp(schedule, timestamp) } diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index bab98efb6..3a1903e0f 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -2041,6 +2041,38 @@ func Test_getLastSuccessBySchedule(t *testing.T) { } } +// Test_updateTotalBackupMetric_prunesStaleTimestamps verifies that the periodic +// resync removes backupLastSuccessfulTimestamp entries for schedules that no longer +// have any completed backups (e.g. after the schedule and its backups are deleted). +func Test_updateTotalBackupMetric_prunesStaleTimestamps(t *testing.T) { + baseTime, err := time.Parse(time.RFC1123, time.RFC1123) + require.NoError(t, err) + + m := metrics.NewServerMetrics() + + // Simulate a previous resync that set the metric for "deleted-schedule" + m.SetBackupLastSuccessfulTimestamp("deleted-schedule", baseTime) + require.Equal(t, 1, m.BackupLastSuccessfulTimestampCount()) + + // Current backups only contain entries for "active-schedule" + backups := []velerov1api.Backup{ + *builder.ForBackup("velero", "b1"). + ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "active-schedule")). + Phase(velerov1api.BackupPhaseCompleted). + CompletionTimestamp(baseTime). + Result(), + } + + // Replicate the resync logic: reset then set + m.ResetBackupLastSuccessfulTimestamp() + for schedule, timestamp := range getLastSuccessBySchedule(backups) { + m.SetBackupLastSuccessfulTimestamp(schedule, timestamp) + } + + // Only "active-schedule" should remain; "deleted-schedule" should be pruned + assert.Equal(t, 1, m.BackupLastSuccessfulTimestampCount()) +} + // Unit tests to make sure that the backup's status is updated correctly during reconcile. // To clear up confusion whether status can be updated with Patch alone without status writer and not kbClient.Status().Patch() func TestPatchResourceWorksWithStatus(t *testing.T) { diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index d54eb02b5..d95867fd1 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -765,6 +765,23 @@ func (m *ServerMetrics) ResetBackupLastSuccessfulTimestamp() { } } +// BackupLastSuccessfulTimestampCount returns the number of active time series +// in the backupLastSuccessfulTimestamp gauge. +func (m *ServerMetrics) BackupLastSuccessfulTimestampCount() int { + g, ok := m.metrics[backupLastSuccessfulTimestamp].(*prometheus.GaugeVec) + if !ok { + return 0 + } + ch := make(chan prometheus.Metric, 100) + g.Collect(ch) + close(ch) + count := 0 + for range ch { + count++ + } + return count +} + // SetBackupTarballSizeBytesGauge records the size, in bytes, of a backup tarball. func (m *ServerMetrics) SetBackupTarballSizeBytesGauge(backupSchedule string, size int64) { if g, ok := m.metrics[backupTarballSizeBytesGauge].(*prometheus.GaugeVec); ok {