Skip to content

Commit

Permalink
Fix issue vmware-tanzu#7163.
Browse files Browse the repository at this point in the history 
Update CSIVolumeSnapshotsCompleted in backup's status and the metric
during backup finlize stage according to async operations content.

Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
Xun Jiang committed Dec 6, 2023
1 parent 8d8d68d commit cd13350
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 20 deletions.
23 changes: 13 additions & 10 deletions pkg/backup/snapshots.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,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, globalCRClient kbclient.Client, 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,
globalCRClient kbclient.Client,
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) {
Expand Down Expand Up @@ -69,13 +78,7 @@ func UpdateBackupCSISnapshotsStatus(client kbclient.Client, globalCRClient kbcli
}
}
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
}
12 changes: 9 additions & 3 deletions pkg/controller/backup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,9 @@ func (b *backupReconciler) runBackup(backup *pkgbackup.Request) error {
backup.Status.VolumeSnapshotsCompleted++
}
}
volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses := pkgbackup.UpdateBackupCSISnapshotsStatus(b.kbClient, b.globalCRClient, backup.Backup, backupLog)
volumeSnapshots, volumeSnapshotContents, volumeSnapshotClasses := pkgbackup.GetBackupCSIResources(b.kbClient, b.globalCRClient, 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.
Expand Down Expand Up @@ -763,15 +765,14 @@ 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)
serverMetrics.RegisterVolumeSnapshotFailures(backupScheduleName, backup.Status.VolumeSnapshotsAttempted-backup.Status.VolumeSnapshotsCompleted)

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 {
Expand All @@ -782,6 +783,11 @@ 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)
}
}
}

Expand Down
23 changes: 22 additions & 1 deletion pkg/controller/backup_finalizer_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (

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"
Expand Down Expand Up @@ -188,9 +190,11 @@ func (r *backupFinalizerReconciler) Reconcile(ctx context.Context, req ctrl.Requ
r.metrics.RegisterBackupLastStatus(backupScheduleName, metrics.BackupLastStatusFailure)
}
backup.Status.CompletionTimestamp = &metav1.Time{Time: r.clock.Now()}

updateCSIVolumeSnapshotsCompleted(backup, operations)

recordBackupMetrics(log, backup, outBackupFile, r.metrics, true)

pkgbackup.UpdateBackupCSISnapshotsStatus(r.client, r.globalCRClient, backup, log)
// update backup metadata in object store
backupJSON := new(bytes.Buffer)
if err := encode.To(backup, "json", backupJSON); err != nil {
Expand All @@ -214,3 +218,20 @@ 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(
backup *velerov1api.Backup,
operations []*itemoperation.BackupOperation) {
completedNum := 0

for index := range operations {
if operations[index].Spec.ResourceIdentifier.String() == kuberesource.VolumeSnapshots.String() &&
operations[index].Status.Phase == itemoperation.OperationPhaseCompleted {
completedNum++
}
}

backup.Status.CSIVolumeSnapshotsCompleted = completedNum
}
51 changes: 45 additions & 6 deletions pkg/controller/backup_finalizer_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@ 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
expectError bool
expectPhase velerov1api.BackupPhase
expectedCompletedVS int
}{
{
name: "Finalizing backup is completed",
Expand Down Expand Up @@ -145,6 +146,43 @@ 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()).
Phase(velerov1api.BackupPhaseFinalizing).Result(),
backupLocation: defaultBackupLocation,
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 {
Expand Down Expand Up @@ -184,6 +222,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)
})
}
}

0 comments on commit cd13350

Please sign in to comment.