OLS-3236: Reconcile agentic console plugin in lightspeed-operator#1758
OLS-3236: Reconcile agentic console plugin in lightspeed-operator#1758blublinsky wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@blublinsky: This pull request references OLS-3236 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. |
📝 WalkthroughWalkthroughThe PR adds shared console-plugin resource helpers, introduces agentic console resource generation and reconciliation wiring, extends image and CRD plumbing, and updates docs and local-development behavior for the new flow. ChangesConsole plugin and agentic console rollout
Sequence Diagram(s)sequenceDiagram
participant OLSConfigReconciler
participant agenticconsole
participant utils
OLSConfigReconciler->>agenticconsole: ReconcileAgenticConsoleUIResources()
agenticconsole->>utils: RunReconcileTasks(...)
OLSConfigReconciler->>agenticconsole: ReconcileAgenticConsoleUIDeploymentAndPlugin()
agenticconsole->>utils: ReconcileConsolePluginDeployment(...)
agenticconsole->>utils: WaitForConsolePluginTLSSecret(...)
agenticconsole->>utils: ActivateConsolePlugin(...)
OLSConfigReconciler->>agenticconsole: RemoveAgenticConsole()
agenticconsole->>utils: RunDeleteTasks(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.ai/spec/README.md:
- Line 52: The cross-reference entry for console-ui.md and agentic-console-ui.md
is too narrow because those docs now cover ConsolePlugin lifecycle and cleanup,
not only deployment generation. Update the README cross-reference table near the
existing deployment-generation mapping by splitting the console docs into a
separate entry or adding the reconciliation guide alongside
how/deployment-generation.md, and verify the referenced architecture and command
descriptions against AGENTS.md, ARCHITECTURE.md, and CONTRIBUTING.md.
In `@cmd/main.go`:
- Line 188: The `--agentic-console-image` flag is defined in `main` but not
wired into the operator deployment args, so add it to the container argument
lists used by the OLM bundle and the canonical manager manifest. Update the args
block that currently includes `--console-image`, `--service-image`, and
`--postgres-image` so it also passes `--agentic-console-image`, keeping the
ordering and style consistent with the existing image override flags.
In `@internal/controller/utils/console_plugin_reconciler.go`:
- Around line 35-53: The continueOnError path in RunReconcileTasks is
double-logging task failures and returning only a stringified task list, which
drops the original error chain. Keep a single log per failed task, collect the
task errors in a slice, and return one aggregated error using errors.Join while
preserving each task name via wrapping; also alias the standard errors import
(for example, stderrors) so it does not conflict with
k8s.io/apimachinery/pkg/api/errors.
In `@internal/controller/utils/utils.go`:
- Around line 1000-1002: The GenerateConsolePluginDeployment helper currently
dereferences opts.Resources without a defensive check, which can lead to a nil
pointer panic if the utility is called with missing resource settings. Add a nil
guard in GenerateConsolePluginDeployment before using opts.Resources, and fall
back to DefaultConsolePluginResourceRequirements() when it is nil so the
deployment setup remains safe even for direct callers.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d9b9e113-e1d2-4736-9ab9-24ace63131c5
⛔ Files ignored due to path filters (2)
api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated.deepcopy.goconfig/crd/bases/ols.openshift.io_olsconfigs.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (33)
.ai/spec/README.md.ai/spec/how/project-structure.md.ai/spec/how/reconciliation.md.ai/spec/what/agentic-console-ui.md.ai/spec/what/console-ui.mdAGENTS.mdARCHITECTURE.mdapi/v1alpha1/olsconfig_types.gocmd/main.gointernal/controller/agenticconsole/assets.gointernal/controller/agenticconsole/assets_test.gointernal/controller/agenticconsole/deployment.gointernal/controller/agenticconsole/reconciler.gointernal/controller/agenticconsole/reconciler_test.gointernal/controller/agenticconsole/suite_test.gointernal/controller/appserver/reconciler.gointernal/controller/console/assets.gointernal/controller/console/assets_test.gointernal/controller/console/deployment.gointernal/controller/console/reconciler.gointernal/controller/console/reconciler_test.gointernal/controller/olsconfig_controller.gointernal/controller/olsconfig_helpers.gointernal/controller/reconciler/interface.gointernal/controller/utils/console_plugin_reconciler.gointernal/controller/utils/console_plugin_test.gointernal/controller/utils/constants.gointernal/controller/utils/resource_defaults_test.gointernal/controller/utils/suite_test.gointernal/controller/utils/testing.gointernal/controller/utils/types.gointernal/controller/utils/utils.gointernal/controller/watchers/watchers.go
💤 Files with no reviewable changes (1)
- internal/controller/console/assets_test.go
blublinsky
force-pushed
the
agentic-bundle
branch
from
1d9cced to
7219225
Compare
|
@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. |
vimalk78
left a comment
vimalk78
left a comment
There was a problem hiding this comment.
AI Code Review — OLS-3349 (Score: 87/100)
Mode: jira | Verdict: needs_review | Round: 1
Acceptance Criteria: 8/10 PASS, 0 FAIL, 2 NEEDS_REVIEW
| # | Criterion | Status | Evidence |
|---|---|---|---|
| 1 | spec.ols.deployment.agenticConsole field in OLSConfig CR |
PASS | AgenticConsoleContainer Config added to DeploymentConfig in olsconfig_types.go, CRD YAML updated |
| 2 | --agentic-console-image flag overrides default image |
PASS | Flag in cmd/main.go, plumbed through overrideImages() and OLSConfigReconcilerOptions |
| 3 | Agentic console Deployment created | PASS | GenerateAgenticConsoleUIDeployment() uses shared GenerateConsolePluginDeployment(); tested |
| 4 | Service on port 9443 with serving cert annotation | PASS | Service in assets.go with serving-cert-secret-name annotation; tested |
| 5 | ConsolePlugin CR registered and activated | PASS | GenerateAgenticConsoleUIPlugin() + activateAgenticConsoleUI(); tested |
| 6 | AgenticConsolePluginReady status condition |
PASS | Constant in utils/types.go, registered in olsconfig_controller.go |
| 7 | OverallStatus requires AgenticConsolePluginReady=True | PASS | Adding the step with ConditionType automatically includes it in aggregation |
| 8 | Finalizer removes ConsolePlugin on deletion | PASS | RemoveAgenticConsole() called in finalizeOLSConfig(); tested |
| 9 | Existing agentic-console resources adopted on upgrade | NEEDS_REVIEW | Resource names match old agentic-operator names. Get-then-create-or-update pattern should adopt. Owner reference transfer and cluster-scoped ConsolePlugin adoption require runtime verification. |
| 10 | CSV contains --agentic-console-image and relatedImages entry |
NEEDS_REVIEW | Explicitly deferred to follow-up PR per PR description and spec. |
Code Quality Issues
1. E2E tests not included (should-fix)
- Jira scope section mentions E2E tests (OLSConfig CR → agentic console pod → ConsolePlugin registered → condition True → deletion cleanup). No E2E tests in this PR. May be planned for follow-up.
2. Missing proxy/OCP_VERSION env vars (nice-to-have)
- Chat console sets
HTTP_PROXY/HTTPS_PROXY/NO_PROXYandOCP_VERSION. Agentic console sets none. Verified this is correct — the agentic console image is a plain nginx static server with no multi-version logic and no outbound connections. Worth a brief code comment explaining the difference.
3. Bundled unrelated change (nice-to-have)
LOCAL_DEV_MODEguard added toappserver/reconciler.go:reconcileMetricsReaderSecret. Small and low-risk but unrelated to agentic console work.
Summary
Well-structured PR that follows established patterns. The shared console plugin helpers in utils/console_plugin_reconciler.go are a clean extraction that reduces duplication. Unit tests are comprehensive. All CI checks pass.
Automated review by /ols-review · OLS-3349
|
Re: E2E tests not included E2E tests are planned as a follow-up once the agentic console image is available in CI and the full integration path (OLSConfig CR → agentic console pod → ConsolePlugin registered → condition True → deletion cleanup) can be exercised on a real cluster. The unit tests in this PR cover all reconciler logic, resource generation, and lifecycle management. |
|
Re: Missing proxy/OCP_VERSION env vars Not applicable. The chat console needs |
|
Re: Bundled unrelated change (LOCAL_DEV_MODE guard) Acknowledged — this was hit while developing the agentic console locally. |
3 participants
Description
Summary
Migrate agentic console plugin reconciliation from lightspeed-agentic-operator into lightspeed-operator (OLS-3236, operator scope).
36 files — 9 added, 27 modified (~3.1k / ~710 lines).
Operator reconciliation
Add internal/controller/agenticconsole/ — Phase 1 (ConfigMap, NetworkPolicy, ServiceAccount) and Phase 2 (Deployment, Service, TLS wait, ConsolePlugin, Console activation), plus RemoveAgenticConsole() finalizer cleanup.
Wire into OLSConfigReconciler: Phase 1/2 steps, AgenticConsolePluginReady, TLS secret watcher, deployment restart mapping.
Add spec.ols.deployment.agenticConsole (Config) to OLSConfig; regenerate config/crd/bases/ols.openshift.io_olsconfigs.yaml.
Add --agentic-console-image in cmd/main.go (default: Konflux lightspeed-agentic-console:main).
Reuse existing resource names for upgrade adoption (lightspeed-agentic-console-plugin, cert lightspeed-agentic-console-plugin-cert, port 9443, container console).
No ConsolePlugin proxy to app-server (unlike chat console).
Shared console plugin refactor
Extract internal/controller/utils/console_plugin_reconciler.go (+ tests).
Refactor internal/controller/console/ to use shared helpers (~400 lines removed).
Local dev fix
Skip reconcileMetricsReaderSecret when LOCAL_DEV_MODE=true (make run) to stop metrics-reader-token reconcile loop. Add agentic console name to make run
Documentation
AGENTS.md, ARCHITECTURE.md
.ai/spec/what/agentic-console-ui.md (new), console-ui.md, README.md, how/project-structure.md, how/reconciliation.md
Intentionally out of scope (follow-up)
OLM bundle/CSV sync (bundle/manifests/, related_images.json, CSV --agentic-console-image)
Type of change
Related Tickets & Documents
https://redhat.atlassian.net/browse/OLS-3349
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation