Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Remove disk related backup field from ExtraConfig when empty #815

Merged
merged 1 commit into from
Dec 12, 2024
Merged

🌱 Remove disk related backup field from ExtraConfig when empty #815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 27 additions & 39 deletions pkg/providers/vsphere/virtualmachine/backup.go
View file
Open in desktop
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
Open in desktop
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
Loading