Add missing label servicemonitor (open-telemetry#2573)

* Add missing label for Service/Pod Monitors Signed-off-by: Yuri Sa <[email protected]> Add missing label for Service/Pod Monitors Signed-off-by: Yuri Sa <[email protected]> Add missing label for Service/Pod Monitors Signed-off-by: Yuri Sa <[email protected]> * Added manifests.utils to set labels Signed-off-by: Yuri Sa <[email protected]> * Fixed labels names Signed-off-by: Yuri Sa <[email protected]> * Fixed labels for PodMonitor Signed-off-by: Yuri Sa <[email protected]> * Fixed e2e test Signed-off-by: Yuri Sa <[email protected]> * Fixed e2e test Signed-off-by: Yuri Sa <[email protected]> * Fixed selectorMatchLabels Signed-off-by: Yuri Sa <[email protected]> * Removed the extra method Signed-off-by: Yuri Sa <[email protected]> * Adjusted nil Signed-off-by: Yuri Sa <[email protected]> * Changed labels Signed-off-by: Yuri Sa <[email protected]> * Changed labels Signed-off-by: Yuri Sa <[email protected]> * Readded common labels Signed-off-by: Yuri Sa <[email protected]> * Readded common labels Signed-off-by: Yuri Sa <[email protected]> * Added labels assert Signed-off-by: Yuri Sa <[email protected]> --------- Signed-off-by: Yuri Sa <[email protected]>
ItielOlenick · Mar 11, 2024 · 52d0e49 · 52d0e49
1 parent 5decd22
commit 52d0e49
Show file tree

Hide file tree

Showing 12 changed files with 189 additions and 67 deletions.
diff --git a/.chloggen/add-missing-label-servicemonitor.yaml b/.chloggen/add-missing-label-servicemonitor.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Added missing label for Service/Pod Monitors
+
+# One or more tracking issues related to the change
+issues: [2251]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
diff --git a/controllers/builder_test.go b/controllers/builder_test.go
@@ -318,12 +318,13 @@ service:
 						Name:      "test-collector-monitoring",
 						Namespace: "test",
 						Labels: map[string]string{
-							"app.kubernetes.io/component":  "opentelemetry-collector",
-							"app.kubernetes.io/instance":   "test.test",
-							"app.kubernetes.io/managed-by": "opentelemetry-operator",
-							"app.kubernetes.io/name":       "test-collector-monitoring",
-							"app.kubernetes.io/part-of":    "opentelemetry",
-							"app.kubernetes.io/version":    "latest",
+							"app.kubernetes.io/component":                            "opentelemetry-collector",
+							"app.kubernetes.io/instance":                             "test.test",
+							"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+							"app.kubernetes.io/name":                                 "test-collector-monitoring",
+							"app.kubernetes.io/part-of":                              "opentelemetry",
+							"app.kubernetes.io/version":                              "latest",
+							"operator.opentelemetry.io/collector-monitoring-service": "Exists",
 						},
 						Annotations: nil,
 					},
@@ -564,12 +565,13 @@ service:
 						Name:      "test-collector-monitoring",
 						Namespace: "test",
 						Labels: map[string]string{
-							"app.kubernetes.io/component":  "opentelemetry-collector",
-							"app.kubernetes.io/instance":   "test.test",
-							"app.kubernetes.io/managed-by": "opentelemetry-operator",
-							"app.kubernetes.io/name":       "test-collector-monitoring",
-							"app.kubernetes.io/part-of":    "opentelemetry",
-							"app.kubernetes.io/version":    "latest",
+							"app.kubernetes.io/component":                            "opentelemetry-collector",
+							"app.kubernetes.io/instance":                             "test.test",
+							"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+							"app.kubernetes.io/name":                                 "test-collector-monitoring",
+							"app.kubernetes.io/part-of":                              "opentelemetry",
+							"app.kubernetes.io/version":                              "latest",
+							"operator.opentelemetry.io/collector-monitoring-service": "Exists",
 						},
 						Annotations: nil,
 					},
@@ -831,12 +833,13 @@ service:
 						Name:      "test-collector-monitoring",
 						Namespace: "test",
 						Labels: map[string]string{
-							"app.kubernetes.io/component":  "opentelemetry-collector",
-							"app.kubernetes.io/instance":   "test.test",
-							"app.kubernetes.io/managed-by": "opentelemetry-operator",
-							"app.kubernetes.io/name":       "test-collector-monitoring",
-							"app.kubernetes.io/part-of":    "opentelemetry",
-							"app.kubernetes.io/version":    "latest",
+							"app.kubernetes.io/component":                            "opentelemetry-collector",
+							"app.kubernetes.io/instance":                             "test.test",
+							"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+							"app.kubernetes.io/name":                                 "test-collector-monitoring",
+							"app.kubernetes.io/part-of":                              "opentelemetry",
+							"app.kubernetes.io/version":                              "latest",
+							"operator.opentelemetry.io/collector-monitoring-service": "Exists",
 						},
 						Annotations: nil,
 					},
@@ -1313,12 +1316,13 @@ service:
 						Name:      "test-collector-monitoring",
 						Namespace: "test",
 						Labels: map[string]string{
-							"app.kubernetes.io/component":  "opentelemetry-collector",
-							"app.kubernetes.io/instance":   "test.test",
-							"app.kubernetes.io/managed-by": "opentelemetry-operator",
-							"app.kubernetes.io/name":       "test-collector-monitoring",
-							"app.kubernetes.io/part-of":    "opentelemetry",
-							"app.kubernetes.io/version":    "latest",
+							"app.kubernetes.io/component":                            "opentelemetry-collector",
+							"app.kubernetes.io/instance":                             "test.test",
+							"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+							"app.kubernetes.io/name":                                 "test-collector-monitoring",
+							"app.kubernetes.io/part-of":                              "opentelemetry",
+							"app.kubernetes.io/version":                              "latest",
+							"operator.opentelemetry.io/collector-monitoring-service": "Exists",
 						},
 						Annotations: nil,
 					},
@@ -1711,12 +1715,13 @@ prometheus_cr:
 						Name:      "test-collector-monitoring",
 						Namespace: "test",
 						Labels: map[string]string{
-							"app.kubernetes.io/component":  "opentelemetry-collector",
-							"app.kubernetes.io/instance":   "test.test",
-							"app.kubernetes.io/managed-by": "opentelemetry-operator",
-							"app.kubernetes.io/name":       "test-collector-monitoring",
-							"app.kubernetes.io/part-of":    "opentelemetry",
-							"app.kubernetes.io/version":    "latest",
+							"app.kubernetes.io/component":                            "opentelemetry-collector",
+							"app.kubernetes.io/instance":                             "test.test",
+							"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+							"app.kubernetes.io/name":                                 "test-collector-monitoring",
+							"app.kubernetes.io/part-of":                              "opentelemetry",
+							"app.kubernetes.io/version":                              "latest",
+							"operator.opentelemetry.io/collector-monitoring-service": "Exists",
 						},
 						Annotations: nil,
 					},

diff --git a/internal/manifests/collector/podmonitor.go b/internal/manifests/collector/podmonitor.go
@@ -15,7 +15,6 @@
 package collector
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -25,10 +24,11 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
+	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
 	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
 )
 
-// ServiceMonitor returns the service monitor for the given instance.
+// PodMonitor returns the pod monitor for the given instance.
 func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
 	if !params.OtelCol.Spec.Observability.Metrics.EnableMetrics {
 		params.Log.V(2).Info("Metrics disabled for this OTEL Collector",
@@ -42,16 +42,14 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
 	if params.OtelCol.Spec.Mode != v1beta1.ModeSidecar {
 		return nil, nil
 	}
-
+	name := naming.PodMonitor(params.OtelCol.Name)
+	labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, nil)
+	selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
 	pm = monitoringv1.PodMonitor{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: params.OtelCol.Namespace,
-			Name:      naming.PodMonitor(params.OtelCol.Name),
-			Labels: map[string]string{
-				"app.kubernetes.io/name":       naming.PodMonitor(params.OtelCol.Name),
-				"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
-				"app.kubernetes.io/managed-by": "opentelemetry-operator",
-			},
+			Name:      name,
+			Labels:    labels,
 		},
 		Spec: monitoringv1.PodMonitorSpec{
 			JobLabel:        "app.kubernetes.io/instance",
@@ -60,10 +58,7 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) {
 				MatchNames: []string{params.OtelCol.Namespace},
 			},
 			Selector: metav1.LabelSelector{
-				MatchLabels: map[string]string{
-					"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
-				},
+				MatchLabels: selectorLabels,
 			},
 			PodMetricsEndpoints: append(
 				[]monitoringv1.PodMetricsEndpoint{

diff --git a/internal/manifests/collector/podmonitor_test.go b/internal/manifests/collector/podmonitor_test.go
@@ -42,17 +42,33 @@ func TestDesiredPodMonitors(t *testing.T) {
 	assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
 	assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
 	assert.Equal(t, "monitoring", actual.Spec.PodMetricsEndpoints[0].Port)
+	expectedSelectorLabels := map[string]string{
+		"app.kubernetes.io/component":  "opentelemetry-collector",
+		"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
+		"app.kubernetes.io/managed-by": "opentelemetry-operator",
+		"app.kubernetes.io/part-of":    "opentelemetry",
+	}
+	assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
+}
 
-	params, err = newParams("", "testdata/prometheus-exporter.yaml")
+func TestDesiredPodMonitorsWithPrometheus(t *testing.T) {
+	params, err := newParams("", "testdata/prometheus-exporter.yaml")
 	assert.NoError(t, err)
 	params.OtelCol.Spec.Mode = v1beta1.ModeSidecar
 	params.OtelCol.Spec.Observability.Metrics.EnableMetrics = true
-	actual, err = PodMonitor(params)
+	actual, err := PodMonitor(params)
 	assert.NoError(t, err)
 	assert.NotNil(t, actual)
 	assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
 	assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
 	assert.Equal(t, "monitoring", actual.Spec.PodMetricsEndpoints[0].Port)
 	assert.Equal(t, "prometheus-dev", actual.Spec.PodMetricsEndpoints[1].Port)
 	assert.Equal(t, "prometheus-prod", actual.Spec.PodMetricsEndpoints[2].Port)
+	expectedSelectorLabels := map[string]string{
+		"app.kubernetes.io/managed-by": "opentelemetry-operator",
+		"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
+		"app.kubernetes.io/part-of":    "opentelemetry",
+		"app.kubernetes.io/component":  "opentelemetry-collector",
+	}
+	assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
 }
diff --git a/internal/manifests/collector/service.go b/internal/manifests/collector/service.go
@@ -29,10 +29,11 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
 )
 
-// headless label is to differentiate the headless service from the clusterIP service.
+// headless and monitoring labels are to differentiate the headless/monitoring services from the clusterIP service.
 const (
-	headlessLabel  = "operator.opentelemetry.io/collector-headless-service"
-	headlessExists = "Exists"
+	headlessLabel   = "operator.opentelemetry.io/collector-headless-service"
+	monitoringLabel = "operator.opentelemetry.io/collector-monitoring-service"
+	valueExists     = "Exists"
 )
 
 func HeadlessService(params manifests.Params) (*corev1.Service, error) {
@@ -42,7 +43,7 @@ func HeadlessService(params manifests.Params) (*corev1.Service, error) {
 	}
 
 	h.Name = naming.HeadlessService(params.OtelCol.Name)
-	h.Labels[headlessLabel] = headlessExists
+	h.Labels[headlessLabel] = valueExists
 
 	// copy to avoid modifying params.OtelCol.Annotations
 	annotations := map[string]string{
@@ -61,6 +62,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) {
 
 	name := naming.MonitoringService(params.OtelCol.Name)
 	labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
+	labels[monitoringLabel] = valueExists
 
 	out, err := params.OtelCol.Spec.Config.Yaml()
 	if err != nil {

diff --git a/internal/manifests/collector/servicemonitor.go b/internal/manifests/collector/servicemonitor.go
@@ -15,7 +15,6 @@
 package collector
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -25,6 +24,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
+	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
 	"github.com/open-telemetry/opentelemetry-operator/internal/naming"
 )
 
@@ -42,15 +42,16 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
 	if params.OtelCol.Spec.Mode == v1beta1.ModeSidecar {
 		return nil, nil
 	}
+	name := naming.ServiceMonitor(params.OtelCol.Name)
+	labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
+	selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
+	selectorLabels[monitoringLabel] = valueExists
+
 	sm = monitoringv1.ServiceMonitor{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: params.OtelCol.Namespace,
-			Name:      naming.ServiceMonitor(params.OtelCol.Name),
-			Labels: map[string]string{
-				"app.kubernetes.io/name":       naming.ServiceMonitor(params.OtelCol.Name),
-				"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
-				"app.kubernetes.io/managed-by": "opentelemetry-operator",
-			},
+			Name:      name,
+			Labels:    labels,
 		},
 		Spec: monitoringv1.ServiceMonitorSpec{
 			Endpoints: append([]monitoringv1.Endpoint{
@@ -62,10 +63,7 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
 				MatchNames: []string{params.OtelCol.Namespace},
 			},
 			Selector: metav1.LabelSelector{
-				MatchLabels: map[string]string{
-					"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					"app.kubernetes.io/instance":   fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
-				},
+				MatchLabels: selectorLabels,
 			},
 		},
 	}

diff --git a/internal/manifests/collector/servicemonitor_test.go b/internal/manifests/collector/servicemonitor_test.go
@@ -35,16 +35,34 @@ func TestDesiredServiceMonitors(t *testing.T) {
 	assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
 	assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
 	assert.Equal(t, "monitoring", actual.Spec.Endpoints[0].Port)
+	expectedSelectorLabels := map[string]string{
+		"app.kubernetes.io/component":                            "opentelemetry-collector",
+		"app.kubernetes.io/instance":                             "default.test",
+		"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+		"app.kubernetes.io/part-of":                              "opentelemetry",
+		"operator.opentelemetry.io/collector-monitoring-service": "Exists",
+	}
+	assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
+}
 
-	params, err = newParams("", "testdata/prometheus-exporter.yaml")
+func TestDesiredServiceMonitorsWithPrometheus(t *testing.T) {
+	params, err := newParams("", "testdata/prometheus-exporter.yaml")
 	assert.NoError(t, err)
 	params.OtelCol.Spec.Observability.Metrics.EnableMetrics = true
-	actual, err = ServiceMonitor(params)
+	actual, err := ServiceMonitor(params)
 	assert.NoError(t, err)
 	assert.NotNil(t, actual)
 	assert.Equal(t, fmt.Sprintf("%s-collector", params.OtelCol.Name), actual.Name)
 	assert.Equal(t, params.OtelCol.Namespace, actual.Namespace)
 	assert.Equal(t, "monitoring", actual.Spec.Endpoints[0].Port)
 	assert.Equal(t, "prometheus-dev", actual.Spec.Endpoints[1].Port)
 	assert.Equal(t, "prometheus-prod", actual.Spec.Endpoints[2].Port)
+	expectedSelectorLabels := map[string]string{
+		"app.kubernetes.io/component":                            "opentelemetry-collector",
+		"app.kubernetes.io/instance":                             "default.test",
+		"app.kubernetes.io/managed-by":                           "opentelemetry-operator",
+		"app.kubernetes.io/part-of":                              "opentelemetry",
+		"operator.opentelemetry.io/collector-monitoring-service": "Exists",
+	}
+	assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
 }
diff --git a/tests/e2e-prometheuscr/create-pm-prometheus-exporters/01-assert.yaml b/tests/e2e-prometheuscr/create-pm-prometheus-exporters/01-assert.yaml
@@ -42,3 +42,4 @@ spec:
     matchLabels:
       app.kubernetes.io/managed-by: opentelemetry-operator
       app.kubernetes.io/instance: create-pm-prometheus.simplest
+
diff --git a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/01-assert.yaml b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/01-assert.yaml
@@ -17,8 +17,11 @@ spec:
     - create-sm-prometheus
   selector:
     matchLabels:
-      app.kubernetes.io/managed-by: opentelemetry-operator
+      app.kubernetes.io/component: opentelemetry-collector
       app.kubernetes.io/instance: create-sm-prometheus.simplest
+      app.kubernetes.io/managed-by: opentelemetry-operator
+      app.kubernetes.io/part-of: opentelemetry
+      operator.opentelemetry.io/collector-monitoring-service: "Exists"
 ---
 apiVersion: v1
 kind: Service
@@ -55,6 +58,7 @@ metadata:
     app.kubernetes.io/managed-by: opentelemetry-operator
     app.kubernetes.io/name: simplest-collector-monitoring
     app.kubernetes.io/part-of: opentelemetry
+    operator.opentelemetry.io/collector-monitoring-service: "Exists"
   name: simplest-collector-monitoring
   namespace: create-sm-prometheus
 spec:

diff --git a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/02-assert.yaml b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/02-assert.yaml
@@ -16,8 +16,12 @@ spec:
     - create-sm-prometheus
   selector:
     matchLabels:
-      app.kubernetes.io/managed-by: opentelemetry-operator
+      app.kubernetes.io/component: opentelemetry-collector
       app.kubernetes.io/instance: create-sm-prometheus.simplest
+      app.kubernetes.io/managed-by: opentelemetry-operator
+      app.kubernetes.io/part-of: opentelemetry
+      operator.opentelemetry.io/collector-monitoring-service: "Exists"
+
 ---
 apiVersion: v1
 kind: Service
@@ -39,4 +43,23 @@ spec:
   - name: prometheus-prod
     port: 8884
     protocol: TCP
-    targetPort: 8884
+    targetPort: 8884
+---
+apiVersion: v1
+kind: Service
+metadata:
+  labels:
+    app.kubernetes.io/component: opentelemetry-collector
+    app.kubernetes.io/instance: create-sm-prometheus.simplest
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: simplest-collector-monitoring
+    app.kubernetes.io/part-of: opentelemetry
+    operator.opentelemetry.io/collector-monitoring-service: "Exists"
+  name: simplest-collector-monitoring
+  namespace: create-sm-prometheus
+spec:
+  ports:
+  - name: monitoring
+    port: 8888
+    protocol: TCP
+    targetPort: 8888