diff --git a/.chloggen/add-missing-label-servicemonitor.yaml b/.chloggen/add-missing-label-servicemonitor.yaml new file mode 100755 index 0000000000..d7a1a8f28a --- /dev/null +++ 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 index c22dd0161a..20480934cd 100644 --- 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 index 73c63bf56b..9454a6037a 100644 --- 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 index c83950ea85..c9def26881 100644 --- a/internal/manifests/collector/podmonitor_test.go +++ b/internal/manifests/collector/podmonitor_test.go @@ -42,12 +42,21 @@ 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) @@ -55,4 +64,11 @@ func TestDesiredPodMonitors(t *testing.T) { 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 index 8ac13a3e6d..333b3e5254 100644 --- 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 index f7cb6b9d70..21ff254e09 100644 --- 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 index c3fdf0c960..9cd133e080 100644 --- a/internal/manifests/collector/servicemonitor_test.go +++ b/internal/manifests/collector/servicemonitor_test.go @@ -35,11 +35,21 @@ 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) @@ -47,4 +57,12 @@ func TestDesiredServiceMonitors(t *testing.T) { 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 index d89c68dd59..1572a5fb07 100644 --- 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 index 0a3c07d13b..eb0652f517 100644 --- 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 index b79840724e..4c5b8bd5b8 100644 --- 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 \ No newline at end of file + 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 \ No newline at end of file diff --git a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml index f56ec033cd..3e8205803c 100644 --- a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml +++ b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml @@ -17,5 +17,27 @@ spec: - create-sm-prometheus selector: matchLabels: - app.kubernetes.io/managed-by: opentelemetry-operator app.kubernetes.io/instance: create-sm-prometheus.simplest + app.kubernetes.io/managed-by: opentelemetry-operator + app.kubernetes.io/part-of: opentelemetry + app.kubernetes.io/component: opentelemetry-collector + operator.opentelemetry.io/collector-monitoring-service: "Exists" +--- +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 \ No newline at end of file diff --git a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml index 8df94a5984..dcfecf5d81 100644 --- a/tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml +++ b/tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml @@ -16,5 +16,27 @@ 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 +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 \ No newline at end of file