fix lvms-operator catalog resolution in ocp4.22+

[eliajahshan]

Description:

Problem

When deploying a cluster with the LVM operator enabled on OCP 4.22+, the installation gets stuck at the "Waiting for olm operators csv initialization" stage and eventually times out. The root cause is that lvms-operator is no longer published in the redhat-operator-index:v4.22 catalog in the gRPC format required by the legacy OLM v1alpha1 Subscription API.

The OLM Subscription enters ResolutionFailed state with:

no operators found in package lvms-operator in the catalog referenced by subscription lvms-operator

Because OLM can never resolve the Subscription to an InstallPlan, no CSV is ever created in openshift-storage, and the install times out.

Fix

For OCP ≥ 4.22, assisted-service now injects an additional CatalogSource manifest pointing to redhat-operator-index:v4.21 (where lvms-operator is still available). The LVMS Subscription is then directed at this fallback catalog source instead of the default redhat-operators.

For OCP < 4.22 the behavior is completely unchanged — the Subscription continues to use redhat-operators as before.

Files Changed

• internal/operators/lvm/config.go — added 3 constants: LvmsCatalogFallbackMinVersion (4.22.0), LvmsCatalogFallbackName, LvmsCatalogFallbackImage
• internal/operators/lvm/manifest.go:
  • Manifests() — injects 50_openshift-lvm_catalog_source.yaml for OCP ≥ 4.22
  • getSubscriptionInfo() — sets catalog source dynamically based on OCP version
  • LvmSubscription template — source field is now dynamic instead of hardcoded redhat-operators
  • Added LvmCatalogSource template and getCatalogSource() function

Precedent

This is the same workaround pattern already used for LSO (deploy/operator/setup_lso.sh), which went through the identical issue on 4.22 and then again on 5.0. The TODO comment is intentional — the workaround should be removed once lvms-operator is published to the 4.22 catalog.

Testing

• Deploy cluster with LVM enabled on OCP 4.22 — verify lvms-operator CSV is created and operator reaches Succeeded
• Verify 50_openshift-lvm_catalog_source.yaml manifest is generated for OCP 4.22+
• Verify no catalog source manifest is generated for OCP 4.21 and below
• Existing LVM unit tests pass

Summary by CodeRabbit

• New Features
  • Added fallback catalog support for the LVM operator to improve compatibility across different OpenShift versions.
  • The operator subscription now dynamically selects the appropriate catalog source based on your OpenShift version, ensuring optimal compatibility and performance.

[coderabbitai]

Walkthrough

Adds three exported constants to config.go defining the minimum OCP version, catalog name, and index image for the lvms-operator fallback catalog. Updates manifest.go to conditionally inject a CatalogSource manifest and to parameterize the subscription's source field using those constants.

Changes

LVM Fallback CatalogSource Manifest Injection

Layer / File(s)	Summary
Fallback catalog constants internal/operators/lvm/config.go	Adds LvmsCatalogFallbackMinVersion, LvmsCatalogFallbackName, and LvmsCatalogFallbackImage exported constants with explanatory comments and a TODO for future removal.
Conditional CatalogSource injection and subscription parameterization internal/operators/lvm/manifest.go	Manifests() appends 50_openshift-lvm_catalog_source.yaml when the fallback is needed. getSubscriptionInfo() derives OPERATOR_CATALOG_SOURCE from the OCP version. New getCatalogSource() helper renders the fallback manifest. LvmSubscription template replaces hardcoded redhat-operators with {{.OPERATOR_CATALOG_SOURCE}}; new exported LvmCatalogSource YAML template constant is added.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• pastequo
• shay23bra

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality	⚠️ Warning	Tests missing critical coverage for new feature: no tests for OCP 4.22+ catalog source injection, assertion messages inconsistent, test structure has multiple concerns per test case.	Add tests for OCP 4.22+ to verify catalog source manifest generation; test 4.21 to verify no catalog source; add assertion messages to all Expect calls; separate test concerns (manifest existence vs YAML validity).

✅ Passed checks (13 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main fix: addressing lvms-operator catalog resolution for OCP 4.22+, which matches the primary change in the changeset.
Description check	✅ Passed	The PR description comprehensively covers the problem, fix, files changed, precedent, and testing steps. However, it lacks formal checklist completion as required by the template.
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	This PR only modifies implementation files (config.go and manifest.go) and does not add any new Ginkgo tests. Existing tests with dynamic names using fmt.Sprintf were already present in the parent...
Microshift Test Compatibility	✅ Passed	PR modifies only source code (config.go, manifest.go) without adding any Ginkgo e2e tests, making this MicroShift test compatibility check not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added in this PR. The modified test files are internal unit tests in internal/operators/lvm/, not e2e tests. The custom check applies only to new e2e tests.
Topology-Aware Scheduling Compatibility	✅ Passed	PR adds CatalogSource and updates Subscription manifests for OCP 4.22 fallback; no deployment/pod specs with scheduling constraints are introduced.
Ote Binary Stdout Contract	✅ Passed	No stdout writes in process-level code; functions are called from handler context via GenerateManifests, not during init/startup. Template rendering uses bytes.Buffer.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	New Ginkgo tests in manifests_test.go are unit tests that use only local mocked data, perform no external network calls, and contain no IPv4 assumptions. Fully compatible with IPv6-only and disconn...
No-Weak-Crypto	✅ Passed	The PR modifies LVM manifest generation with no cryptographic code, weak crypto algorithms, custom implementations, or secret comparisons involved.
Container-Privileges	✅ Passed	PR modifies LVM operator manifests (Subscription, CatalogSource, Namespace, OperatorGroup, LvmCluster) but contains no container specs with privileged settings, hostPID/Network/IPC, SYS_ADMIN capab...
No-Sensitive-Data-In-Logs	✅ Passed	No logging statements exist in the modified code, and no sensitive data (passwords, tokens, API keys, PII, etc.) is referenced or would be exposed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[openshift-ci]

Hi @eliajahshan. 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.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

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: eliajahshan
Once this PR has been reviewed and has the lgtm label, please assign gamli75 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

[coderabbitai]

🧹 Nitpick comments (1)

internal/operators/lvm/manifest.go (1)

33-43: ⚡ Quick win

Add explicit unit coverage for the new fallback-catalog branch.

Please add tests that assert: (1) Line 33+ behavior includes 50_openshift-lvm_catalog_source.yaml for OCP 4.22.0+, (2) it is absent below 4.22.0, and (3) Line 144 uses redhat-operators-v4-21 only in the fallback case. This is the new risk surface and currently isn’t directly asserted in the provided LVM tests.

Also applies to: 57-65, 72-80, 84-90, 144-163

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/operators/lvm/manifest.go` around lines 33 - 43, Add explicit unit
test coverage for the new fallback-catalog branch logic in the LVM manifest
generation. Create test cases that assert: (1) when the cluster OpenShift
version is 4.22.0 or greater, the needsFallbackCatalog check triggers and the
getCatalogSource function is called to populate openshiftManifests with the
50_openshift-lvm_catalog_source.yaml key, (2) when the cluster version is below
the LvmsCatalogFallbackMinVersion threshold, the fallback catalog source is not
included in the manifests, and (3) the redhat-operators-v4-21 catalog source is
only used in the fallback case path. These tests will cover the new risk surface
created by this feature branch and ensure the conditional logic behaves
correctly across version boundaries.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/operators/lvm/manifest.go`:
- Around line 33-43: Add explicit unit test coverage for the new
fallback-catalog branch logic in the LVM manifest generation. Create test cases
that assert: (1) when the cluster OpenShift version is 4.22.0 or greater, the
needsFallbackCatalog check triggers and the getCatalogSource function is called
to populate openshiftManifests with the 50_openshift-lvm_catalog_source.yaml
key, (2) when the cluster version is below the LvmsCatalogFallbackMinVersion
threshold, the fallback catalog source is not included in the manifests, and (3)
the redhat-operators-v4-21 catalog source is only used in the fallback case
path. These tests will cover the new risk surface created by this feature branch
and ensure the conditional logic behaves correctly across version boundaries.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7aaf8be1-e1ff-4768-b488-e4751a17c5a1

📥 Commits

Reviewing files that changed from the base of the PR and between aa192af and 455d3bf.

📒 Files selected for processing (2)

• internal/operators/lvm/config.go
• internal/operators/lvm/manifest.go