Skip to content

Commit

Permalink
Remove disk related backup field from ExtraConfig when empty
Browse files Browse the repository at this point in the history 
Previously we'd base64+gzip "null" for this case but it is clearer
to remove the EC entry in that case.
  • Loading branch information
Bryan Venteicher committed Dec 11, 2024
1 parent 86a2d34 commit ee4d2b6
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 39 deletions.
66 changes: 27 additions & 39 deletions pkg/providers/vsphere/virtualmachine/backup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,14 @@ func BackupVirtualMachine(opts BackupVirtualMachineOptions) (result error) {
})
}

pvcDiskData, classicDiskData, err := getDesiredDiskDataForBackup(opts, curExCfg)
pvcDiskData, classicDiskData, err := getDesiredDiskDataForBackup(opts)
if err != nil {
opts.VMCtx.Logger.Error(err, "failed to get disk data for backup")
return err
}

if pvcDiskData == "" {
curPvcDiskData, _ := curExCfg.GetString(backupapi.PVCDiskDataExtraConfigKey)
if pvcDiskData == curPvcDiskData {
opts.VMCtx.Logger.V(4).Info("Skipping PVC disk data backup as unchanged")
} else {
ecToUpdate = append(ecToUpdate, &vimtypes.OptionValue{
Expand All @@ -133,7 +134,8 @@ func BackupVirtualMachine(opts BackupVirtualMachineOptions) (result error) {
}

if pkgcfg.FromContext(opts.VMCtx).Features.VMIncrementalRestore {
if classicDiskData == "" {
curClassDiskData, _ := curExCfg.GetString(backupapi.ClassicDiskDataExtraConfigKey)
if classicDiskData == curClassDiskData {
opts.VMCtx.Logger.V(4).Info("Skipping classic disk data backup as unchanged")
} else {
ecToUpdate = append(ecToUpdate, &vimtypes.OptionValue{
Expand Down Expand Up @@ -240,20 +242,20 @@ func canPerformBackup(opts BackupVirtualMachineOptions, backupVersionEcVal, back

storedVersion, err := strconv.ParseInt(backupVersionEcVal, 10, 64)
if err != nil {
opts.VMCtx.Logger.Error(err, fmt.Sprintf("failed to parse backup version extraConfig key to int64 : %v", backupVersionEcVal))
opts.VMCtx.Logger.Error(err, "failed to parse backup version ExtraConfig value to int64", "version", backupVersionEcVal)
return false, err
}

currVersion, err := strconv.ParseInt(backupVersionAnnotation, 10, 64)
if err != nil {
opts.VMCtx.Logger.Error(err, fmt.Sprintf("failed to parse backup version annotation to int64: %v", backupVersionAnnotation))
opts.VMCtx.Logger.Error(err, "failed to parse backup version annotation value to int64", "version", backupVersionAnnotation)
return false, err
}

// if the storedVersion in the backup is older than the annotation, this is a restore.
// If the storedVersion in the backup is older than the annotation, this is a restore.
if currVersion > storedVersion {
opts.VMCtx.Logger.Info(fmt.Sprintf("Skipping VM backup as a restore is detected: current version: %s greater than backup extraconfig version: %s, wait for VM registration",
backupVersionAnnotation, backupVersionEcVal))
opts.VMCtx.Logger.Info("Skipping VM backup because restore is detected: current version is greater than backup ExtraConfig version. Wait for VM registration",
"currentVersion", backupVersionAnnotation, "extraConfigVersion", backupVersionEcVal)
return false, nil
}

Expand Down Expand Up @@ -401,14 +403,7 @@ func getBackupResourceVersions(ecResourceData string) (map[string]string, error)
return resourceVersions, nil
}

func getDesiredDiskDataForBackup(
opts BackupVirtualMachineOptions,
extraConfig pkgutil.OptionValues) (string, string, error) {

// Return an empty string to skip backup if no disk data is specified.
if len(opts.DiskUUIDToPVC) == 0 && len(opts.ClassicDiskUUIDs) == 0 {
return "", "", nil
}
func getDesiredDiskDataForBackup(opts BackupVirtualMachineOptions) (string, string, error) {

deviceList, err := opts.VcVM.Device(opts.VMCtx)
if err != nil {
Expand Down Expand Up @@ -438,36 +433,29 @@ func getDesiredDiskDataForBackup(
}
}

pvcDiskDataJSON, err := json.Marshal(pvcDiskData)
if err != nil {
return "", "", err
}
pvcDiskDataBackup, err := pkgutil.EncodeGzipBase64(string(pvcDiskDataJSON))
if err != nil {
return "", "", err
}

// Return an empty string to skip backup if PVC disk data is unchanged.
curBackup, _ := extraConfig.GetString(backupapi.PVCDiskDataExtraConfigKey)
if pvcDiskDataBackup == curBackup {
pvcDiskDataBackup = ""
}

var classicDiskDataBackup string
if pkgcfg.FromContext(opts.VMCtx).Features.VMIncrementalRestore {
classicDiskDataJSON, err := json.Marshal(classicDiskData)
var pvcDiskDataBackup string
if len(pvcDiskData) > 0 {
pvcDiskDataJSON, err := json.Marshal(pvcDiskData)
if err != nil {
return "", "", err
}
classicDiskDataBackup, err = pkgutil.EncodeGzipBase64(string(classicDiskDataJSON))
pvcDiskDataBackup, err = pkgutil.EncodeGzipBase64(string(pvcDiskDataJSON))
if err != nil {
return "", "", err
}
}

// Return an empty string to skip backup if classic disk data is unchanged.
curBackup, _ := extraConfig.GetString(backupapi.ClassicDiskDataExtraConfigKey)
if classicDiskDataBackup == curBackup {
classicDiskDataBackup = ""
var classicDiskDataBackup string
if len(classicDiskData) > 0 {
if pkgcfg.FromContext(opts.VMCtx).Features.VMIncrementalRestore {
classicDiskDataJSON, err := json.Marshal(classicDiskData)
if err != nil {
return "", "", err
}
classicDiskDataBackup, err = pkgutil.EncodeGzipBase64(string(classicDiskDataJSON))
if err != nil {
return "", "", err
}
}
}

Expand Down
61 changes: 61 additions & 0 deletions pkg/providers/vsphere/virtualmachine/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,40 @@ func backupTests() {
Expect(vmCtx.VM.Annotations[vmopv1.VirtualMachineBackupVersionAnnotation]).To(Equal(vT2))
}
})

It("Should clear PVC disk data in ExtraConfig when no PVCs", func() {
dummyPVC := builder.DummyPersistentVolumeClaim()
diskUUIDToPVC := map[string]corev1.PersistentVolumeClaim{
vcSimDiskUUID: *dummyPVC,
}

backupOpts := virtualmachine.BackupVirtualMachineOptions{
VMCtx: vmCtx,
VcVM: vcVM,
DiskUUIDToPVC: diskUUIDToPVC,
}

if IncrementalRestore {
backupOpts.BackupVersion = vT2
}

Expect(virtualmachine.BackupVirtualMachine(backupOpts)).To(Succeed())

backupOpts.DiskUUIDToPVC = nil

if IncrementalRestore {
backupOpts.BackupVersion = vT3
}

Expect(virtualmachine.BackupVirtualMachine(backupOpts)).To(Succeed())

verifyBackupDataInExtraConfig(ctx, vcVM, backupapi.PVCDiskDataExtraConfigKey, "", true)

if IncrementalRestore {
verifyBackupDataInExtraConfig(ctx, vcVM, backupapi.BackupVersionExtraConfigKey, vT3, false)
Expect(vmCtx.VM.Annotations[vmopv1.VirtualMachineBackupVersionAnnotation]).To(Equal(vT3))
}
})
})
})

Expand Down Expand Up @@ -1001,6 +1035,33 @@ func backupTests() {
Expect(vmCtx.VM.Annotations[vmopv1.VirtualMachineBackupVersionAnnotation]).To(Equal(vT1))
})
})

When("VM already has classic disk data backup in ExtraConfig", func() {
JustBeforeEach(func() {
backupOpts := virtualmachine.BackupVirtualMachineOptions{
VMCtx: vmCtx,
VcVM: vcVM,
ClassicDiskUUIDs: map[string]struct{}{vcSimDiskUUID: {}},
BackupVersion: vT1,
}
Expect(virtualmachine.BackupVirtualMachine(backupOpts)).To(Succeed())
})

It("Should clear classic disk data backup if no classic disks in ExtraConfig", func() {
backupOpts := virtualmachine.BackupVirtualMachineOptions{
VMCtx: vmCtx,
VcVM: vcVM,
ClassicDiskUUIDs: nil,
BackupVersion: vT2,
}
Expect(virtualmachine.BackupVirtualMachine(backupOpts)).To(Succeed())

verifyBackupDataInExtraConfig(ctx, vcVM, backupapi.ClassicDiskDataExtraConfigKey, "", true)

verifyBackupDataInExtraConfig(ctx, vcVM, backupapi.BackupVersionExtraConfigKey, vT2, false)
Expect(vmCtx.VM.Annotations[vmopv1.VirtualMachineBackupVersionAnnotation]).To(Equal(vT2))
})
})
})
}
},
Expand Down

0 comments on commit ee4d2b6

Please sign in to comment.