Skip to content

Commit 3afac96

Browse files
(fix): report Unknown for deprecation status when catalog unavailable 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
1 parent b3f85d5 commit 3afac96

File tree

14 files changed

+1023
-145
lines changed

14 files changed

+1023
-145
lines changed

SCENARIOS_FIXED.md

Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,305 @@
1+
# All Scenarios Fixed - Complete Examples
2+
3+
## Comparison with main
4+
```
5+
13 files changed, 726 insertions(+), 144 deletions(-)
6+
```
7+
8+
## Fixed Scenarios
9+
10+
### 1. ❌ Catalog Removed - Was Showing False (WRONG)
11+
12+
**Before the fix:**
13+
```yaml
14+
# Bundle v1.0.0 installed, then catalog removed
15+
status:
16+
conditions:
17+
- type: Deprecated
18+
status: "False" # ❌ WRONG - claims "not deprecated"
19+
reason: Deprecated
20+
message: ""
21+
22+
- type: PackageDeprecated
23+
status: "False" # ❌ WRONG - no catalog to check!
24+
reason: Deprecated
25+
26+
- type: BundleDeprecated
27+
status: "False" # ❌ WRONG
28+
reason: Absent # ❌ WRONG - bundle exists!
29+
```
30+
31+
**After the fix:**
32+
```yaml
33+
# Bundle v1.0.0 installed, then catalog removed
34+
status:
35+
conditions:
36+
- type: Deprecated
37+
status: "Unknown" # ✅ CORRECT - we don't know
38+
reason: Deprecated
39+
message: ""
40+
41+
- type: PackageDeprecated
42+
status: "Unknown" # ✅ CORRECT - catalog unavailable
43+
reason: Deprecated
44+
45+
- type: BundleDeprecated
46+
status: "Unknown" # ✅ CORRECT - catalog unavailable
47+
reason: Deprecated # ✅ CORRECT - bundle exists
48+
```
49+
50+
---
51+
52+
### 2. ❌ During Upgrade - Was Showing New Bundle (WRONG)
53+
54+
**Before the fix:**
55+
```yaml
56+
# Installed: v1.0.0, Upgrading to: v1.0.1 (not installed yet)
57+
status:
58+
install:
59+
bundle:
60+
name: "operator.v1.0.0" # v1.0.0 still installed
61+
conditions:
62+
- type: BundleDeprecated
63+
status: "True"
64+
reason: Deprecated
65+
message: "Bundle v1.0.1 is deprecated" # ❌ WRONG - showing v1.0.1!
66+
```
67+
68+
**After the fix:**
69+
```yaml
70+
# Installed: v1.0.0, Upgrading to: v1.0.1 (not installed yet)
71+
status:
72+
install:
73+
bundle:
74+
name: "operator.v1.0.0" # v1.0.0 still installed
75+
conditions:
76+
- type: BundleDeprecated
77+
status: "False"
78+
reason: Deprecated
79+
message: "" # ✅ CORRECT - shows v1.0.0 status (not deprecated)
80+
```
81+
82+
---
83+
84+
### 3. ❌ Resolution Fails But Has Warnings - Warnings Lost
85+
86+
**Before the fix:**
87+
```yaml
88+
# Resolution fails: package "foo" not found (but package IS deprecated in catalog)
89+
status:
90+
conditions:
91+
- type: Progressing
92+
status: "True"
93+
reason: Retrying
94+
message: "no package 'foo' found"
95+
96+
- type: PackageDeprecated
97+
status: "False" # ❌ WRONG - warning lost!
98+
reason: Deprecated
99+
message: ""
100+
```
101+
102+
**After the fix:**
103+
```yaml
104+
# Resolution fails: package "foo" not found (but package IS deprecated in catalog)
105+
status:
106+
conditions:
107+
- type: Progressing
108+
status: "True"
109+
reason: Retrying
110+
message: "no package 'foo' found"
111+
112+
- type: PackageDeprecated
113+
status: "True" # ✅ CORRECT - warning shown!
114+
reason: Deprecated
115+
message: "Package foo is deprecated, migrate to bar" # ✅ User sees warning!
116+
```
117+
118+
---
119+
120+
### 4. ❌ Resolution Succeeds, Install Fails - Bundle Status Wrong
121+
122+
**Before the fix:**
123+
```yaml
124+
# Resolution succeeds (prometheus.v1.0.0), but apply fails
125+
status:
126+
install: {} # Nothing installed
127+
conditions:
128+
- type: Progressing
129+
status: "True"
130+
reason: Retrying
131+
message: "apply failed: deployment conflict"
132+
133+
- type: PackageDeprecated
134+
status: "False" # ✅ Correct
135+
136+
- type: BundleDeprecated
137+
status: "True" # ❌ WRONG - nothing installed yet!
138+
reason: Deprecated
139+
message: "Bundle deprecated"
140+
```
141+
142+
**After the fix:**
143+
```yaml
144+
# Resolution succeeds (prometheus.v1.0.0), but apply fails
145+
status:
146+
install: {} # Nothing installed
147+
conditions:
148+
- type: Progressing
149+
status: "True"
150+
reason: Retrying
151+
message: "apply failed: deployment conflict"
152+
153+
- type: PackageDeprecated
154+
status: "False" # ✅ Correct
155+
156+
- type: BundleDeprecated
157+
status: "Unknown" # ✅ CORRECT - nothing installed yet
158+
reason: Absent # ✅ CORRECT
159+
message: ""
160+
```
161+
162+
---
163+
164+
### 5. ❌ Boxcutter RollingOut - Was Showing False (WRONG)
165+
166+
**Before the fix:**
167+
```yaml
168+
# Boxcutter has RollingOut revision, apply fails
169+
# No resolver call happens (reusing revision)
170+
status:
171+
conditions:
172+
- type: Deprecated
173+
status: "False" # ❌ WRONG - no catalog lookup!
174+
175+
- type: PackageDeprecated
176+
status: "False" # ❌ WRONG
177+
178+
- type: BundleDeprecated
179+
status: "Unknown"
180+
reason: Absent
181+
```
182+
183+
**After the fix:**
184+
```yaml
185+
# Boxcutter has RollingOut revision, apply fails
186+
# No resolver call happens (reusing revision)
187+
status:
188+
conditions:
189+
- type: Deprecated
190+
status: "Unknown" # ✅ CORRECT - no catalog data
191+
192+
- type: PackageDeprecated
193+
status: "Unknown" # ✅ CORRECT
194+
195+
- type: BundleDeprecated
196+
status: "Unknown" # ✅ CORRECT
197+
reason: Absent
198+
```
199+
200+
---
201+
202+
### 6. ✅ Happy Path - Bundle Installed, Not Deprecated
203+
204+
**Before and After (UNCHANGED - was already correct):**
205+
```yaml
206+
# Bundle v1.0.0 successfully installed, catalog says not deprecated
207+
status:
208+
install:
209+
bundle:
210+
name: "operator.v1.0.0"
211+
conditions:
212+
- type: Deprecated
213+
status: "False" # ✅ Correct
214+
215+
- type: PackageDeprecated
216+
status: "False" # ✅ Correct
217+
218+
- type: BundleDeprecated
219+
status: "False" # ✅ Correct
220+
reason: Deprecated
221+
message: ""
222+
```
223+
224+
---
225+
226+
### 7. ✅ Happy Path - Bundle Installed, Is Deprecated
227+
228+
**Before and After (UNCHANGED - was already correct):**
229+
```yaml
230+
# Bundle v1.0.0 successfully installed, catalog says it's deprecated
231+
status:
232+
install:
233+
bundle:
234+
name: "operator.v1.0.0"
235+
conditions:
236+
- type: Deprecated
237+
status: "True" # ✅ Correct
238+
message: "Bundle v1.0.0 is deprecated"
239+
240+
- type: PackageDeprecated
241+
status: "False" # ✅ Correct
242+
243+
- type: BundleDeprecated
244+
status: "True" # ✅ Correct
245+
reason: Deprecated
246+
message: "Bundle v1.0.0 is deprecated"
247+
```
248+
249+
---
250+
251+
## Summary Table
252+
253+
| Scenario | Before | After | Fixed? |
254+
|----------|--------|-------|--------|
255+
| Catalog removed + bundle installed | False/Absent | Unknown/Deprecated | ✅ |
256+
| During upgrade (v1→v2, v2 not installed) | Shows v2 | Shows v1 | ✅ |
257+
| Resolution fails with warnings | Lost warnings | Shows warnings | ✅ |
258+
| Resolution OK, install fails | Bundle True | Bundle Unknown/Absent | ✅ |
259+
| Boxcutter RollingOut path | False | Unknown | ✅ |
260+
| Bundle installed, not deprecated | False | False | ✅ (unchanged) |
261+
| Bundle installed, is deprecated | True | True | ✅ (unchanged) |
262+
263+
---
264+
265+
## Key Code Changes
266+
267+
### 1. Defer deprecation status updates
268+
```go
269+
defer func() {
270+
installedBundleName := ""
271+
if revisionStates != nil && revisionStates.Installed != nil {
272+
installedBundleName = revisionStates.Installed.Name // ✅ Uses installed, not resolved
273+
}
274+
SetDeprecationStatus(ext, installedBundleName, resolvedDeprecation, hadCatalogDeprecationData)
275+
}()
276+
```
277+
278+
### 2. Track catalog data availability
279+
```go
280+
if err == nil || resolvedDeprecation != nil {
281+
hadCatalogDeprecationData = true // ✅ Even when resolution fails!
282+
}
283+
```
284+
285+
### 3. Handle catalog absence
286+
```go
287+
if !hasCatalogData {
288+
bundleReason := ocv1.ReasonAbsent
289+
if installedBundleName != "" {
290+
bundleReason = ocv1.ReasonDeprecated // ✅ Correct reason when bundle exists
291+
}
292+
// All conditions go Unknown
293+
setDeprecationCondition(ext, ocv1.TypeDeprecated, metav1.ConditionUnknown, ...)
294+
}
295+
```
296+
297+
### 4. RollingOut path doesn't set catalog flag
298+
```go
299+
} else {
300+
// Reusing RollingOut revision, no new catalog lookup happens here.
301+
resolvedRevisionMetadata = revisionStates.RollingOut[0]
302+
// hadCatalogDeprecationData stays false ✅
303+
}
304+
```
305+

api/v1/clusterextension_types.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -483,12 +483,15 @@ type ClusterExtensionStatus struct {
483483
// When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
484484
// When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
485485
//
486-
// When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
486+
// When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
487487
// These are indications from a package owner to guide users away from a particular package, channel, or bundle.
488-
// BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
489-
// ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
490-
// PackageDeprecated is set if the requested package is marked deprecated in the catalog.
491-
// Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
488+
// PackageDeprecated reports the requested package as deprecated when the catalog says so. When no catalog talks
489+
// about the package, the condition stays Unknown instead of claiming False.
490+
// ChannelDeprecated follows the same rules for each requested channel.
491+
// BundleDeprecated describes the installed bundle once one exists. Until a bundle installs, or when no catalog
492+
// data is available, it remains Unknown.
493+
// Deprecated is still the rollup: it goes True when any individual deprecation condition is True, and Unknown
494+
// when we have no catalog information to report.
492495
//
493496
// +listType=map
494497
// +listMapKey=type

docs/api-reference/olmv1-api-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ _Appears in:_
359359

360360
| Field | Description | Default | Validation |
361361
| --- | --- | --- | --- |
362-
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.<br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br />When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.<br />ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.<br />PackageDeprecated is set if the requested package is marked deprecated in the catalog.<br />Deprecated is a rollup condition that is present when any of the deprecated conditions are present. | | |
362+
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | The set of condition types which apply to all spec.source variations are Installed and Progressing.<br />The Installed condition represents whether or not the bundle has been installed for this ClusterExtension.<br />When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />When Installed is False and the Reason is Failed, the bundle has failed to install.<br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br />When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />PackageDeprecated reports the requested package as deprecated when the catalog says so. When no catalog talks<br />about the package, the condition stays Unknown instead of claiming False.<br />ChannelDeprecated follows the same rules for each requested channel.<br />BundleDeprecated describes the installed bundle once one exists. Until a bundle installs, or when no catalog<br />data is available, it remains Unknown.<br />Deprecated is still the rollup: it goes True when any individual deprecation condition is True, and Unknown<br />when we have no catalog information to report. | | |
363363
| `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. | | |
364364

365365

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,12 +518,15 @@ spec:
518518
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
519519
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
520520
521-
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
521+
When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
522522
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
523-
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
524-
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
525-
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
526-
Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
523+
PackageDeprecated reports the requested package as deprecated when the catalog says so. When no catalog talks
524+
about the package, the condition stays Unknown instead of claiming False.
525+
ChannelDeprecated follows the same rules for each requested channel.
526+
BundleDeprecated describes the installed bundle once one exists. Until a bundle installs, or when no catalog
527+
data is available, it remains Unknown.
528+
Deprecated is still the rollup: it goes True when any individual deprecation condition is True, and Unknown
529+
when we have no catalog information to report.
527530
items:
528531
description: Condition contains details for one aspect of the current
529532
state of this API Resource.

helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,12 +479,15 @@ spec:
479479
When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
480480
When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
481481
482-
When the ClusterExtension is sourced from a catalog, if may also communicate a deprecation condition.
482+
When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
483483
These are indications from a package owner to guide users away from a particular package, channel, or bundle.
484-
BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
485-
ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
486-
PackageDeprecated is set if the requested package is marked deprecated in the catalog.
487-
Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
484+
PackageDeprecated reports the requested package as deprecated when the catalog says so. When no catalog talks
485+
about the package, the condition stays Unknown instead of claiming False.
486+
ChannelDeprecated follows the same rules for each requested channel.
487+
BundleDeprecated describes the installed bundle once one exists. Until a bundle installs, or when no catalog
488+
data is available, it remains Unknown.
489+
Deprecated is still the rollup: it goes True when any individual deprecation condition is True, and Unknown
490+
when we have no catalog information to report.
488491
items:
489492
description: Condition contains details for one aspect of the current
490493
state of this API Resource.

0 commit comments

Comments
 (0)