OCPMCP-290: add anthropic/gemini vertex jobs#81118
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughThe CI config adds two optional MCP checker evaluation steps that mount ChangesOpenShift MCP eval CI steps
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-main.yaml`:
- Around line 67-74: The eval job is masking real failures by forcing mcpchecker
to succeed with “|| true”, so update the job to preserve the actual exit status
while keeping cleanup separate. In the affected check steps for mcpchecker,
replace the inline success override with a trap-based cleanup that always runs
make stop-server, and let mcpchecker return its real status so the optional jobs
still report failures correctly.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 811f0b2c-ad67-458f-9213-c8216f02c0e5
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/openshift-mcp-server/openshift-openshift-mcp-server-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-main.yaml
|
/pj-rehearse |
|
@dlom: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/hold |
968d084 to
f08c1ea
Compare
|
/pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
|
/pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/hold cancel |
|
containers/kubernetes-mcp-server#1241 merged, but needs to be downstreamed before this can be re-evaluated |
|
This appears sound, but cannot merge w/o the extra rehearsal jobs, so I'll switch this from draft but with a hold until those pass. |
|
/hold |
|
/pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
openshift/openshift-mcp-server#379 merged, re-trying rehearsals /pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Signed-off-by: grokspawn <jordan@nimblewidget.com>
|
mcpchecker/mcpchecker#322 merged to fix relative path references for judge config /pj-rehearse |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@grokspawn looking at the logs from the google job which did run mcpchecker, I think we need to install kubectl in the job: Many tasks rely on it being present |
|
Last rehearsal pulled mcpchecker v0.0.18 which didn't have the relative pathing corrected yet. v0.0.19 is out which does, so re-rehearsing. /pj-rehearse and also re-running failed CI /retest |
|
@grokspawn: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retitle OCPMCP-290: add anthropic/gemini vertex jobs |
|
@grokspawn: This pull request references OCPMCP-290 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. |
|
/hold cancel |
|
/hold |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: grokspawn 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 |
|
@grokspawn: |
Cali0707
left a comment
There was a problem hiding this comment.
LOVE the approach here, just some notes about the model/agent config methods
| workflow: ipi-aws | ||
| ``` | ||
|
|
||
| Requires the `openshift-mcp-server-mcpchecker-credentials` secret in the `test-credentials` |
There was a problem hiding this comment.
I dont think this is true, see: https://github.com/mcpchecker/mcpchecker/blob/main/docs/how-to/configure-agents.md#llm-agent (note that for openai, we only need OPENAI_API_KEY, etc.)
This seems to be referring to the older config from mcpchekcer
| Reads model API credentials from the openshift-mcp-server-mcpchecker-credentials | ||
| secret and writes them as env var exports to ${SHARED_DIR}/mcpchecker-creds.env | ||
| for consumption by the openshift-mcp-server-mcpchecker-eval step. | ||
| Expected secret keys: MODEL_BASE_URL, MODEL_KEY, JUDGE_BASE_URL, |
There was a problem hiding this comment.
As other spot, we probably only need to extract OPENAI_API_KEY key (which if the env var key is the literal in the secret, we probably need to rename in vault as well)
| export MODEL_BASE_URL="$(cat "${SECRETS_DIR}/MODEL_BASE_URL")" | ||
| export MODEL_KEY="$(cat "${SECRETS_DIR}/MODEL_KEY")" | ||
| export JUDGE_BASE_URL="$(cat "${SECRETS_DIR}/JUDGE_BASE_URL")" | ||
| export JUDGE_API_KEY="$(cat "${SECRETS_DIR}/JUDGE_API_KEY")" | ||
| export JUDGE_MODEL_NAME="$(cat "${SECRETS_DIR}/JUDGE_MODEL_NAME")" |
Signed-off-by: grokspawn <jordan@nimblewidget.com>
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@grokspawn: The following tests 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. |
Builds off #78018, but switches to a multi-step inline test instead of a container test to access mounted credentials.
Summary by CodeRabbit
This PR updates OpenShift CI configuration for the MCP server to add two optional Vertex-based evaluation steps (Google Gemini and Anthropic Claude). In practical terms, it switches to a multi-step inline
mcpcheckerworkflow so the job can access mounted credentials during the test run.What changed
ci-operator/config/openshift/openshift-mcp-server/openshift-openshift-mcp-server-main.yamlto add two new optional MCP checker evaluation steps:mcpchecker-eval-googlemcpchecker-eval-anthropicocp-mcpcredential (fromtest-credentials) at/var/run/ocp-mcpset -euo pipefail)GOOGLE_CLOUD_LOCATION=us-east1andGEMINI_USE_VERTEX=1GOOGLE_CLOUD_LOCATION=us-east1andANTHROPIC_USE_VERTEX=1/var/run/ocp-mcp/service-account.jsonGOOGLE_CLOUD_PROJECTby parsing the mounted service account JSON via an inlinepython3 -ccommandGOFLAGS='-mod=readonly' make buildGOFLAGS='' make mcpcheckermake run-server TOOLSETS=core,config)trap 'make stop-server || true' EXIT)_output/tools/bin/mcpchecker checkin JSON mode (--output json) with--label-selector suite=coregrace_period: 1m0sand resources (cpu: 500m,memory: 200Mi)Eval suites used
mcpchecker-eval-google:evals/core-eval-testing/builtin-google/eval-core.yamlmcpchecker-eval-anthropic:evals/core-eval-testing/builtin-anthropic/eval-core.yamlCI impact
optional: true, so they can be exercised without necessarily blocking the overall MCP server CI run if they fail.