diff --git a/changelogs/unreleased/7202-blackpiglet b/changelogs/unreleased/7202-blackpiglet new file mode 100644 index 000000000..ac5d18011 --- /dev/null +++ b/changelogs/unreleased/7202-blackpiglet @@ -0,0 +1,2 @@ +Update CSIVolumeSnapshotsCompleted in backup's status and the metric +during backup finalize stage according to async operations content. \ No newline at end of file diff --git a/pkg/backup/snapshots.go b/pkg/backup/snapshots.go index a5c659705..7bdfc585e 100644 --- a/pkg/backup/snapshots.go +++ b/pkg/backup/snapshots.go @@ -15,9 +15,18 @@ import ( "github.com/vmware-tanzu/velero/pkg/util/boolptr" ) -// Common function to update the status of CSI snapshots -// returns VolumeSnapshot, VolumeSnapshotContent, VolumeSnapshotClasses referenced -func UpdateBackupCSISnapshotsStatus(client kbclient.Client, volumeSnapshotLister snapshotv1listers.VolumeSnapshotLister, backup *velerov1api.Backup, backupLog logrus.FieldLogger) (volumeSnapshots []snapshotv1api.VolumeSnapshot, volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent, volumeSnapshotClasses []snapshotv1api.VolumeSnapshotClass) { +// GetBackupCSIResources is used to get CSI snapshot related resources. +// Returns VolumeSnapshot, VolumeSnapshotContent, VolumeSnapshotClasses referenced +func GetBackupCSIResources( + client kbclient.Client, + volumeSnapshotLister snapshotv1listers.VolumeSnapshotLister, + backup *velerov1api.Backup, + backupLog logrus.FieldLogger, +) ( + volumeSnapshots []snapshotv1api.VolumeSnapshot, + volumeSnapshotContents []snapshotv1api.VolumeSnapshotContent, + volumeSnapshotClasses []snapshotv1api.VolumeSnapshotClass, +) { if boolptr.IsSetToTrue(backup.Spec.SnapshotMoveData) { backupLog.Info("backup SnapshotMoveData is set to true, skip VolumeSnapshot resource persistence.") } else if features.IsEnabled(velerov1api.CSIFeatureFlag) { @@ -56,13 +65,7 @@ func UpdateBackupCSISnapshotsStatus(client kbclient.Client, volumeSnapshotLister } } backup.Status.CSIVolumeSnapshotsAttempted = len(volumeSnapshots) - csiVolumeSnapshotsCompleted := 0 - for _, vs := range volumeSnapshots { - if vs.Status != nil && boolptr.IsSetToTrue(vs.Status.ReadyToUse) { - csiVolumeSnapshotsCompleted++ - } - } - backup.Status.CSIVolumeSnapshotsCompleted = csiVolumeSnapshotsCompleted } + return volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses } diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 306b1af2e..4c088365a 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -824,7 +824,6 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string cmd.CheckError(err) r := controller.NewBackupFinalizerReconciler( s.mgr.GetClient(), - s.csiSnapshotLister, clock.RealClock{}, backupper, newPluginManager, diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index a708a24e7..acc5febac 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -661,7 +661,9 @@ func (b *backupReconciler) runBackup(backup *pkgbackup.Request) error { backup.Status.VolumeSnapshotsCompleted++ } } - volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses := pkgbackup.UpdateBackupCSISnapshotsStatus(b.kbClient, b.volumeSnapshotLister, backup.Backup, backupLog) + volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses := pkgbackup.GetBackupCSIResources(b.kbClient, b.volumeSnapshotLister, backup.Backup, backupLog) + // Update CSIVolumeSnapshotsAttempted + backup.Status.CSIVolumeSnapshotsAttempted = len(volumeSnapshots) // Iterate over backup item operations and update progress. // Any errors on operations at this point should be added to backup errors. @@ -760,6 +762,7 @@ func recordBackupMetrics(log logrus.FieldLogger, backup *velerov1api.Backup, bac backupDurationSeconds := float64(backupDuration / time.Second) serverMetrics.RegisterBackupDuration(backupScheduleName, backupDurationSeconds) } + if !finalize { serverMetrics.RegisterVolumeSnapshotAttempts(backupScheduleName, backup.Status.VolumeSnapshotsAttempted) serverMetrics.RegisterVolumeSnapshotSuccesses(backupScheduleName, backup.Status.VolumeSnapshotsCompleted) @@ -767,8 +770,6 @@ func recordBackupMetrics(log logrus.FieldLogger, backup *velerov1api.Backup, bac if features.IsEnabled(velerov1api.CSIFeatureFlag) { serverMetrics.RegisterCSISnapshotAttempts(backupScheduleName, backup.Name, backup.Status.CSIVolumeSnapshotsAttempted) - serverMetrics.RegisterCSISnapshotSuccesses(backupScheduleName, backup.Name, backup.Status.CSIVolumeSnapshotsCompleted) - serverMetrics.RegisterCSISnapshotFailures(backupScheduleName, backup.Name, backup.Status.CSIVolumeSnapshotsAttempted-backup.Status.CSIVolumeSnapshotsCompleted) } if backup.Status.Progress != nil { @@ -779,6 +780,9 @@ func recordBackupMetrics(log logrus.FieldLogger, backup *velerov1api.Backup, bac if backup.Status.Warnings > 0 { serverMetrics.RegisterBackupWarning(backupScheduleName) } + } else if features.IsEnabled(velerov1api.CSIFeatureFlag) { + serverMetrics.RegisterCSISnapshotSuccesses(backupScheduleName, backup.Name, backup.Status.CSIVolumeSnapshotsCompleted) + serverMetrics.RegisterCSISnapshotFailures(backupScheduleName, backup.Name, backup.Status.CSIVolumeSnapshotsAttempted-backup.Status.CSIVolumeSnapshotsCompleted) } } diff --git a/pkg/controller/backup_finalizer_controller.go b/pkg/controller/backup_finalizer_controller.go index eb99f6ee5..a27aa4521 100644 --- a/pkg/controller/backup_finalizer_controller.go +++ b/pkg/controller/backup_finalizer_controller.go @@ -29,10 +29,10 @@ import ( ctrl "sigs.k8s.io/controller-runtime" kbclient "sigs.k8s.io/controller-runtime/pkg/client" - snapshotv1listers "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1" - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" pkgbackup "github.com/vmware-tanzu/velero/pkg/backup" + "github.com/vmware-tanzu/velero/pkg/itemoperation" + "github.com/vmware-tanzu/velero/pkg/kuberesource" "github.com/vmware-tanzu/velero/pkg/metrics" "github.com/vmware-tanzu/velero/pkg/persistence" "github.com/vmware-tanzu/velero/pkg/plugin/clientmgmt" @@ -42,21 +42,19 @@ import ( // backupFinalizerReconciler reconciles a Backup object type backupFinalizerReconciler struct { - client kbclient.Client - volumeSnapshotLister snapshotv1listers.VolumeSnapshotLister - clock clocks.WithTickerAndDelayedExecution - backupper pkgbackup.Backupper - newPluginManager func(logrus.FieldLogger) clientmgmt.Manager - backupTracker BackupTracker - metrics *metrics.ServerMetrics - backupStoreGetter persistence.ObjectBackupStoreGetter - log logrus.FieldLogger + client kbclient.Client + clock clocks.WithTickerAndDelayedExecution + backupper pkgbackup.Backupper + newPluginManager func(logrus.FieldLogger) clientmgmt.Manager + backupTracker BackupTracker + metrics *metrics.ServerMetrics + backupStoreGetter persistence.ObjectBackupStoreGetter + log logrus.FieldLogger } // NewBackupFinalizerReconciler initializes and returns backupFinalizerReconciler struct. func NewBackupFinalizerReconciler( client kbclient.Client, - volumeSnapshotLister snapshotv1listers.VolumeSnapshotLister, clock clocks.WithTickerAndDelayedExecution, backupper pkgbackup.Backupper, newPluginManager func(logrus.FieldLogger) clientmgmt.Manager, @@ -188,10 +186,12 @@ func (r *backupFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Requ r.metrics.RegisterBackupPartialFailure(backupScheduleName) r.metrics.RegisterBackupLastStatus(backupScheduleName, metrics.BackupLastStatusFailure) } + backup.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()} + backup.Status.CSIVolumeSnapshotsCompleted = updateCSIVolumeSnapshotsCompleted(operations) + recordBackupMetrics(log, backup, outBackupFile, r.metrics, true) - pkgbackup.UpdateBackupCSISnapshotsStatus(r.client, r.volumeSnapshotLister, backup, log) // update backup metadata in object store backupJSON := new(bytes.Buffer) if err := encode.To(backup, "json", backupJSON); err != nil { @@ -215,3 +215,19 @@ func (r *backupFinalizerReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&velerov1api.Backup{}). Complete(r) } + +// updateCSIVolumeSnapshotsCompleted calculate the completed VS number according to +// the backup's async operation list. +func updateCSIVolumeSnapshotsCompleted( + operations []*itemoperation.BackupOperation) int { + completedNum := 0 + + for index := range operations { + if operations[index].Spec.ResourceIdentifier.String() == kuberesource.VolumeSnapshots.String() && + operations[index].Status.Phase == itemoperation.OperationPhaseCompleted { + completedNum++ + } + } + + return completedNum +} diff --git a/pkg/controller/backup_finalizer_controller_test.go b/pkg/controller/backup_finalizer_controller_test.go index f759d0318..207f3a3ac 100644 --- a/pkg/controller/backup_finalizer_controller_test.go +++ b/pkg/controller/backup_finalizer_controller_test.go @@ -23,7 +23,6 @@ import ( "testing" "time" - snapshotv1listers "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -37,6 +36,7 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" + "github.com/vmware-tanzu/velero/pkg/features" "github.com/vmware-tanzu/velero/pkg/itemoperation" "github.com/vmware-tanzu/velero/pkg/kuberesource" "github.com/vmware-tanzu/velero/pkg/metrics" @@ -44,14 +44,12 @@ import ( "github.com/vmware-tanzu/velero/pkg/plugin/framework" "github.com/vmware-tanzu/velero/pkg/plugin/velero" velerotest "github.com/vmware-tanzu/velero/pkg/test" - velerotestmocks "github.com/vmware-tanzu/velero/pkg/test/mocks" ) -func mockBackupFinalizerReconciler(fakeClient kbclient.Client, fakeVolumeSnapshotLister snapshotv1listers.VolumeSnapshotLister, fakeClock *testclocks.FakeClock) (*backupFinalizerReconciler, *fakeBackupper) { +func mockBackupFinalizerReconciler(fakeClient kbclient.Client, fakeClock *testclocks.FakeClock) (*backupFinalizerReconciler, *fakeBackupper) { backupper := new(fakeBackupper) return NewBackupFinalizerReconciler( fakeClient, - fakeVolumeSnapshotLister, fakeClock, backupper, func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager }, @@ -68,12 +66,14 @@ func TestBackupFinalizerReconcile(t *testing.T) { defaultBackupLocation := builder.ForBackupStorageLocation(velerov1api.DefaultNamespace, "default").Result() tests := []struct { - name string - backup *velerov1api.Backup - backupOperations []*itemoperation.BackupOperation - backupLocation *velerov1api.BackupStorageLocation - expectError bool - expectPhase velerov1api.BackupPhase + name string + backup *velerov1api.Backup + backupOperations []*itemoperation.BackupOperation + backupLocation *velerov1api.BackupStorageLocation + enableCSI bool + expectError bool + expectPhase velerov1api.BackupPhase + expectedCompletedVS int }{ { name: "Finalizing backup is completed", @@ -147,6 +147,50 @@ func TestBackupFinalizerReconcile(t *testing.T) { }, }, }, + { + name: "Test calculate backup.Status.BackupItemOperationsCompleted", + backup: builder.ForBackup(velerov1api.DefaultNamespace, "backup-3"). + StorageLocation("default"). + ObjectMeta(builder.WithUID("foo")). + StartTimestamp(fakeClock.Now()). + WithStatus(velerov1api.BackupStatus{ + StartTimestamp: &metav1Now, + CompletionTimestamp: &metav1Now, + CSIVolumeSnapshotsAttempted: 1, + Phase: velerov1api.BackupPhaseFinalizing, + }). + Result(), + backupLocation: defaultBackupLocation, + enableCSI: true, + expectPhase: velerov1api.BackupPhaseCompleted, + expectedCompletedVS: 1, + backupOperations: []*itemoperation.BackupOperation{ + { + Spec: itemoperation.BackupOperationSpec{ + BackupName: "backup-3", + BackupUID: "foo", + BackupItemAction: "foo", + ResourceIdentifier: velero.ResourceIdentifier{ + GroupResource: kuberesource.VolumeSnapshots, + Namespace: "ns-1", + Name: "vs-1", + }, + PostOperationItems: []velero.ResourceIdentifier{ + { + GroupResource: kuberesource.Secrets, + Namespace: "ns-1", + Name: "secret-1", + }, + }, + OperationID: "operation-3", + }, + Status: itemoperation.OperationStatus{ + Phase: itemoperation.OperationPhaseCompleted, + Created: &metav1Now, + }, + }, + }, + }, } for _, test := range tests { @@ -162,11 +206,14 @@ func TestBackupFinalizerReconcile(t *testing.T) { initObjs = append(initObjs, test.backupLocation) } + if test.enableCSI { + features.Enable(velerov1api.CSIFeatureFlag) + defer features.Enable() + } + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, initObjs...) - fakeVolumeSnapshotLister := velerotestmocks.NewVolumeSnapshotLister(t) - - reconciler, backupper := mockBackupFinalizerReconciler(fakeClient, fakeVolumeSnapshotLister, fakeClock) + reconciler, backupper := mockBackupFinalizerReconciler(fakeClient, fakeClock) pluginManager.On("CleanupClients").Return(nil) backupStore.On("GetBackupItemOperations", test.backup.Name).Return(test.backupOperations, nil) backupStore.On("GetBackupContents", mock.Anything).Return(io.NopCloser(bytes.NewReader([]byte("hello world"))), nil) @@ -186,6 +233,7 @@ func TestBackupFinalizerReconcile(t *testing.T) { require.NoError(t, err) assert.Equal(t, test.expectPhase, backupAfter.Status.Phase) + assert.Equal(t, test.expectedCompletedVS, backupAfter.Status.CSIVolumeSnapshotsCompleted) }) } } diff --git a/pkg/controller/backup_sync_controller_test.go b/pkg/controller/backup_sync_controller_test.go index bc362edb4..b07f11929 100644 --- a/pkg/controller/backup_sync_controller_test.go +++ b/pkg/controller/backup_sync_controller_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1" "github.com/sirupsen/logrus" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -33,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" testclocks "k8s.io/utils/clock/testing" - ctrl "sigs.k8s.io/controller-runtime" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -429,6 +429,9 @@ var _ = Describe("Backup Sync Reconciler", func() { backupNames = append(backupNames, backup.backup.Name) backupStore.On("GetBackupMetadata", backup.backup.Name).Return(backup.backup, nil) backupStore.On("GetPodVolumeBackups", backup.backup.Name).Return(backup.podVolumeBackups, nil) + backupStore.On("BackupExists", "bucket-1", backup.backup.Name).Return(true, nil) + backupStore.On("GetCSIVolumeSnapshotClasses", backup.backup.Name).Return([]*snapshotv1api.VolumeSnapshotClass{}, nil) + backupStore.On("GetCSIVolumeSnapshotContents", backup.backup.Name).Return([]*snapshotv1api.VolumeSnapshotContent{}, nil) } backupStore.On("ListBackups").Return(backupNames, nil) }