Skip to content

Remove OLS operator code and RBAC rules#95

Closed
lpiwowar wants to merge 2 commits into
openstack-lightspeed:lcore-migrationfrom
lpiwowar:lpiwowar/remove-ols-code
Closed

Remove OLS operator code and RBAC rules#95
lpiwowar wants to merge 2 commits into
openstack-lightspeed:lcore-migrationfrom
lpiwowar:lpiwowar/remove-ols-code

Conversation

@lpiwowar

@lpiwowar lpiwowar commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Remove controller logic (funcs.go, ols_install.go, openstacklightspeed_controller.go) and RBAC permissions (OLSConfig access, OLM operator resources) that are no longer needed.

This prepares the operator for migration to lightspeed-core by stripping out all OLS specific functionality.

Summary by CodeRabbit

  • Refactor

    • Removed automatic OpenShift Lightspeed operator installation and lifecycle management; operators must now be provisioned separately by administrators.
    • Simplified controller reconciliation by removing integrated operator and configuration management.
  • Tests

    • KUTTL integration tests now skip by default.
  • Chores

    • Removed operator provisioning scripts and related environment configuration.

Remove controller logic (funcs.go, ols_install.go,
openstacklightspeed_controller.go) and RBAC permissions (OLSConfig
access, OLM operator resources) that are no longer needed.

This prepares the operator for migration to lightspeed-core by
stripping out all OLS-specific functionality.
Modified Makefile kuttl-test-run target to skip test execution and
output a warning message instead of running the full test suite.

Tests need to be updated to work with the new LCORE backend before
they can be re-enabled. Skipping them temporarily prevents CI
failures during the migration period.
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The PR removes OLS operator lifecycle management capabilities, eliminates OLSConfig-related RBAC permissions, simplifies controller reconciliation logic by deleting operator installation and configuration management functions, and removes associated build and deployment scripts.

Changes

Cohort / File(s) Summary
OLS Operator Lifecycle Management Removal
internal/controller/ols_install.go
Entire module removed (427 lines) containing OLS operator installation, subscription management, install plan approval, and uninstallation logic via OLM.
Controller & Helper Functions Refactor
internal/controller/openstacklightspeed_controller.go, internal/controller/funcs.go, internal/controller/ocp_version.go
Removed OLS operator reconciliation, OLSConfig management, and system prompt support from controller and funcs. Moved BuildRAGConfigs function to ocp_version.go. Simplified deletion flow and removed time-based requeue logic.
RBAC & Permissions Cleanup
config/rbac/role.yaml
Removed namespaced Role and its RBAC rules for ols.openshift.io (olsconfigs) and operators.coreos.com (CSV/subscriptions/installplans). ClusterRole remains unchanged.
Build Scripts & Configuration
Makefile, scripts/env.sh, scripts/gen-ols.sh
Made kuttl-test-run a no-op, removed OPENSHIFT_LIGHTSPEED_OPERATOR_VERSION environment variable, and deleted script that generated OLM subscription manifests for OLS operator.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 With whiskers twitching, the OLS installation sprawls away,
No more subscriptions to manage, no plans to sway,
The controller's cleaner, the RBAC refined,
A simpler lightspeed dance, leaving complexity behind! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main changes: removal of OLS operator code and RBAC rules, which are the primary focus of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci

openshift-ci Bot commented Apr 13, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Apr 13, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lpiwowar

The full list of commands accepted by this bot can be found here.

The pull request process is described 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

@lpiwowar

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/controller/ocp_version.go`:
- Around line 160-166: The BuildRAGConfigs function still references the
constant OpenStackLightspeedVectorDBPath which was removed with funcs.go;
restore that constant into a retained file (for example add it near the top of
internal/controller/ocp_version.go or another kept file) so the controller
compiles. Specifically, reintroduce the OpenStackLightspeedVectorDBPath constant
with the same value/semantics used previously and ensure any other uses in
BuildRAGConfigs and related functions (e.g., any map entries referencing
OpenStackLightspeedVectorDBPath) import it from the retained package scope.

In `@internal/controller/openstacklightspeed_controller.go`:
- Around line 141-142: The delete branch currently only removes the finalizer
and returns, which orphaned pre-migration OLS resources; modify the flow so
reconcileDelete(ctx, helper, instance) performs a best-effort cleanup/migration
of resources owned by OpenStackLightspeed (use the owner/OLS constants from
funcs.go) before removing the finalizer; ensure reconcileDelete is invoked and
awaited/success-checked in both the early delete path (where you added the
simple finalizer removal) and the similar block around lines 250-259 so that
teardown/migration is attempted for older installs prior to finalizer removal.

In `@Makefile`:
- Around line 294-297: The kuttl-test-run Makefile target currently always
succeeds and hides missing e2e tests; change it to fail fast unless an explicit
opt-in variable (e.g., KUTTL_SKIP or SKIP_KUTTL) is set to "true". Update the
kuttl-test-run target so it checks the env var: if the skip var is set print a
clear WARNING and exit 0, otherwise print an error and exit 1 (or invoke the
real kuttl test command when available); reference the Makefile target name
kuttl-test-run so callers/CI can be updated to export the skip flag when
intentionally skipping. Ensure the message makes it obvious tests were skipped
by opt-in.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9eb7880d-24fe-48c2-8068-f878ca683c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 55ffb15 and 6b2b964.

📒 Files selected for processing (8)
  • Makefile
  • config/rbac/role.yaml
  • internal/controller/funcs.go
  • internal/controller/ocp_version.go
  • internal/controller/ols_install.go
  • internal/controller/openstacklightspeed_controller.go
  • scripts/env.sh
  • scripts/gen-ols.sh
💤 Files with no reviewable changes (5)
  • scripts/env.sh
  • scripts/gen-ols.sh
  • config/rbac/role.yaml
  • internal/controller/funcs.go
  • internal/controller/ols_install.go

Comment thread internal/controller/ocp_version.go
Comment thread internal/controller/openstacklightspeed_controller.go
Comment thread Makefile
@lpiwowar lpiwowar closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant