OLS console#101
Merged
openshift-merge-bot[bot] merged 6 commits intoMay 27, 2026
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds OpenShift console plugin integration to the OpenStack Lightspeed operator. It introduces build infrastructure for OpenShift registries, OCP API dependencies, console image selection by OCP version (PatternFly 5 vs 6), resource builders for Deployment/Service/NetworkPolicy/ConsolePlugin with HTTPS/TLS and locale rewriting, two-phase reconciliation logic with idempotent activation/deactivation, controller wiring, RBAC extensions, and comprehensive unit and KUTTL integration tests. ChangesConsole Plugin Integration
Sequence Diagram(s)sequenceDiagram
participant Controller as OpenStackLightspeed Controller
participant R1 as ReconcileConsoleResources
participant R2 as ReconcileConsoleDeployment
participant K8s as Kubernetes API
participant Console as OpenShift Console CR
Controller->>R1: Phase 1: Create/Patch base resources
R1->>K8s: ConfigMap, NetworkPolicy, ServiceAccount
R1-->>Controller: Continue-on-error
Controller->>R2: Phase 2: Deploy and activate
R2->>K8s: Detect OCP version, select console image
R2->>K8s: Deployment + init container (locale rewrite)
R2->>K8s: Service with serving-certificate annotation
R2->>K8s: Poll TLS Secret (tls.key, tls.crt)
R2->>K8s: ConsolePlugin CR
R2->>Console: Fetch Console CR
R2->>Console: Add plugin to spec.plugins
R2->>Console: Update Console CR (activate)
R2-->>Controller: Fail-fast on error
Console-->>K8s: Plugin visible in OpenShift console UI
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
lpiwowar
reviewed
May 11, 2026
Contributor
lpiwowar
left a comment
lpiwowar
left a comment
There was a problem hiding this comment.
Overall looks good to me:). I think it is great we will have the logic for automatic build + deployment of the operator for the KUTTL tests. I wrote anything that crossed my mind + I did not test this change manually.
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
|
CodeRabbit does not run by default on the |
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/controller/console_deployment.go (1)
112-112: ⚡ Quick winVerify PullAlways is intentional for production use.
The console plugin deployment uses
ImagePullPolicy: PullAlways, which will pull the image on every pod restart. This can cause unnecessary registry load and slower startup times in production environments where images are stable.Consider using
PullIfNotPresentor making the policy configurable unless there's a specific requirement for always pulling the latest image.🤖 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/controller/console_deployment.go` at line 112, The deployment currently sets ImagePullPolicy to corev1.PullAlways in the console plugin spec; change this to a safer default (e.g., corev1.PullIfNotPresent) or make the policy configurable so production doesn’t always re-pull images. Locate the pod spec building code that sets ImagePullPolicy (reference: ImagePullPolicy and corev1.PullAlways) and replace the constant with a configurable value (read from a config struct, flag, or env var) or switch to corev1.PullIfNotPresent as the default.internal/controller/console_reconciler.go (1)
154-155: 💤 Low valueVerify version comparison logic for OCP 5+.
The comment states "OCP < 4 can't run this operator, so major != 4 effectively means OCP 5+". However, the condition
major != 4 || minor >= 19will return true for any major version other than 4 (including hypothetical versions less than 4, like 3.x or 2.x).While the assumption that OCP < 4 won't run this operator is reasonable, the logic should be more explicit for clarity:
♻️ Proposed clarification
major, err1 := strconv.Atoi(parts[0]) minor, err2 := strconv.Atoi(parts[1]) - // OCP < 4 can't run this operator, so major != 4 effectively means OCP 5+ - if err1 == nil && err2 == nil && (major != 4 || minor >= 19) { + // Use PF6 for OCP 4.19+ or OCP 5+ + if err1 == nil && err2 == nil && (major > 4 || (major == 4 && minor >= 19)) { return apiv1beta1.OpenStackLightspeedDefaultValues.ConsoleImageURL }🤖 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/controller/console_reconciler.go` around lines 154 - 155, The version check in console_reconciler.go using (major != 4 || minor >= 19) is too broad and matches non-4 older majors; replace that expression with an explicit test for OCP 4.19+ or any 5+ release: use ((major == 4 && minor >= 19) || major >= 5) while keeping the surrounding err1/err2 nil checks (the line starting with if err1 == nil && err2 == nil && ...). This clarifies intent and ensures only OCP 4.19+ or major >=5 satisfy the condition.scripts/ocp-catalog-build.sh (1)
44-44: ⚡ Quick winUse single quotes in trap command.
The trap command should use single quotes to defer variable expansion until the trap executes. Although
WORKDIRis set once and doesn't change in this script, single quotes are the recommended practice for trap commands.🔧 Proposed fix
-trap "rm -rf ${WORKDIR}" EXIT +trap 'rm -rf ${WORKDIR}' EXIT🤖 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 `@scripts/ocp-catalog-build.sh` at line 44, Replace the double-quoted trap invocation so the cleanup command is wrapped in single quotes to defer expansion of WORKDIR until the trap runs; update the trap invocation that currently reads trap "rm -rf ${WORKDIR}" to use single quotes and reference the WORKDIR variable so the removal happens at EXIT with delayed expansion.
🤖 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.
Inline comments:
In
`@test/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yaml`:
- Around line 151-164: Update the assertion for the Deployment named
lightspeed-console-plugin (and its initContainer rewrite-locales) to enforce
hardened securityContext values: assert pod.spec.securityContext.runAsNonRoot:
true, and for each container/initContainer assert securityContext.runAsNonRoot:
true, securityContext.allowPrivilegeEscalation: false,
securityContext.seccompProfile.type: RuntimeDefault,
securityContext.capabilities.drop: [ALL], and
securityContext.readOnlyRootFilesystem: true where applicable; ensure these
expectations are applied to both the initContainers (rewrite-locales) and the
main containers in the Deployment spec within
assert-openstack-lightspeed-instance.yaml so regressions to default/root-capable
settings are caught.
---
Nitpick comments:
In `@internal/controller/console_deployment.go`:
- Line 112: The deployment currently sets ImagePullPolicy to corev1.PullAlways
in the console plugin spec; change this to a safer default (e.g.,
corev1.PullIfNotPresent) or make the policy configurable so production doesn’t
always re-pull images. Locate the pod spec building code that sets
ImagePullPolicy (reference: ImagePullPolicy and corev1.PullAlways) and replace
the constant with a configurable value (read from a config struct, flag, or env
var) or switch to corev1.PullIfNotPresent as the default.
In `@internal/controller/console_reconciler.go`:
- Around line 154-155: The version check in console_reconciler.go using (major
!= 4 || minor >= 19) is too broad and matches non-4 older majors; replace that
expression with an explicit test for OCP 4.19+ or any 5+ release: use ((major ==
4 && minor >= 19) || major >= 5) while keeping the surrounding err1/err2 nil
checks (the line starting with if err1 == nil && err2 == nil && ...). This
clarifies intent and ensures only OCP 4.19+ or major >=5 satisfy the condition.
In `@scripts/ocp-catalog-build.sh`:
- Line 44: Replace the double-quoted trap invocation so the cleanup command is
wrapped in single quotes to defer expansion of WORKDIR until the trap runs;
update the trap invocation that currently reads trap "rm -rf ${WORKDIR}" to use
single quotes and reference the WORKDIR variable so the removal happens at EXIT
with delayed expansion.
🪄 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: 359e6143-8234-45e4-9fb6-6df39dfb23db
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (20)
Makefileapi/v1beta1/openstacklightspeed_types.gobundle/manifests/openstack-lightspeed-operator.clusterserviceversion.yamlcmd/main.goconfig/rbac/role.yamlgo.modinternal/controller/console_deployment.gointernal/controller/console_reconciler.gointernal/controller/console_reconciler_test.gointernal/controller/constants.gointernal/controller/errors.gointernal/controller/lcore_reconciler.gointernal/controller/openstacklightspeed_controller.gointernal/controller/openstacklightspeed_controller_test.gointernal/controller/suite_test.goscripts/ocp-catalog-build.shscripts/ocp-registry-push.shtest/kuttl/common/openstack-lightspeed-instance/assert-openstack-lightspeed-instance.yamltest/kuttl/common/openstack-lightspeed-instance/errors-openstack-lightspeed-instance.yamltest/kuttl/tests/update-openstacklightspeed/10-assert-openstacklightspeed-update.yaml
Current code fails on `OpenStackLightspeed Controller When reconciling a resource [BeforeEach] should successfully reconcile the resource` with failure `OpenStackLightspeed.lightspeed.openstack.org "test-resource" is invalid: spec.llmEndpointType: Unsupported value: "": supported values: "azure_openai", "bam", "openai", "watsonx", "rhoai_vllm", "rhelai_vllm", "fake_provider"`
lightspeed-stack needs to read the cluster version (via the config.openshift.io API) when an admin user interacts with it. Grant list and get on clusterversions to the SAR ClusterRole. Jira: OSPRH-29574 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Allows running kuttl tests using the OpenShift internal image registry instead of an external registry like quay.io. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deploy the OpenShift Lightspeed console plugin as part of the operator reconciliation loop. Adds the Deployment, Service, ConsolePlugin CR, nginx ConfigMap, NetworkPolicy, and ServiceAccount using the existing CreateOrPatch pattern. Image used for the console depends on the OCP version. Versions prior to 4.19 use one with PatterFly 5 and for later one with PatternFly 6. Jira: OSPRH-29746 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
To avoid having the console saying it's OpenShift Lightspeed we will be
doing a replacement of the OpenShift word in the internationalization
file.
This mechanism is a bit hacky, but it's a patch that's easy to revert if
we decide to.
Add an init container that rewrites "OpenShift" references to
"OpenStack" in the locales JSON file using awk.
The awk script splits each line at the key-value boundary and only
replaces in values, preserving i18n lookup keys.
Using this mechanism should not be a problem for non English locales (at
least according to Claude), since the i18n fallback mechanism works like
this:
1. This plugin doesn't initialize i18next itself. It relies on the
parent OpenShift Console (via ConsoleRemotePlugin in
webpack.config.ts) to set up i18next globally. The plugin just uses
useTranslation('plugin__lightspeed-console-plugin') to access its
namespace.
2. When a non-English locale is requested, i18next looks for
locales/<locale>/plugin__lightspeed-console-plugin.json. Since only
locales/en/ exists, i18next falls back to English — this is standard
i18next behavior where missing locale files cause a fallback to the
default language.
3. Additionally, the translation keys in
locales/en/plugin__lightspeed-console-plugin.json are the English
strings themselves (e.g., "Close": "Close"). So even if the fallback
mechanism somehow failed entirely, i18next's last resort is to
display the key as-is — which in this case is already readable
English text.
As long as OLS console doesn't have a bug in their code that doesn't use
the internationalization with a string and that string has OpenShift in
the wording we should be fine.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
|
/retest |
lpiwowar
reviewed
May 26, 2026
Contributor
lpiwowar
left a comment
lpiwowar
left a comment
There was a problem hiding this comment.
LGTM! Thank you! 👍
- Waiting for the KUTTL tests to pass.
- README.md would be nice (but not blocking in the hindsight).
In this patch we update the README.md file to clarify the kuttl related targets, how they are connected and how they can be used.
Contributor
|
/lgtm |
lpiwowar
approved these changes
May 27, 2026
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Akrog, lpiwowar 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 |
openshift-merge-bot
Bot
merged commit 0a68dfd
into
openstack-lightspeed:lcore-migration
7 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primary objective of this PR is adding the OLS Console deployment to the operator making the Lightspeed console display OpenStack Lightspeed instead of OpenShift Lightspeed.
As part of this work we also:
kuttl-test-ocp) so it's easy to run kuttl tests on an OCP cluster without uploading to a remote repository (using OCP's internal repository).Jira: OSPRH-29574
Jira: OSPRH-29746
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests