OLS-3205: Remove multi-image console selection and use single image -operator#1705
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@blublinsky: This pull request references OLS-3205 which is a valid jira issue. 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (3)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR simplifies console plugin image selection by removing PatternFly-version-specific logic. The operator now deploys a single console image via ChangesSimplify console image selection model
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
|
where we select the console image to use in this PR? |
According to the ticket its up to UI to do this. We tell it which OS version it is running on, and it does the magic https://redhat.atlassian.net/browse/OLS-3210 |
|
/override "ci/prow/bundle-e2e-4-21" |
|
@blublinsky: Overrode contexts on behalf of blublinsky: ci/prow/bundle-e2e-4-21 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. |
blublinsky
force-pushed
the
operator-console-multi-image
branch
from
c6b9ad2 to
63b3c2e
Compare
|
/lgtm |
blublinsky
force-pushed
the
operator-console-multi-image
branch
from
63b3c2e to
c5a7b2a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cmd/main.go (2)
181-186:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftBlock merge until the shipped manifests stop passing the removed console-image flags.
This binary no longer accepts
--console-image-pf5/--console-image-4-19, and the PR notes say the current CSV still passes obsolete args. That leaves the shipped operator unable to start untilconfig/manager/andbundle/manifests/are updated in lockstep.As per coding guidelines,
cmd/**/*.go: "New flags should be reflected in deployment/CSV args underconfig/manager/andbundle/manifests/when they affect the shipped operator."🤖 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 `@cmd/main.go` around lines 181 - 186, The PR removes old flags (console-image-pf5 / console-image-4-19) and introduces the new flag bound to variable consoleImage via flag.StringVar("--console-image", ...); before merging, update the shipped operator manifests so the deployment/CSV no longer pass the removed flags and instead pass the new --console-image arg (or omit if not needed). Specifically, update the operator args in config/manager/ and bundle/manifests/ (CSV/deployment templates) to match the flag names used in cmd/main.go (consoleImage / --console-image) and remove any references to console-image-pf5 and console-image-4-19 so the running operator’s command-line matches the code.Source: Coding guidelines
301-305:⚠️ Potential issue | 🟠 MajorFix missing OpenShift version env wiring in console pod (
OCP_VERSION)
internal/controller/console/deployment.gosets the console containerEnvtoutils.GetProxyEnvVars()only, with no additional env var appended for OpenShift major/minor.internal/controller/utils/utils.go:GetProxyEnvVars()only forwards proxy env vars (HTTPS_PROXY/HTTP_PROXY/NO_PROXYvariants) and there are no repo hits forOCP_VERSION(or similar OpenShift version env var names).cmd/main.godetects OpenShift major/minor, but that information is not injected into the console container environment anywhere in the console deployment code.Inject
OCP_VERSION=<major>.<minor>into the console container env (if the documented fallback requires it), or update the docs/selection logic to match the actual mechanism (e.g., versioned image selection instead of env fallback).🤖 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 `@cmd/main.go` around lines 301 - 305, cmd/main.go computes OpenShift major/minor but never wires it into the console Pod env; update the controller wiring so the computed ocpVersion (fmt.Sprintf("%d.%d", major, minor)) is passed into the console controller initialization and used when building the container Env in internal/controller/console/deployment.go (append an EnvVar{Name:"OCP_VERSION", Value: ocpVersion} to the slice returned by utils.GetProxyEnvVars()); keep utils.GetProxyEnvVars() as-is and ensure the controller constructor (the function you call from cmd/main.go to create/register the console reconciler) accepts the ocpVersion string and forwards it to the deployment builder.
🤖 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.
Outside diff comments:
In `@cmd/main.go`:
- Around line 181-186: The PR removes old flags (console-image-pf5 /
console-image-4-19) and introduces the new flag bound to variable consoleImage
via flag.StringVar("--console-image", ...); before merging, update the shipped
operator manifests so the deployment/CSV no longer pass the removed flags and
instead pass the new --console-image arg (or omit if not needed). Specifically,
update the operator args in config/manager/ and bundle/manifests/
(CSV/deployment templates) to match the flag names used in cmd/main.go
(consoleImage / --console-image) and remove any references to console-image-pf5
and console-image-4-19 so the running operator’s command-line matches the code.
- Around line 301-305: cmd/main.go computes OpenShift major/minor but never
wires it into the console Pod env; update the controller wiring so the computed
ocpVersion (fmt.Sprintf("%d.%d", major, minor)) is passed into the console
controller initialization and used when building the container Env in
internal/controller/console/deployment.go (append an EnvVar{Name:"OCP_VERSION",
Value: ocpVersion} to the slice returned by utils.GetProxyEnvVars()); keep
utils.GetProxyEnvVars() as-is and ensure the controller constructor (the
function you call from cmd/main.go to create/register the console reconciler)
accepts the ocpVersion string and forwards it to the deployment builder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cd2558dd-6325-4b8f-ba0f-713802c0293c
📒 Files selected for processing (6)
.ai/spec/what/console-ui.md.ai/spec/what/system-overview.mdcmd/main.goconfig/default/deployment-patch.yamlhack/image_placeholders.jsoninternal/controller/utils/constants.go
💤 Files with no reviewable changes (3)
- internal/controller/utils/constants.go
- hack/image_placeholders.json
- config/default/deployment-patch.yaml
|
/override "ci/prow/bundle-e2e-4-21" |
|
@blublinsky: Overrode contexts on behalf of blublinsky: ci/prow/bundle-e2e-4-21 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xrajesh 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-ci
Bot
added
the
approved
|
/lgtm |
|
/retest |
|
/override "Red Hat Konflux / lightspeed-agentic-operator / lightspeed-operator"(https://github.com/openshift/lightspeed-operator/pull/1705/checks?check_run_id=81005473880) |
|
/override "Red Hat Konflux / lightspeed-agentic-operator / lightspeed-operator" |
|
@blublinsky: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
|
@blublinsky: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
|
/override "Red Hat Konflux / lightspeed-agentic-operator / lightspeed-operator" |
|
@blublinsky: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
|
/override "Red Hat Konflux / lightspeed-agentic-operator-e2e-tests / lightspeed-operator" |
|
@blublinsky: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
blublinsky
force-pushed
the
operator-console-multi-image
branch
from
c5a7b2a to
fac0a75
Compare
|
/override ci/prow/bundle-e2e-4-21 |
|
@blublinsky: Overrode contexts on behalf of blublinsky: ci/prow/bundle-e2e-4-21 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. |
|
/lgtm |
|
/override ci/prow/bundle-e2e-4-21 |
|
@blublinsky: Overrode contexts on behalf of blublinsky: ci/prow/bundle-e2e-4-21 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. |
|
@blublinsky: all tests passed! 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. |
5 participants
Summary
Deploy a single console plugin image from the operator instead of selecting among three images based on OCP minor version. The unified console image handles UI variant selection at runtime (see companion OLS-3205 for
OCP_VERSIONenv).--console-image-pf5and--console-image-4-19CLI flagsoverrideImages()anddefaultImagesto a single console imageConsoleUIImagePF5DefaultandConsoleUIImage419DefaultconstantsOperator-only PR. A companion bundle PR is required before release to remove obsolete deployment args (
--console-image-pf5,--console-image-4-19) and pf5/419 entries fromrelated_images.json/ CSVrelatedImages. Installing this operator build with the current CSV will fail until that bundle PR lands.Type of change
Related Tickets & Documents
https://redhat.atlassian.net/browse/OLS-3210
Checklist before requesting a review
Testing
Summary by CodeRabbit
--console-imageflag (removing prior version/PF-specific console image options).