diff --git a/.chloggen/fix-managed-by-gross.yaml b/.chloggen/fix-managed-by-gross.yaml new file mode 100755 index 0000000000..5a89247d87 --- /dev/null +++ b/.chloggen/fix-managed-by-gross.yaml @@ -0,0 +1,19 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# 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, auto-instrumentation, opamp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: This removes the static label app.kubernetes.io/managed-by from user applied CRs (collector, instrumentation, bridge) + +# One or more tracking issues related to the change +issues: [3014] + +# (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: | + The existence of app.kubernetes.io/managed-by=opentelemetry-operator was preventing the helm managed-by + labels from persisting correctly. this resulted in unnecessary conflicts while also resulting in an + operator failure to pull the correct images for auto-instrumentation. diff --git a/apis/v1alpha1/instrumentation_webhook.go b/apis/v1alpha1/instrumentation_webhook.go index d2b4a5a74d..004992f795 100644 --- a/apis/v1alpha1/instrumentation_webhook.go +++ b/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() } diff --git a/apis/v1alpha1/opampbridge_webhook.go b/apis/v1alpha1/opampbridge_webhook.go index 1fbbcf75c0..b27c9bd989 100644 --- a/apis/v1alpha1/opampbridge_webhook.go +++ b/apis/v1alpha1/opampbridge_webhook.go @@ -52,23 +52,23 @@ 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) @@ -76,7 +76,7 @@ func (o OpAMPBridgeWebhook) ValidateDelete(ctx context.Context, obj runtime.Obje 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 } @@ -84,10 +84,6 @@ func (o OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) 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" - } - 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 diff --git a/apis/v1alpha1/opampbridge_webhook_test.go b/apis/v1alpha1/opampbridge_webhook_test.go index 34d667b04d..335e055ceb 100644 --- a/apis/v1alpha1/opampbridge_webhook_test.go +++ b/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, diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index 930c2c819f..ec5bf47ddd 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -94,9 +94,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. diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 8262a6fd07..12c2a25bd6 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/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{ @@ -144,9 +142,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, @@ -178,9 +174,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, @@ -210,9 +204,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, @@ -247,9 +239,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, @@ -290,9 +280,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, @@ -329,9 +317,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, @@ -379,9 +365,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, @@ -424,9 +408,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, @@ -468,9 +450,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, diff --git a/apis/v1beta1/metrics.go b/apis/v1beta1/metrics.go index 395306059d..53675277f8 100644 --- a/apis/v1beta1/metrics.go +++ b/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) } diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go index 971689169d..72735700a4 100644 --- a/pkg/collector/upgrade/upgrade.go +++ b/pkg/collector/upgrade/upgrade.go @@ -42,14 +42,8 @@ const RecordBufferSize int = 10 // 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) } diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index 948a8e7288..f4b963f1c9 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -79,14 +79,8 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde // ManagedInstances upgrades managed instances by the opentelemetry-operator. func (u *InstrumentationUpgrade) ManagedInstances(ctx context.Context) error { u.Logger.Info("looking for managed Instrumentation instances to upgrade") - - opts := []client.ListOption{ - client.MatchingLabels(map[string]string{ - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }), - } list := &v1alpha1.InstrumentationList{} - 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) }