Skip to content

Commit 09d0758

Browse files
jaronoff97jaronoff97
authored
Remove unnecessary instances of app.kubernetes.io/managed-by (#3074)
* remove unnecessary instances of app.kubernetes.io/managed-by for user applied resources * chlog * fix upgrade issue * Fix lint * Fix test * repro * fix upgrade * duplication * revert change * chlog entry * another chlog entry
1 parent 88ca1af commit 09d0758

File tree

13 files changed

+164
-103
lines changed

13 files changed

+164
-103
lines changed

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: auto-instrumentation
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Fixes a bug that was preventing auto instrumentation from getting correct images.
9+
10+
# One or more tracking issues related to the change
11+
issues: [3014]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
This PR removes the restriction on the operator to only upgrade manually applied CRDs. This meant
18+
that resources applied by helm were not upgraded at all. The solution was to remove the restriction
19+
we had on querying the label app.kubernetes.io/managed-by=opentelemetry-operator, thereby upgrading
20+
ALL CRDs in the cluster.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: collector
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Fixes a bug that was preventing upgrade patches from reliably applying.
9+
10+
# One or more tracking issues related to the change
11+
issues: [3074]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext: |
17+
A bug was discovered in the process of testing the PR that was failing to remove the environment
18+
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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ func (w InstrumentationWebhook) defaulter(r *Instrumentation) error {
9191
if r.Labels == nil {
9292
r.Labels = map[string]string{}
9393
}
94-
if r.Labels["app.kubernetes.io/managed-by"] == "" {
95-
r.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
96-
}
97-
9894
if r.Spec.Java.Image == "" {
9995
r.Spec.Java.Image = w.cfg.AutoInstrumentationJavaImage()
10096
}

‎apis/v1alpha1/opampbridge_webhook.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,42 +52,38 @@ func (o *OpAMPBridgeWebhook) Default(ctx context.Context, obj runtime.Object) er
5252
return o.defaulter(opampBridge)
5353
}
5454

55-
func (c OpAMPBridgeWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
55+
func (o *OpAMPBridgeWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
5656
opampBridge, ok := obj.(*OpAMPBridge)
5757
if !ok {
5858
return nil, fmt.Errorf("expected an OpAMPBridge, received %T", obj)
5959
}
60-
return c.validate(opampBridge)
60+
return o.validate(opampBridge)
6161
}
6262

63-
func (c OpAMPBridgeWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
63+
func (o *OpAMPBridgeWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
6464
opampBridge, ok := newObj.(*OpAMPBridge)
6565
if !ok {
6666
return nil, fmt.Errorf("expected an OpAMPBridge, received %T", newObj)
6767
}
68-
return c.validate(opampBridge)
68+
return o.validate(opampBridge)
6969
}
7070

71-
func (o OpAMPBridgeWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
71+
func (o *OpAMPBridgeWebhook) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
7272
opampBridge, ok := obj.(*OpAMPBridge)
7373
if !ok || opampBridge == nil {
7474
return nil, fmt.Errorf("expected an OpAMPBridge, received %T", obj)
7575
}
7676
return o.validate(opampBridge)
7777
}
7878

79-
func (o OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
79+
func (o *OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
8080
if len(r.Spec.UpgradeStrategy) == 0 {
8181
r.Spec.UpgradeStrategy = UpgradeStrategyAutomatic
8282
}
8383

8484
if r.Labels == nil {
8585
r.Labels = map[string]string{}
8686
}
87-
if r.Labels["app.kubernetes.io/managed-by"] == "" {
88-
r.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
89-
}
90-
9187
one := int32(1)
9288
if r.Spec.Replicas == nil {
9389
r.Spec.Replicas = &one
@@ -104,7 +100,7 @@ func (o OpAMPBridgeWebhook) defaulter(r *OpAMPBridge) error {
104100
return nil
105101
}
106102

107-
func (o OpAMPBridgeWebhook) validate(r *OpAMPBridge) (admission.Warnings, error) {
103+
func (o *OpAMPBridgeWebhook) validate(r *OpAMPBridge) (admission.Warnings, error) {
108104
warnings := admission.Warnings{}
109105

110106
// validate OpAMP server endpoint

‎apis/v1alpha1/opampbridge_webhook_test.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
5050
opampBridge: OpAMPBridge{},
5151
expected: OpAMPBridge{
5252
ObjectMeta: metav1.ObjectMeta{
53-
Labels: map[string]string{
54-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
55-
},
53+
Labels: map[string]string{},
5654
},
5755
Spec: OpAMPBridgeSpec{
5856
Replicas: &one,
@@ -71,9 +69,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
7169
},
7270
expected: OpAMPBridge{
7371
ObjectMeta: metav1.ObjectMeta{
74-
Labels: map[string]string{
75-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
76-
},
72+
Labels: map[string]string{},
7773
},
7874
Spec: OpAMPBridgeSpec{
7975
Replicas: &five,
@@ -93,9 +89,7 @@ func TestOpAMPBridgeDefaultingWebhook(t *testing.T) {
9389
},
9490
expected: OpAMPBridge{
9591
ObjectMeta: metav1.ObjectMeta{
96-
Labels: map[string]string{
97-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
98-
},
92+
Labels: map[string]string{},
9993
},
10094
Spec: OpAMPBridgeSpec{
10195
Replicas: &one,

‎apis/v1beta1/collector_webhook.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,6 @@ func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
6464
if otelcol.Labels == nil {
6565
otelcol.Labels = map[string]string{}
6666
}
67-
if otelcol.Labels["app.kubernetes.io/managed-by"] == "" {
68-
otelcol.Labels["app.kubernetes.io/managed-by"] = "opentelemetry-operator"
69-
}
7067

7168
// We can default to one because dependent objects Deployment and HorizontalPodAutoScaler
7269
// default to 1 as well.

‎apis/v1beta1/collector_webhook_test.go

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
111111
otelcol: OpenTelemetryCollector{},
112112
expected: OpenTelemetryCollector{
113113
ObjectMeta: metav1.ObjectMeta{
114-
Labels: map[string]string{
115-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
116-
},
114+
Labels: map[string]string{},
117115
},
118116
Spec: OpenTelemetryCollectorSpec{
119117
OpenTelemetryCommonFields: OpenTelemetryCommonFields{
@@ -139,9 +137,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
139137
},
140138
expected: OpenTelemetryCollector{
141139
ObjectMeta: metav1.ObjectMeta{
142-
Labels: map[string]string{
143-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
144-
},
140+
Labels: map[string]string{},
145141
},
146142
Spec: OpenTelemetryCollectorSpec{
147143
Mode: ModeSidecar,
@@ -168,9 +164,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
168164
},
169165
expected: OpenTelemetryCollector{
170166
ObjectMeta: metav1.ObjectMeta{
171-
Labels: map[string]string{
172-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
173-
},
167+
Labels: map[string]string{},
174168
},
175169
Spec: OpenTelemetryCollectorSpec{
176170
Mode: ModeSidecar,
@@ -195,9 +189,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
195189
},
196190
expected: OpenTelemetryCollector{
197191
ObjectMeta: metav1.ObjectMeta{
198-
Labels: map[string]string{
199-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
200-
},
192+
Labels: map[string]string{},
201193
},
202194
Spec: OpenTelemetryCollectorSpec{
203195
Mode: ModeDeployment,
@@ -227,9 +219,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
227219
},
228220
expected: OpenTelemetryCollector{
229221
ObjectMeta: metav1.ObjectMeta{
230-
Labels: map[string]string{
231-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
232-
},
222+
Labels: map[string]string{},
233223
},
234224
Spec: OpenTelemetryCollectorSpec{
235225
Mode: ModeDeployment,
@@ -265,9 +255,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
265255
},
266256
expected: OpenTelemetryCollector{
267257
ObjectMeta: metav1.ObjectMeta{
268-
Labels: map[string]string{
269-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
270-
},
258+
Labels: map[string]string{},
271259
},
272260
Spec: OpenTelemetryCollectorSpec{
273261
Mode: ModeDeployment,
@@ -305,9 +293,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
305293
},
306294
expected: OpenTelemetryCollector{
307295
ObjectMeta: metav1.ObjectMeta{
308-
Labels: map[string]string{
309-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
310-
},
296+
Labels: map[string]string{},
311297
},
312298
Spec: OpenTelemetryCollectorSpec{
313299
Mode: ModeDeployment,
@@ -350,9 +336,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
350336
},
351337
expected: OpenTelemetryCollector{
352338
ObjectMeta: metav1.ObjectMeta{
353-
Labels: map[string]string{
354-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
355-
},
339+
Labels: map[string]string{},
356340
},
357341
Spec: OpenTelemetryCollectorSpec{
358342
Mode: ModeDeployment,
@@ -390,9 +374,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
390374
},
391375
expected: OpenTelemetryCollector{
392376
ObjectMeta: metav1.ObjectMeta{
393-
Labels: map[string]string{
394-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
395-
},
377+
Labels: map[string]string{},
396378
},
397379
Spec: OpenTelemetryCollectorSpec{
398380
Mode: ModeDeployment,
@@ -423,9 +405,7 @@ func TestCollectorDefaultingWebhook(t *testing.T) {
423405
},
424406
expected: OpenTelemetryCollector{
425407
ObjectMeta: metav1.ObjectMeta{
426-
Labels: map[string]string{
427-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
428-
},
408+
Labels: map[string]string{},
429409
},
430410
Spec: OpenTelemetryCollectorSpec{
431411
Mode: ModeDeployment,

‎apis/v1beta1/metrics.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,8 @@ func NewMetrics(prv metric.MeterProvider, ctx context.Context, cl client.Reader)
121121

122122
// Init metrics from the first time the operator starts.
123123
func (m *Metrics) init(ctx context.Context, cl client.Reader) error {
124-
opts := []client.ListOption{
125-
client.MatchingLabels(map[string]string{
126-
"app.kubernetes.io/managed-by": "opentelemetry-operator",
127-
}),
128-
}
129124
list := &OpenTelemetryCollectorList{}
130-
if err := cl.List(ctx, list, opts...); err != nil {
125+
if err := cl.List(ctx, list); err != nil {
131126
return fmt.Errorf("failed to list: %w", err)
132127
}
133128

‎main.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import (
3636
"k8s.io/client-go/kubernetes"
3737
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
3838
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
39-
"k8s.io/client-go/tools/record"
4039
k8sapiflag "k8s.io/component-base/cli/flag"
4140
ctrl "sigs.k8s.io/controller-runtime"
4241
"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -454,7 +453,7 @@ func addDependencies(_ context.Context, mgr ctrl.Manager, cfg config.Config, v v
454453
Log: ctrl.Log.WithName("collector-upgrade"),
455454
Version: v,
456455
Client: mgr.GetClient(),
457-
Recorder: record.NewFakeRecorder(collectorupgrade.RecordBufferSize),
456+
Recorder: mgr.GetEventRecorderFor("opentelemetry-operator"),
458457
}
459458
return up.ManagedInstances(c)
460459
}))

0 commit comments

Comments
 (0)