OLS-3189: add agentic workflow CRDs with sync tool#1695
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@raptorsun: This pull request references OLS-3189 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. |
|
Warning Review limit reached
More reviews will be available in 33 minutes and 2 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (25)
📝 WalkthroughWalkthroughAdds Makefile-driven synchronization of agentic operator CRDs and samples (via a new script and Make target), introduces multiple agentic CustomResourceDefinitions with detailed OpenAPI validation, adds sample manifests and kustomization entries, and updates the operator bundle/CSV to own the new resources. ChangesAgentic Operator Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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
🧹 Nitpick comments (2)
bundle/manifests/lightspeed-operator.clusterserviceversion.yaml (2)
98-120: 💤 Low valueConsider including all referenced agents in the examples.
The Proposal example references three agent names ("smart" for analysis, "default" for execution, "fast" for verification), but the Agent example (lines 7-20) only defines "default". While this technically works, showing all three agents in the alm-examples would provide a more complete and self-contained demonstration of the workflow.
📋 Suggested additional Agent examples
Consider adding these additional agents to the alm-examples array (after the existing Agent at line 20):
} }, + { + "apiVersion": "agentic.openshift.io/v1alpha1", + "kind": "Agent", + "metadata": { + "name": "smart" + }, + "spec": { + "llmProvider": { + "name": "vertex-ai" + }, + "maxTurns": 200, + "model": "claude-opus-4-6" + } + }, + { + "apiVersion": "agentic.openshift.io/v1alpha1", + "kind": "Agent", + "metadata": { + "name": "fast" + }, + "spec": { + "llmProvider": { + "name": "vertex-ai" + }, + "maxTurns": 100, + "model": "claude-sonnet-4-6" + } + }, { "apiVersion": "agentic.openshift.io/v1alpha1",🤖 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 `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml` around lines 98 - 120, The Proposal example uses three agent names (spec.analysis.agent "smart", spec.execution.agent "default", spec.verification.agent "fast") but the alm-examples only defines the "default" Agent; add Agent resources for the "smart" and "fast" agents to the alm-examples array so the CSV is self-contained—create two additional objects with "apiVersion": "agentic.openshift.io/v1alpha1", "kind": "Agent", metadata.name "smart" and "fast" and include plausible spec fields matching the existing "default" Agent example.
212-229: ⚖️ Poor tradeoffNew agentic CRDs lack UI metadata descriptors.
The nine newly-added agentic CRDs (Agent, AnalysisResult, ApprovalPolicy, EscalationResult, ExecutionResult, LLMProvider, Proposal, ProposalApproval, VerificationResult) are missing
displayName,description,specDescriptors, andstatusDescriptorsfields, unlike the OLSConfig CRD (lines 230-626). This means the OpenShift Console UI won't provide helpful field descriptions, display names, or input hints when users create or edit these resources.Since these CRDs are synchronized from the external lightspeed-agentic-operator repository (per PR description), this may be intentional. However, consider whether enhanced Console integration is desired for end users.
Also applies to: 627-635
🤖 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 `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml` around lines 212 - 229, The new agentic CRDs (kinds: Agent, AnalysisResult, ApprovalPolicy, EscalationResult, ExecutionResult, LLMProvider, Proposal, ProposalApproval, VerificationResult) in the CSV lack Console UI metadata; add displayName and description and provide specDescriptors and statusDescriptors blocks for each CRD in the CSV manifest to match the pattern used by OLSConfig (use its displayName/description/specDescriptors/statusDescriptors as the template), ensuring each field/attribute has a human-readable label, description, and appropriate path/type so the OpenShift Console shows helpful UI hints when creating or editing these resources.
🤖 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 `@config/samples/agentic_v1alpha1_proposal.yaml`:
- Around line 10-15: The sample YAML references non-existent agents: update the
analysis.agent and verification.agent values (currently "smart" and "fast") to
use the shipped sample agent name "default" (matching execution.agent), or
alternatively add Agent sample definitions named "smart" and "fast" to the
samples; locate the keys analysis.agent and verification.agent in the proposal
sample and either change their values to "default" or add corresponding Agent
resources with those names.
In `@hack/sync_agentic_crds.sh`:
- Around line 40-46: The script currently deletes and reinserts entries around
the scaffold marker SCAFFOLD_LINE but doesn't verify the marker exists, so
removals can proceed without reinsertion; update the logic that handles
SCAFFOLD_LINE (and the analogous block later around lines 54-62) to first check
that the marker string exists in the target file referenced by KUSTOMIZATION
(e.g., grep or a test on sed -n) and if not, echo a clear error and exit
non‑zero immediately (fail fast) before performing the deletion/loop; ensure the
check references the SCAFFOLD_LINE variable and the same KUSTOMIZATION file and
apply the same guard in both the first loop (for f in
"${DEST}"/agentic.openshift.io_*.yaml) and the second similar block so you don’t
proceed when scaffold markers are missing.
- Around line 23-46: The script currently copies new CRDs into ${DEST} but never
removes deleted upstream CRDs; before copying, remove any existing files
matching "${DEST}/agentic.openshift.io_*.yaml" so deleted CRDs are purged, then
proceed to copy from ${SRC} and rebuild the KUSTOMIZATION at ${KUSTOMIZATION} as
you already do: remove lines matching '^- bases/agentic\.openshift\.io_'
(SCAFFOLD_LINE handling stays) and re-insert ENTRY lines for each remaining file
in ${DEST}; ensure the count logic still fails when no files are present and
that the loop that produces ENTRY uses the files in ${DEST} (not ${SRC}) so only
current files are added.
In `@Makefile`:
- Line 9: AGENTIC_OPERATOR_REF is currently defaulting to the mutable branch
name "main"; change the Makefile so AGENTIC_OPERATOR_REF defaults to an
immutable release reference (e.g., a specific tag or commit hash) instead of
"main". Locate the AGENTIC_OPERATOR_REF variable in the Makefile and replace the
value "main" with a fixed tag or commit (or a clearly documented placeholder
like a stable version string) so running make bundle is reproducible and
upstream changes won’t alter shipped CRDs.
---
Nitpick comments:
In `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml`:
- Around line 98-120: The Proposal example uses three agent names
(spec.analysis.agent "smart", spec.execution.agent "default",
spec.verification.agent "fast") but the alm-examples only defines the "default"
Agent; add Agent resources for the "smart" and "fast" agents to the alm-examples
array so the CSV is self-contained—create two additional objects with
"apiVersion": "agentic.openshift.io/v1alpha1", "kind": "Agent", metadata.name
"smart" and "fast" and include plausible spec fields matching the existing
"default" Agent example.
- Around line 212-229: The new agentic CRDs (kinds: Agent, AnalysisResult,
ApprovalPolicy, EscalationResult, ExecutionResult, LLMProvider, Proposal,
ProposalApproval, VerificationResult) in the CSV lack Console UI metadata; add
displayName and description and provide specDescriptors and statusDescriptors
blocks for each CRD in the CSV manifest to match the pattern used by OLSConfig
(use its displayName/description/specDescriptors/statusDescriptors as the
template), ensuring each field/attribute has a human-readable label,
description, and appropriate path/type so the OpenShift Console shows helpful UI
hints when creating or editing these resources.
🪄 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: a586078d-67d6-450b-a58e-79c925cfa358
⛔ Files ignored due to path filters (9)
config/crd/bases/agentic.openshift.io_agents.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_analysisresults.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_approvalpolicies.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_escalationresults.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_executionresults.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_llmproviders.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_proposalapprovals.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_proposals.yamlis excluded by!config/crd/bases/**config/crd/bases/agentic.openshift.io_verificationresults.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (25)
MakefileREADME.mdbundle/manifests/agentic.openshift.io_agents.yamlbundle/manifests/agentic.openshift.io_analysisresults.yamlbundle/manifests/agentic.openshift.io_approvalpolicies.yamlbundle/manifests/agentic.openshift.io_escalationresults.yamlbundle/manifests/agentic.openshift.io_executionresults.yamlbundle/manifests/agentic.openshift.io_llmproviders.yamlbundle/manifests/agentic.openshift.io_proposalapprovals.yamlbundle/manifests/agentic.openshift.io_proposals.yamlbundle/manifests/agentic.openshift.io_verificationresults.yamlbundle/manifests/lightspeed-operator.clusterserviceversion.yamlconfig/crd/kustomization.yamlconfig/manifests/bases/lightspeed-operator.clusterserviceversion.yamlconfig/samples/agentic_v1alpha1_agent.yamlconfig/samples/agentic_v1alpha1_analysisresult.yamlconfig/samples/agentic_v1alpha1_approvalpolicy.yamlconfig/samples/agentic_v1alpha1_escalationresult.yamlconfig/samples/agentic_v1alpha1_executionresult.yamlconfig/samples/agentic_v1alpha1_llmprovider.yamlconfig/samples/agentic_v1alpha1_proposal.yamlconfig/samples/agentic_v1alpha1_proposalapproval.yamlconfig/samples/agentic_v1alpha1_verificationresult.yamlconfig/samples/kustomization.yamlhack/sync_agentic_crds.sh
| analysis: | ||
| agent: smart | ||
| execution: | ||
| agent: default | ||
| verification: | ||
| agent: fast |
There was a problem hiding this comment.
Align referenced agent names with shipped sample agents.
On Line 11 and Line 15, smart and fast are referenced, while the provided sample flow context only defines default. This makes the sample workflow inconsistent and likely non-reproducible when applied as-is. Prefer using existing sample agent names (or add matching Agent samples).
🤖 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 `@config/samples/agentic_v1alpha1_proposal.yaml` around lines 10 - 15, The
sample YAML references non-existent agents: update the analysis.agent and
verification.agent values (currently "smart" and "fast") to use the shipped
sample agent name "default" (matching execution.agent), or alternatively add
Agent sample definitions named "smart" and "fast" to the samples; locate the
keys analysis.agent and verification.agent in the proposal sample and either
change their values to "default" or add corresponding Agent resources with those
names.
| sed -i '/^- bases\/agentic\.openshift\.io_/d' "${KUSTOMIZATION}" | ||
| SCAFFOLD_LINE="#+kubebuilder:scaffold:crdkustomizeresource" | ||
| for f in "${DEST}"/agentic.openshift.io_*.yaml; do | ||
| [ -f "$f" ] || continue | ||
| ENTRY="- bases/$(basename "$f")" | ||
| sed -i "s|${SCAFFOLD_LINE}|${ENTRY}\n${SCAFFOLD_LINE}|" "${KUSTOMIZATION}" | ||
| done |
There was a problem hiding this comment.
Fail fast if scaffold markers are missing before rewrites.
If a scaffold marker is absent, the delete step still runs but reinsertion never occurs, leaving kustomization missing entries without a hard failure.
Suggested fix
if [ -f "${KUSTOMIZATION}" ]; then
+ grep -q '^#+kubebuilder:scaffold:crdkustomizeresource$' "${KUSTOMIZATION}" || {
+ echo "ERROR: CRD scaffold marker missing in ${KUSTOMIZATION}" >&2
+ exit 1
+ }
sed -i '/^- bases\/agentic\.openshift\.io_/d' "${KUSTOMIZATION}"
@@
if [ -d "${SAMPLES_SRC}" ]; then
+ grep -q '^#+kubebuilder:scaffold:manifestskustomizesamples$' "${SAMPLES_KUSTOMIZATION}" || {
+ echo "ERROR: samples scaffold marker missing in ${SAMPLES_KUSTOMIZATION}" >&2
+ exit 1
+ }
sed -i '/^- agentic_/d' "${SAMPLES_KUSTOMIZATION}" 2>/dev/null || trueAlso applies to: 54-62
🤖 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 `@hack/sync_agentic_crds.sh` around lines 40 - 46, The script currently deletes
and reinserts entries around the scaffold marker SCAFFOLD_LINE but doesn't
verify the marker exists, so removals can proceed without reinsertion; update
the logic that handles SCAFFOLD_LINE (and the analogous block later around lines
54-62) to first check that the marker string exists in the target file
referenced by KUSTOMIZATION (e.g., grep or a test on sed -n) and if not, echo a
clear error and exit non‑zero immediately (fail fast) before performing the
deletion/loop; ensure the check references the SCAFFOLD_LINE variable and the
same KUSTOMIZATION file and apply the same guard in both the first loop (for f
in "${DEST}"/agentic.openshift.io_*.yaml) and the second similar block so you
don’t proceed when scaffold markers are missing.
| VERSION ?= latest | ||
|
|
||
| AGENTIC_OPERATOR_REPO ?= https://github.com/openshift/lightspeed-agentic-operator | ||
| AGENTIC_OPERATOR_REF ?= main |
There was a problem hiding this comment.
Use an immutable default for AGENTIC_OPERATOR_REF.
Using main makes make bundle non-reproducible and allows unreviewed upstream changes to alter shipped CRDs over time.
Suggested fix
-AGENTIC_OPERATOR_REF ?= main
+# Pin to a released tag or commit SHA for reproducible syncs.
+AGENTIC_OPERATOR_REF ?= v0.1.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AGENTIC_OPERATOR_REF ?= main | |
| # Pin to a released tag or commit SHA for reproducible syncs. | |
| AGENTIC_OPERATOR_REF ?= v0.1.0 |
🤖 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 `@Makefile` at line 9, AGENTIC_OPERATOR_REF is currently defaulting to the
mutable branch name "main"; change the Makefile so AGENTIC_OPERATOR_REF defaults
to an immutable release reference (e.g., a specific tag or commit hash) instead
of "main". Locate the AGENTIC_OPERATOR_REF variable in the Makefile and replace
the value "main" with a fixed tag or commit (or a clearly documented placeholder
like a stable version string) so running make bundle is reproducible and
upstream changes won’t alter shipped CRDs.
raptorsun
force-pushed
the
agentic-crd
branch
2 times, most recently
from
f2af5b6 to
4ec95f6
Compare
Signed-off-by: Haoyu Sun <hasun@redhat.com>
|
/retest |
|
@raptorsun: The following test failed, say
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. |
onmete
left a comment
onmete
left a comment
There was a problem hiding this comment.
Review — OLS-3189
The core sync mechanism looks good — all 5 Jira ACs are met. One issue to address before merge:
Sample-sync logic references a directory that doesn't exist upstream
hack/sync_agentic_crds.sh lines 51-69 contain logic to sync samples from ${TMPDIR}/config/samples/agentic_*.yaml, but the lightspeed-agentic-operator repo has no config/samples/ directory (samples live under examples/setup/). The if [ -d "${SAMPLES_SRC}" ] guard makes this a silent no-op.
The 9 sample files in this PR were manually created, not synced — which contradicts the README statement "do not hand-edit the agentic CRD or sample files."
Options:
- Add a
config/samples/directory to lightspeed-agentic-operator so the sync actually works, or - Remove the sample-sync logic from the script and drop the "don't hand-edit samples" claim from the README (acknowledge samples are manually maintained).
Either way, the current state is inconsistent — the script promises sample sync but can't deliver it.
3 participants
Description
This PR adds the CRDs required by the agentic workflow and a tool script to synchronize from the lightspeed-agentic-operator repository.
The CRDs are synchronized when calling
make bundle.To sync agentic CRDs and samples from the pinned ref (defaults to
main) without regenerating the bundle:To sync from a specific tag or commit:
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
New Features
Documentation