OCPBUGS-85646: display operators in catalog when Tech Preview enabled - #16976
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@simkam: This pull request references Jira Issue OCPBUGS-85646, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review. WalkthroughThe catalog hooks replace ChangesCatalog flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR corrects operator catalog loading and duplicate type handling and adds regression coverage; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
/jira refresh |
|
@simkam: This pull request references Jira Issue OCPBUGS-85646, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
|
@simkam: This pull request references Jira Issue OCPBUGS-85646, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
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. |
sampadasawant-36
left a comment
There was a problem hiding this comment.
Reviewed the diff — the fix looks correct overall. A couple of observations:
The core fix is right — inverting loading: true → loaded: false and updating the guard in useCatalogCategories (if (loading || error) → if (!loaded || error)) properly aligns with what CatalogExtensionHookResolver expects. The OLMV1_ENABLED disallowed flag in console-extensions.json also makes sense to avoid duplicate catalog entries.
Two minor suggestions:
-
Unit test for the hook — The semantic inversion (
loading→loaded) is the heart of the fix but is only covered by e2e. A unit test foruseCatalogItemsthat verifiesloadedstarts asfalseand transitions totrueon fetch completion (and on error) would protect against regression without needing a cluster. -
test.skipinsidebeforeEach— This works in Playwright but the idiomatic pattern is to calltest.skip(condition, reason)at the top of each test rather than inside the hook. Calling it inbeforeEachcan sometimes produce misleading output when multiple tests are skipped.
Otherwise the e2e mock approach using page.route() is clean and the test coverage of filtering is a nice addition. LGTM from a logic standpoint!
|
updated per review - added unit tests, moved skip logic in e2e |
|
/test e2e-gcp-console |
|
/test e2e-playwright |
3 similar comments
|
/test e2e-playwright |
|
/test e2e-playwright |
|
/test e2e-playwright |
|
@coderabbitai review |
|
|
|
||
| const CATALOG_ITEMS_URL_PATTERN = '**/api/olm/catalog-items/**'; | ||
|
|
||
| test.describe('OLMv1 Software Catalog items', { tag: ['@admin'] }, () => { |
There was a problem hiding this comment.
wouldn't it better to put it in the existing catalog.spec.ts to avoid fragmentation?
|
|
||
| const CATALOG_ITEMS_URL_PATTERN = '**/api/olm/catalog-items/**'; | ||
|
|
||
| test.describe('OLMv1 Software Catalog items', { tag: ['@admin'] }, () => { |
There was a problem hiding this comment.
is this suite actually testing something strictly related to OLMv1 catalog? Is it maybe just testing the generic behavior of the Catalog Page with v1 data?
There was a problem hiding this comment.
no, test generalized and moved to catalog.spec.ts.
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
useCatalogItems returned `loading = true` (in progress) as the second element of its tuple, but CatalogExtensionHookResolver treats the second element as `loaded = true` (ready). This caused the catalog to call onValueResolved immediately with an empty array, then never again once real items arrived from /api/olm/catalog-items/. Rename `loading` to `loaded` and invert the initial state (false → true on completion) in both useCatalogItems and useCatalogCategories. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The OLMv0 console.catalog/item-type for "operator" had no flag guard, causing it to remain active alongside the OLMv1 item-type when OLMV1_ENABLED=true. This produced a duplicate "Operators" entry in the catalog type selector. Add the same disallowed: [OLMV1_ENABLED] flag that already gates the OLMv0 item-provider, so only the OLMv1 item-type is active when OLMv1 is enabled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ering Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/pipeline required |
|
Scheduling tests matching the |
|
/retest |
1 similar comment
|
/retest |
|
/unhold |
|
@logonoff: This PR has been marked as verified by DetailsIn response to this:
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. |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, simkam, stefanonardo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@simkam: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
bc711f8
into
openshift:main
|
@simkam: Jira Issue Verification Checks: Jira Issue OCPBUGS-85646 Jira Issue OCPBUGS-85646 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
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. |
|
Fix included in release 5.1.0-0.nightly-2026-08-25-102753 |
|
/cherry-pick release-5.0 |
|
@sdodson: new pull request created: #17100 DetailsIn response to this:
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. |


https://redhat.atlassian.net/browse/OCPBUGS-85646
Analysis / Root cause:
useCatalogItemsreturned[items, loading, error]butCatalogExtensionHookResolverexpects[data, loaded, error]. The inverted boolean semantics caused the catalog to report 0 items as "resolved" immediately and ignore real data when it arrived.Solution description:
loading→loadedand invert initial state inuseCatalogItemsanduseCatalogCategoriesdisallowed: [OLMV1_ENABLED]flag to OLMv0 operator item-type to prevent both OLMv0 and OLMv1 types appearing simultaneouslyScreenshots / screen recording:


Before (no operators)
After:
Test setup:
Test cases:
Playwright tests included.
Browser conformance:
Additional info:
Reviewers and assignees:
Summary by CodeRabbit
Bug Fixes
Tests