Remove unnecessary instances of app.kubernetes.io/managed-by

.chloggen/fix-managed-by-gross.yaml

@@ -0,0 +1,18 @@
+# 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. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fixes a bug that was preventing upgrade patches from reliably applying.
+
+# One or more tracking issues related to the change
+issues: [3074]
+
+# (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: |
+  A bug was discovered in the process of testing the PR that was failing to remove the environment
+  variables introduced in the 0.104.0 upgrade. The fix was to take a deepcopy of the object and update that.

apis/v1alpha1/instrumentation_webhook.go

@@ -91,10 +91,6 @@ func (w InstrumentationWebhook) defaulter(r *Instrumentation) error {
 	if r.Labels == nil {
 		r.Labels = map[string]string{}
 	}
-	if r.Labels["app.kubernetes.io/managed-by"] == "" {
-		r.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
-	}
-
 	if r.Spec.Java.Image == "" {
 		r.Spec.Java.Image = w.cfg.AutoInstrumentationJavaImage()
 	}

apis/v1alpha1/opampbridge_webhook.go

@@ -52,42 +52,38 @@ func (o *OpAMPBridgeWebhook) Default(ctx context.Context, obj runtime.Object) er
 	return o.defaulter(opampBridge)
 }
 
-func (c OpAMPBridgeWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+func (o *OpAMPBridgeWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
 	opampBridge, ok := obj.(*OpAMPBridge)
 	if !ok {
 		return nil, fmt.Errorf("expected an OpAMPBridge, received %T", obj)
 	}
-	return c.validate(opampBridge)
+	return o.validate(opampBridge)
 }
 
-func (c OpAMPBridgeWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+func (o *OpAMPBridgeWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
 	opampBridge, ok := newObj.(*OpAMPBridge)
 	if !ok {
 		return nil, fmt.Errorf("expected an OpAMPBridge, received %T", newObj)
 	}
-	return c.validate(opampBridge)
+	return o.validate(opampBridge)
 }
 
-func (o OpAMPBridgeWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+func (o *OpAMPBridgeWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
 	opampBridge, ok := obj.(*OpAMPBridge)
 	if !ok || opampBridge == nil {
 		return nil, fmt.Errorf("expected an OpAMPBridge, received %T", obj)
 	}
 	return o.validate(opampBridge)
 }
 
-func (o OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
+func (o *OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
 	if len(r.Spec.UpgradeStrategy) == 0 {
 		r.Spec.UpgradeStrategy = UpgradeStrategyAutomatic
 	}
 
 	if r.Labels == nil {
 		r.Labels = map[string]string{}
 	}
-	if r.Labels["app.kubernetes.io/managed-by"] == "" {
-		r.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
-	}
-
 	one := int32(1)
 	if r.Spec.Replicas == nil {
 		r.Spec.Replicas = &one
@@ -104,7 +100,7 @@ func (o OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
 	return nil
 }
 
-func (o OpAMPBridgeWebhook) validate(r *OpAMPBridge) (admission.Warnings, error) {
+func (o *OpAMPBridgeWebhook) validate(r *OpAMPBridge) (admission.Warnings, error) {
 	warnings := admission.Warnings{}
 
 	// validate OpAMP server endpoint

apis/v1alpha1/opampbridge_webhook_test.go

@@ -50,9 +50,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
 			opampBridge: OpAMPBridge{},
 			expected: OpAMPBridge{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpAMPBridgeSpec{
 					Replicas:        &one,
@@ -71,9 +69,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpAMPBridge{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpAMPBridgeSpec{
 					Replicas:        &five,
@@ -93,9 +89,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpAMPBridge{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpAMPBridgeSpec{
 					Replicas:        &one,

apis/v1beta1/collector_webhook.go

@@ -64,9 +64,6 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
 	if otelcol.Labels == nil {
 		otelcol.Labels = map[string]string{}
 	}
-	if otelcol.Labels["app.kubernetes.io/managed-by"] == "" {
-		otelcol.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
-	}
 
 	// We can default to one because dependent objects Deployment and HorizontalPodAutoScaler
 	// default to 1 as well.

apis/v1beta1/collector_webhook_test.go

@@ -111,9 +111,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			otelcol: OpenTelemetryCollector{},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					OpenTelemetryCommonFields: OpenTelemetryCommonFields{
@@ -139,9 +137,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode:            ModeSidecar,
@@ -168,9 +164,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode:            ModeSidecar,
@@ -195,9 +189,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode:            ModeDeployment,
@@ -227,9 +219,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
@@ -265,9 +255,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
@@ -305,9 +293,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
@@ -350,9 +336,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
@@ -390,9 +374,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,
@@ -423,9 +405,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
 			},
 			expected: OpenTelemetryCollector{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: map[string]string{
-						"app.kubernetes.io/managed-by": "opentelemetry-operator",
-					},
+					Labels: map[string]string{},
 				},
 				Spec: OpenTelemetryCollectorSpec{
 					Mode: ModeDeployment,

apis/v1beta1/metrics.go

@@ -121,13 +121,8 @@ func NewMetrics(prv metric.MeterProvider, ctx context.Context, cl client.Reader)
 
 // Init metrics from the first time the operator starts.
 func (m *Metrics) init(ctx context.Context, cl client.Reader) error {
-	opts := []client.ListOption{
-		client.MatchingLabels(map[string]string{
-			"app.kubernetes.io/managed-by": "opentelemetry-operator",
-		}),
-	}
 	list := &OpenTelemetryCollectorList{}
-	if err := cl.List(ctx, list, opts...); err != nil {
+	if err := cl.List(ctx, list); err != nil {
 		return fmt.Errorf("failed to list: %w", err)
 	}
 

main.go

@@ -36,7 +36,6 @@ import (
 	"k8s.io/client-go/kubernetes"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
-	"k8s.io/client-go/tools/record"
 	k8sapiflag "k8s.io/component-base/cli/flag"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -454,7 +453,7 @@ func addDependencies(_ context.Context, mgr ctrl.Manager, cfg config.Config, v v
 			Log:      ctrl.Log.WithName("collector-upgrade"),
 			Version:  v,
 			Client:   mgr.GetClient(),
-			Recorder: record.NewFakeRecorder(collectorupgrade.RecordBufferSize),
+			Recorder: mgr.GetEventRecorderFor("opentelemetry-operator"),
 		}
 		return up.ManagedInstances(c)
 	}))

pkg/collector/upgrade/upgrade.go

@@ -42,14 +42,8 @@ const RecordBufferSize int = 100
 // ManagedInstances finds all the otelcol instances for the current operator and upgrades them, if necessary.
 func (u VersionUpgrade) ManagedInstances(ctx context.Context) error {
 	u.Log.Info("looking for managed instances to upgrade")
-
-	opts := []client.ListOption{
-		client.MatchingLabels(map[string]string{
-			"app.kubernetes.io/managed-by": "opentelemetry-operator",
-		}),
-	}
 	list := &v1beta1.OpenTelemetryCollectorList{}
-	if err := u.Client.List(ctx, list, opts...); err != nil {
+	if err := u.Client.List(ctx, list); err != nil {
 		return fmt.Errorf("failed to list: %w", err)
 	}
 
@@ -73,7 +67,6 @@ func (u VersionUpgrade) ManagedInstances(ctx context.Context) error {
 			u.Recorder.Event(&original, "Error", "Upgrade", msg)
 			continue
 		}
-
 		if !reflect.DeepEqual(upgraded, list.Items[i]) {
 			// the resource update overrides the status, so, keep it so that we can reset it later
 			st := upgraded.Status
@@ -89,7 +82,6 @@ func (u VersionUpgrade) ManagedInstances(ctx context.Context) error {
 				itemLogger.Error(err, "failed to apply changes to instance's status object")
 				continue
 			}
-
 			itemLogger.Info("instance upgraded", "version", upgraded.Status.Version)
 		}
 	}
@@ -114,60 +106,61 @@ func (u VersionUpgrade) ManagedInstance(_ context.Context, otelcol v1beta1.OpenT
 		return otelcol, err
 	}
 
+	updated := *(otelcol.DeepCopy())
 	if instanceV.GreaterThan(&Latest.Version) {
 		// Update with the latest known version, which is what we have from versions.txt
-		u.Log.V(4).Info("no upgrade routines are needed for the OpenTelemetry instance", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version, "latest", Latest.Version.String())
+		u.Log.V(4).Info("no upgrade routines are needed for the OpenTelemetry instance", "name", updated.Name, "namespace", updated.Namespace, "version", updated.Status.Version, "latest", Latest.Version.String())
 
 		otelColV, err := semver.NewVersion(u.Version.OpenTelemetryCollector)
 		if err != nil {
-			return otelcol, err
+			return updated, err
 		}
 		if instanceV.LessThan(otelColV) {
-			u.Log.Info("upgraded OpenTelemetry Collector version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version)
-			otelcol.Status.Version = u.Version.OpenTelemetryCollector
+			u.Log.Info("upgraded OpenTelemetry Collector version", "name", updated.Name, "namespace", updated.Namespace, "version", updated.Status.Version)
+			updated.Status.Version = u.Version.OpenTelemetryCollector
 		} else {
-			u.Log.V(4).Info("skipping upgrade for OpenTelemetry Collector instance", "name", otelcol.Name, "namespace", otelcol.Namespace)
+			u.Log.V(4).Info("skipping upgrade for OpenTelemetry Collector instance", "name", updated.Name, "namespace", updated.Namespace)
 		}
 
-		return otelcol, nil
+		return updated, nil
 	}
 
 	for _, available := range versions {
 		if available.GreaterThan(instanceV) {
 			if available.upgrade != nil {
 				otelcolV1alpha1 := &v1alpha1.OpenTelemetryCollector{}
-				if err := otelcolV1alpha1.ConvertFrom(&otelcol); err != nil {
-					return otelcol, err
+				if err := otelcolV1alpha1.ConvertFrom(&updated); err != nil {
+					return updated, err
 				}
 
 				upgradedV1alpha1, err := available.upgrade(u, otelcolV1alpha1)
 				if err != nil {
-					u.Log.Error(err, "failed to upgrade managed otelcol instances", "name", otelcol.Name, "namespace", otelcol.Namespace)
-					return otelcol, err
+					u.Log.Error(err, "failed to upgrade managed otelcol instances", "name", updated.Name, "namespace", updated.Namespace)
+					return updated, err
 				}
 				upgradedV1alpha1.Status.Version = available.String()
 
-				if err := upgradedV1alpha1.ConvertTo(&otelcol); err != nil {
-					return otelcol, err
+				if err := upgradedV1alpha1.ConvertTo(&updated); err != nil {
+					return updated, err
 				}
-				u.Log.V(1).Info("step upgrade", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", available.String())
+				u.Log.V(1).Info("step upgrade", "name", updated.Name, "namespace", updated.Namespace, "version", available.String())
 			} else {
 
-				upgraded, err := available.upgradeV1beta1(u, &otelcol) //available.upgrade(params., &otelcol)
+				upgraded, err := available.upgradeV1beta1(u, &updated) //available.upgrade(params., &updated)
 				if err != nil {
-					u.Log.Error(err, "failed to upgrade managed otelcol instances", "name", otelcol.Name, "namespace", otelcol.Namespace)
-					return otelcol, err
+					u.Log.Error(err, "failed to upgrade managed otelcol instances", "name", updated.Name, "namespace", updated.Namespace)
+					return updated, err
 				}
 
-				u.Log.V(1).Info("step upgrade", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", available.String())
+				u.Log.V(1).Info("step upgrade", "name", updated.Name, "namespace", updated.Namespace, "version", available.String())
 				upgraded.Status.Version = available.String()
-				otelcol = *upgraded
+				updated = *upgraded
 			}
 		}
 	}
 	// Update with the latest known version, which is what we have from versions.txt
-	otelcol.Status.Version = u.Version.OpenTelemetryCollector
+	updated.Status.Version = u.Version.OpenTelemetryCollector
 
-	u.Log.V(1).Info("final version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version)
-	return otelcol, nil
+	u.Log.V(1).Info("final version", "name", updated.Name, "namespace", updated.Namespace, "version", updated.Status.Version)
+	return updated, nil
 }