Skip to content

Commit

Permalink
Fixes internal bug for modified selector (open-telemetry#2874)
Browse files Browse the repository at this point in the history
* fixes bug

* chlog
  • Loading branch information
jaronoff97 authored Apr 17, 2024
1 parent 0867ff6 commit 6f213e4
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
16 changes: 16 additions & 0 deletions .chloggen/fix-selector-bug.yaml
Original file line number Diff line number Diff line change
@@ -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:
15 changes: 14 additions & 1 deletion controllers/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions internal/manifests/mutate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6f213e4

Please sign in to comment.