From 3cd76c87fbc06b1482a8d13ff17debe74a44d4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Antunes?= Date: Tue, 17 Dec 2024 15:24:31 +0000 Subject: [PATCH] chore(backup): cleanup volume annotations (#5056) * chore(backup): add helper methods to extract info based on velero annotations * chore(backup): add more volume tests * chore(backup): add more volume tests * chore(backup): move some annotations to types * chore(backup): cleanup volume annotations --- pkg/kotsadmsnapshot/backup.go | 122 +++--------- pkg/kotsadmsnapshot/backup_test.go | 304 ++++++++++++++++++++++++++++- pkg/kotsadmsnapshot/types/types.go | 7 + 3 files changed, 328 insertions(+), 105 deletions(-) diff --git a/pkg/kotsadmsnapshot/backup.go b/pkg/kotsadmsnapshot/backup.go index a5736af297..de7497a757 100644 --- a/pkg/kotsadmsnapshot/backup.go +++ b/pkg/kotsadmsnapshot/backup.go @@ -143,9 +143,9 @@ func CreateApplicationBackup(ctx context.Context, a *apptypes.App, isScheduled b veleroBackup.Spec.IncludedNamespaces = prepareIncludedNamespaces(includedNamespaces) - snapshotTrigger := "manual" + snapshotTrigger := types.BackupTriggerManual if isScheduled { - snapshotTrigger = "schedule" + snapshotTrigger = types.BackupTriggerSchedule } veleroBackup.Name = "" @@ -156,7 +156,7 @@ func CreateApplicationBackup(ctx context.Context, a *apptypes.App, isScheduled b if veleroBackup.Annotations == nil { veleroBackup.Annotations = make(map[string]string, 0) } - veleroBackup.Annotations["kots.io/snapshot-trigger"] = snapshotTrigger + veleroBackup.Annotations[types.BackupTriggerAnnotation] = snapshotTrigger veleroBackup.Annotations["kots.io/app-id"] = a.ID veleroBackup.Annotations["kots.io/app-sequence"] = strconv.FormatInt(parentSequence, 10) veleroBackup.Annotations["kots.io/snapshot-requested"] = time.Now().UTC().Format(time.RFC3339) @@ -659,9 +659,9 @@ func appendCommonAnnotations(k8sClient kubernetes.Interface, annotations map[str return nil, errors.Wrap(err, "failed to find kotsadm image") } - snapshotTrigger := "manual" + snapshotTrigger := types.BackupTriggerManual if metadata.isScheduled { - snapshotTrigger = "schedule" + snapshotTrigger = types.BackupTriggerSchedule } appSequences := map[string]int64{} @@ -689,11 +689,11 @@ func appendCommonAnnotations(k8sClient kubernetes.Interface, annotations map[str if annotations == nil { annotations = make(map[string]string, 0) } - annotations["kots.io/snapshot-trigger"] = snapshotTrigger + annotations[types.BackupTriggerAnnotation] = snapshotTrigger annotations["kots.io/snapshot-requested"] = metadata.backupReqestedAt.Format(time.RFC3339) annotations["kots.io/kotsadm-image"] = kotsadmImage annotations["kots.io/kotsadm-deploy-namespace"] = metadata.kotsadmNamespace - annotations["kots.io/apps-sequences"] = marshalledAppSequences + annotations[types.BackupAppsSequencesAnnotation] = marshalledAppSequences annotations["kots.io/apps-versions"] = marshalledAppVersions annotations["kots.io/is-airgap"] = strconv.FormatBool(kotsadm.IsAirgap()) embeddedRegistryHost, _, _ := kotsutil.GetEmbeddedRegistryCreds(k8sClient) @@ -773,7 +773,7 @@ func ListBackupsForApp(ctx context.Context, kotsadmNamespace string, appID strin backup.Status = "New" } - trigger, ok := veleroBackup.Annotations["kots.io/snapshot-trigger"] + trigger, ok := veleroBackup.Annotations[types.BackupTriggerAnnotation] if ok { backup.Trigger = trigger } @@ -783,56 +783,16 @@ func ListBackupsForApp(ctx context.Context, kotsadmNamespace string, appID strin backup.SupportBundleID = supportBundleID } - volumeCount, volumeCountOk := veleroBackup.Annotations["kots.io/snapshot-volume-count"] - if volumeCountOk { - i, err := strconv.Atoi(volumeCount) + if backup.Status != "New" && backup.Status != "InProgress" { + volumeSummary, err := getSnapshotVolumeSummary(ctx, &veleroBackup) if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-count") + return nil, errors.Wrap(err, "failed to get volume summary") } - backup.VolumeCount = i - } - volumeSuccessCount, volumeSuccessCountOk := veleroBackup.Annotations["kots.io/snapshot-volume-success-count"] - if volumeSuccessCountOk { - i, err := strconv.Atoi(volumeSuccessCount) - if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-success-count") - } - backup.VolumeSuccessCount = i - } - - volumeBytes, volumeBytesOk := veleroBackup.Annotations["kots.io/snapshot-volume-bytes"] - if volumeBytesOk { - i, err := strconv.ParseInt(volumeBytes, 10, 64) - if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-bytes") - } - backup.VolumeBytes = i - backup.VolumeSizeHuman = units.HumanSize(float64(i)) - } - - if backup.Status != "New" && backup.Status != "InProgress" { - if !volumeBytesOk || !volumeSuccessCountOk { - // save computed summary as annotations if snapshot is finished - volumeSummary, err := getSnapshotVolumeSummary(ctx, &veleroBackup) - if err != nil { - return nil, errors.Wrap(err, "failed to get volume summary") - } - - backup.VolumeCount = volumeSummary.VolumeCount - backup.VolumeSuccessCount = volumeSummary.VolumeSuccessCount - backup.VolumeBytes = volumeSummary.VolumeBytes - backup.VolumeSizeHuman = volumeSummary.VolumeSizeHuman - - // This is failing with "the server could not find the requested resource (put backups.velero.io scheduled-1586536961)" - // veleroBackup.Annotations["kots.io/snapshot-volume-count"] = strconv.Itoa(backup.VolumeCount) - // veleroBackup.Annotations["kots.io/snapshot-volume-success-count"] = strconv.Itoa(backup.VolumeSuccessCount) - // veleroBackup.Annotations["kots.io/snapshot-volume-bytes"] = strconv.FormatInt(backup.VolumeBytes, 10) - - // if _, err = veleroClient.Backups(backendStorageLocation.Namespace).UpdateStatus(&veleroBackup); err != nil { - // return nil, errors.Wrap(err, "failed to update velero backup") - // } - } + backup.VolumeCount = volumeSummary.VolumeCount + backup.VolumeSuccessCount = volumeSummary.VolumeSuccessCount + backup.VolumeBytes = volumeSummary.VolumeBytes + backup.VolumeSizeHuman = volumeSummary.VolumeSizeHuman } backups = append(backups, &backup) @@ -898,40 +858,12 @@ func ListInstanceBackups(ctx context.Context, kotsadmNamespace string) ([]*types backup.Status = "New" } - trigger, ok := veleroBackup.Annotations["kots.io/snapshot-trigger"] + trigger, ok := veleroBackup.Annotations[types.BackupTriggerAnnotation] if ok { backup.Trigger = trigger } - volumeCount, volumeCountOk := veleroBackup.Annotations["kots.io/snapshot-volume-count"] - if volumeCountOk { - i, err := strconv.Atoi(volumeCount) - if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-count") - } - backup.VolumeCount = i - } - - volumeSuccessCount, volumeSuccessCountOk := veleroBackup.Annotations["kots.io/snapshot-volume-success-count"] - if volumeSuccessCountOk { - i, err := strconv.Atoi(volumeSuccessCount) - if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-success-count") - } - backup.VolumeSuccessCount = i - } - - volumeBytes, volumeBytesOk := veleroBackup.Annotations["kots.io/snapshot-volume-bytes"] - if volumeBytesOk { - i, err := strconv.ParseInt(volumeBytes, 10, 64) - if err != nil { - return nil, errors.Wrap(err, "failed to convert volume-bytes") - } - backup.VolumeBytes = i - backup.VolumeSizeHuman = units.HumanSize(float64(i)) - } - - appAnnotationStr, _ := veleroBackup.Annotations["kots.io/apps-sequences"] + appAnnotationStr, _ := veleroBackup.Annotations[types.BackupAppsSequencesAnnotation] if len(appAnnotationStr) > 0 { var apps map[string]int64 if err := json.Unmarshal([]byte(appAnnotationStr), &apps); err != nil { @@ -956,19 +888,17 @@ func ListInstanceBackups(ctx context.Context, kotsadmNamespace string) ([]*types } } + // get volume information if backup.Status != "New" && backup.Status != "InProgress" { - if !volumeBytesOk || !volumeSuccessCountOk { - // save computed summary as annotations if snapshot is finished - volumeSummary, err := getSnapshotVolumeSummary(ctx, &veleroBackup) - if err != nil { - return nil, errors.Wrap(err, "failed to get volume summary") - } - - backup.VolumeCount = volumeSummary.VolumeCount - backup.VolumeSuccessCount = volumeSummary.VolumeSuccessCount - backup.VolumeBytes = volumeSummary.VolumeBytes - backup.VolumeSizeHuman = volumeSummary.VolumeSizeHuman + volumeSummary, err := getSnapshotVolumeSummary(ctx, &veleroBackup) + if err != nil { + return nil, errors.Wrap(err, "failed to get volume summary") } + + backup.VolumeCount = volumeSummary.VolumeCount + backup.VolumeSuccessCount = volumeSummary.VolumeSuccessCount + backup.VolumeBytes = volumeSummary.VolumeBytes + backup.VolumeSizeHuman = volumeSummary.VolumeSizeHuman } // group the velero backups by the name we present to the user diff --git a/pkg/kotsadmsnapshot/backup_test.go b/pkg/kotsadmsnapshot/backup_test.go index 86928a5477..3475f9e892 100644 --- a/pkg/kotsadmsnapshot/backup_test.go +++ b/pkg/kotsadmsnapshot/backup_test.go @@ -3014,6 +3014,280 @@ func Test_getBackupNameFromPrefix(t *testing.T) { } } +func TestListBackupsForApp(t *testing.T) { + scheme := runtime.NewScheme() + corev1.AddToScheme(scheme) + embeddedclusterv1beta1.AddToScheme(scheme) + + // setup timestamps + startTs := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + completionTs := time.Date(2024, 1, 2, 0, 0, 0, 0, time.UTC) + expirationTs := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + // setup common mock objects + kotsadmNamespace := "kotsadm-test" + testBsl := &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: "velero", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: "aws", + Default: true, + }, + } + veleroNamespaceConfigmap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kotsadm-velero-namespace", + Namespace: kotsadmNamespace, + }, + Data: map[string]string{ + "veleroNamespace": "velero", + }, + } + veleroDeployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "velero", + Namespace: "velero", + }, + } + + tests := []struct { + name string + appID string + veleroClientBuilder veleroclient.VeleroClientBuilder + k8sClientBuilder k8sclient.K8sClientsetBuilder + expectedBackups []*types.Backup + wantErr string + }{ + { + name: "fails to create k8s clientset", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: nil, + Err: fmt.Errorf("error creating k8s clientset"), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset().VeleroV1(), + }, + wantErr: "failed to create clientset", + }, + { + name: "fails to create velero client", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset(), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: nil, + Err: fmt.Errorf("error creating velero client"), + }, + wantErr: "failed to create velero clientset", + }, + { + name: "fails to find backup storage location", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset(), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset().VeleroV1(), + }, + wantErr: "no backup store location found", + }, + { + name: "empty backup list", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset( + veleroNamespaceConfigmap, + veleroDeployment, + ), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset( + testBsl, + ).VeleroV1(), + }, + expectedBackups: []*types.Backup{}, + }, + { + name: "backups not matching the app id are excluded", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset( + veleroNamespaceConfigmap, + veleroDeployment, + ), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset( + testBsl, + &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-backup-app-1", + Namespace: "velero", + Annotations: map[string]string{ + "kots.io/app-id": "app-1", + }, + }, + Status: velerov1.BackupStatus{ + Phase: velerov1.BackupPhaseCompleted, + }, + }, + &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-backup-app-2", + Namespace: "velero", + Annotations: map[string]string{ + "kots.io/app-id": "app-2", + }, + }, + Status: velerov1.BackupStatus{ + Phase: velerov1.BackupPhaseCompleted, + }, + }, + &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "instance-backup", + Namespace: "velero", + Annotations: map[string]string{ + types.InstanceBackupAnnotation: "true", + }, + }, + Status: velerov1.BackupStatus{ + Phase: velerov1.BackupPhaseCompleted, + }, + }, + ).VeleroV1(), + }, + expectedBackups: []*types.Backup{ + { + AppID: "app-1", + Name: "app-backup-app-1", + Status: "Completed", + VolumeSizeHuman: "0B", + }, + }, + }, + { + name: "timestamps are populated", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset( + veleroNamespaceConfigmap, + veleroDeployment, + ), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset( + testBsl, + &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-backup-app-1", + Namespace: "velero", + Annotations: map[string]string{ + "kots.io/app-id": "app-1", + }, + }, + Status: velerov1.BackupStatus{ + Phase: velerov1.BackupPhaseCompleted, + StartTimestamp: &metav1.Time{Time: startTs}, + CompletionTimestamp: &metav1.Time{Time: completionTs}, + Expiration: &metav1.Time{Time: expirationTs}, + }, + }, + ).VeleroV1(), + }, + expectedBackups: []*types.Backup{ + { + AppID: "app-1", + Name: "app-backup-app-1", + Status: "Completed", + StartedAt: &startTs, + FinishedAt: &completionTs, + ExpiresAt: &expirationTs, + VolumeSizeHuman: "0B", + }, + }, + }, + { + name: "volume info is populated from pod volume backups", + appID: "app-1", + k8sClientBuilder: &k8sclient.MockBuilder{ + Client: fake.NewSimpleClientset( + veleroNamespaceConfigmap, + veleroDeployment, + ), + }, + veleroClientBuilder: &veleroclient.MockBuilder{ + Client: velerofake.NewSimpleClientset( + testBsl, + &velerov1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-backup-app-1", + Namespace: "velero", + Annotations: map[string]string{ + "kots.io/snapshot-trigger": "schedule", + "kots.io/app-id": "app-1", + }, + }, + Status: velerov1.BackupStatus{ + Phase: velerov1.BackupPhaseCompleted, + }, + }, + &velerov1.PodVolumeBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-backup-app-1-pod-volume-backup", + Namespace: "velero", + Labels: map[string]string{ + "velero.io/backup-name": "app-backup-app-1", + }, + }, + Status: velerov1.PodVolumeBackupStatus{ + Phase: velerov1.PodVolumeBackupPhaseCompleted, + Progress: velerov1.PodVolumeOperationProgress{ + BytesDone: 2000, + }, + }, + }, + ).VeleroV1(), + }, + expectedBackups: []*types.Backup{ + { + AppID: "app-1", + Name: "app-backup-app-1", + Status: "Completed", + Trigger: "schedule", + VolumeSizeHuman: "2kB", + VolumeBytes: 2000, + VolumeSuccessCount: 1, + VolumeCount: 1, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + asrt := assert.New(t) + // setup mock clients + k8sclient.SetBuilder(test.k8sClientBuilder) + veleroclient.SetBuilder(test.veleroClientBuilder) + + backups, err := ListBackupsForApp(context.Background(), kotsadmNamespace, test.appID) + + if test.wantErr != "" { + asrt.Error(err) + asrt.Contains(err.Error(), test.wantErr) + } else { + asrt.NoError(err) + } + asrt.Equal(test.expectedBackups, backups) + }) + } +} + func TestListInstanceBackups(t *testing.T) { scheme := runtime.NewScheme() corev1.AddToScheme(scheme) @@ -3329,7 +3603,7 @@ func TestListInstanceBackups(t *testing.T) { }, }, { - name: "volume info is populated", + name: "volume info is populated from pod volume backups", k8sClientBuilder: &k8sclient.MockBuilder{ Client: fake.NewSimpleClientset( veleroNamespaceConfigmap, @@ -3344,17 +3618,29 @@ func TestListInstanceBackups(t *testing.T) { Name: "some-backup-with-volumes", Namespace: "velero", Annotations: map[string]string{ - types.InstanceBackupAnnotation: "true", - "kots.io/snapshot-trigger": "manual", - "kots.io/snapshot-volume-count": "2", - "kots.io/snapshot-volume-success-count": "1", - "kots.io/snapshot-volume-bytes": "1000", + types.InstanceBackupAnnotation: "true", + "kots.io/snapshot-trigger": "manual", }, }, Status: velerov1.BackupStatus{ Phase: velerov1.BackupPhaseCompleted, }, }, + &velerov1.PodVolumeBackup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-backup-with-volumes-pod-volume-backup", + Namespace: "velero", + Labels: map[string]string{ + "velero.io/backup-name": "some-backup-with-volumes", + }, + }, + Status: velerov1.PodVolumeBackupStatus{ + Phase: velerov1.PodVolumeBackupPhaseCompleted, + Progress: velerov1.PodVolumeOperationProgress{ + BytesDone: 2000, + }, + }, + }, ).VeleroV1(), }, expectedBackups: []*types.ReplicatedBackup{ @@ -3366,10 +3652,10 @@ func TestListInstanceBackups(t *testing.T) { Name: "some-backup-with-volumes", Status: "Completed", Trigger: "manual", - VolumeSizeHuman: "1kB", - VolumeBytes: 1000, + VolumeSizeHuman: "2kB", + VolumeBytes: 2000, VolumeSuccessCount: 1, - VolumeCount: 2, + VolumeCount: 1, IncludedApps: []types.App{}, }, }, diff --git a/pkg/kotsadmsnapshot/types/types.go b/pkg/kotsadmsnapshot/types/types.go index c5c44c771a..23b339e5e6 100644 --- a/pkg/kotsadmsnapshot/types/types.go +++ b/pkg/kotsadmsnapshot/types/types.go @@ -40,6 +40,13 @@ const ( // InstanceBackupVersionCurrent is the current backup version. When future breaking changes are // introduced, we can increment this number on backup creation. InstanceBackupVersionCurrent = InstanceBackupVersion1 + // BackupTriggerAnnotation is the annotation used to store the trigger of the backup. + BackupTriggerAnnotation = "kots.io/snapshot-trigger" + // BackupTriggerManual indicates that the backup was triggered manually. + BackupTriggerManual = "manual" + // BackupTriggerSchedule indicates that the backup was triggered by a schedule. + BackupTriggerSchedule = "schedule" + BackupAppsSequencesAnnotation = "kots.io/apps-sequences" ) type App struct {