From c4572d7b20b4565a167bf69f0882d072d0511618 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Thu, 29 Aug 2024 11:59:05 +0200 Subject: [PATCH] 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 #766 Signed-off-by: Andrea Panattoni --- .../metrics-exporter/metrics-daemonset.yaml | 2 +- .../sriovoperatorconfig_controller_test.go | 61 ++++++++++++++----- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/bindata/manifests/metrics-exporter/metrics-daemonset.yaml b/bindata/manifests/metrics-exporter/metrics-daemonset.yaml index 6e433f07d..8393bd1bb 100644 --- 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 index 582d9781d..9ba05490b 100644 --- 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") @@ -501,3 +513,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 +}