mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-07-30 12:02:44 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #10000 from shubham-pampattiwar/fix/cleanup-stale-backup-metrics

Browse Source 
Fix stale backupLastSuccessfulTimestamp metric after schedule deletion
This commit is contained in:
Shubham Pampattiwar
2026-07-22 23:26:02 -07:00
committed by GitHub
parent d69f6abe5c 0f01b534c3
commit c676fdd03e
5 changed files with 138 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changelogs/unreleased/10000-shubham-pampattiwar
+1
View File
@@ -0,0 +1 @@
Fix stale backupLastSuccessfulTimestamp metric after schedule deletion
pkg/controller/backup_controller.go
+59 -43
View File
@@ -84,33 +84,34 @@ var autoExcludeClusterScopedResources = []string{
}
type backupReconciler struct {
ctx context.Context
logger logrus.FieldLogger
discoveryHelper discovery.Helper
backupper pkgbackup.Backupper
kbClient kbclient.Client
clock clock.WithTickerAndDelayedExecution
backupLogLevel logrus.Level
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager
backupTracker BackupTracker
defaultBackupLocation string
defaultVolumesToFsBackup bool
defaultBackupTTL time.Duration
defaultVGSLabelKey string
defaultCSISnapshotTimeout time.Duration
resourceTimeout time.Duration
defaultItemOperationTimeout time.Duration
defaultSnapshotLocations map[string]string
metrics *metrics.ServerMetrics
backupStoreGetter persistence.ObjectBackupStoreGetter
formatFlag logging.Format
credentialFileStore credentials.FileStore
maxConcurrentK8SConnections int
defaultSnapshotMoveData bool
globalCRClient kbclient.Client
itemBlockWorkerCount int
concurrentBackups int
globalVolumePoliciesConfigMap string
ctx context.Context
logger logrus.FieldLogger
discoveryHelper discovery.Helper
backupper pkgbackup.Backupper
kbClient kbclient.Client
clock clock.WithTickerAndDelayedExecution
backupLogLevel logrus.Level
newPluginManager func(logrus.FieldLogger) clientmgmt.Manager
backupTracker BackupTracker
defaultBackupLocation string
defaultVolumesToFsBackup bool
defaultBackupTTL time.Duration
defaultVGSLabelKey string
defaultCSISnapshotTimeout time.Duration
resourceTimeout time.Duration
defaultItemOperationTimeout time.Duration
defaultSnapshotLocations map[string]string
metrics *metrics.ServerMetrics
backupStoreGetter persistence.ObjectBackupStoreGetter
formatFlag logging.Format
credentialFileStore credentials.FileStore
maxConcurrentK8SConnections int
defaultSnapshotMoveData bool
globalCRClient kbclient.Client
itemBlockWorkerCount int
concurrentBackups int
globalVolumePoliciesConfigMap string
knownSchedulesWithSuccessfulBackup sets.Set[string]
}
func NewBackupReconciler(
@@ -204,28 +205,43 @@ func (b *backupReconciler) updateTotalBackupMetric() {
time.Sleep(5 * time.Second)
wait.Until(
func() {
// recompute backup_total metric
backups := &velerov1api.BackupList{}
err := b.kbClient.List(context.Background(), backups, &kbclient.ListOptions{LabelSelector: labels.Everything()})
if err != nil {
b.logger.Error(err, "Error computing backup_total metric")
} else {
b.metrics.SetBackupTotal(int64(len(backups.Items)))
}
// recompute backup_last_successful_timestamp metric for each
// schedule (including the empty schedule, i.e. ad-hoc backups)
for schedule, timestamp := range getLastSuccessBySchedule(backups.Items) {
b.metrics.SetBackupLastSuccessfulTimestamp(schedule, timestamp)
}
},
b.resyncBackupMetrics,
backupResyncPeriod,
b.ctx.Done(),
)
}()
}
func (b *backupReconciler) resyncBackupMetrics() {
backups := &velerov1api.BackupList{}
err := b.kbClient.List(context.Background(), backups, &kbclient.ListOptions{LabelSelector: labels.Everything()})
if err != nil {
b.logger.Error(err, "Error computing backup_total metric")
return
}
b.metrics.SetBackupTotal(int64(len(backups.Items)))
currentSchedules := getLastSuccessBySchedule(backups.Items)
for schedule, timestamp := range currentSchedules {
b.metrics.SetBackupLastSuccessfulTimestamp(schedule, timestamp)
}
// Remove metrics for schedules that no longer have successful backups
if b.knownSchedulesWithSuccessfulBackup != nil {
for schedule := range b.knownSchedulesWithSuccessfulBackup {
if _, exists := currentSchedules[schedule]; !exists {
b.metrics.DeleteBackupLastSuccessfulTimestamp(schedule)
}
}
}
b.knownSchedulesWithSuccessfulBackup = sets.New[string]()
for schedule := range currentSchedules {
b.knownSchedulesWithSuccessfulBackup.Insert(schedule)
}
}
// getLastSuccessBySchedule finds the most recent completed backup for each schedule
// and returns a map of schedule name -> completion time of the most recent completed
// backup. This map includes an entry for ad-hoc/non-scheduled backups, where the key
pkg/controller/backup_controller_test.go
+43
View File
@@ -31,6 +31,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
@@ -2041,6 +2042,48 @@ func Test_getLastSuccessBySchedule(t *testing.T) {
}
}
// Test_resyncBackupMetrics_prunesStaleTimestamps verifies that resyncBackupMetrics
// removes backupLastSuccessfulTimestamp entries for schedules that no longer have
// any completed backups (e.g. after the schedule and its backups are deleted).
func Test_resyncBackupMetrics_prunesStaleTimestamps(t *testing.T) {
baseTime, err := time.Parse(time.RFC1123, time.RFC1123)
require.NoError(t, err)
m := metrics.NewServerMetrics()
gauge := m.Metrics()["backup_last_successful_timestamp"]
activeBackup := builder.ForBackup("velero", "b1").
ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "active-schedule")).
Phase(velerov1api.BackupPhaseCompleted).
CompletionTimestamp(baseTime).
Result()
deletedBackup := builder.ForBackup("velero", "b2").
ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "deleted-schedule")).
Phase(velerov1api.BackupPhaseCompleted).
CompletionTimestamp(baseTime).
Result()
fakeClient := velerotest.NewFakeControllerRuntimeClient(t, activeBackup, deletedBackup)
c := &backupReconciler{
kbClient: fakeClient,
logger: logrus.StandardLogger(),
metrics: m,
}
// First resync: sets metrics for both schedules
c.resyncBackupMetrics()
assert.Equal(t, 2, testutil.CollectAndCount(gauge))
// Simulate schedule deletion: remove the backup for "deleted-schedule"
require.NoError(t, fakeClient.Delete(t.Context(), deletedBackup))
// Second resync: prunes "deleted-schedule" metric, keeps "active-schedule"
c.resyncBackupMetrics()
assert.Equal(t, 1, testutil.CollectAndCount(gauge))
}
// 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) {
pkg/metrics/metrics.go
+8
View File
@@ -758,6 +758,14 @@ func (m *ServerMetrics) RegisterPodVolumeOpLatencyGauge(node, pvbName, opName, b
}
}
// DeleteBackupLastSuccessfulTimestamp removes the backupLastSuccessfulTimestamp
// metric for a single schedule.
func (m *ServerMetrics) DeleteBackupLastSuccessfulTimestamp(scheduleName string) {
if g, ok := m.metrics[backupLastSuccessfulTimestamp].(*prometheus.GaugeVec); ok {
g.DeleteLabelValues(scheduleName)
}
}
// 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 {
pkg/metrics/metrics_test.go
+27
View File
@@ -21,6 +21,7 @@ import (
"time"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
dto "github.com/prometheus/client_model/go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -457,6 +458,32 @@ func getHistogramCount(t *testing.T, vec *prometheus.HistogramVec, scheduleLabel
return 0
}
// TestDeleteBackupLastSuccessfulTimestamp verifies that DeleteBackupLastSuccessfulTimestamp
// removes only the specified schedule's metric.
func TestDeleteBackupLastSuccessfulTimestamp(t *testing.T) {
m := NewServerMetrics()
now := time.Now()
m.SetBackupLastSuccessfulTimestamp("schedule-1", now)
m.SetBackupLastSuccessfulTimestamp("schedule-2", now.Add(-time.Hour))
m.SetBackupLastSuccessfulTimestamp("", now.Add(-2*time.Hour))
g := m.metrics[backupLastSuccessfulTimestamp].(*prometheus.GaugeVec)
assert.Equal(t, 3, testutil.CollectAndCount(g))
m.DeleteBackupLastSuccessfulTimestamp("schedule-1")
assert.Equal(t, 2, testutil.CollectAndCount(g))
assert.Equal(t, float64(now.Add(-time.Hour).Unix()), testutil.ToFloat64(g.WithLabelValues("schedule-2")))
assert.Equal(t, float64(now.Add(-2*time.Hour).Unix()), testutil.ToFloat64(g.WithLabelValues("")))
m.DeleteBackupLastSuccessfulTimestamp("schedule-2")
assert.Equal(t, 1, testutil.CollectAndCount(g))
assert.Equal(t, float64(now.Add(-2*time.Hour).Unix()), testutil.ToFloat64(g.WithLabelValues("")))
m.DeleteBackupLastSuccessfulTimestamp("")
assert.Equal(t, 0, testutil.CollectAndCount(g))
}
// TestRepoMaintenanceMetrics verifies that repo maintenance metrics are properly recorded.
func TestRepoMaintenanceMetrics(t *testing.T) {
tests := []struct {