OTA-2084: pkg/agenticrun/controller: Inline AgenticRun prompt, dropping the ConfigMap - #1432
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe AgenticRun controller now uses an embedded advisor prompt instead of a ConfigMap. Prompt configuration, retrieval logic, related manifests, and the ConfigMap getter passed during CVO construction are removed. ChangesAgenticRun prompt configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CVO
participant AgenticRunController
participant getAgenticRuns
CVO->>AgenticRunController: construct without ConfigMap getter
AgenticRunController->>getAgenticRuns: pass embedded prompt
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
wking
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@wking: This pull request references OTA-2084 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
the
approved
![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 (1)
pkg/agenticrun/controller.go (1)
87-93: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the obsolete ConfigMap getter from the remaining constructor call.
NewControllernow accepts five arguments, butpkg/cvo/availableupdates_test.goat Line 205-221 still passes the removed ConfigMap getter as a sixth argument. This causes a compile-time “too many arguments” failure; remove that function from the test call.🤖 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 `@pkg/agenticrun/controller.go` around lines 87 - 93, Update the NewController call in availableupdates_test.go to pass only its five current parameters, removing the obsolete ConfigMap getter argument while preserving the remaining argument order.
🤖 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 `@pkg/agenticrun/controller.go`:
- Around line 87-93: Update the NewController call in availableupdates_test.go
to pass only its five current parameters, removing the obsolete ConfigMap getter
argument while preserving the remaining argument order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: eb82933b-99f4-43bd-8bc4-d7e87b8f850a
📒 Files selected for processing (4)
install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yamlinstall/0000_00_cluster-version-operator_50_lightspeed-prompts.yamlpkg/agenticrun/controller.gopkg/cvo/cvo.go
💤 Files with no reviewable changes (3)
- install/0000_00_cluster-version-operator_45_openshift-lightspeed_namespace.yaml
- install/0000_00_cluster-version-operator_50_lightspeed-prompts.yaml
- pkg/cvo/cvo.go
f2a808f to
d3ab552
Compare
openshift-ci
Bot
added
the
needs-rebase
d3ab552 to
895848b
Compare
openshift-ci
Bot
removed
the
needs-rebase
8f4b4e6 to
859556e
Compare
…figMap I'm not entirely clear on when we stopped needing a ConfigMap. Before c3fe4c0 (Use imported Proposal API from lightspeed-agentic-operator, 2026-05-08, openshift#1387), there had been a systemPromptRef for passing in a prompt, and that had needed a ConfigMap. But since c3fe4c0, the prompt has been passed into the Proposal / AgeneticRun via a string property. And since then, the ConfigMap had been the cluster-version operator just passing itself the prompt string. By removing the prompt ConfigMap and just compiling the string into the cluster-version operator, we remove that useless indirection, and can successfully run on default-feature-set clusters where the TechPreviewNoUpgrade installer manifest wasn't getting appied: $ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep controller.go | tail -n5 I0726 00:46:30.141228 1 controller.go:255] Failed to get prompt ConfigMap openshift-lightspeed/cluster-update-advisory-prompt: configmaps "cluster-update-advisory-prompt" not found I0726 00:46:30.141246 1 controller.go:188] Finished syncing CVO configuration (5.4221ms) I0726 00:47:11.101773 1 controller.go:186] Started syncing CVO configuration "ClusterVersionOperator/agenticrun-lifecycle-controller" I0726 00:47:11.107717 1 controller.go:255] Failed to get prompt ConfigMap openshift-lightspeed/cluster-update-advisory-prompt: configmaps "cluster-update-advisory-prompt" not found I0726 00:47:11.107740 1 controller.go:188] Finished syncing CVO configuration (6.001307ms)
|
/lgtm It looks good to me, I was able to test it in a cluster and it produced the agenticrun properly deriving the prompt from inline code and not from configmap. However I had to tweak a little to get agentic controller running, here #1434 for it. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jrangelramos, wking 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 |
|
/hold I already verified in 5.0.0 live cluster, agentic runs get created properly with the inline prompt. Added hold for @wking in case he wants to verify either. |
openshift-ci
Bot
added
the
do-not-merge/hold
openshift-ci-robot
added
the
verified
|
@jrangelramos: This PR has been marked as verified by 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. |
|
ClusterBot $ oc image info registry.build10.ci.openshift.org/ci-ln-m01zi82/release:latest | grep Digest
Digest: sha256:6ab40c482e6db74b808d38b262322a99f0c90b48213846a8c0d7aec360aa0bb3I updated a demo cluster that has the AgenticRun CRD installed, setting some unsafe knobs to allow updating to an unsighed, CI image in this throw-away cluster: $ oc patch clusterversion version --type json -p '[{"op": "add", "path": "/spec/overrides/-", "value": [{"group": "config.openshift.io", "kind": "ClusterImagePolicy", "name": "openshift", "namespace": "", "unmanaged": true}]}]'
$ oc delete clusterimagepolicy openshift
$ oc adm upgrade --force --allow-explicit-upgrade --to-image registry.build10.ci.openshift.org/ci-ln-m01zi82/release@sha256:6ab40c482e6db74b808d38b262322a99f0c90b48213846a8c0d7aec360aa0bb3And after that update completes, it looks good to me: $ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep controller.go | tail
$ oc -n openshift-cluster-version logs -l k8s-app=cluster-version-operator --tail -1 | grep controller.go | tail -n4
I0727 20:36:27.603243 1 controller.go:241] Skipping agentic run creation: cluster is progressing (Working towards 5.0.0-0-2026-07-27-192510-test-ci-ln-m01zi82-latest: 894 of 1033 done (86% complete), waiting on machine-config)
I0727 20:36:27.603258 1 controller.go:191] Finished syncing CVO configuration (20.646268ms)
I0727 20:40:22.328826 1 controller.go:189] Started syncing CVO configuration "ClusterVersionOperator/agenticrun-lifecycle-controller"
I0727 20:40:34.227007 1 controller.go:191] Finished syncing CVO configuration (11.89818018s)
$ oc -n openshift-lightspeed get agenticruns
NAMESPACE NAME AGE
openshift-lightspeed ota-5-0-0-0-2026-07-27-192510-test-ci-ln-m01zi82-latest-to-5xxx 62sHmm, not clear why there's only one AgenticRun, when there are two "supported" updated via dummy Fauxinnati data: $ oc adm upgrade recommend
Failed to check for at least some preconditions: no token is currently in use for this session
Upstream update service: https://fauxinnati-fauxinnati.apps.ota-stage.q2z4.p1.openshiftapps.com/api/upgrades_info/graph
Channel: simple
Updates to 5.1:
Version: 5.1.0
Image: quay.io/openshift-release-dev/ocp-release@sha256:00000000000000000000000000000000000000000000000000000000004c4f28
Reason: RiskNameNotCompatibleWithReasonProperty
Message: Parent credentials secret must be restored prior to upgrade: kube-system/aws-creds
Updates to 5.0:
VERSION ISSUES
5.0.1 no known issues relevant to this clusterBut whatever, better than failing on the prompt ConfigMap issue. /hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
CustomResourceValidationRules and NewOLMConfigAPI failures are unrelated to this pull: /override ci/prow/e2e-aws-ovn-techpreview |
|
@wking: Overrode contexts on behalf of wking: ci/prow/e2e-aws-ovn-techpreview 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. |
|
@wking: 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. |
3 participants
I'm not entirely clear on when we stopped needing a ConfigMap. Before c3fe4c0 (#1387), there had been a
systemPromptReffor passing in a prompt, and that had needed a ConfigMap. But since c3fe4c0, the prompt has been passed into the Proposal / AgeneticRun via a string property. And since then, the ConfigMap had been the cluster-version operator just passing itself the prompt string. By removing the prompt ConfigMap and just compiling the string into the cluster-version operator, we remove that useless indirection, and can successfully run on default-feature-set clusters where theTechPreviewNoUpgradeinstaller manifest wasn't getting appied:Summary by CodeRabbit