test: Fix flake in patching out finalizers (#5823)

aws · Mar 12, 2024 · cc4076d · cc4076d
1 parent 916bfd7
commit cc4076d
Show file tree

Hide file tree

Showing 9 changed files with 22 additions and 17 deletions.
diff --git a/go.mod b/go.mod
@@ -28,7 +28,7 @@ require (
 	k8s.io/utils v0.0.0-20240102154912-e7106e64919e
 	knative.dev/pkg v0.0.0-20231010144348-ca8c009405dd
 	sigs.k8s.io/controller-runtime v0.17.2
-	sigs.k8s.io/karpenter v0.35.1-0.20240307191015-7ab5eca34e53
+	sigs.k8s.io/karpenter v0.35.1-0.20240311230445-343eb7580913
 	sigs.k8s.io/yaml v1.4.0
 )
 

diff --git a/go.sum b/go.sum
@@ -757,8 +757,8 @@ sigs.k8s.io/controller-runtime v0.17.2 h1:FwHwD1CTUemg0pW2otk7/U5/i5m2ymzvOXdbeG
 sigs.k8s.io/controller-runtime v0.17.2/go.mod h1:+MngTvIQQQhfXtwfdGw/UOQ/aIaqsYywfCINOtwMO/s=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
-sigs.k8s.io/karpenter v0.35.1-0.20240307191015-7ab5eca34e53 h1:B2Hwd9cIdVg26iFUGG4H8ieqlYx8OMv172GZ26sOv9o=
-sigs.k8s.io/karpenter v0.35.1-0.20240307191015-7ab5eca34e53/go.mod h1:HcZ33DJe0gATRfcPoA+/9+bq6dTtkBbo/BoPQ3AImcM=
+sigs.k8s.io/karpenter v0.35.1-0.20240311230445-343eb7580913 h1:ldnc5SBfq/5lMXt1NSQMPr/ONvGwfoD6Cf/uCptFEGY=
+sigs.k8s.io/karpenter v0.35.1-0.20240311230445-343eb7580913/go.mod h1:DYnwDaoy2AhZwL2Ie96RGVu15+K6P5AJkSAtpPCRy8k=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
 sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=

diff --git a/test/pkg/environment/common/expectations.go b/test/pkg/environment/common/expectations.go
@@ -135,7 +135,7 @@ func (env *Environment) ExpectSettingsReplaced(vars ...v1.EnvVar) {
 
 	if !equality.Semantic.DeepEqual(d, stored) {
 		By("replacing environment variables for karpenter deployment")
-		Expect(env.Client.Patch(env.Context, d, client.MergeFrom(stored))).To(Succeed())
+		Expect(env.Client.Patch(env.Context, d, client.StrategicMergeFrom(stored))).To(Succeed())
 		env.EventuallyExpectKarpenterRestarted()
 	}
 }
@@ -159,7 +159,7 @@ func (env *Environment) ExpectSettingsOverridden(vars ...v1.EnvVar) {
 	}
 	if !equality.Semantic.DeepEqual(d, stored) {
 		By("overriding environment variables for karpenter deployment")
-		Expect(env.Client.Patch(env.Context, d, client.MergeFrom(stored))).To(Succeed())
+		Expect(env.Client.Patch(env.Context, d, client.StrategicMergeFrom(stored))).To(Succeed())
 		env.EventuallyExpectKarpenterRestarted()
 	}
 }
@@ -179,7 +179,7 @@ func (env *Environment) ExpectSettingsRemoved(vars ...v1.EnvVar) {
 	})
 	if !equality.Semantic.DeepEqual(d, stored) {
 		By("removing environment variables for karpenter deployment")
-		Expect(env.Client.Patch(env.Context, d, client.MergeFrom(stored))).To(Succeed())
+		Expect(env.Client.Patch(env.Context, d, client.StrategicMergeFrom(stored))).To(Succeed())
 		env.EventuallyExpectKarpenterRestarted()
 	}
 }
@@ -329,7 +329,7 @@ func (env *Environment) EventuallyExpectRollout(name, namespace string) {
 		"kubectl.kubernetes.io/restartedAt": time.Now().Format(time.RFC3339),
 	}
 	deploy.Spec.Template.Annotations = lo.Assign(deploy.Spec.Template.Annotations, restartedAtAnnotation)
-	Expect(env.Client.Patch(env.Context, deploy, client.MergeFrom(stored))).To(Succeed())
+	Expect(env.Client.Patch(env.Context, deploy, client.StrategicMergeFrom(stored))).To(Succeed())
 
 	By("waiting for the newly generated deployment to rollout")
 	Eventually(func(g Gomega) {
@@ -777,7 +777,7 @@ func (env *Environment) ExpectDaemonSetEnvironmentVariableUpdated(obj client.Obj
 		Expect(len(ds.Spec.Template.Spec.Containers)).To(BeNumerically("==", 1))
 		containers = append(containers, ds.Spec.Template.Spec.Containers[0].Name)
 	}
-	patch := client.MergeFrom(ds.DeepCopy())
+	patch := client.StrategicMergeFrom(ds.DeepCopy())
 	containerNames := sets.New(containers...)
 	for ci := range ds.Spec.Template.Spec.Containers {
 		c := &ds.Spec.Template.Spec.Containers[ci]
@@ -824,7 +824,7 @@ func (env *Environment) ForcePodsToSpread(nodes ...*v1.Node) {
 		Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(node), node)).To(Succeed())
 		stored := node.DeepCopy()
 		node.Spec.Unschedulable = true
-		Expect(env.Client.Patch(env.Context, node, client.MergeFrom(stored))).To(Succeed())
+		Expect(env.Client.Patch(env.Context, node, client.StrategicMergeFrom(stored))).To(Succeed())
 		for _, pod := range nodePods[maxPodsPerNode:] {
 			env.ExpectDeleted(pod)
 		}
@@ -842,7 +842,7 @@ func (env *Environment) ForcePodsToSpread(nodes ...*v1.Node) {
 	for _, n := range nodes {
 		stored := n.DeepCopy()
 		n.Spec.Unschedulable = false
-		Expect(env.Client.Patch(env.Context, n, client.MergeFrom(stored))).To(Succeed())
+		Expect(env.Client.Patch(env.Context, n, client.StrategicMergeFrom(stored))).To(Succeed())
 	}
 }
 

diff --git a/test/pkg/environment/common/setup.go b/test/pkg/environment/common/setup.go
@@ -145,13 +145,18 @@ func (env *Environment) ExpectTestingFinalizerRemoved(obj client.Object) error {
 	if err := env.Client.Get(env, client.ObjectKeyFromObject(obj), metaObj); err != nil {
 		return client.IgnoreNotFound(err)
 	}
-
 	deepCopy := metaObj.DeepCopy()
 	metaObj.Finalizers = lo.Reject(metaObj.Finalizers, func(finalizer string, _ int) bool {
 		return finalizer == TestingFinalizer
 	})
 
 	if !equality.Semantic.DeepEqual(metaObj, deepCopy) {
+		// If the Group is the "core" APIs, then we can strategic merge patch
+		// CRDs do not currently have support for strategic merge patching, so we can't blindly do it
+		// https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#advanced-features-and-flexibility:~:text=Yes-,strategic%2Dmerge%2Dpatch,-The%20new%20endpoints
+		if metaObj.GroupVersionKind().Group == "" {
+			return client.IgnoreNotFound(env.Client.Patch(env, metaObj, client.StrategicMergeFrom(deepCopy)))
+		}
 		return client.IgnoreNotFound(env.Client.Patch(env, metaObj, client.MergeFrom(deepCopy)))
 	}
 	return nil

diff --git a/test/suites/chaos/suite_test.go b/test/suites/chaos/suite_test.go
@@ -150,7 +150,7 @@ func (t *taintAdder) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	if err := t.kubeClient.Get(ctx, req.NamespacedName, node); err != nil {
 		return reconcile.Result{}, client.IgnoreNotFound(err)
 	}
-	mergeFrom := client.MergeFrom(node.DeepCopy())
+	mergeFrom := client.StrategicMergeFrom(node.DeepCopy())
 	taint := v1.Taint{
 		Key:    "test",
 		Value:  "true",

diff --git a/test/suites/drift/suite_test.go b/test/suites/drift/suite_test.go
@@ -615,7 +615,7 @@ var _ = Describe("Drift", func() {
 				g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(nodeTwo), nodeTwo)).To(Succeed())
 				stored := nodeTwo.DeepCopy()
 				nodeTwo.Spec.Taints = lo.Reject(nodeTwo.Spec.Taints, func(t v1.Taint, _ int) bool { return t.Key == "example.com/another-taint-2" })
-				g.Expect(env.Client.Patch(env.Context, nodeTwo, client.MergeFrom(stored))).To(Succeed())
+				g.Expect(env.Client.Patch(env.Context, nodeTwo, client.StrategicMergeFrom(stored))).To(Succeed())
 			}).Should(Succeed())
 		}
 		env.EventuallyExpectNotFound(pod, node)

diff --git a/test/suites/expiration/suite_test.go b/test/suites/expiration/suite_test.go
@@ -660,7 +660,7 @@ var _ = Describe("Expiration", func() {
 					g.Expect(env.Client.Get(env.Context, client.ObjectKeyFromObject(node), node)).To(Succeed())
 					stored := node.DeepCopy()
 					node.Spec.Taints = lo.Reject(node.Spec.Taints, func(t v1.Taint, _ int) bool { return t.Key == "example.com/taint" })
-					g.Expect(env.Client.Patch(env.Context, node, client.MergeFrom(stored))).To(Succeed())
+					g.Expect(env.Client.Patch(env.Context, node, client.StrategicMergeFrom(stored))).To(Succeed())
 				}
 			}).Should(Succeed())
 

diff --git a/test/suites/integration/emptiness_test.go b/test/suites/integration/emptiness_test.go
@@ -111,7 +111,7 @@ var _ = Describe("Emptiness", func() {
 		By("making the nodeclaim empty")
 		persisted := deployment.DeepCopy()
 		deployment.Spec.Replicas = ptr.Int32(0)
-		Expect(env.Client.Patch(env, deployment, client.MergeFrom(persisted))).To(Succeed())
+		Expect(env.Client.Patch(env, deployment, client.StrategicMergeFrom(persisted))).To(Succeed())
 
 		env.EventuallyExpectEmpty(nodeClaim)
 

diff --git a/test/suites/integration/storage_test.go b/test/suites/integration/storage_test.go
@@ -241,7 +241,7 @@ func ExpectSetEBSDriverLimit(limit int) {
 		containers[i].Args = append(containers[i].Args, fmt.Sprintf("--volume-attach-limit=%d", limit))
 		break
 	}
-	Expect(env.Client.Patch(env.Context, ds, client.MergeFrom(stored))).To(Succeed())
+	Expect(env.Client.Patch(env.Context, ds, client.StrategicMergeFrom(stored))).To(Succeed())
 }
 
 func ExpectRemoveEBSDriverLimit() {
@@ -260,5 +260,5 @@ func ExpectRemoveEBSDriverLimit() {
 		})
 		break
 	}
-	Expect(env.Client.Patch(env.Context, ds, client.MergeFrom(stored))).To(Succeed())
+	Expect(env.Client.Patch(env.Context, ds, client.StrategicMergeFrom(stored))).To(Succeed())
 }