OCPBUGS-39539: clear rogue controller ownerReferences during merge

[savio87]

Summary

Fixes OCPBUGS-39539: CVO wedges with Failing=True when reconciling a CRD that has a rogue ownerReference with controller: true set by another operator (e.g. OLM).

The Kubernetes API rejects updates when two ownerReferences both claim controller: true. The mergeOwnerRefs function was additive-only — it never cleared conflicting controller flags on existing refs, so the rogue ref persisted alongside CVO's own controller ref, causing the update to fail indefinitely.

This fix adds a post-merge pass: when a required ownerReference has Controller=true, any existing ownerReference (not in the required set) that also has Controller=true gets its Controller field cleared. The rogue reference itself is preserved — only its controller claim is demoted.

Test plan

• Added unit test: rogue Controller=true ref is cleared when CVO's required ref claims controller
• Added unit test: existing refs without Controller are left untouched
• All existing TestMergeOwnerRefs tests pass

Summary by CodeRabbit

• Bug Fixes
  • Prevented resources from retaining multiple controller references during metadata merging.
  • Existing controller references are cleared when a new required controller reference is added.
  • Non-controller owner references remain unchanged.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@savio87: This pull request references Jira Issue OCPBUGS-39539, which is invalid:

• expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

Fixes OCPBUGS-39539: CVO wedges with Failing=True when reconciling a CRD that has a rogue ownerReference with controller: true set by another operator (e.g. OLM).

The Kubernetes API rejects updates when two ownerReferences both claim controller: true. The mergeOwnerRefs function was additive-only — it never cleared conflicting controller flags on existing refs, so the rogue ref persisted alongside CVO's own controller ref, causing the update to fail indefinitely.

This fix adds a post-merge pass: when a required ownerReference has Controller=true, any existing ownerReference (not in the required set) that also has Controller=true gets its Controller field cleared. The rogue reference itself is preserved — only its controller claim is demoted.

Test plan

• Added unit test: rogue Controller=true ref is cleared when CVO's required ref claims controller
• Added unit test: existing refs without Controller are left untouched
• All existing TestMergeOwnerRefs tests pass

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 4e23ab4f-011e-4a96-a67c-188d4ae851fa

📥 Commits

Reviewing files that changed from the base of the PR and between f94dc81 and 5db859d.

📒 Files selected for processing (2)

• lib/resourcemerge/meta.go
• lib/resourcemerge/meta_test.go

---

Walkthrough

The owner-reference merge now clears conflicting existing controller flags when a required controller reference is added. Tests cover both conflicting controller references and preserved non-controller references.

Changes

Owner reference reconciliation

Layer / File(s)	Summary
Controller flag reconciliation lib/resourcemerge/meta.go, lib/resourcemerge/meta_test.go	The merge clears Controller=true from existing references with different UIDs when a required controller reference is present. Tests verify that both references remain and that existing non-controller references are preserved.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to 5db85

The change is localized to owner-reference merging and adds focused unit coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the bug and the main change: clearing rogue controller owner references during merge.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	The changed test uses only t.Run("test#%d", idx); idx comes from a fixed table order. No Ginkgo titles or runtime values, such as names, dates, UUIDs, or IPs, were added.
Test Structure And Quality	✅ Passed	The changed test is a standard Go table-driven unit test, not Ginkgo. It creates no resources or waits, uses diagnostic t.Fatalf messages, and matches package conventions.
Microshift Test Compatibility	✅ Passed	The PR adds only a standard Go TestMergeOwnerRefs unit test using testing.T; it adds no Ginkgo e2e tests or MicroShift-incompatible API usage.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds a standard Go table-driven unit test, not a new Ginkgo e2e test; no SNO multi-node compatibility check applies.
Topology-Aware Scheduling Compatibility	✅ Passed	The diff changes only owner-reference merge logic and unit tests; it adds no manifests, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDB constraints.
Ote Binary Stdout Contract	✅ Passed	The diff only changes owner-reference logic and unit-test cases; no added stdout, logging, main, init, or suite-setup writes exist.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The added coverage is a standard Go table-driven unit test, not a Ginkgo e2e test, and it adds no IPv4 assumptions or external network access.
No-Weak-Crypto	✅ Passed	The PR changes only Kubernetes owner-reference merging and tests; the added code has no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparison.
Container-Privileges	✅ Passed	The pull request changes only Go source and tests; it adds no container or Kubernetes manifest privilege settings.
No-Sensitive-Data-In-Logs	✅ Passed	The diff adds owner-reference logic and synthetic test data only; it introduces no runtime logging or sensitive values in logs.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci-robot]

@savio87: This pull request references Jira Issue OCPBUGS-39539, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (5.0.0) matches configured target version for branch (5.0.0)
• bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (jiajliu@redhat.com), skipping review request.

Details

In response to this:

Summary

Fixes OCPBUGS-39539: CVO wedges with Failing=True when reconciling a CRD that has a rogue ownerReference with controller: true set by another operator (e.g. OLM).

The Kubernetes API rejects updates when two ownerReferences both claim controller: true. The mergeOwnerRefs function was additive-only — it never cleared conflicting controller flags on existing refs, so the rogue ref persisted alongside CVO's own controller ref, causing the update to fail indefinitely.

This fix adds a post-merge pass: when a required ownerReference has Controller=true, any existing ownerReference (not in the required set) that also has Controller=true gets its Controller field cleared. The rogue reference itself is preserved — only its controller claim is demoted.

Test plan

• Added unit test: rogue Controller=true ref is cleared when CVO's required ref claims controller
• Added unit test: existing refs without Controller are left untouched
• All existing TestMergeOwnerRefs tests pass

Summary by CodeRabbit

• Bug Fixes
• Prevented resources from retaining multiple controller references during metadata merging.
• Existing controller references are cleared when a new required controller reference is added.
• Non-controller owner references remain unchanged.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Hi @savio87. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: savio87
Once this PR has been reviewed and has the lgtm label, please assign pratikmahajan for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[savio87]

This clears rogue controller ownerReferences that cause CVO to wedge with "only one reference can have Controller set to true" during reconciliation. The rogue ref is preserved — only its controller claim is demoted. Could a maintainer check and run /ok-to-test? Thanks!