Override the app label name (#1778)

* Allow the overriding of the app name label, consolidate too * chloggen * override TA too * Comment to force build
open-telemetry · Jun 1, 2023 · b471aba · b471aba
1 parent d6d9f2f
commit b471aba
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 61 deletions.
diff --git a/.chloggen/1777-override-label.yaml b/.chloggen/1777-override-label.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: fixes a previously undocumented behavior that a collector could not override the collector's app name
+
+# One or more tracking issues related to the change
+issues: [1777]
+
+# (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/pkg/collector/daemonset.go b/pkg/collector/daemonset.go
@@ -27,8 +27,8 @@ import (
 
 // DaemonSet builds the deployment for the given instance.
 func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.DaemonSet {
-	labels := Labels(otelcol, cfg.LabelsFilter())
-	labels["app.kubernetes.io/name"] = naming.Collector(otelcol)
+	name := naming.Collector(otelcol)
+	labels := Labels(otelcol, name, cfg.LabelsFilter())
 
 	annotations := Annotations(otelcol)
 	podAnnotations := PodAnnotations(otelcol)

diff --git a/pkg/collector/deployment.go b/pkg/collector/deployment.go
@@ -27,15 +27,15 @@ import (
 
 // Deployment builds the deployment for the given instance.
 func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.Deployment {
-	labels := Labels(otelcol, cfg.LabelsFilter())
-	labels["app.kubernetes.io/name"] = naming.Collector(otelcol)
+	name := naming.Collector(otelcol)
+	labels := Labels(otelcol, name, cfg.LabelsFilter())
 
 	annotations := Annotations(otelcol)
 	podAnnotations := PodAnnotations(otelcol)
 
 	return appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.Collector(otelcol),
+			Name:        name,
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: annotations,

diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go
@@ -31,8 +31,8 @@ import (
 func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) client.Object {
 	autoscalingVersion := cfg.AutoscalingVersion()
 
-	labels := Labels(otelcol, cfg.LabelsFilter())
-	labels["app.kubernetes.io/name"] = naming.Collector(otelcol)
+	name := naming.Collector(otelcol)
+	labels := Labels(otelcol, name, cfg.LabelsFilter())
 	annotations := Annotations(otelcol)
 	var result client.Object
 

diff --git a/pkg/collector/labels.go b/pkg/collector/labels.go
@@ -32,7 +32,7 @@ func isFilteredLabel(label string, filterLabels []string) bool {
 }
 
 // Labels return the common labels to all objects that are part of a managed OpenTelemetryCollector.
-func Labels(instance v1alpha1.OpenTelemetryCollector, filterLabels []string) map[string]string {
+func Labels(instance v1alpha1.OpenTelemetryCollector, name string, filterLabels []string) map[string]string {
 	// new map every time, so that we don't touch the instance's label
 	base := map[string]string{}
 	if nil != instance.Labels {
@@ -54,6 +54,11 @@ func Labels(instance v1alpha1.OpenTelemetryCollector, filterLabels []string) map
 		base["app.kubernetes.io/version"] = "latest"
 	}
 
+	// Don't override the app name if it already exists
+	if _, ok := base["app.kubernetes.io/name"]; !ok {
+		base["app.kubernetes.io/name"] = name
+	}
+
 	return base
 }
 

diff --git a/pkg/collector/labels_test.go b/pkg/collector/labels_test.go
@@ -24,20 +24,25 @@ import (
 	. "github.com/open-telemetry/opentelemetry-operator/pkg/collector"
 )
 
+const (
+	collectorName      = "my-instance"
+	collectorNamespace = "my-ns"
+)
+
 func TestLabelsCommonSet(t *testing.T) {
 	// prepare
 	otelcol := v1alpha1.OpenTelemetryCollector{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-instance",
-			Namespace: "my-ns",
+			Name:      collectorName,
+			Namespace: collectorNamespace,
 		},
 		Spec: v1alpha1.OpenTelemetryCollectorSpec{
 			Image: "ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.47.0",
 		},
 	}
 
 	// test
-	labels := Labels(otelcol, []string{})
+	labels := Labels(otelcol, collectorName, []string{})
 	assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"])
 	assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"])
 	assert.Equal(t, "0.47.0", labels["app.kubernetes.io/version"])
@@ -49,16 +54,16 @@ func TestLabelsTagUnset(t *testing.T) {
 	// prepare
 	otelcol := v1alpha1.OpenTelemetryCollector{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-instance",
-			Namespace: "my-ns",
+			Name:      collectorName,
+			Namespace: collectorNamespace,
 		},
 		Spec: v1alpha1.OpenTelemetryCollectorSpec{
 			Image: "ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator",
 		},
 	}
 
 	// test
-	labels := Labels(otelcol, []string{})
+	labels := Labels(otelcol, collectorName, []string{})
 	assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"])
 	assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"])
 	assert.Equal(t, "latest", labels["app.kubernetes.io/version"])
@@ -70,16 +75,20 @@ func TestLabelsPropagateDown(t *testing.T) {
 	// prepare
 	otelcol := v1alpha1.OpenTelemetryCollector{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels: map[string]string{"myapp": "mycomponent"},
+			Labels: map[string]string{
+				"myapp":                  "mycomponent",
+				"app.kubernetes.io/name": "test",
+			},
 		},
 	}
 
 	// test
-	labels := Labels(otelcol, []string{})
+	labels := Labels(otelcol, collectorName, []string{})
 
 	// verify
-	assert.Len(t, labels, 6)
+	assert.Len(t, labels, 7)
 	assert.Equal(t, "mycomponent", labels["myapp"])
+	assert.Equal(t, "test", labels["app.kubernetes.io/name"])
 }
 
 func TestLabelsFilter(t *testing.T) {
@@ -90,10 +99,10 @@ func TestLabelsFilter(t *testing.T) {
 	}
 
 	// This requires the filter to be in regex match form and not the other simpler wildcard one.
-	labels := Labels(otelcol, []string{".*.bar.io"})
+	labels := Labels(otelcol, collectorName, []string{".*.bar.io"})
 
 	// verify
-	assert.Len(t, labels, 6)
+	assert.Len(t, labels, 7)
 	assert.NotContains(t, labels, "test.bar.io")
 	assert.Equal(t, "bar", labels["test.foo.io"])
 }

diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go
@@ -66,14 +66,8 @@ func ConfigMaps(ctx context.Context, params Params) error {
 
 func desiredConfigMap(_ context.Context, params Params) corev1.ConfigMap {
 	name := naming.ConfigMap(params.Instance)
-	version := strings.Split(params.Instance.Spec.Image, ":")
-	labels := collector.Labels(params.Instance, []string{})
-	labels["app.kubernetes.io/name"] = name
-	if len(version) > 1 {
-		labels["app.kubernetes.io/version"] = version[len(version)-1]
-	} else {
-		labels["app.kubernetes.io/version"] = "latest"
-	}
+	labels := collector.Labels(params.Instance, name, []string{})
+
 	config, err := ReplaceConfig(params.Instance)
 	if err != nil {
 		params.Log.V(2).Info("failed to update prometheus config to use sharded targets: ", "err", err)
@@ -95,8 +89,7 @@ func desiredConfigMap(_ context.Context, params Params) corev1.ConfigMap {
 func desiredTAConfigMap(params Params) (corev1.ConfigMap, error) {
 	name := naming.TAConfigMap(params.Instance)
 	version := strings.Split(params.Instance.Spec.Image, ":")
-	labels := targetallocator.Labels(params.Instance)
-	labels["app.kubernetes.io/name"] = name
+	labels := targetallocator.Labels(params.Instance, name)
 	if len(version) > 1 {
 		labels["app.kubernetes.io/version"] = version[len(version)-1]
 	} else {

diff --git a/pkg/collector/reconcile/opentelemetry.go b/pkg/collector/reconcile/opentelemetry.go
@@ -67,8 +67,7 @@ func updateScaleSubResourceStatus(ctx context.Context, cli client.Client, change
 	name := naming.Collector(*changed)
 
 	// Set the scale selector
-	labels := collector.Labels(*changed, []string{})
-	labels["app.kubernetes.io/name"] = name
+	labels := collector.Labels(*changed, name, []string{})
 	selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: labels})
 	if err != nil {
 		return fmt.Errorf("failed to get selector for labelSelector: %w", err)

diff --git a/pkg/collector/reconcile/service.go b/pkg/collector/reconcile/service.go
@@ -74,8 +74,8 @@ func Services(ctx context.Context, params Params) error {
 }
 
 func desiredService(ctx context.Context, params Params) *corev1.Service {
-	labels := collector.Labels(params.Instance, []string{})
-	labels["app.kubernetes.io/name"] = naming.Service(params.Instance)
+	name := naming.Service(params.Instance)
+	labels := collector.Labels(params.Instance, name, []string{})
 
 	config, err := adapters.ConfigFromString(params.Instance.Spec.Config)
 	if err != nil {
@@ -136,11 +136,10 @@ func desiredService(ctx context.Context, params Params) *corev1.Service {
 }
 
 func desiredTAService(params Params) corev1.Service {
-	labels := targetallocator.Labels(params.Instance)
-	labels["app.kubernetes.io/name"] = naming.TAService(params.Instance)
+	name := naming.TAService(params.Instance)
+	labels := targetallocator.Labels(params.Instance, name)
 
-	selector := targetallocator.Labels(params.Instance)
-	selector["app.kubernetes.io/name"] = naming.TargetAllocator(params.Instance)
+	selector := targetallocator.Labels(params.Instance, name)
 
 	return corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
@@ -182,12 +181,12 @@ func headless(ctx context.Context, params Params) *corev1.Service {
 }
 
 func monitoringService(ctx context.Context, params Params) *corev1.Service {
-	labels := collector.Labels(params.Instance, []string{})
-	labels["app.kubernetes.io/name"] = naming.MonitoringService(params.Instance)
+	name := naming.MonitoringService(params.Instance)
+	labels := collector.Labels(params.Instance, name, []string{})
 
 	return &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.MonitoringService(params.Instance),
+			Name:        name,
 			Namespace:   params.Instance.Namespace,
 			Labels:      labels,
 			Annotations: params.Instance.Annotations,

diff --git a/pkg/collector/reconcile/service_test.go b/pkg/collector/reconcile/service_test.go
@@ -251,8 +251,7 @@ func service(name string, ports []v1.ServicePort) v1.Service {
 }
 
 func serviceWithInternalTrafficPolicy(name string, ports []v1.ServicePort, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) v1.Service {
-	labels := collector.Labels(params().Instance, []string{})
-	labels["app.kubernetes.io/name"] = name
+	labels := collector.Labels(params().Instance, name, []string{})
 
 	return v1.Service{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/collector/serviceaccount.go b/pkg/collector/serviceaccount.go
@@ -33,12 +33,12 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string {
 
 // ServiceAccount returns the service account for the given instance.
 func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount {
-	labels := Labels(otelcol, []string{})
-	labels["app.kubernetes.io/name"] = naming.ServiceAccount(otelcol)
+	name := naming.ServiceAccount(otelcol)
+	labels := Labels(otelcol, name, []string{})
 
 	return corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.ServiceAccount(otelcol),
+			Name:        name,
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: otelcol.Annotations,

diff --git a/pkg/collector/statefulset.go b/pkg/collector/statefulset.go
@@ -27,15 +27,15 @@ import (
 
 // StatefulSet builds the statefulset for the given instance.
 func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.StatefulSet {
-	labels := Labels(otelcol, cfg.LabelsFilter())
-	labels["app.kubernetes.io/name"] = naming.Collector(otelcol)
+	name := naming.Collector(otelcol)
+	labels := Labels(otelcol, name, cfg.LabelsFilter())
 
 	annotations := Annotations(otelcol)
 	podAnnotations := PodAnnotations(otelcol)
 
 	return appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.Collector(otelcol),
+			Name:        name,
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: annotations,

diff --git a/pkg/targetallocator/deployment.go b/pkg/targetallocator/deployment.go
@@ -27,12 +27,12 @@ import (
 
 // Deployment builds the deployment for the given instance.
 func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.Deployment {
-	labels := Labels(otelcol)
-	labels["app.kubernetes.io/name"] = naming.TargetAllocator(otelcol)
+	name := naming.TargetAllocator(otelcol)
+	labels := Labels(otelcol, name)
 
 	return appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      naming.TargetAllocator(otelcol),
+			Name:      name,
 			Namespace: otelcol.Namespace,
 			Labels:    labels,
 		},

diff --git a/pkg/targetallocator/labels.go b/pkg/targetallocator/labels.go
@@ -20,7 +20,7 @@ import (
 )
 
 // Labels return the common labels to all TargetAllocator objects that are part of a managed OpenTelemetryCollector.
-func Labels(instance v1alpha1.OpenTelemetryCollector) map[string]string {
+func Labels(instance v1alpha1.OpenTelemetryCollector, name string) map[string]string {
 	// new map every time, so that we don't touch the instance's label
 	base := map[string]string{}
 	if nil != instance.Labels {
@@ -34,5 +34,9 @@ func Labels(instance v1alpha1.OpenTelemetryCollector) map[string]string {
 	base["app.kubernetes.io/part-of"] = "opentelemetry"
 	base["app.kubernetes.io/component"] = "opentelemetry-targetallocator"
 
+	if _, ok := base["app.kubernetes.io/name"]; !ok {
+		base["app.kubernetes.io/name"] = name
+	}
+
 	return base
 }
diff --git a/pkg/targetallocator/labels_test.go b/pkg/targetallocator/labels_test.go
@@ -23,35 +23,45 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
 )
 
+const (
+	name      = "my-instance"
+	namespace = "my-ns"
+)
+
 func TestLabelsCommonSet(t *testing.T) {
 	// prepare
 	otelcol := v1alpha1.OpenTelemetryCollector{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-instance",
-			Namespace: "my-ns",
+			Name:      name,
+			Namespace: namespace,
 		},
 	}
 
 	// test
-	labels := Labels(otelcol)
+	labels := Labels(otelcol, name)
 	assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"])
 	assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"])
 	assert.Equal(t, "opentelemetry", labels["app.kubernetes.io/part-of"])
 	assert.Equal(t, "opentelemetry-targetallocator", labels["app.kubernetes.io/component"])
+	assert.Equal(t, name, labels["app.kubernetes.io/name"])
 }
 
 func TestLabelsPropagateDown(t *testing.T) {
 	// prepare
 	otelcol := v1alpha1.OpenTelemetryCollector{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels: map[string]string{"myapp": "mycomponent"},
+			Labels: map[string]string{
+				"myapp":                  "mycomponent",
+				"app.kubernetes.io/name": "test",
+			},
 		},
 	}
 
 	// test
-	labels := Labels(otelcol)
+	labels := Labels(otelcol, name)
 
 	// verify
-	assert.Len(t, labels, 5)
+	assert.Len(t, labels, 6)
 	assert.Equal(t, "mycomponent", labels["myapp"])
+	assert.Equal(t, "test", labels["app.kubernetes.io/name"])
 }
diff --git a/pkg/targetallocator/serviceaccount.go b/pkg/targetallocator/serviceaccount.go
@@ -33,12 +33,12 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string {
 
 // ServiceAccount returns the service account for the given instance.
 func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount {
-	labels := Labels(otelcol)
-	labels["app.kubernetes.io/name"] = naming.TargetAllocatorServiceAccount(otelcol)
+	name := naming.TargetAllocatorServiceAccount(otelcol)
+	labels := Labels(otelcol, name)
 
 	return corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:        naming.TargetAllocatorServiceAccount(otelcol),
+			Name:        name,
 			Namespace:   otelcol.Namespace,
 			Labels:      labels,
 			Annotations: otelcol.Annotations,