OLS-3697 Add RHOKP standalone operand spec and update related specs - #1876
OLS-3697 Add RHOKP standalone operand spec and update related specs#1876blublinsky wants to merge 1 commit into
Conversation
New rhokp.md operand spec mirroring ocp-mcp pattern. Updates to app-server, crd-api, reconciliation, tls, and ocpmcp specs for the RHOKP standalone HTTPS cutover and ServiceMonitor additions. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe specifications replace the RHOKP app-server sidecar with a standalone HTTPS Deployment and Service, update CRD and app-server configuration, add conditional reconciliation and monitoring behavior, and define CA trust, certificate rotation, sandbox handoff, and cleanup semantics. ChangesRHOKP standalone HTTPS cutover
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/what/app-server.md:
- Line 23: Update the app-server restart-detection contract near the solr_hybrid
RHOKP endpoint description to include RHOKP CA-content changes and RHOKP
certificate rotation as restart triggers, matching the terminology and behavior
documented in rhokp.md and tls.md while retaining the existing MCP CA trigger.
- Line 12: Update the app-server deployment composition described in this
document to remove RHOKP from the sidecar count and sidecar list. Keep RHOKP
represented only as the standalone operator-managed Deployment referenced by the
OKP configuration, so the app-server contract no longer implies local RHOKP
deployment.
In @.ai/spec/what/crd-api.md:
- Line 145: Update the replica constraint in the CRD contract to explicitly
include RHOKP alongside console, database, alerts adapter, agentic console, and
otel collector. Keep the existing operator-forced replica value of 1 and ensure
the earlier RHOKP planned note remains consistent.
In @.ai/spec/what/rhokp.md:
- Line 52: Update the NetworkPolicy specifications in .ai/spec/what/rhokp.md at
lines 52-52 and .ai/spec/what/ocpmcp.md at lines 54-55 to permit ingress from
the openshift-monitoring namespace for the Prometheus ServiceMonitor scraper;
cover the RHOKP metrics endpoint and the MCP /metrics endpoint while preserving
existing operator-namespace access.
- Around line 25-26: Define one explicit RHOKP readiness contract, choosing
either a dedicated RHOKPReady condition or dependency of ApiReady and overall
readiness on RHOKP health; update .ai/spec/what/rhokp.md lines 25-26 to replace
the unconditional no-status-condition rule, and update
.ai/spec/what/reconciliation.md line 31 to apply the identical Phase 2/status
behavior.
- Line 55: Align the OKP handoff statements at the referenced rule and the
agentic-handoff planned-work entry: either mark the ConfigMap and CA injection
behavior as planned, or explicitly state that it is outside the current
implementation scope. Ensure both sections consistently describe the same
implementation status.
🪄 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: Pro Plus
Run ID: f3b7bc4f-f406-48ca-9803-7c84996e8bb3
📒 Files selected for processing (6)
.ai/spec/what/app-server.md.ai/spec/what/crd-api.md.ai/spec/what/ocpmcp.md.ai/spec/what/reconciliation.md.ai/spec/what/rhokp.md.ai/spec/what/tls.md
| 3. The data collector sidecar (lightspeed-to-dataverse-exporter) is added when data collection is enabled AND the telemetry pull secret exists in the openshift-config namespace with a cloud.openshift.com auth entry. | ||
| 4. The OpenShift MCP server runs as a standalone HTTPS Deployment/Service (`ocpmcp` package) when `spec.ols.introspectionEnabled` is true. The app-server connects via `https://openshift-mcp-server.<ns>.svc:8443/mcp` and trusts `openshift-mcp-server-ca` (`service-ca.crt`). See `ocpmcp.md`. | ||
| 5. OKP (Offline Knowledge Portal) / Solr hybrid RAG is operator-managed (no CR toggle besides `byokRAGOnly`). When OKP is enabled, the RHOKP sidecar serves Solr HTTP on localhost:9080 for the `search_openshift_documentation` tool path. It requires ~75 GiB ephemeral storage. OKP is on by default; set `spec.ols.byokRAGOnly` to true to skip the RHOKP sidecar, `solr_hybrid` config, and OCP documentation retrieval via Solr. | ||
| 5. [PLANNED: OLS-3697] OKP (Offline Knowledge Portal) / Solr hybrid RAG is operator-managed (no CR toggle besides `byokRAGOnly`). When OKP is enabled, the RHOKP standalone Deployment serves Solr via HTTPS at `https://lightspeed-rhokp.<ns>.svc:8443`. The app-server connects as a client, trusting the RHOKP service-ca cert via `extra_ca`. OKP is on by default; set `spec.ols.byokRAGOnly` to true to skip the RHOKP standalone operand, `solr_hybrid` config, and OCP documentation retrieval via Solr. See `rhokp.md`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the stale RHOKP sidecar description.
The deployment composition still says the app-server has a RHOKP sidecar, contradicting the standalone Deployment described here and in rhokp.md. Change the sidecar count/list so the app-server contract cannot be interpreted as deploying RHOKP locally.
🤖 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 @.ai/spec/what/app-server.md at line 12, Update the app-server deployment
composition described in this document to remove RHOKP from the sidecar count
and sidecar list. Keep RHOKP represented only as the standalone operator-managed
Deployment referenced by the OKP configuration, so the app-server contract no
longer implies local RHOKP deployment.
| 12. If `spec.ols.querySystemPrompt` is set, the custom prompt is written as a second key in the config ConfigMap and referenced by file path in the config. | ||
| 13. BYOK reference content indexes from `spec.ols.rag` are written to `reference_content.indexes` when present. OCP product documentation is served exclusively via `solr_hybrid` (OKP); the operator does not emit a built-in OCP FAISS index. | ||
| 14. Unless `byokRAGOnly` is true, the operator generates a `solr_hybrid` config section in `olsconfig.yaml` pointing to `http://localhost:9080` with default hybrid retrieval tuning parameters. | ||
| 14. [PLANNED: OLS-3697] Unless `byokRAGOnly` is true, the operator generates a `solr_hybrid` config section in `olsconfig.yaml` pointing to `https://lightspeed-rhokp.<ns>.svc:8443` with default hybrid retrieval tuning parameters. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Document RHOKP CA changes in app-server restart detection.
The new RHOKP HTTPS endpoint requires the RHOKP CA, but the app-server change-detection contract only names the MCP CA. Align it with rhokp.md and tls.md by documenting RHOKP CA-content changes and certificate rotation as restart triggers.
🤖 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 @.ai/spec/what/app-server.md at line 23, Update the app-server
restart-detection contract near the solr_hybrid RHOKP endpoint description to
include RHOKP CA-content changes and RHOKP certificate rotation as restart
triggers, matching the terminology and behavior documented in rhokp.md and
tls.md while retaining the existing MCP CA trigger.
| `otelCollector` | `otelCollector` | `Config` | OTEL Collector container ([OLS-3510](https://redhat.atlassian.net/browse/OLS-3510)). Replicas forced to 1 | ||
|
|
||
| 20. Replicas are user-configurable for the API container (`spec.ols.deployment.api.replicas`). For console, database, alerts adapter, agentic console, and otel collector, the operator always overrides replicas to 1 regardless of spec value. | ||
| 20. Replicas are user-configurable for the API container (`spec.ols.deployment.api.replicas`). For console, database, alerts adapter, agentic console, otel collector, and RHOKP [PLANNED: OLS-3697], the operator always overrides replicas to 1 regardless of spec value. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Update the replica constraint to include RHOKP.
This line makes RHOKP replicas operator-forced to 1, but the later replica constraint still lists only console, database, alerts adapter, agentic console, and OTEL collector. Update that constraint so the CRD contract is unambiguous.
🤖 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 @.ai/spec/what/crd-api.md at line 145, Update the replica constraint in the
CRD contract to explicitly include RHOKP alongside console, database, alerts
adapter, agentic console, and otel collector. Keep the existing operator-forced
replica value of 1 and ensure the earlier RHOKP planned note remains consistent.
| 1. When `spec.ols.byokRAGOnly` is false (or absent), Phase 1 and Phase 2 reconcile the standalone RHOKP operand. | ||
| 2. When true, Phase 1 calls `rhokp.Remove()`; Phase 2 skips deployment reconciliation. No status condition is emitted (RHOKP readiness is implicit in `ApiReady` — the app-server cannot become ready if its configured Solr backend is unreachable). |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Define RHOKP readiness propagation once.
RHOKP is deployment-health checked but has no status condition, so a failed operand can be invisible to OverallStatus. Either add RHOKPReady or explicitly make ApiReady and overall readiness depend on RHOKP health.
.ai/spec/what/rhokp.md#L25-L26: replace the unconditional “no status condition” rule with the chosen readiness contract..ai/spec/what/reconciliation.md#L31-L31: include the same RHOKP readiness behavior in the Phase 2/status rule.
📍 Affects 2 files
.ai/spec/what/rhokp.md#L25-L26(this comment).ai/spec/what/reconciliation.md#L31-L31
🤖 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 @.ai/spec/what/rhokp.md around lines 25 - 26, Define one explicit RHOKP
readiness contract, choosing either a dedicated RHOKPReady condition or
dependency of ApiReady and overall readiness on RHOKP health; update
.ai/spec/what/rhokp.md lines 25-26 to replace the unconditional
no-status-condition rule, and update .ai/spec/what/reconciliation.md line 31 to
apply the identical Phase 2/status behavior.
| 17. App-server Deployment tracks RHOKP CA content hash (`ols.openshift.io/rhokp-ca-configmap-hash`) only while OKP is enabled. | ||
|
|
||
| ### Monitoring | ||
| 18. ServiceMonitor `lightspeed-rhokp-monitor` — scrapes RHOKP metrics via HTTPS on port 8443, path `/solr/admin/metrics` (Solr built-in Prometheus metrics reporter). Uses service-ca TLS for the scrape connection. Skipped if Prometheus Operator CRDs are not installed. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Permit the Prometheus scraper through both operand NetworkPolicies.
The ServiceMonitors target workloads protected by policies that allow only operator-namespace ingress, while OpenShift monitoring runs in openshift-monitoring.
.ai/spec/what/rhokp.md#L52-L52: update the RHOKP NetworkPolicy to allow the Prometheus scraper..ai/spec/what/ocpmcp.md#L54-L55: update the MCP NetworkPolicy similarly for/metrics.
📍 Affects 2 files
.ai/spec/what/rhokp.md#L52-L52(this comment).ai/spec/what/ocpmcp.md#L54-L55
🤖 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 @.ai/spec/what/rhokp.md at line 52, Update the NetworkPolicy specifications
in .ai/spec/what/rhokp.md at lines 52-52 and .ai/spec/what/ocpmcp.md at lines
54-55 to permit ingress from the openshift-monitoring namespace for the
Prometheus ServiceMonitor scraper; cover the RHOKP metrics endpoint and the MCP
/metrics endpoint while preserving existing operator-namespace access.
| 18. ServiceMonitor `lightspeed-rhokp-monitor` — scrapes RHOKP metrics via HTTPS on port 8443, path `/solr/admin/metrics` (Solr built-in Prometheus metrics reporter). Uses service-ca TLS for the scrape connection. Skipped if Prometheus Operator CRDs are not installed. | ||
|
|
||
| ### Agentic Handoff | ||
| 19. When OKP is enabled, the inter-operator handoff ConfigMap (`lightspeed-sandbox-config`) includes `rhokp-endpoint` key with the HTTPS URL and the RHOKP CA cert volume + mount in the base `sandbox-pod-spec` PodSpec. When `byokRAGOnly` is true, both are absent. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align handoff status with the planned-work section.
This section promises the handoff ConfigMap and CA injection as behavioral rules, while line 82 says agentic handoff remains planned. Mark the behavior consistently as planned or make the implementation scope explicit.
Also applies to: 82-82
🤖 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 @.ai/spec/what/rhokp.md at line 55, Align the OKP handoff statements at the
referenced rule and the agentic-handoff planned-work entry: either mark the
ConfigMap and CA injection behavior as planned, or explicitly state that it is
outside the current implementation scope. Ensure both sections consistently
describe the same implementation status.
red-hat-konflux
Bot
added
approved
|
label added: |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: 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 |
|
@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. |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. 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. |
1 similar comment
|
PR needs rebase. 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. |
1 participant
Summary
rhokp.mdoperand spec mirroring the ocp-mcp standalone HTTPS patternapp-server.md: sidecar → standalone references, HTTPS URL, CA wiring, handoff ConfigMap extensioncrd-api.md:spec.ols.deployment.rhokptype change fromContainerConfigtoConfigreconciliation.md: new Phase 1/2 RHOKP component grouptls.md: RHOKP service-ca cert, CA mount, cert rotation watcherocpmcp.md: planned ServiceMonitor for MCP/metricsTest plan
Made with Cursor
Summary by CodeRabbit