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.

Sign up for GitHub

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
merged 15 commits into from
Mar 11, 2024
Merged

Add missing label servicemonitor #2573

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ea1e00a
Add missing label for Service/Pod Monitors
yuriolisa Jan 29, 2024
3f6fd0b
Added manifests.utils to set labels
yuriolisa Jan 30, 2024
32bf984
Fixed labels names
yuriolisa Jan 31, 2024
b456ba6
Fixed labels for PodMonitor
yuriolisa Feb 12, 2024
fc03579
Fixed e2e test
yuriolisa Feb 12, 2024
00d2a04
Fixed e2e test
yuriolisa Feb 13, 2024
2ee13bb
Fixed selectorMatchLabels
yuriolisa Feb 13, 2024
81ae4ab
Removed the extra method
yuriolisa Feb 29, 2024
a8a518a
Adjusted nil
yuriolisa Mar 1, 2024
525eb94
Merge branch 'main' into add-missing-label-servicemonitor
yuriolisa Mar 1, 2024
fd39eb9
Changed labels
yuriolisa Mar 7, 2024
50a375d
Changed labels
yuriolisa Mar 7, 2024
956b94e
Readded common labels
yuriolisa Mar 7, 2024
d41662b
Readded common labels
yuriolisa Mar 7, 2024
38464fb
Added labels assert
yuriolisa Mar 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 36 additions & 35 deletions controllers/builder_test.go
View file
Open in desktop
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
11 changes: 7 additions & 4 deletions internal/manifests/collector/service.go
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions internal/manifests/collector/servicemonitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
}
name := naming.ServiceMonitor(params.OtelCol.Name)
labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{})
selectorMatchLabels := manifestutils.SelectorMatchLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector)
Copy link
Contributor

Choose a reason for hiding this comment

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

we should still be using this to match on the overall labels we expect the service to have, otherwise our match would be too wide

sm = monitoringv1.ServiceMonitor{
ObjectMeta: metav1.ObjectMeta{
Namespace: params.OtelCol.Namespace,
Expand All @@ -61,7 +60,9 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro
MatchNames: []string{params.OtelCol.Namespace},
},
Selector: metav1.LabelSelector{
MatchLabels: selectorMatchLabels,
MatchLabels: map[string]string{
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
},
},
},
}
Expand Down
14 changes: 2 additions & 12 deletions internal/manifests/collector/servicemonitor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
"fmt"
"testing"

"github.com/open-telemetry/opentelemetry-operator/internal/naming"

"github.com/stretchr/testify/assert"
)

Expand All @@ -38,11 +36,7 @@ func TestDesiredServiceMonitors(t *testing.T) {
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": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": naming.MonitoringService(params.OtelCol.Name),
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}
Expand All @@ -60,11 +54,7 @@ func TestDesiredServiceMonitorsWithPrometheus(t *testing.T) {
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": fmt.Sprintf("%s.%s", params.OtelCol.Namespace, params.OtelCol.Name),
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": naming.MonitoringService(params.OtelCol.Name),
"operator.opentelemetry.io/collector-monitoring-service": "Exists",
}
assert.Equal(t, expectedSelectorLabels, actual.Spec.Selector.MatchLabels)
}
11 changes: 0 additions & 11 deletions internal/manifests/manifestutils/labels.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,3 @@ func SelectorLabels(instance metav1.ObjectMeta, component string) map[string]str
"app.kubernetes.io/component": component,
}
}

// SelectorMatchLabels return the common labels that should be used on ServiceMonitor when the Spec.Observability.Metrics.EnableMetrics is true.
func SelectorMatchLabels(instance metav1.ObjectMeta, component string) map[string]string {
return map[string]string{
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/instance": naming.Truncate("%s.%s", 63, instance.Namespace, instance.Name),
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/component": component,
"app.kubernetes.io/name": naming.MonitoringService(instance.Name),
}
}
20 changes: 0 additions & 20 deletions internal/manifests/manifestutils/labels_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,23 +168,3 @@ func TestSelectorLabels(t *testing.T) {
// verify
assert.Equal(t, expected, result)
}

func TestSelectorMatchLabels(t *testing.T) {
// prepare
expected := map[string]string{
"app.kubernetes.io/component": "opentelemetry-collector",
"app.kubernetes.io/instance": "my-namespace.my-opentelemetry",
"app.kubernetes.io/managed-by": "opentelemetry-operator",
"app.kubernetes.io/part-of": "opentelemetry",
"app.kubernetes.io/name": "my-opentelemetry-collector-monitoring",
}
otelcol := v1alpha1.OpenTelemetryCollector{
ObjectMeta: metav1.ObjectMeta{Name: "my-opentelemetry", Namespace: "my-namespace"},
}

// test
result := SelectorMatchLabels(otelcol.ObjectMeta, "opentelemetry-collector")

// verify
assert.Equal(t, expected, result)
}
4 changes: 1 addition & 3 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/01-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
operator.opentelemetry.io/collector-monitoring-service: "Exists"
---
apiVersion: v1
kind: Service
Expand Down
5 changes: 2 additions & 3 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/02-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
operator.opentelemetry.io/collector-monitoring-service: "Exists"

---
apiVersion: v1
kind: Service
Expand Down
4 changes: 1 addition & 3 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/05-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
operator.opentelemetry.io/collector-monitoring-service: "Exists"
4 changes: 1 addition & 3 deletions tests/e2e-prometheuscr/create-sm-prometheus-exporters/06-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,4 @@ spec:
- create-sm-prometheus
selector:
matchLabels:
app.kubernetes.io/managed-by: opentelemetry-operator
app.kubernetes.io/instance: create-sm-prometheus.simplest
app.kubernetes.io/name: simplest-collector-monitoring
operator.opentelemetry.io/collector-monitoring-service: "Exists"
Loading