Skip to content

🐛 OPRUN-4783: Surface deployment owner conflicts on ClusterExtensions - #3094

Open
fao89 wants to merge 1 commit into
operator-framework:mainfrom
fao89:fix/clusterextension-owner-conflict-status
Open

fao89 wants to merge 1 commit into
operator-framework:mainfrom
fao89:fix/clusterextension-owner-conflict-status

Conversation

@fao89

@fao89 fao89 commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Description

An ownership conflict on the highest active ClusterObjectSet blocks revision creation, but the owning ClusterExtension reports Progressing=True/RollingOut and Installed=False/Absent on a first install. Surface the conflict as Progressing=True/Retrying and Installed=False/Failed, preserving the diagnostic message and any previously installed bundle during upgrades. Add regression coverage for recovery through rollout completion and document the mapping.

Follow-up to the status-mapping review comment on #3074.

Related to OPRUN-4783.

Validation

  • make test-unit passed with race detection using umask 022.
  • make lint and make verify passed.
  • The focused regression tests reproduced the incorrect RollingOut reason before the fix and passed afterward.

Full cluster E2E suites were not run.

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

Summary by CodeRabbit

  • Bug Fixes
    • Owner conflicts now show as retrying, with the conflict details preserved. On a first install, the installation is marked failed; during upgrades, the previously installed bundle and status remain in place until the new rollout completes. Reconciliation resumes automatically once the conflict is cleared.
  • Documentation
    • Clarified how owner conflicts affect installation status and upgrade behavior.

Map ClusterObjectDeployment OwnerConflict to Progressing=True/Retrying
and Installed=False/Failed for first installs. Preserve the deployment's
diagnostic message and the previously installed bundle during upgrades.

Cover status recovery through rollout completion and document the mapping.

Refs: operator-framework#3074
Signed-off-by: Fabricio Aguiar <fabricio.aguiar@gmail.com>

rh-pre-commit.version: 2.3.2
rh-pre-commit.check-secrets: ENABLED
@netlify

netlify Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 9d3b4d8
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/6ae8d2416300094d3a708b51
😎 Deploy Preview https://deploy-preview-3094--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@openshift-ci
openshift-ci Bot requested review from fgiudici and perdasilva November 2, 2026 18:11
@openshift-ci

openshift-ci Bot commented Nov 2, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign perdasilva 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:

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

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: b9a2a3e2-c150-401c-ba30-22af856720e5
Commits

Reviewing files that changed from the base of the PR and between a9a8ed1 and 9d3b4d8.

Files selected for processing: 3
  • docs/draft/concepts/clusterobjectsets.md
  • internal/operator-controller/controllers/clusterobjectdeployment_status.go
  • internal/operator-controller/controllers/clusterobjectdeployment_status_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The controller now maps OwnerConflict to retrying status and handles it as a failure when no bundle is installed. Tests cover first installs and upgrades through rollout completion. Documentation describes the reported conditions and reconciliation behavior.

Changes: OwnerConflict status handling

Layer / File(s) Summary
Map and verify owner-conflict status
internal/operator-controller/controllers/clusterobjectdeployment_status.go, internal/operator-controller/controllers/clusterobjectdeployment_status_test.go, docs/draft/concepts/clusterobjectsets.md
The controller maps OwnerConflict to Retrying and maps it to Failed when no bundle is installed. Tests cover first install, upgrade, and rollout progress. The documentation describes the status conditions and reconciliation behavior.

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

Change: Bug fix

Suggested reviewers: rashmigottipati, perdasilva

Merge Risk: Minimal · up to 9d3b4

Ownership conflicts are reported as intended, with no actionable merge-blocking risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the bug fix and matches the main change: surfacing deployment owner conflicts on ClusterExtensions.
Description check Passed The description explains the motivation, expected status changes, upgrade behavior, tests, validation results, related issues, and the fact that full E2E suites were not run. It includes the required …
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.
Full details: Description check

Explanation

The description explains the motivation, expected status changes, upgrade behavior, tests, validation results, related issues, and the fact that full E2E suites were not run. It includes the required template sections.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant