🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak #2296

camilamacedo86 · 2025-11-01T07:58:25Z

Resolves the TODO in SetDeprecationStatus that requested:

Deprecation status should reflect the installed bundle, not the resolved bundle
Show deprecation warnings even when resolution fails

This fix also addresses the scenario where a bundle resolves correctly but
fails to install: package/channel deprecations are populated correctly while
bundle deprecation stays Unknown/Absent because no bundle is installed yet.

Problem

Before this fix:

When catalog was removed/unreachable, conditions showed False (claiming "not deprecated")
During upgrades v1→v2, BundleDeprecated showed v2 status before v2 was installed
Install/validation errors leaked into deprecation conditions
Resolution failures couldn't show deprecation warnings

Solution

Defer deprecation status updates until reconciliation completes, using a
closure that reads the installed bundle name and catalog data. This ensures:

Correct bundle reference: BundleDeprecated reflects installed bundle, not resolved
Catalog absence handling: Unknown when catalog unavailable (not False)
Error separation: Install errors stay in Progressing, never in deprecation conditions
Warnings on failure: Deprecation data shown even when resolution fails

Scenarios Handled

Scenario	PackageDeprecated	ChannelDeprecated	BundleDeprecated	Notes
No catalog + no bundle	Unknown/Deprecated	Unknown/Deprecated	Unknown/Absent	Catalog unavailable
No catalog + bundle installed	Unknown/Deprecated	Unknown/Deprecated	Unknown/Deprecated	Bundle exists but catalog absent
Resolution succeeds, install fails	From catalog	From catalog	Unknown/Absent	No bundle installed yet
During upgrade v1→v2	From catalog	From catalog	Shows v1 status	Not v2 (not installed)
Resolution fails with warnings	True if deprecated	True if deprecated	Unknown/Absent	Warnings reach users
Resolution fails, no catalog	Unknown	Unknown	Unknown	Catalog completely absent

Resolver Contract

The fix relies on the resolver to signal catalog availability:

Returns deprecation != nil when catalog is available (even if resolution fails)
Returns deprecation = nil when catalog is unavailable
This allows the controller to distinguish "catalog says not deprecated" from "catalog absent"

Example scenarios fixed

1. Catalog Removed - Was Showing False (WRONG)

Before the fix:

# Bundle v1.0.0 installed, then catalog removed
status:
  conditions:
    - type: Deprecated
      status: "False"           # ❌ WRONG - claims "not deprecated"
      reason: Deprecated
      message: ""
      
    - type: PackageDeprecated
      status: "False"           # ❌ WRONG - no catalog to check!
      reason: Deprecated
      
    - type: BundleDeprecated
      status: "False"           # ❌ WRONG
      reason: Absent            # ❌ WRONG - bundle exists!

After the fix:

# Bundle v1.0.0 installed, then catalog removed
status:
  conditions:
    - type: Deprecated
      status: "Unknown"         # ✅ CORRECT - we don't know
      reason: Deprecated
      message: ""
      
    - type: PackageDeprecated
      status: "Unknown"         # ✅ CORRECT - catalog unavailable
      reason: Deprecated
      
    - type: BundleDeprecated
      status: "Unknown"         # ✅ CORRECT - catalog unavailable
      reason: Deprecated        # ✅ CORRECT - bundle exists

2. During Upgrade - Was Showing New Bundle (WRONG)

Before the fix:

# Installed: v1.0.0, Upgrading to: v1.0.1 (not installed yet)
status:
  install:
    bundle:
      name: "operator.v1.0.0"  # v1.0.0 still installed
  conditions:
    - type: BundleDeprecated
      status: "True"
      reason: Deprecated
      message: "Bundle v1.0.1 is deprecated"  # ❌ WRONG - showing v1.0.1!

After the fix:

# Installed: v1.0.0, Upgrading to: v1.0.1 (not installed yet)
status:
  install:
    bundle:
      name: "operator.v1.0.0"  # v1.0.0 still installed
  conditions:
    - type: BundleDeprecated
      status: "False"
      reason: Deprecated
      message: ""  # ✅ CORRECT - shows v1.0.0 status (not deprecated)

3. Resolution Fails But Has Warnings - Warnings Lost

Before the fix:

# Resolution fails: package "foo" not found (but package IS deprecated in catalog)
status:
  conditions:
    - type: Progressing
      status: "True"
      reason: Retrying
      message: "no package 'foo' found"
      
    - type: PackageDeprecated
      status: "False"           # ❌ WRONG - warning lost!
      reason: Deprecated
      message: ""

After the fix:

# Resolution fails: package "foo" not found (but package IS deprecated in catalog)
status:
  conditions:
    - type: Progressing
      status: "True"
      reason: Retrying
      message: "no package 'foo' found"
      
    - type: PackageDeprecated
      status: "True"            # ✅ CORRECT - warning shown!
      reason: Deprecated
      message: "Package foo is deprecated, migrate to bar"  # ✅ User sees warning!

4. Resolution Succeeds, Install Fails - Bundle Status Wrong

Before the fix:

# Resolution succeeds (prometheus.v1.0.0), but apply fails
status:
  install: {}  # Nothing installed
  conditions:
    - type: Progressing
      status: "True"
      reason: Retrying
      message: "apply failed: deployment conflict"
      
    - type: PackageDeprecated
      status: "False"           # ✅ Correct
      
    - type: BundleDeprecated
      status: "True"            # ❌ WRONG - nothing installed yet!
      reason: Deprecated
      message: "Bundle deprecated"

After the fix:

# Resolution succeeds (prometheus.v1.0.0), but apply fails
status:
  install: {}  # Nothing installed
  conditions:
    - type: Progressing
      status: "True"
      reason: Retrying
      message: "apply failed: deployment conflict"
      
    - type: PackageDeprecated
      status: "False"           # ✅ Correct
      
    - type: BundleDeprecated
      status: "Unknown"         # ✅ CORRECT - nothing installed yet
      reason: Absent            # ✅ CORRECT
      message: ""

5. Boxcutter RollingOut - Was Showing False (WRONG)

Before the fix:

# Boxcutter has RollingOut revision, apply fails
# No resolver call happens (reusing revision)
status:
  conditions:
    - type: Deprecated
      status: "False"           # ❌ WRONG - no catalog lookup!
      
    - type: PackageDeprecated
      status: "False"           # ❌ WRONG
      
    - type: BundleDeprecated
      status: "Unknown"
      reason: Absent

After the fix:

# Boxcutter has RollingOut revision, apply fails
# No resolver call happens (reusing revision)
status:
  conditions:
    - type: Deprecated
      status: "Unknown"         # ✅ CORRECT - no catalog data
      
    - type: PackageDeprecated
      status: "Unknown"         # ✅ CORRECT
      
    - type: BundleDeprecated
      status: "Unknown"         # ✅ CORRECT
      reason: Absent

Motivation

Closes: #2008

netlify · 2025-11-01T07:58:30Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`baf566c`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/690e4598e077450008bb97b1
😎 Deploy Preview	https://deploy-preview-2296--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

Copilot

Pull Request Overview

This PR refactors deprecation status handling in ClusterExtension reconciliation to ensure deprecation conditions accurately reflect catalog data and installed bundle state, preventing install/validation errors from leaking into deprecation conditions.

Moved deprecation status updates to a deferred function that runs at the end of reconciliation
Changed BundleDeprecated condition to use Unknown status with ReasonAbsent when no bundle is installed
Updated test expectations to handle multiple possible error messages for sourceType validation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
internal/operator-controller/controllers/clusterextension_controller.go	Refactored deprecation status logic with deferred updates and new condition semantics for uninstalled bundles
internal/operator-controller/controllers/clusterextension_controller_test.go	Added tests for deprecation handling with resolution failures and applier failures; updated test expectations for BundleDeprecated conditions
test/e2e/cluster_extension_install_test.go	Updated e2e tests to verify deprecation conditions in success and failure scenarios
internal/operator-controller/controllers/clusterextension_admission_test.go	Enhanced sourceType validation test to handle multiple possible error message formats

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

internal/operator-controller/controllers/clusterextension_admission_test.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

codecov · 2025-11-01T08:46:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.32%. Comparing base (b3f85d5) to head (baf566c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2296      +/-   ##
==========================================
+ Coverage   71.14%   74.32%   +3.17%     
==========================================
  Files          91       91              
  Lines        7046     7076      +30     
==========================================
+ Hits         5013     5259     +246     
+ Misses       1618     1406     -212     
+ Partials      415      411       -4

Flag	Coverage Δ
e2e	`46.13% <82.50%> (+0.19%)`	⬆️
experimental-e2e	`48.34% <82.50%> (?)`
unit	`58.73% <100.00%> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

internal/operator-controller/controllers/clusterextension_controller_test.go:1

The assertion require.Contains(ct, []string{...}, cond.Reason) is semantically backwards. The Contains function expects a slice as the second argument and the search item as the first. This should be require.Contains(ct, cond.Reason, []string{ocv1.ReasonFailed, ocv1.ReasonAbsent}) or use require.True(ct, slices.Contains([]string{ocv1.ReasonFailed, ocv1.ReasonAbsent}, cond.Reason)).

package controllers_test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller.go

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextension_controller_test.go

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/operator-controller/controllers/clusterextension_controller_test.go:1856

Corrected spelling of 'channedl' to 'channel'.

						Message: "another bad channedl!",

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… and prevent error leak When a catalog is removed or unreachable, deprecation conditions now correctly show Unknown instead of False. BundleDeprecated now reflects the installed bundle and uses the correct reason. - Defer deprecation status updates until end of reconciliation - Track hadCatalogDeprecationData to distinguish catalog absence from 'not deprecated' - Report installed bundle deprecation, not the bundle being installed - BundleDeprecated uses Deprecated reason when bundle exists, Absent only when none - Prevent install/validation errors from leaking into deprecation conditions Scenarios fixed: - No catalog + no bundle → BundleDeprecated: Unknown/Absent - No catalog + bundle installed → BundleDeprecated: Unknown/Deprecated (was False/Absent) - During upgrade v1→v2 → shows v1 status, not v2 - Resolution fails with warnings → deprecation warnings still reach users Assisted-by: Cursor

Copilot

Pull Request Overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

camilamacedo86 · 2025-11-09T18:58:19Z

crd-diff / crd-diff (pull_request)F

That fails due change in the description to properly supplement it.
Not due breaking changes.
We should be able to ovewrite

/overwrite crd-diff

Copilot AI review requested due to automatic review settings November 1, 2025 07:58

camilamacedo86 requested a review from a team as a code owner November 1, 2025 07:58

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2025

openshift-ci bot requested a review from dtfranz November 1, 2025 07:58

openshift-ci bot requested a review from oceanc80 November 1, 2025 07:58

Copilot AI reviewed Nov 1, 2025

View reviewed changes

camilamacedo86 force-pushed the fix-issue-conditions branch from 0891f37 to 4f61e6a Compare November 1, 2025 08:27

Copilot AI review requested due to automatic review settings November 1, 2025 08:33

camilamacedo86 force-pushed the fix-issue-conditions branch from 4f61e6a to 2b26273 Compare November 1, 2025 08:33

Copilot AI reviewed Nov 1, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from 2b26273 to ad9ccfd Compare November 1, 2025 08:46

camilamacedo86 changed the title ~~WIP: 🐛 (fix):Fix deprecation conditions leaking install errors~~ 🐛 (fix):Fix deprecation conditions leaking install errors Nov 1, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 1, 2025

camilamacedo86 requested review from Copilot, joelanford and perdasilva November 1, 2025 08:47

Copilot AI reviewed Nov 1, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Show resolved Hide resolved

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from ad9ccfd to a6d0678 Compare November 1, 2025 09:31

camilamacedo86 requested a review from Copilot November 1, 2025 09:32

Copilot AI reviewed Nov 1, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from a6d0678 to 287bede Compare November 3, 2025 09:38

camilamacedo86 requested a review from Copilot November 3, 2025 10:10

Copilot AI reviewed Nov 3, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

perdasilva reviewed Nov 3, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from 287bede to b2da76f Compare November 3, 2025 12:16

perdasilva reviewed Nov 3, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

perdasilva reviewed Nov 3, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

perdasilva reviewed Nov 3, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from bd2e4eb to ebae212 Compare November 7, 2025 16:50

camilamacedo86 changed the title ~~WIP: 🐛 Fix deprecation conditions leaking install errors~~ 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak Nov 7, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2025

Copilot AI review requested due to automatic review settings November 7, 2025 16:55

camilamacedo86 force-pushed the fix-issue-conditions branch from ebae212 to e9dfabb Compare November 7, 2025 16:55

Copilot AI reviewed Nov 7, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller.go Outdated Show resolved Hide resolved

camilamacedo86 changed the title ~~🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak~~ WIP 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak Nov 7, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2025

camilamacedo86 force-pushed the fix-issue-conditions branch from e9dfabb to 661d7ba Compare November 7, 2025 17:12

operator-framework deleted a comment from Copilot AI Nov 7, 2025

camilamacedo86 requested a review from Copilot November 7, 2025 17:16

Copilot AI reviewed Nov 7, 2025

View reviewed changes

internal/operator-controller/controllers/clusterextension_controller_test.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issue-conditions branch from 661d7ba to 1ea2a14 Compare November 7, 2025 17:28

Copilot AI review requested due to automatic review settings November 7, 2025 17:41

camilamacedo86 force-pushed the fix-issue-conditions branch from 1ea2a14 to c1efa30 Compare November 7, 2025 17:41

camilamacedo86 requested review from Copilot and removed request for Copilot November 7, 2025 17:42

Copilot AI reviewed Nov 7, 2025

View reviewed changes

camilamacedo86 force-pushed the fix-issue-conditions branch from c1efa30 to 3afac96 Compare November 7, 2025 17:53

camilamacedo86 changed the title ~~WIP 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak~~ 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak Nov 7, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2025

camilamacedo86 changed the title ~~🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak~~ WIP 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak Nov 7, 2025

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2025

camilamacedo86 changed the title ~~WIP 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak~~ 🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak Nov 7, 2025

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 7, 2025

Copilot AI review requested due to automatic review settings November 7, 2025 19:16

camilamacedo86 force-pushed the fix-issue-conditions branch from 3afac96 to baf566c Compare November 7, 2025 19:16

Copilot AI reviewed Nov 7, 2025

View reviewed changes

🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak #2296

Are you sure you want to change the base?

🐛 report Unknown for deprecation status when catalog unavailable and prevent error leak #2296

Conversation

camilamacedo86 commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Scenarios Handled

Resolver Contract

Example scenarios fixed

1. Catalog Removed - Was Showing False (WRONG)

2. During Upgrade - Was Showing New Bundle (WRONG)

3. Resolution Fails But Has Warnings - Warnings Lost

4. Resolution Succeeds, Install Fails - Bundle Status Wrong

5. Boxcutter RollingOut - Was Showing False (WRONG)

Motivation

Uh oh!

netlify bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

codecov bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

camilamacedo86 commented Nov 9, 2025

Uh oh!

camilamacedo86 commented Nov 1, 2025 •

edited

Loading

netlify bot commented Nov 1, 2025 •

edited

Loading

codecov bot commented Nov 1, 2025 •

edited

Loading