Merge pull request #732 from zeeke/metrics-exporter-rules

adrianchiris · web-flow · commit aecb4bb904f2 · 2024-09-19T09:16:31.000+03:00
[metrics 4/x] Metrics exporter rules
diff --git a/bindata/manifests/metrics-exporter/metrics-prometheus-rule.yaml b/bindata/manifests/metrics-exporter/metrics-prometheus-rule.yaml
@@ -0,0 +1,38 @@
+---
+{{ if and .IsPrometheusOperatorInstalled .PrometheusOperatorDeployRules }}
+apiVersion: monitoring.coreos.com/v1
+kind: PrometheusRule
+metadata:
+  name: sriov-vf-rules
+  namespace: {{.Namespace}}
+spec:
+  groups:
+  - name: sriov-network-metrics-operator.rules
+    interval: 30s
+    rules:
+    - expr: |
+        sriov_vf_tx_packets * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_tx_packets
+    - expr: |
+        sriov_vf_rx_packets * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_rx_packets
+    - expr: |
+        sriov_vf_tx_bytes * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_tx_bytes
+    - expr: |
+        sriov_vf_rx_bytes * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_rx_bytes
+    - expr: |
+        sriov_vf_tx_dropped * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_tx_dropped
+    - expr: |
+        sriov_vf_rx_dropped * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_rx_dropped
+    - expr: |
+        sriov_vf_rx_broadcast * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_rx_broadcast
+    - expr: |
+        sriov_vf_rx_multicast * on (pciAddr) group_left(pod,namespace,dev_type) sriov_kubepoddevice
+      record: network:sriov_vf_rx_multicast
+{{ end }}
+
diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go
@@ -241,6 +241,7 @@ func (r *SriovOperatorConfigReconciler) syncMetricsExporter(ctx context.Context,
 	data.Data["IsOpenshift"] = r.PlatformHelper.IsOpenshiftCluster()
 
 	data.Data["IsPrometheusOperatorInstalled"] = strings.ToLower(os.Getenv("METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED")) == trueString
+	data.Data["PrometheusOperatorDeployRules"] = strings.ToLower(os.Getenv("METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES")) == trueString
 	data.Data["PrometheusOperatorServiceAccount"] = os.Getenv("METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT")
 	data.Data["PrometheusOperatorNamespace"] = os.Getenv("METRICS_EXPORTER_PROMETHEUS_OPERATOR_NAMESPACE")
 
diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go
@@ -380,6 +380,8 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 				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")
+					DeferCleanup(os.Setenv, "METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES", os.Getenv("METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES"))
+					os.Setenv("METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES", "true")
 
 					err := util.WaitForNamespacedObject(&rbacv1.Role{}, k8sClient, testNamespace, "prometheus-k8s", util.RetryInterval, util.APITimeout)
 					Expect(err).ToNot(HaveOccurred())
@@ -394,6 +396,14 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 							Version: "v1",
 						},
 						client.ObjectKey{Namespace: testNamespace, Name: "sriov-network-metrics-exporter"})
+
+					assertResourceExists(
+						schema.GroupVersionKind{
+							Group:   "monitoring.coreos.com",
+							Kind:    "PrometheusRule",
+							Version: "v1",
+						},
+						client.ObjectKey{Namespace: testNamespace, Name: "sriov-vf-rules"})
 				})
 			})
 		})
diff --git a/deploy/operator.yaml b/deploy/operator.yaml
@@ -78,6 +78,8 @@ spec:
               value: $METRICS_EXPORTER_KUBE_RBAC_PROXY_IMAGE
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED
               value: "$METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED"
+            - name: METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES
+              value: "$METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES"
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT
               value: $METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_NAMESPACE
diff --git a/deploy/role.yaml b/deploy/role.yaml
@@ -29,9 +29,12 @@ rules:
   - monitoring.coreos.com
   resources:
   - servicemonitors
+  - prometheusrules
   verbs:
   - get
   - create
+  - update
+  - delete
 - apiGroups:
   - apps
   resourceNames:
diff --git a/deployment/sriov-network-operator-chart/README.md b/deployment/sriov-network-operator-chart/README.md
@@ -89,6 +89,7 @@ We have introduced the following Chart parameters.
 | `operator.metricsExporter.prometheusOperator.enabled` | bool | false | Wheter the operator shoud configure Prometheus resources or not (e.g. `ServiceMonitors`). |
 | `operator.metricsExporter.prometheusOperator.serviceAccount` | string | `prometheus-k8s` | The service account used by the Prometheus Operator. This is used to give Prometheus the permission to list resource in the SR-IOV operator namespace |
 | `operator.metricsExporter.prometheusOperator.namespace` | string | `monitoring` | The namespace where the Prometheus Operator is installed. Setting this variable makes the operator deploy `monitoring.coreos.com` resources. |
+| `operator.metricsExporter.prometheusOperator.deployRules` | bool | false | Whether the operator should deploy `PrometheusRules` to scrape namespace version of metrics. |
 
 #### Admission Controllers parameters
 
diff --git a/deployment/sriov-network-operator-chart/templates/operator.yaml b/deployment/sriov-network-operator-chart/templates/operator.yaml
@@ -83,6 +83,8 @@ spec:
             {{- if .Values.operator.metricsExporter.prometheusOperator.enabled }}
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED
               value: {{ .Values.operator.metricsExporter.prometheusOperator.enabled | quote}}
+            - name: METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULES
+              value: {{ .Values.operator.metricsExporter.prometheusOperator.deployRules | quote}}
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT
               value: {{ .Values.operator.metricsExporter.prometheusOperator.serviceAccount }}
             - name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_NAMESPACE
diff --git a/deployment/sriov-network-operator-chart/templates/role.yaml b/deployment/sriov-network-operator-chart/templates/role.yaml
@@ -32,9 +32,12 @@ rules:
       - monitoring.coreos.com
     resources:
       - servicemonitors
+      - prometheusrules
     verbs:
       - get
       - create
+      - update
+      - delete
   - apiGroups:
       - apps
     resourceNames:
diff --git a/deployment/sriov-network-operator-chart/values.yaml b/deployment/sriov-network-operator-chart/values.yaml
@@ -35,6 +35,7 @@ operator:
       enabled: false
       serviceAccount: "prometheus-k8s"
       namespace: "monitoring"
+      deployRules: false
   admissionControllers:
     enabled: false
     certificates:
diff --git a/hack/run-e2e-conformance-virtual-ocp.sh b/hack/run-e2e-conformance-virtual-ocp.sh
@@ -191,6 +191,7 @@ export DEV_MODE=TRUE
 export CLUSTER_HAS_EMULATED_PF=TRUE
 export OPERATOR_LEADER_ELECTION_ENABLE=true
 export METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED=true
+export METRICS_EXPORTER_PROMETHEUS_DEPLOY_RULE=true
 export METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT=${METRICS_EXPORTER_PROMETHEUS_OPERATOR_SERVICE_ACCOUNT:-"prometheus-k8s"}
 export METRICS_EXPORTER_PROMETHEUS_OPERATOR_NAMESPACE=${METRICS_EXPORTER_PROMETHEUS_OPERATOR_NAMESPACE:-"openshfit-monitoring"}
 
diff --git a/test/conformance/tests/test_exporter_metrics.go b/test/conformance/tests/test_exporter_metrics.go
@@ -2,9 +2,12 @@ package tests
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"net/url"
 	"strings"
 
+	sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/cluster"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/discovery"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/namespaces"
@@ -13,6 +16,7 @@ import (
 
 	dto "github.com/prometheus/client_model/go"
 	"github.com/prometheus/common/expfmt"
+	"github.com/prometheus/common/model"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -22,6 +26,8 @@ import (
 )
 
 var _ = Describe("[sriov] Metrics Exporter", Ordered, func() {
+	var node string
+	var nic *sriovv1.InterfaceExt
 
 	BeforeAll(func() {
 		if cluster.VirtualCluster() {
@@ -48,13 +54,11 @@ var _ = Describe("[sriov] Metrics Exporter", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		WaitForSRIOVStable()
-	})
 
-	It("collects metrics regarding receiving traffic via VF", func() {
 		sriovInfos, err := cluster.DiscoverSriov(clients, operatorNamespace)
 		Expect(err).ToNot(HaveOccurred())
 
-		node, nic, err := sriovInfos.FindOneSriovNodeAndDevice()
+		node, nic, err = sriovInfos.FindOneSriovNodeAndDevice()
 		Expect(err).ToNot(HaveOccurred())
 		By("Using device " + nic.Name + " on node " + node)
 
@@ -65,7 +69,13 @@ var _ = Describe("[sriov] Metrics Exporter", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 		waitForNetAttachDef("test-me-network", namespaces.Test)
 
+		DeferCleanup(namespaces.Clean, operatorNamespace, namespaces.Test, clients, discovery.Enabled())
+	})
+
+	It("collects metrics regarding receiving traffic via VF", func() {
+
 		pod := createTestPod(node, []string{"test-me-network"})
+		DeferCleanup(namespaces.CleanPods, namespaces.Test, clients)
 
 		ips, err := network.GetSriovNicIPs(pod, "net1")
 		Expect(err).ToNot(HaveOccurred())
@@ -88,6 +98,28 @@ var _ = Describe("[sriov] Metrics Exporter", Ordered, func() {
 		Expect(finalRxPackets).Should(BeNumerically(">", initialRxPackets))
 	})
 
+	It("PrometheusRule should provide namespaced metrics", func() {
+		pod := createTestPod(node, []string{"test-me-network"})
+		DeferCleanup(namespaces.CleanPods, namespaces.Test, clients)
+
+		namespacedMetricNames := []string{
+			"network:sriov_vf_rx_bytes",
+			"network:sriov_vf_tx_bytes",
+			"network:sriov_vf_rx_packets",
+			"network:sriov_vf_tx_packets",
+			"network:sriov_vf_rx_dropped",
+			"network:sriov_vf_tx_dropped",
+			"network:sriov_vf_rx_broadcast",
+			"network:sriov_vf_rx_multicast",
+		}
+
+		Eventually(func(g Gomega) {
+			for _, metricName := range namespacedMetricNames {
+				values := runPromQLQuery(fmt.Sprintf(`%s{namespace="%s",pod="%s"}`, metricName, pod.Namespace, pod.Name))
+				g.Expect(values).ToNot(BeEmpty(), "no value for metric %s", metricName)
+			}
+		}, "40s", "1s").Should(Succeed())
+	})
 })
 
 func getMetricsForNode(nodeName string) map[string]*dto.MetricFamily {
@@ -185,3 +217,33 @@ func areLabelsMatching(labels []*dto.LabelPair, labelsToMatch map[string]string)
 
 	return true
 }
+
+func runPromQLQuery(query string) model.Vector {
+	prometheusPods, err := clients.Pods("").List(context.Background(), metav1.ListOptions{
+		LabelSelector: "app.kubernetes.io/component=prometheus",
+	})
+	ExpectWithOffset(1, err).ToNot(HaveOccurred())
+	ExpectWithOffset(1, prometheusPods.Items).ToNot(HaveLen(0), "At least one Prometheus operator pod expected")
+
+	prometheusPod := prometheusPods.Items[0]
+
+	url := fmt.Sprintf("localhost:9090/api/v1/query?%s", (url.Values{"query": []string{query}}).Encode())
+	command := []string{"curl", url}
+	stdout, stderr, err := pod.ExecCommand(clients, &prometheusPod, command...)
+	ExpectWithOffset(1, err).ToNot(HaveOccurred(),
+		"promQL query failed: [%s/%s] command: [%v]\nstdout: %s\nstderr: %s", prometheusPod.Namespace, prometheusPod.Name, command, stdout, stderr)
+
+	result := struct {
+		Status string `json:"status"`
+		Data   struct {
+			ResultType string       `json:"resultType"`
+			Result     model.Vector `json:"result"`
+		} `json:"data"`
+	}{}
+
+	json.Unmarshal([]byte(stdout), &result)
+	ExpectWithOffset(1, err).ToNot(HaveOccurred())
+	ExpectWithOffset(1, result.Status).To(Equal("success"), "cURL for [%s] failed: %s", url, stdout)
+
+	return result.Data.Result
+}
diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go
@@ -311,6 +311,14 @@ var _ = Describe("[sriov] operator", func() {
 					g.Expect(err).ToNot(HaveOccurred())
 				}).Should(Succeed())
 			})
+
+			It("should remove ServiceMonitor when the feature is turned off", func() {
+				setFeatureFlag("metricsExporter", false)
+				Eventually(func(g Gomega) {
+					_, err := clients.ServiceMonitors(operatorNamespace).Get(context.Background(), "sriov-network-metrics-exporter", metav1.GetOptions{})
+					g.Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+				}).Should(Succeed())
+			})
 		})
 	})
 
diff --git a/test/util/crds/monitoring.coreos.com_prometheusrules.yaml b/test/util/crds/monitoring.coreos.com_prometheusrules.yaml
