Merge pull request kubevirt#8116 from EdDev/tests-replace-update-with…

…-patch

tests: Replace `Update` with `Patch`

pkg/controller/controller.go

@@ -311,11 +311,6 @@ func GeneratePatchBytes(ops []string) []byte {
 	return []byte(fmt.Sprintf("[%s]", strings.Join(ops, ", ")))
 }
 
-func EscapeJSONPointer(ptr string) string {
-	s := strings.ReplaceAll(ptr, "~", "~0")
-	return strings.ReplaceAll(s, "/", "~1")
-}
-
 func SetVMIPhaseTransitionTimestamp(oldVMI *v1.VirtualMachineInstance, newVMI *v1.VirtualMachineInstance) {
 	if oldVMI.Status.Phase != newVMI.Status.Phase {
 		for _, transitionTimeStamp := range newVMI.Status.PhaseTransitionTimestamps {

pkg/util/types/patch.go

@@ -22,6 +22,7 @@ package types
 import (
 	"encoding/json"
 	"fmt"
+	"strings"
 )
 
 type PatchOperation struct {
@@ -70,3 +71,8 @@ func UnmarshalPatch(patch []byte) ([]PatchOperation, error) {
 
 	return p, err
 }
+
+func EscapeJSONPointer(ptr string) string {
+	s := strings.ReplaceAll(ptr, "~", "~0")
+	return strings.ReplaceAll(s, "/", "~1")
+}

pkg/virt-controller/watch/drain/disruptionbudget/BUILD.bazel

@@ -9,6 +9,7 @@ go_library(
         "//pkg/controller:go_default_library",
         "//pkg/util/migrations:go_default_library",
         "//pkg/util/pdbs:go_default_library",
+        "//pkg/util/types:go_default_library",
         "//pkg/virt-config:go_default_library",
         "//staging/src/kubevirt.io/api/core/v1:go_default_library",
         "//staging/src/kubevirt.io/client-go/kubecli:go_default_library",
@@ -34,8 +35,8 @@ go_test(
     ],
     deps = [
         ":go_default_library",
-        "//pkg/controller:go_default_library",
         "//pkg/testutils:go_default_library",
+        "//pkg/util/types:go_default_library",
         "//pkg/virt-config:go_default_library",
         "//staging/src/kubevirt.io/api/core/v1:go_default_library",
         "//staging/src/kubevirt.io/client-go/api:go_default_library",

pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget.go

@@ -23,6 +23,7 @@ import (
 	"kubevirt.io/kubevirt/pkg/controller"
 	"kubevirt.io/kubevirt/pkg/util/migrations"
 	"kubevirt.io/kubevirt/pkg/util/pdbs"
+	kubevirttypes "kubevirt.io/kubevirt/pkg/util/types"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 )
 
@@ -451,7 +452,7 @@ func (c *DisruptionBudgetController) deletePDB(key string, pdb *policyv1.PodDisr
 func (c *DisruptionBudgetController) shrinkPDB(vmi *virtv1.VirtualMachineInstance, pdb *policyv1.PodDisruptionBudget) error {
 	if pdb != nil && pdb.DeletionTimestamp == nil && pdb.Spec.MinAvailable.IntValue() != 1 {
 		patch := []byte(fmt.Sprintf(`[{ "op": "replace", "path": "/spec/minAvailable", "value": 1 }, { "op": "remove", "path": "/metadata/labels/%s" }]`,
-			controller.EscapeJSONPointer(virtv1.MigrationNameLabel)))
+			kubevirttypes.EscapeJSONPointer(virtv1.MigrationNameLabel)))
 
 		_, err := c.clientset.PolicyV1().PodDisruptionBudgets(pdb.Namespace).Patch(context.Background(), pdb.Name, types.JSONPatchType, patch, v1.PatchOptions{})
 		if err != nil {

pkg/virt-controller/watch/drain/disruptionbudget/disruptionbudget_test.go

@@ -23,8 +23,8 @@ import (
 	v1 "kubevirt.io/api/core/v1"
 	"kubevirt.io/client-go/kubecli"
 
-	ctrl_util "kubevirt.io/kubevirt/pkg/controller"
 	"kubevirt.io/kubevirt/pkg/testutils"
+	kubevirttypes "kubevirt.io/kubevirt/pkg/util/types"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 	"kubevirt.io/kubevirt/pkg/virt-controller/watch/drain/disruptionbudget"
 )
@@ -111,7 +111,7 @@ var _ = Describe("Disruptionbudget", func() {
 			Expect(patch.GetPatchType()).To(Equal(types.JSONPatchType))
 
 			expectedPatch := fmt.Sprintf(`[{ "op": "replace", "path": "/spec/minAvailable", "value": 1 }, { "op": "remove", "path": "/metadata/labels/%s" }]`,
-				ctrl_util.EscapeJSONPointer(v1.MigrationNameLabel))
+				kubevirttypes.EscapeJSONPointer(v1.MigrationNameLabel))
 			Expect(string(patch.GetPatch())).To(Equal(expectedPatch))
 			return true, &policyv1.PodDisruptionBudget{}, nil
 		})

pkg/virt-controller/watch/migration.go

@@ -1116,7 +1116,7 @@ func (c *MigrationController) sync(key string, migration *virtv1.VirtualMachineI
 				if vmi.Annotations == nil {
 					patches = append(patches, fmt.Sprintf(`{ "op": "add", "path": "/metadata/annotations", "value":  { "%s": "true"} }`, virtv1.DeprecatedNonRootVMIAnnotation))
 				} else if _, ok := vmi.Annotations[virtv1.DeprecatedNonRootVMIAnnotation]; !ok {
-					patches = append(patches, fmt.Sprintf(`{ "op": "add", "path": "/metadata/annotations/%s", "value": "true"}`, controller.EscapeJSONPointer(virtv1.DeprecatedNonRootVMIAnnotation)))
+					patches = append(patches, fmt.Sprintf(`{ "op": "add", "path": "/metadata/annotations/%s", "value": "true"}`, kubevirttypes.EscapeJSONPointer(virtv1.DeprecatedNonRootVMIAnnotation)))
 				}
 			} else {
 				// The cluster is configured for root VMs, ensure the VMI is root.
@@ -1127,7 +1127,7 @@ func (c *MigrationController) sync(key string, migration *virtv1.VirtualMachineI
 
 				if vmi.Annotations != nil {
 					if _, ok := vmi.Annotations[virtv1.DeprecatedNonRootVMIAnnotation]; ok {
-						patches = append(patches, fmt.Sprintf(`{ "op": "remove", "path": "/metadata/annotations/%s"}`, controller.EscapeJSONPointer(virtv1.DeprecatedNonRootVMIAnnotation)))
+						patches = append(patches, fmt.Sprintf(`{ "op": "remove", "path": "/metadata/annotations/%s"}`, kubevirttypes.EscapeJSONPointer(virtv1.DeprecatedNonRootVMIAnnotation)))
 					}
 				}
 			}

tests/BUILD.bazel

@@ -175,6 +175,7 @@ go_test(
         "//pkg/util/hardware:go_default_library",
         "//pkg/util/migrations:go_default_library",
         "//pkg/util/net/dns:go_default_library",
+        "//pkg/util/types:go_default_library",
         "//pkg/virt-config:go_default_library",
         "//pkg/virt-controller/leaderelectionconfig:go_default_library",
         "//pkg/virt-controller/services:go_default_library",

tests/infra_test.go

@@ -70,6 +70,7 @@ import (
 	"kubevirt.io/client-go/kubecli"
 
 	clusterutil "kubevirt.io/kubevirt/pkg/util/cluster"
+	k6ttypes "kubevirt.io/kubevirt/pkg/util/types"
 	"kubevirt.io/kubevirt/pkg/virt-controller/leaderelectionconfig"
 	nodelabellerutil "kubevirt.io/kubevirt/pkg/virt-handler/node-labeller/util"
 	"kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/components"
@@ -394,27 +395,19 @@ var _ = Describe("[Serial][sig-compute]Infrastructure", func() {
 			AfterEach(func() {
 				if selectedNodeName != "" {
 					By("removing the taint from the tainted node")
-					err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-						selectedNode, err := virtClient.CoreV1().Nodes().Get(context.Background(), selectedNodeName, metav1.GetOptions{})
-						if err != nil {
-							return err
-						}
+					selectedNode, err := virtClient.CoreV1().Nodes().Get(context.Background(), selectedNodeName, metav1.GetOptions{})
+					Expect(err).NotTo(HaveOccurred())
 
-						var taints []k8sv1.Taint
-						for _, taint := range selectedNode.Spec.Taints {
-							if taint.Key != "CriticalAddonsOnly" {
-								taints = append(taints, taint)
-							}
+					var taints []k8sv1.Taint
+					for _, taint := range selectedNode.Spec.Taints {
+						if taint.Key != "CriticalAddonsOnly" {
+							taints = append(taints, taint)
 						}
-
-						nodeCopy := selectedNode.DeepCopy()
-						nodeCopy.ResourceVersion = ""
-						nodeCopy.Spec.Taints = taints
-
-						_, err = virtClient.CoreV1().Nodes().Update(context.Background(), nodeCopy, metav1.UpdateOptions{})
-						return err
-					})
-					Expect(err).ShouldNot(HaveOccurred())
+					}
+					patchData, err := k6ttypes.GenerateTestReplacePatch("/spec/taints", selectedNode.Spec.Taints, taints)
+					Expect(err).NotTo(HaveOccurred())
+					selectedNode, err = virtClient.CoreV1().Nodes().Patch(context.Background(), selectedNode.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
+					Expect(err).NotTo(HaveOccurred())
 				}
 			})
 
@@ -490,23 +483,19 @@ var _ = Describe("[Serial][sig-compute]Infrastructure", func() {
 				go signalTerminatedPods(stopCn, lw.ResultChan(), terminatedPodsCn)
 
 				By("tainting the selected node")
-				err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
-					selectedNode, err := virtClient.CoreV1().Nodes().Get(context.Background(), selectedNodeName, metav1.GetOptions{})
-					if err != nil {
-						return err
-					}
+				selectedNode, err := virtClient.CoreV1().Nodes().Get(context.Background(), selectedNodeName, metav1.GetOptions{})
+				Expect(err).NotTo(HaveOccurred())
 
-					selectedNodeCopy := selectedNode.DeepCopy()
-					selectedNodeCopy.Spec.Taints = append(selectedNodeCopy.Spec.Taints, k8sv1.Taint{
-						Key:    "CriticalAddonsOnly",
-						Value:  "",
-						Effect: k8sv1.TaintEffectNoExecute,
-					})
-
-					_, err = virtClient.CoreV1().Nodes().Update(context.Background(), selectedNodeCopy, metav1.UpdateOptions{})
-					return err
+				taints := append(selectedNode.Spec.Taints, k8sv1.Taint{
+					Key:    "CriticalAddonsOnly",
+					Value:  "",
+					Effect: k8sv1.TaintEffectNoExecute,
 				})
-				Expect(err).ShouldNot(HaveOccurred())
+
+				patchData, err := k6ttypes.GenerateTestReplacePatch("/spec/taints", selectedNode.Spec.Taints, taints)
+				Expect(err).ToNot(HaveOccurred())
+				selectedNode, err = virtClient.CoreV1().Nodes().Patch(context.Background(), selectedNode.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
+				Expect(err).ToNot(HaveOccurred())
 
 				Consistently(terminatedPodsCn, 5*time.Second).ShouldNot(Receive(), "pods should not terminate")
 				stopCn <- true

tests/instancetype_test.go

@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/pointer"
 
 	v1 "kubevirt.io/api/core/v1"
@@ -23,6 +24,7 @@ import (
 	"kubevirt.io/client-go/kubecli"
 
 	instancetypepkg "kubevirt.io/kubevirt/pkg/instancetype"
+	k6ttypes "kubevirt.io/kubevirt/pkg/util/types"
 	"kubevirt.io/kubevirt/tests"
 	cd "kubevirt.io/kubevirt/tests/containerdisk"
 	"kubevirt.io/kubevirt/tests/libvmi"
@@ -546,8 +548,9 @@ var _ = Describe("[crit:medium][vendor:[email protected]][level:component][sig-c
 
 			By("Updating the VirtualMachineInstancetype vCPU count")
 			newInstancetypeCPUGuest := originalInstancetypeCPUGuest + 1
-			instancetype.Spec.CPU.Guest = newInstancetypeCPUGuest
-			updatedInstancetype, err := virtClient.VirtualMachineInstancetype(util.NamespaceTestDefault).Update(context.Background(), instancetype, metav1.UpdateOptions{})
+			patchData, err := k6ttypes.GenerateTestReplacePatch("/spec/cpu/guest", originalInstancetypeCPUGuest, newInstancetypeCPUGuest)
+			Expect(err).ToNot(HaveOccurred())
+			updatedInstancetype, err := virtClient.VirtualMachineInstancetype(util.NamespaceTestDefault).Patch(context.Background(), instancetype.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(updatedInstancetype.Spec.CPU.Guest).To(Equal(newInstancetypeCPUGuest))
 

tests/pool_test.go

@@ -33,6 +33,8 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 
+	k6ttypes "kubevirt.io/kubevirt/pkg/util/types"
+
 	"kubevirt.io/kubevirt/tests/libvmi"
 	"kubevirt.io/kubevirt/tests/util"
 
@@ -44,6 +46,11 @@ import (
 	cd "kubevirt.io/kubevirt/tests/containerdisk"
 )
 
+const (
+	newLabelKey   = "newlabel"
+	newLabelValue = "newvalue"
+)
+
 var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 	var err error
 	var virtClient kubecli.KubevirtClient
@@ -325,8 +332,13 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 		newPool, err = virtClient.VirtualMachinePool(newPool.ObjectMeta.Namespace).Get(context.Background(), newPool.ObjectMeta.Name, v12.GetOptions{})
 		Expect(err).ToNot(HaveOccurred())
 
-		newPool.Spec.VirtualMachineTemplate.ObjectMeta.Labels["newlabel"] = "newvalue"
-		newPool, err = virtClient.VirtualMachinePool(newPool.ObjectMeta.Namespace).Update(context.Background(), newPool, metav1.UpdateOptions{})
+		patchData, err := k6ttypes.GeneratePatchPayload(k6ttypes.PatchOperation{
+			Op:    k6ttypes.PatchAddOp,
+			Path:  fmt.Sprintf("/spec/virtualMachineTemplate/metadata/labels/%s", newLabelKey),
+			Value: newLabelValue,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		newPool, err = virtClient.VirtualMachinePool(newPool.ObjectMeta.Namespace).Patch(context.Background(), newPool.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
 		Expect(err).ToNot(HaveOccurred())
 
 		By("Ensuring VM picks up label")
@@ -337,7 +349,7 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 				return err
 			}
 
-			_, ok := vm.Labels["newlabel"]
+			_, ok := vm.Labels[newLabelKey]
 			if !ok {
 				return fmt.Errorf("Expected vm pool update to roll out to VMs")
 			}
@@ -379,8 +391,13 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		// Make a VMI template change
-		newPool.Spec.VirtualMachineTemplate.Spec.Template.ObjectMeta.Labels["newlabel"] = "newvalue"
-		newPool, err = virtClient.VirtualMachinePool(newPool.ObjectMeta.Namespace).Update(context.Background(), newPool, metav1.UpdateOptions{})
+		patchData, err := k6ttypes.GeneratePatchPayload(k6ttypes.PatchOperation{
+			Op:    k6ttypes.PatchAddOp,
+			Path:  fmt.Sprintf("/spec/virtualMachineTemplate/spec/template/metadata/labels/%s", newLabelKey),
+			Value: newLabelValue,
+		})
+		Expect(err).ToNot(HaveOccurred())
+		newPool, err = virtClient.VirtualMachinePool(newPool.ObjectMeta.Namespace).Patch(context.Background(), newPool.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
 		Expect(err).ToNot(HaveOccurred())
 
 		By("Ensuring VM picks up label")
@@ -391,7 +408,7 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 				return err
 			}
 
-			_, ok := vm.Spec.Template.ObjectMeta.Labels["newlabel"]
+			_, ok := vm.Spec.Template.ObjectMeta.Labels[newLabelKey]
 			if !ok {
 				return fmt.Errorf("Expected vm pool update to roll out to VMs")
 			}
@@ -409,7 +426,7 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 			if vmi.UID == vmiUID {
 				return fmt.Errorf("Waiting on VMI to get deleted and recreated")
 			}
-			_, ok := vmi.ObjectMeta.Labels["newlabel"]
+			_, ok := vmi.ObjectMeta.Labels[newLabelKey]
 			if !ok {
 				return fmt.Errorf("Expected vmi to pick up the new updated label")
 			}
@@ -472,8 +489,9 @@ var _ = Describe("[sig-compute]VirtualMachinePool", func() {
 
 		// set new replica count while still being paused
 		By("Updating the number of replicas")
-		pool.Spec.Replicas = tests.NewInt32(1)
-		_, err = virtClient.VirtualMachinePool(pool.ObjectMeta.Namespace).Update(context.Background(), pool, metav1.UpdateOptions{})
+		patchData, err := k6ttypes.GenerateTestReplacePatch("/spec/replicas", pool.Spec.Replicas, tests.NewInt32(1))
+		Expect(err).ToNot(HaveOccurred())
+		pool, err = virtClient.VirtualMachinePool(pool.ObjectMeta.Namespace).Patch(context.Background(), pool.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
 		Expect(err).ToNot(HaveOccurred())
 
 		// make sure that we don't scale up

tests/replicaset_test.go

@@ -47,6 +47,7 @@ import (
 	"kubevirt.io/client-go/kubecli"
 
 	"kubevirt.io/kubevirt/pkg/controller"
+	k6ttypes "kubevirt.io/kubevirt/pkg/util/types"
 	"kubevirt.io/kubevirt/tests"
 )
 
@@ -339,8 +340,9 @@ var _ = Describe("[rfe_id:588][crit:medium][vendor:[email protected]][level:comp
 
 		// set new replica count while still being paused
 		By("Updating the number of replicas")
-		rs.Spec.Replicas = tests.NewInt32(2)
-		_, err = virtClient.ReplicaSet(rs.ObjectMeta.Namespace).Update(rs)
+		patchData, err := k6ttypes.GenerateTestReplacePatch("/spec/replicas", rs.Spec.Replicas, tests.NewInt32(2))
+		Expect(err).ToNot(HaveOccurred())
+		rs, err = virtClient.ReplicaSet(rs.Namespace).Patch(rs.Name, types.JSONPatchType, patchData)
 		Expect(err).ToNot(HaveOccurred())
 
 		// make sure that we don't scale up

tests/storage/BUILD.bazel

@@ -66,7 +66,6 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
-        "//vendor/k8s.io/client-go/util/retry:go_default_library",
         "//vendor/k8s.io/utils/pointer:go_default_library",
         "//vendor/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
     ],

tests/storage/datavolume.go

@@ -35,14 +35,17 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/utils/pointer"
 
 	v1 "kubevirt.io/api/core/v1"
 	"kubevirt.io/client-go/kubecli"
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
 
+	k6ttypes "kubevirt.io/kubevirt/pkg/util/types"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
+
 	"kubevirt.io/kubevirt/tests"
 	"kubevirt.io/kubevirt/tests/clientcmd"
 	"kubevirt.io/kubevirt/tests/console"
@@ -112,12 +115,14 @@ var _ = SIGDescribe("DataVolume Integration", func() {
 			By("Expecting the VirtualMachineInstance console")
 			Expect(console.LoginToCirros(vmi)).To(Succeed())
 
-			pvc, err := virtClient.CoreV1().PersistentVolumeClaims(util.NamespaceTestDefault).Get(context.Background(), dataVolume.Name, metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred())
-
 			By("Expanding PVC")
-			pvc.Spec.Resources.Requests[k8sv1.ResourceStorage] = resource.MustParse("2Gi")
-			_, err = virtClient.CoreV1().PersistentVolumeClaims(util.NamespaceTestDefault).Update(context.Background(), pvc, metav1.UpdateOptions{})
+			patchData, err := k6ttypes.GeneratePatchPayload(k6ttypes.PatchOperation{
+				Op:    k6ttypes.PatchAddOp,
+				Path:  "/spec/resources/requests/storage",
+				Value: resource.MustParse("2Gi"),
+			})
+			Expect(err).ToNot(HaveOccurred())
+			_, err = virtClient.CoreV1().PersistentVolumeClaims(util.NamespaceTestDefault).Patch(context.Background(), dataVolume.Name, types.JSONPatchType, patchData, metav1.PatchOptions{})
 			Expect(err).ToNot(HaveOccurred())
 
 			By("Waiting for notification about size change")