diff --git a/.chloggen/fix-selector-bug.yaml b/.chloggen/fix-selector-bug.yaml new file mode 100755 index 0000000000..8df1821aaf --- /dev/null +++ b/.chloggen/fix-selector-bug.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. 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: resolves a bug that would create a junk selector for the service by merging rather than overriding. + +# One or more tracking issues related to the change +issues: [2873] + +# (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/controllers/reconcile_test.go b/controllers/reconcile_test.go index 0043d789ec..8dd272e77f 100644 --- a/controllers/reconcile_test.go +++ b/controllers/reconcile_test.go @@ -152,9 +152,16 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { // confirm the initial strategy is unset assert.Equal(t, d.Spec.Strategy.RollingUpdate.MaxUnavailable.IntVal, int32(0)) assert.Equal(t, d.Spec.Strategy.RollingUpdate.MaxSurge.IntVal, int32(0)) - exists, err = populateObjectIfExists(t, &v1.Service{}, namespacedObjectName(naming.Service(params.Name), params.Namespace)) + svc := &v1.Service{} + exists, err = populateObjectIfExists(t, svc, namespacedObjectName(naming.Service(params.Name), params.Namespace)) assert.NoError(t, err) assert.True(t, exists) + assert.Equal(t, svc.Spec.Selector, map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/part-of": "opentelemetry", + }) exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, namespacedObjectName(naming.ServiceAccount(params.Name), params.Namespace)) assert.NoError(t, err) assert.True(t, exists) @@ -183,6 +190,12 @@ func TestOpenTelemetryCollectorReconciler_Reconcile(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) assert.Contains(t, actual.Spec.Ports, extraPorts.ServicePort) + assert.Equal(t, actual.Spec.Selector, map[string]string{ + "app.kubernetes.io/component": "opentelemetry-collector", + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/part-of": "opentelemetry", + }) }, }, wantErr: assert.NoError, diff --git a/internal/manifests/mutate.go b/internal/manifests/mutate.go index 09f48daf52..227af1ebda 100644 --- a/internal/manifests/mutate.go +++ b/internal/manifests/mutate.go @@ -89,7 +89,7 @@ func MutateFuncFor(existing, desired client.Object) controllerutil.MutateFn { case *corev1.Service: svc := existing.(*corev1.Service) wantSvc := desired.(*corev1.Service) - return mutateService(svc, wantSvc) + mutateService(svc, wantSvc) case *corev1.ServiceAccount: sa := existing.(*corev1.ServiceAccount) @@ -256,12 +256,9 @@ func mutatePodMonitor(existing, desired *monitoringv1.PodMonitor) { existing.Spec = desired.Spec } -func mutateService(existing, desired *corev1.Service) error { +func mutateService(existing, desired *corev1.Service) { existing.Spec.Ports = desired.Spec.Ports - if err := mergeWithOverride(&existing.Spec.Selector, desired.Spec.Selector); err != nil { - return err - } - return nil + existing.Spec.Selector = desired.Spec.Selector } func mutateDaemonset(existing, desired *appsv1.DaemonSet) error {