KSM: Replace reactors

Using fake client has a benefit to actually assert on the output of the "function" rather than arbitrary slice of bytes. For example when asserting for "randomlabel" the patch can be testing for this value or replacing it. This can easily turn to unexpected coverage of the code. Signed-off-by: Luboslav Pivarc <[email protected]>
KarstenB · Oct 19, 2023 · 84ad2f9 · 84ad2f9
1 parent 20c5e5f
commit 84ad2f9
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 8 deletions.
diff --git a/pkg/virt-handler/heartbeat/BUILD.bazel b/pkg/virt-handler/heartbeat/BUILD.bazel
@@ -40,6 +40,7 @@ go_test(
         "//staging/src/kubevirt.io/client-go/testutils:go_default_library",
         "//vendor/github.com/onsi/ginkgo/v2:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/github.com/onsi/gomega/types:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",

diff --git a/pkg/virt-handler/heartbeat/ksm_test.go b/pkg/virt-handler/heartbeat/ksm_test.go
@@ -40,6 +40,8 @@ import (
 	kubevirtv1 "kubevirt.io/api/core/v1"
 
 	"kubevirt.io/kubevirt/pkg/testutils"
+
+	gomegatypes "github.com/onsi/gomega/types"
 )
 
 const (
@@ -123,15 +125,18 @@ var _ = Describe("KSM", func() {
 		fakeClient := fake.NewSimpleClientset(node)
 
 		heartbeat := NewHeartBeat(fakeClient.CoreV1(), deviceController(true), config(virtconfig.CPUManager), "mynode")
-		testutils.DoNotExpectNodePatch(fakeClient, kubevirtv1.KSMEnabledLabel)
+
 		heartbeat.do()
+		node, err := fakeClient.CoreV1().Nodes().Get(context.TODO(), "mynode", metav1.GetOptions{})
+		Expect(err).ToNot(HaveOccurred())
+		Expect(node.Labels).To(HaveKeyWithValue(kubevirtv1.KSMEnabledLabel, "false"))
 
-		err := os.WriteFile(filepath.Join(fakeSysKSMDir, "run"), []byte("1\n"), 0644)
+		err = os.WriteFile(filepath.Join(fakeSysKSMDir, "run"), []byte("1\n"), 0644)
 		Expect(err).ToNot(HaveOccurred())
 
 		createCustomMemInfo(false)
 
-		testutils.ExpectNodePatch(fakeClient, kubevirtv1.KSMEnabledLabel)
+		// TODO: What is this supposed to do?
 		heartbeat.do()
 	})
 
@@ -159,7 +164,9 @@ var _ = Describe("KSM", func() {
 			clusterConfig, _, _ = testutils.NewFakeClusterConfigUsingKV(kv)
 		})
 
-		DescribeTable("with memory pressure, should", func(initialKsmValue string, nodeLabels, nodeAnnotations map[string]string, expectedNodePatch []string, expectedKsmValue string) {
+		DescribeTable("with memory pressure, should", func(initialKsmValue string,
+			nodeLabels, nodeAnnotations map[string]string,
+			labelsMatcher gomegatypes.GomegaMatcher, annotationsMatcher gomegatypes.GomegaMatcher, expectedKsmValue string) {
 			node := &v1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "mynode",
@@ -173,23 +180,31 @@ var _ = Describe("KSM", func() {
 			createCustomMemInfo(true)
 			heartbeat := NewHeartBeat(fakeClient.CoreV1(), deviceController(true), clusterConfig, "mynode")
 
-			testutils.ExpectNodePatch(fakeClient, expectedNodePatch...)
 			heartbeat.do()
+
+			node, err = fakeClient.CoreV1().Nodes().Get(context.TODO(), "mynode", metav1.GetOptions{})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(node.Labels).To(labelsMatcher)
+			Expect(node.Annotations).To(annotationsMatcher)
+
 			running, err := os.ReadFile(filepath.Join(fakeSysKSMDir, "run"))
 			Expect(err).NotTo(HaveOccurred())
 			Expect(string(bytes.TrimSpace(running))).To(Equal(expectedKsmValue))
 		},
 			Entry("enable ksm if the node labels match ksmConfiguration.nodeLabelSelector",
 				"0\n", map[string]string{"test_label": "true"}, make(map[string]string),
-				[]string{kubevirtv1.KSMEnabledLabel, kubevirtv1.KSMHandlerManagedAnnotation}, "1",
+				HaveKeyWithValue(kubevirtv1.KSMEnabledLabel, "true"), HaveKeyWithValue(kubevirtv1.KSMHandlerManagedAnnotation, "true"),
+				"1",
 			),
 			Entry("disable ksm if the node labels does not match ksmConfiguration.nodeLabelSelector and the node has the KSMHandlerManagedAnnotation annotation",
 				"1\n", map[string]string{"test_label": "false"}, map[string]string{kubevirtv1.KSMHandlerManagedAnnotation: "true"},
-				[]string{kubevirtv1.KSMHandlerManagedAnnotation}, "0",
+				HaveKeyWithValue(kubevirtv1.KSMEnabledLabel, "false"), HaveKeyWithValue(kubevirtv1.KSMHandlerManagedAnnotation, "false"),
+				"0",
 			),
 			Entry("not change ksm if the node labels does not match ksmConfiguration.nodeLabelSelector and the node does not have the KSMHandlerManagedAnnotation annotation",
 				"1\n", map[string]string{"test_label": "false"}, make(map[string]string),
-				nil, "1",
+				HaveKeyWithValue(kubevirtv1.KSMEnabledLabel, "true"), HaveKeyWithValue(kubevirtv1.KSMHandlerManagedAnnotation, "false"),
+				"1",
 			),
 		)