Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing label servicemonitor #2573

Merged
16 changes: 16 additions & 0 deletions .chloggen/add-missing-label-servicemonitor.yaml
Original file line number Diff line number Diff line change
@@ -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:
71 changes: 36 additions & 35 deletions controllers/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,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,
},
Expand Down Expand Up @@ -563,12 +564,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,
},
Expand Down Expand Up @@ -830,12 +832,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,
},
Expand Down Expand Up @@ -1309,12 +1312,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,
},
Expand Down Expand Up @@ -1706,12 +1710,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,
},
Expand Down Expand Up @@ -1944,11 +1949,7 @@ prometheus_cr:
},
Selector: v1.LabelSelector{
MatchLabels: map[string]string{
"app.kubernetes.io/component": "opentelemetry-targetallocator",
"app.kubernetes.io/instance": "test.test",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/name": "test-targetallocator",
"app.kubernetes.io/part-of": "opentelemetry",
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
},
},
NamespaceSelector: monitoringv1.NamespaceSelector{
Expand Down
21 changes: 8 additions & 13 deletions internal/manifests/collector/podmonitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package collector

import (
"fmt"
"strings"

"github.com/go-logr/logr"
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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{
Expand Down
20 changes: 18 additions & 2 deletions internal/manifests/collector/podmonitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
11 changes: 7 additions & 4 deletions internal/manifests/collector/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ 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"
headlessExists = "Exists"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we can consolidate here

monitoringLabel = "operator.opentelemetry.io/collector-monitoring-service"
monitoringExists = "Exists"
)

func HeadlessService(params manifests.Params) (*corev1.Service, error) {
Expand All @@ -44,7 +46,7 @@ func HeadlessService(params manifests.Params) (*corev1.Service, error) {
h.Name = naming.HeadlessService(params.OtelCol.Name)
h.Labels[headlessLabel] = headlessExists

// copy to avoid modifying params.OtelCol.Annotations
// copy to avoid modifying params.OtelCol.annotations
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uppercase?

annotations := map[string]string{
"service.beta.openshift.io/serving-cert-secret-name": fmt.Sprintf("%s-tls", h.Name),
}
Expand All @@ -61,6 +63,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] = monitoringExists

out, err := params.OtelCol.Spec.Config.Yaml()
if err != nil {
Expand Down
15 changes: 6 additions & 9 deletions internal/manifests/collector/servicemonitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package collector

import (
"fmt"
"strings"

"github.com/go-logr/logr"
Expand All @@ -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"
)

Expand All @@ -42,15 +42,13 @@ 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{})
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{
Expand All @@ -63,8 +61,7 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
},
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),
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
},
},
},
Expand Down
14 changes: 12 additions & 2 deletions internal/manifests/collector/servicemonitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,26 @@ 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{
"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{
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ spec:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-pm-prometheus.simplest

Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
operator.opentelemetry.io/collector-monitoring-service: "Exists"
---
apiVersion: v1
kind: Service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
operator.opentelemetry.io/collector-monitoring-service: "Exists"

---
apiVersion: v1
kind: Service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
operator.opentelemetry.io/collector-monitoring-service: "Exists"
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,4 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
operator.opentelemetry.io/collector-monitoring-service: "Exists"
Loading