Run eval with vertex instead of Gemini#228
Run eval with vertex instead of Gemini#228openshift-merge-bot[bot] merged 1 commit intorh-ecosystem-edge:mainfrom
Conversation
WalkthroughAdds Vertex AI credential handling to Prow test artifacts: Changes
Sequence Diagram(s)sequenceDiagram
participant K8s as Kubernetes Job (container)
participant Entrypoint as entrypoint.sh
participant FS as Container filesystem (.env)
participant Eval as python eval.py
Note over K8s: Secret mounted at /opt/app-root/google-vertex-service-account.json
K8s->>Entrypoint: start container
Entrypoint->>FS: write "GEMINI_API_KEY=dummy" to .env
Entrypoint->>FS: append "GOOGLE_APPLICATION_CREDENTIALS=/opt/app-root/google-vertex-service-account.json" to .env
Entrypoint->>Eval: exec python eval.py ... --judge_provider="vertex"
Eval->>K8s: uses GOOGLE_APPLICATION_CREDENTIALS for Vertex AI calls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/prow/entrypoint.sh(1 hunks)test/prow/template.yaml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / assisted-chat-test-image-saas-main-on-pull-request
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
🔇 Additional comments (2)
test/prow/template.yaml (2)
74-77: Params LGTM; verify secret/key names exist and match the mounted filename.
- Names look reasonable. Ensure the secret key name equals the filename you expect under the mount (used in the path).
79-79: OCM_BASE_URL value change noted.Looks fine for stage. Confirm this aligns with the AGENT envs for the CI cluster.
|
/retest |
test/prow/template.yaml
Outdated
| volumeMounts: | ||
| - name: google-vertex-service-account | ||
| mountPath: /opt/app-root/google-vertex-service-account.json | ||
| subPath: ${VERTEX_API_SECRET_KEY_NAME} |
There was a problem hiding this comment.
Fix indentation: volumeMounts must live inside the container spec
Right now volumeMounts is aligned with the pod spec (same level as containers), so the generated manifest is invalid—Kubernetes rejects volumeMounts outside an individual container. Indent the block so it stays under - name: assisted-chat-eval-test (and mark it read-only while you’re there).
- volumeMounts:
- - name: google-vertex-service-account
- mountPath: /opt/app-root/google-vertex-service-account.json
- subPath: ${VERTEX_API_SECRET_KEY_NAME}
+ volumeMounts:
+ - name: google-vertex-service-account
+ mountPath: /opt/app-root/google-vertex-service-account.json
+ subPath: ${VERTEX_API_SECRET_KEY_NAME}
+ readOnly: true📝 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.
| volumeMounts: | |
| - name: google-vertex-service-account | |
| mountPath: /opt/app-root/google-vertex-service-account.json | |
| subPath: ${VERTEX_API_SECRET_KEY_NAME} | |
| volumeMounts: | |
| - name: google-vertex-service-account | |
| mountPath: /opt/app-root/google-vertex-service-account.json | |
| subPath: ${VERTEX_API_SECRET_KEY_NAME} | |
| readOnly: true |
🤖 Prompt for AI Agents
In test/prow/template.yaml around lines 50 to 53, the volumeMounts block is
incorrectly placed at the pod level instead of under the specific container (-
name: assisted-chat-eval-test); move/indent the entire volumeMounts block so it
is nested inside that container's spec and add readOnly: true to the mount entry
to ensure the secret is mounted as read-only.
This is an attempt to run the eval test judge model with vertex instead of Gemini If this works, we can remove Gemini and its credentials from the CI to align ourselves better with what we run in prod.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
test/prow/Dockerfile (1)
5-5: Pin dependency versions for reproducible CI buildsUnpinned
google-cloud-aiplatform(andyq) may introduce breaking changes. Pin versions or use a constraints/requirements file.Example:
-RUN pip install yq google-cloud-aiplatform +RUN pip install \ + yq==4.44.3 \ + google-cloud-aiplatform==1.70.0test/prow/entrypoint.sh (2)
23-24: Drop.envwrites and the dummy GEMINI key
- Writing
.envis unnecessary;GOOGLE_APPLICATION_CREDENTIALSis already provided via env as a file path.GEMINI_API_KEY=dummyis misleading now that--judge_provider=vertexis used.Remove both lines.
-echo "GEMINI_API_KEY=dummy" > .env -echo "GOOGLE_APPLICATION_CREDENTIALS=${GOOGLE_APPLICATION_CREDENTIALS}" >> .env
19-19: Quote paths/vars to avoid word-splitting and globbing issuesSafer, more robust shell usage.
-cd $TEMP_DIR +cd "$TEMP_DIR" - -cp $TEST_DIR/eval_data.yaml $TEMP_DIR/eval_data.yaml -sed -i "s/uniq-cluster-name/${UNIQUE_ID}/g" $TEMP_DIR/eval_data.yaml -sed -i "s|: ../scripts|: ${WORK_DIR}/test/scripts|g" $TEMP_DIR/eval_data.yaml - -python $TEST_DIR/eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}" --agent_auth_token_file $TEMP_DIR/ocm_token.txt --eval_data_yaml $TEMP_DIR/eval_data.yaml --judge_provider="vertex" +cp "$TEST_DIR/eval_data.yaml" "$TEMP_DIR/eval_data.yaml" +sed -i "s/uniq-cluster-name/${UNIQUE_ID}/g" "$TEMP_DIR/eval_data.yaml" +sed -i "s|: ../scripts|: ${WORK_DIR}/test/scripts|g" "$TEMP_DIR/eval_data.yaml" +python "$TEST_DIR/eval.py" \ + --agent_endpoint "${AGENT_URL}:${AGENT_PORT}" \ + --agent_auth_token_file "$TEMP_DIR/ocm_token.txt" \ + --eval_data_yaml "$TEMP_DIR/eval_data.yaml" \ + --judge_provider="vertex"Also applies to: 26-30
test/prow/template.yaml (3)
50-53: Mount secret read-onlyAdd
readOnly: trueon the volumeMount for defense-in-depth.volumeMounts: - name: google-vertex-service-account mountPath: /opt/app-root/google-vertex-service-account.json subPath: ${VERTEX_API_SECRET_KEY_NAME} + readOnly: true
35-39: Remove Gemini secret from the container if no longer usedIf the judge is now Vertex-only, drop the
GEMINI_API_KEYenv to simplify CI and reduce secret surface.- - name: GEMINI_API_KEY - valueFrom: - secretKeyRef: - key: ${GEMINI_API_SECRET_KEY_NAME} - name: ${GEMINI_API_SECRET_NAME}
75-82: Also remove Gemini parameters if unusedClean up template parameters to avoid confusion.
-- name: GEMINI_API_SECRET_NAME - value: gemini -- name: GEMINI_API_SECRET_KEY_NAME - value: api_key
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
test/prow/Dockerfile(1 hunks)test/prow/entrypoint.sh(1 hunks)test/prow/template.yaml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Red Hat Konflux / assisted-chat-test-image-saas-main-on-pull-request
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
🔇 Additional comments (2)
test/prow/template.yaml (2)
40-41: Good: ADC points to a filesystem pathSetting
GOOGLE_APPLICATION_CREDENTIALSto the mounted file path matches ADC requirements.
54-57: Volumes placement looks correct
volumesis at the pod spec level, sibling tocontainers. LGTM.
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: carbonin, omertuc 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 |
openshift-merge-bot
bot
merged commit f89e5a2
into
rh-ecosystem-edge:main
2 participants
This is an attempt to run the eval test judge model with vertex instead of Gemini
If this works, we can remove Gemini and its credentials from the CI to align ourselves better with what we run in prod.
Summary by CodeRabbit
Tests
Chores