Drop certain keys from clone annotation filters

Some annotations are purely needed for functionality, and we cannot get rid of them easily. Signed-off-by: Alex Kalenyuk <[email protected]>
zyjj1 · Dec 10, 2023 · f1e36f5 · f1e36f5
1 parent c5ae4a4
commit f1e36f5
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/pkg/virt-controller/watch/clone/clone_test.go b/pkg/virt-controller/watch/clone/clone_test.go
@@ -615,7 +615,8 @@ var _ = Describe("Clone", func() {
 				Expect(restore.Spec.VirtualMachineSnapshotName).To(Equal(snapshotName))
 
 				patchedVM, err := offlinePatchVM(sourceVM, restore.Spec.Patches)
-				Expect(patchedVM.Spec).To(Equal(patchedVM.Spec))
+				Expect(err).ToNot(HaveOccurred())
+				Expect(patchedVM.Spec).To(Equal(expectedVM.Spec))
 
 				return true, create.GetObject(), nil
 			})
@@ -694,8 +695,11 @@ var _ = Describe("Clone", func() {
 					if i%2 == 0 {
 						iface.MacAddress = generateNewMacAddress()
 						newMacAddresses[iface.Name] = iface.MacAddress
-						expectedInterfaces[i] = iface
+					} else {
+						// empty MAC for others
+						iface.MacAddress = ""
 					}
+					expectedInterfaces[i] = iface
 				}
 
 				sourceVM.Spec.Template.Spec.Domain.Devices.Interfaces = originalInterfaces
@@ -805,6 +809,35 @@ var _ = Describe("Clone", func() {
 				Entry("with annotations", annotations),
 			)
 
+			It("should not strip lastRestoreUID annotation from newly created VM", func() {
+				if sourceVM.Annotations == nil {
+					sourceVM.Annotations = make(map[string]string)
+				}
+				sourceVM.Annotations["restore.kubevirt.io/lastRestoreUID"] = "bar"
+				// controller mutates original ptrs annotations
+				sourceVMCpy := sourceVM.DeepCopy()
+				vmClone.Spec.AnnotationFilters = []string{"somekey/*"}
+				addVM(sourceVM)
+				addClone(vmClone)
+
+				client.Fake.PrependReactor("create", restoreResource, func(action testing.Action) (handled bool, ret runtime.Object, err error) {
+					create, ok := action.(testing.CreateAction)
+					Expect(ok).To(BeTrue())
+
+					restore := create.GetObject().(*snapshotv1alpha1.VirtualMachineRestore)
+					Expect(restore.Spec.VirtualMachineSnapshotName).To(Equal(snapshotName))
+
+					expectedPatches := []string{`{"op": "replace", "path": "/spec/template/spec/domain/devices/interfaces/0/macAddress", "value": ""}`}
+					Expect(restore.Spec.Patches).To(Equal(expectedPatches))
+					patchedVM, err := offlinePatchVM(sourceVMCpy, restore.Spec.Patches)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(patchedVM.Annotations).To(HaveKey("restore.kubevirt.io/lastRestoreUID"))
+
+					return true, create.GetObject(), nil
+				})
+				controller.Execute()
+			})
+
 		})
 
 		Context("Firmware UUID", func() {

diff --git a/pkg/virt-controller/watch/clone/vm-target.go b/pkg/virt-controller/watch/clone/vm-target.go
@@ -99,6 +99,8 @@ func generateTemplateLabelPatches(labels map[string]string, filters []string) (p
 
 func generateAnnotationPatches(annotations map[string]string, filters []string) (patches []string) {
 	const basePath = "/metadata/annotations"
+	// Some keys are needed for restore functionality
+	delete(annotations, "restore.kubevirt.io/lastRestoreUID")
 	return generateStrStrMapPatches(annotations, filters, basePath)
 }
 
@@ -107,12 +109,12 @@ func generateTemplateAnnotationPatches(annotations map[string]string, filters []
 	return generateStrStrMapPatches(annotations, filters, basePath)
 }
 
-func generateStrStrMapPatches(m map[string]string, filters []string, baseJsonPath string) (patches []string) {
+func generateStrStrMapPatches(m map[string]string, filters []string, baseJSONPath string) (patches []string) {
 	appendRemovalPatch := func(key string) {
 		const patchPattern = `{"op": "remove", "path": "%s/%s"}`
 
 		key = addKeyEscapeCharacters(key)
-		patches = append(patches, fmt.Sprintf(patchPattern, baseJsonPath, key))
+		patches = append(patches, fmt.Sprintf(patchPattern, baseJSONPath, key))
 	}
 
 	if filters == nil {
@@ -146,7 +148,7 @@ func generateStrStrMapPatches(m map[string]string, filters []string, baseJsonPat
 
 	includedKeys := map[string]struct{}{}
 	// Negation filters have precedence, therefore regular filters would be applied first
-	for key, _ := range m {
+	for key := range m {
 		for _, filter := range regularFilters {
 			if matchRegex(filter, key) {
 				includedKeys[key] = struct{}{}
@@ -161,7 +163,7 @@ func generateStrStrMapPatches(m map[string]string, filters []string, baseJsonPat
 	}
 
 	// Appending removal patches
-	for originalKey, _ := range m {
+	for originalKey := range m {
 		if _, isIncluded := includedKeys[originalKey]; !isIncluded {
 			appendRemovalPatch(originalKey)
 		}

diff --git a/tests/clone_test.go b/tests/clone_test.go
@@ -251,7 +251,6 @@ var _ = Describe("[Serial]VirtualMachineClone Tests", Serial, func() {
 		}
 		expectEqualTemplateAnnotations := func(targetVM, sourceVM *virtv1.VirtualMachine, keysToExclude ...string) {
 			expectEqualStrMap(targetVM.Spec.Template.ObjectMeta.Annotations, sourceVM.Spec.Template.ObjectMeta.Annotations, fmt.Sprintf(cloneShouldEqualSourceMsgPattern, "template.annotations"), keysToExclude...)
-
 		}
 
 		expectSpecsToEqualExceptForMacAddress := func(vm1, vm2 *virtv1.VirtualMachine) {
-Original file line number
+Diff line change
@@ Expand Up @@
     		}
     		expectEqualTemplateAnnotations := func(targetVM, sourceVM *virtv1.VirtualMachine, keysToExclude ...string) {
     			expectEqualStrMap(targetVM.Spec.Template.ObjectMeta.Annotations, sourceVM.Spec.Template.ObjectMeta.Annotations, fmt.Sprintf(cloneShouldEqualSourceMsgPattern, "template.annotations"), keysToExclude...)
     		}
     		expectSpecsToEqualExceptForMacAddress := func(vm1, vm2 *virtv1.VirtualMachine) {
@@ Expand Down @@