metrics: Fix NodeSelector with boolean values

When using a node selector with boolean values, e.g.: ``` apiVersion: sriovnetwork.openshift.io/v1 kind: SriovOperatorConfig metadata: name: default spec: configDaemonNodeSelector: feature.node.kubernetes.io/network-sriov.capable: "true" ``` the value needs to be quoted before forwarding it to the metrics-exporter node selector field. Fixes openshift#766 Signed-off-by: Andrea Panattoni <[email protected]>
zeeke · Oct 11, 2024 · 740018f · 740018f
1 parent 9c0c831
commit 740018f
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 17 deletions.
diff --git a/bindata/manifests/metrics-exporter/metrics-daemonset.yaml b/bindata/manifests/metrics-exporter/metrics-daemonset.yaml
@@ -88,7 +88,7 @@ spec:
             readOnly: true
       nodeSelector:
         {{- range $key, $value := .NodeSelectorField }}
-          {{ $key }}: {{ $value }}
+          {{ $key }}: "{{ $value }}"
         {{- end }}
       restartPolicy: Always
       volumes:

diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go
@@ -226,12 +226,9 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 
 		It("should be able to update the node selector of sriov-network-config-daemon", func() {
 			By("specify the configDaemonNodeSelector")
-			config := &sriovnetworkv1.SriovOperatorConfig{}
-			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
-
-			config.Spec.ConfigDaemonNodeSelector = map[string]string{"node-role.kubernetes.io/worker": ""}
-			err := k8sClient.Update(ctx, config)
-			Expect(err).NotTo(HaveOccurred())
+			nodeSelector := map[string]string{"node-role.kubernetes.io/worker": ""}
+			restore := updateConfigDaemonNodeSelector(nodeSelector)
+			DeferCleanup(restore)
 
 			daemonSet := &appsv1.DaemonSet{}
 			Eventually(func() map[string]string {
@@ -241,19 +238,17 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 					return nil
 				}
 				return daemonSet.Spec.Template.Spec.NodeSelector
-			}, util.APITimeout, util.RetryInterval).Should(Equal(config.Spec.ConfigDaemonNodeSelector))
+			}, util.APITimeout, util.RetryInterval).Should(Equal(nodeSelector))
 		})
 
 		It("should be able to do multiple updates to the node selector of sriov-network-config-daemon", func() {
 			By("changing the configDaemonNodeSelector")
-			config := &sriovnetworkv1.SriovOperatorConfig{}
-			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
-			config.Spec.ConfigDaemonNodeSelector = map[string]string{"labelA": "", "labelB": "", "labelC": ""}
-			err := k8sClient.Update(ctx, config)
-			Expect(err).NotTo(HaveOccurred())
-			config.Spec.ConfigDaemonNodeSelector = map[string]string{"labelA": "", "labelB": ""}
-			err = k8sClient.Update(ctx, config)
-			Expect(err).NotTo(HaveOccurred())
+			firstNodeSelector := map[string]string{"labelA": "", "labelB": "", "labelC": ""}
+			restore := updateConfigDaemonNodeSelector(firstNodeSelector)
+			DeferCleanup(restore)
+
+			secondNodeSelector := map[string]string{"labelA": "", "labelB": ""}
+			updateConfigDaemonNodeSelector(secondNodeSelector)
 
 			daemonSet := &appsv1.DaemonSet{}
 			Eventually(func() map[string]string {
@@ -262,7 +257,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 					return nil
 				}
 				return daemonSet.Spec.Template.Spec.NodeSelector
-			}, util.APITimeout, util.RetryInterval).Should(Equal(config.Spec.ConfigDaemonNodeSelector))
+			}, util.APITimeout, util.RetryInterval).Should(Equal(secondNodeSelector))
 		})
 
 		It("should not render disable-plugins cmdline flag of sriov-network-config-daemon if disablePlugin not provided in spec", func() {
@@ -365,6 +360,23 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 					Expect(err).ToNot(HaveOccurred())
 				})
 
+				It("should deploy the sriov-network-metrics-exporter using the Spec.ConfigDaemonNodeSelector field", func() {
+					nodeSelector := map[string]string{
+						"node-role.kubernetes.io/worker": "",
+						"bool-key":                       "true",
+					}
+
+					restore := updateConfigDaemonNodeSelector(nodeSelector)
+					DeferCleanup(restore)
+
+					Eventually(func(g Gomega) {
+						metricsDaemonset := appsv1.DaemonSet{}
+						err := util.WaitForNamespacedObject(&metricsDaemonset, k8sClient, testNamespace, "sriov-network-metrics-exporter", util.RetryInterval, util.APITimeout)
+						g.Expect(err).NotTo(HaveOccurred())
+						g.Expect(metricsDaemonset.Spec.Template.Spec.NodeSelector).To((Equal(nodeSelector)))
+					}).Should(Succeed())
+				})
+
 				It("should deploy extra configuration when the Prometheus operator is installed", func() {
 					DeferCleanup(os.Setenv, "METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED", os.Getenv("METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED"))
 					os.Setenv("METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED", "true")
@@ -511,3 +523,20 @@ func assertResourceExists(gvk schema.GroupVersionKind, key client.ObjectKey) {
 	err := k8sClient.Get(context.Background(), key, u)
 	Expect(err).NotTo(HaveOccurred())
 }
+
+func updateConfigDaemonNodeSelector(newValue map[string]string) func() {
+	config := &sriovnetworkv1.SriovOperatorConfig{}
+	err := k8sClient.Get(context.Background(), types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)
+	Expect(err).NotTo(HaveOccurred())
+
+	previousValue := config.Spec.ConfigDaemonNodeSelector
+	ret := func() {
+		updateConfigDaemonNodeSelector(previousValue)
+	}
+
+	config.Spec.ConfigDaemonNodeSelector = newValue
+	err = k8sClient.Update(context.Background(), config)
+	Expect(err).NotTo(HaveOccurred())
+
+	return ret
+}