Fix TargetAllocator reconcilation loop and missing RBAC for events.k8…#4950
Open
IshwarKanse wants to merge 2 commits intoopen-telemetry:mainfrom
Open
Fix TargetAllocator reconcilation loop and missing RBAC for events.k8…#4950IshwarKanse wants to merge 2 commits intoopen-telemetry:mainfrom
IshwarKanse wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
…s.io Assisted by Claude Code.
d2cc048 to
f63e68f
Compare
Contributor
|
For the event fix, could you add a e2e test verifying that the operator can emit them? I'd like to avoid this breakage in the future. |
Contributor
Author
Added the assertion to check the events. |
swiatekm
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two bugs:
1. AnyConfig.DeepCopyInto shallow copy causes TargetAllocator infinite reconciliation loop
AnyConfig.DeepCopyIntousedmaps.Copy()which only copied the top-level map entries, leaving nested maps/slices as shared references. WhenApplyDefaultsinjected TLS profile settings (min_version) into the collector's scrape config viaapplyTLSProfileToScrapeConfigs, it mutated the informer cache through the shared reference.This caused the TargetAllocator config hash annotation (
opentelemetry-targetallocator-config/hash) to alternate between two values on every reconciliation — one withmin_version: TLS12(from the mutated cache) and one without (from a fresh cache read). The Deployment generation counter incremented continuously (~2/sec), flip-flopping between two ReplicaSets.Root cause: Introduced by #4871 which added
PrometheusParser.applyTLSProfileToScrapeConfigs— a function that mutates deeply-nested maps in-place. The shallowDeepCopyexisted before but was harmless until nested map mutation was introduced.Fix: Changed
AnyConfig.DeepCopyIntoto perform a true deep copy via JSON round-trip (json.Marshal→json.Unmarshal), ensuring nested maps/slices are fully independent copies.Triggered when:
TLS_CONFIGURE_OPERANDS=true(default on OpenShift with TLS profile injection enabled).2. Missing RBAC for events.k8s.io API group
The operator uses
k8s.io/client-go/tools/events(via controller-runtime'smgr.GetEventRecorder()), which targets theevents.k8s.ioAPI group. The ClusterRole only granted permission for the core API group (""), causing"Server rejected event (will not retry!)"errors when recording events on managed resources.Fix: Added
+kubebuilder:rbac:groups=events.k8s.io,resources=events,verbs=create;patchmarkers to all three controllers and regenerated manifests.Testing
chainsaw test --skip-delete tests/e2e-targetallocator/targetallocator-kubernetessd— passes (previously failed withobservedGeneration: 21instead of1)1with a single ReplicaSet after the fixAnyConfig.DeepCopyInto:TestAnyConfigDeepCopyInto_NestedMapIndependence— verifies mutating nested maps in the copy does not affect the source (reproduces the exact bug)TestAnyConfigDeepCopyInto_NilObject— nil Object stays nilTestAnyConfigDeepCopyInto_EmptyObject— empty map is independentTestAnyConfigDeepCopyInto_PreservesValues— all value types survive the deep copygo test ./apis/v1beta1/...,go test ./internal/manifests/targetallocator/...)