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

[release-v0.7] Ignore VMIs from backup when owner VMs are excluded #315

Open
wants to merge 2 commits into
base: release-v0.7
Choose a base branch
from
Open

[release-v0.7] Ignore VMIs from backup when owner VMs are excluded #315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3788c3b
Fix VMI backups when VMs are excluded
alromeros Aug 14, 2024
3de3b06
Update functional tests
alromeros Aug 16, 2024
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
42 changes: 28 additions & 14 deletions pkg/plugin/vmi_backup_item_action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ func (p *VMIBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Back
return nil, nil, errors.WithStack(err)
}

// There's no point in backing up a VMI when it's owned by a VM excluded from the backup
shouldExclude, err := shouldExcludeVMI(vmi, backup)
if err != nil {
return nil, nil, errors.WithStack(err)
}
if shouldExclude {
return nil, nil, nil
}

if !util.IsVMIPaused(vmi) {
if !util.IsResourceInBackup("pods", backup) && util.IsResourceInBackup("persistentvolumeclaims", backup) {
return nil, nil, fmt.Errorf("VM is running but launcher pod is not included in the backup")
Expand All @@ -98,19 +107,6 @@ func (p *VMIBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Back
}

if isVMIOwned(vmi) {
if !util.IsResourceInBackup("virtualmachines", backup) {
return nil, nil, fmt.Errorf("VMI owned by a VM and the VM is not included in the backup")
}

excluded, err := isVMExcludedByLabel(vmi)
if err != nil {
return nil, nil, errors.WithStack(err)
}

if excluded {
return nil, nil, fmt.Errorf("VMI owned by a VM and the VM is not included in the backup")
}

util.AddAnnotation(item, AnnIsOwned, "true")
} else if !util.IsMetadataBackup(backup) {
restore, err := util.RestorePossible(vmi.Spec.Volumes, backup, vmi.Namespace, func(volume kvcore.Volume) bool { return false }, p.log)
Expand All @@ -122,7 +118,7 @@ func (p *VMIBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Back
}
}

extra, err := p.addLauncherPod(vmi, extra)
extra, err = p.addLauncherPod(vmi, extra)
if err != nil {
return nil, nil, err
}
Expand All @@ -132,6 +128,24 @@ func (p *VMIBackupItemAction) Execute(item runtime.Unstructured, backup *v1.Back
return item, extra, nil
}

// shouldExcludeVMI checks wether a VMI owned by VM should be backed up or ignored
func shouldExcludeVMI(vmi *kvcore.VirtualMachineInstance, backup *v1.Backup) (bool, error) {
if !isVMIOwned(vmi) {
return false, nil
}

if !util.IsResourceInBackup("virtualmachines", backup) {
return true, nil
}

excluded, err := isVMExcludedByLabel(vmi)
if err != nil {
return false, err
}

return excluded, nil
}

func isVMIOwned(vmi *kvcore.VirtualMachineInstance) bool {
return len(vmi.OwnerReferences) > 0
}
Expand Down
44 changes: 22 additions & 22 deletions pkg/plugin/vmi_backup_item_action_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ func TestVMIBackupItemAction(t *testing.T) {
},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances", "virtualmachines"},
ExcludedResources: []string{"pods"},
},
},
Expand Down Expand Up @@ -163,7 +164,11 @@ func TestVMIBackupItemAction(t *testing.T) {
unstructured.Unstructured{
Object: pausedVMI,
},
velerov1.Backup{},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances", "virtualmachines", "pods"},
},
},
excludedPod,
returnFalse,
returnFalse,
Expand Down Expand Up @@ -193,6 +198,7 @@ func TestVMIBackupItemAction(t *testing.T) {
},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances", "virtualmachines", "persistentvolumeclaims"},
ExcludedResources: []string{"pods"},
},
},
Expand All @@ -207,10 +213,14 @@ func TestVMIBackupItemAction(t *testing.T) {
unstructured.Unstructured{
Object: ownedVMI,
},
velerov1.Backup{},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances", "virtualmachines"},
},
},
excludedPod,
returnFalse,
returnTrue,
returnFalse,
true,
"VM is running but launcher pod is not included in the backup",
nullValidator,
Expand All @@ -237,6 +247,7 @@ func TestVMIBackupItemAction(t *testing.T) {
},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances", "virtualmachines"},
ExcludedResources: []string{"pods", "persistentvolumeclaims"},
},
},
Expand All @@ -247,48 +258,37 @@ func TestVMIBackupItemAction(t *testing.T) {
"",
nullValidator,
},
{"Owned VMI: backup must include VM",
{"Owned VMI: Won't backup VMI if VMs are excluded",
unstructured.Unstructured{
Object: ownedVMI,
},
velerov1.Backup{
Spec: velerov1.BackupSpec{
IncludedResources: []string{"virtualmachineinstances"},
ExcludedResources: []string{"virtualmachines"},
},
},
launcherPod,
returnFalse,
returnFalse,
true,
"VMI owned by a VM and the VM is not included in the backup",
false,
"",
nullValidator,
},
{"Owned VMI: VM must not be excluded from backup",
{"Owned VMI: Will ignore VMI if VM is excluded by label",
unstructured.Unstructured{
Object: ownedVMI,
},
velerov1.Backup{
Spec: velerov1.BackupSpec{
ExcludedResources: []string{"virtualmachines"},
IncludedResources: []string{"virtualmachineinstances", "virtualmachines"},
},
},
launcherPod,
returnFalse,
returnFalse,
true,
"VMI owned by a VM and the VM is not included in the backup",
nullValidator,
},
{"Owned VMI: VM must not be excluded by label",
unstructured.Unstructured{
Object: ownedVMI,
},
velerov1.Backup{},
launcherPod,
returnFalse,
returnTrue,
true,
"VMI owned by a VM and the VM is not included in the backup",
false,
"",
nullValidator,
},
{"Owned VMI must add 'is owned' annotation",
Expand Down
72 changes: 64 additions & 8 deletions tests/resource_filtering_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ var _ = Describe("Resource includes", func() {
Expect(err).ToNot(HaveOccurred())
})

It("[test_id:10198]Selecting VMI (with DV+PVC+Pod) but not VM: Backing up VMI should fail", func() {
It("[test_id:10198]Selecting VMI (with DV+PVC+Pod) but not VM: Empty backup without failure", func() {
By("Creating VirtualMachine")
vmSpec := newVMSpecBlankDVTemplate(includedVMName, "100Mi")
_, err := framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec)
Expand All @@ -880,11 +880,25 @@ var _ = Describe("Resource includes", func() {
resources := "datavolumes,virtualmachineinstances,pods,persistentvolumeclaims,persistentvolumes,volumesnapshots,volumesnapshotcontents"
err = framework.CreateBackupForResources(timeout, backupName, resources, f.Namespace.Name, snapshotLocation, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhasePartiallyFailed)
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Deleting VirtualMachineInstance")
err = framework.DeleteVirtualMachineInstance(f.KvClient, f.Namespace.Name, includedVMName)
Expect(err).ToNot(HaveOccurred())

By("Creating restore")
err = framework.CreateRestoreForBackup(timeout, backupName, restoreName, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForRestorePhase(timeout, restoreName, f.BackupNamespace, velerov1api.RestorePhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Verifying VMI not present")
_, err = framework.GetVirtualMachineInstance(f.KvClient, f.Namespace.Name, "test-vmi")
Expect(err).To(HaveOccurred())
})

It("[test_id:10199]Selecting VMI (without DV+PVC+Pod) but not VM: Backing up VMI should fail", func() {
It("[test_id:10199]Selecting VMI (without DV+PVC+Pod) but not VM: Empty backup without failure", func() {
By("Creating VirtualMachine")
vmSpec := newVMSpecBlankDVTemplate(includedVMName, "100Mi")
_, err := framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec)
Expand All @@ -901,8 +915,22 @@ var _ = Describe("Resource includes", func() {
resources := "virtualmachineinstances"
err = framework.CreateBackupForResources(timeout, backupName, resources, f.Namespace.Name, snapshotLocation, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhasePartiallyFailed)
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Deleting VirtualMachineInstance")
err = framework.DeleteVirtualMachineInstance(f.KvClient, f.Namespace.Name, includedVMName)
Expect(err).ToNot(HaveOccurred())

By("Creating restore")
err = framework.CreateRestoreForBackup(timeout, backupName, restoreName, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForRestorePhase(timeout, restoreName, f.BackupNamespace, velerov1api.RestorePhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Verifying VMI not present")
_, err = framework.GetVirtualMachineInstance(f.KvClient, f.Namespace.Name, "test-vmi")
Expect(err).To(HaveOccurred())
})
})

Expand Down Expand Up @@ -2129,7 +2157,7 @@ var _ = Describe("Resource excludes", func() {
Expect(err).ToNot(HaveOccurred())
})

It("[test_id:10227]Running VM excluded: backup should fail", func() {
It("[test_id:10227]Running VM excluded: empty backup without failure", func() {
By("Creating VirtualMachines")
vmSpec := newVMSpecBlankDVTemplate(includedVMName, "100Mi")
_, err := framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec)
Expand All @@ -2145,8 +2173,22 @@ var _ = Describe("Resource excludes", func() {
resources := "virtualmachine"
err = framework.CreateBackupForNamespaceExcludeResources(timeout, backupName, f.Namespace.Name, resources, snapshotLocation, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhasePartiallyFailed)
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Deleting VirtualMachineInstance")
err = framework.DeleteVirtualMachineInstance(f.KvClient, f.Namespace.Name, includedVMName)
Expect(err).ToNot(HaveOccurred())

By("Creating restore")
err = framework.CreateRestoreForBackup(timeout, backupName, restoreName, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForRestorePhase(timeout, restoreName, f.BackupNamespace, velerov1api.RestorePhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Verifying VMI not present")
_, err = framework.GetVirtualMachineInstance(f.KvClient, f.Namespace.Name, "test-vmi")
Expect(err).To(HaveOccurred())
})

It("[test_id:10228]Stopped VM excluded: DV+PVC should be restored", func() {
Expand Down Expand Up @@ -2940,7 +2982,7 @@ var _ = Describe("Resource excludes", func() {
Expect(err).ToNot(HaveOccurred())
})

It("[test_id:10254]VMI included, VM excluded: backup should fail", func() {
It("[test_id:10254]VMI included, VM excluded: empty backup without failure", func() {
By("Creating VirtualMachines")
vmSpec := newVMSpecBlankDVTemplate("test-vm", "100Mi")
_, err := framework.CreateVirtualMachineFromDefinition(f.KvClient, f.Namespace.Name, vmSpec)
Expand All @@ -2959,8 +3001,22 @@ var _ = Describe("Resource excludes", func() {
By("Creating backup")
err = framework.CreateBackupForNamespace(timeout, backupName, f.Namespace.Name, snapshotLocation, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhasePartiallyFailed)
err = framework.WaitForBackupPhase(timeout, backupName, f.BackupNamespace, velerov1api.BackupPhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Deleting VirtualMachineInstance")
err = framework.DeleteVirtualMachineInstance(f.KvClient, f.Namespace.Name, includedVMName)
Expect(err).ToNot(HaveOccurred())

By("Creating restore")
err = framework.CreateRestoreForBackup(timeout, backupName, restoreName, f.BackupNamespace, true)
Expect(err).ToNot(HaveOccurred())
err = framework.WaitForRestorePhase(timeout, restoreName, f.BackupNamespace, velerov1api.RestorePhaseCompleted)
Expect(err).ToNot(HaveOccurred())

By("Verifying VMI not present")
_, err = framework.GetVirtualMachineInstance(f.KvClient, f.Namespace.Name, "test-vmi")
Expect(err).To(HaveOccurred())
})
})

Expand Down
Loading