Skip to content

log previous assisted-chat-eval-test container's logs #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
giladravid16 wants to merge 9 commits into from
Closed

log previous assisted-chat-eval-test container's logs #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1a91e8e
log previous assisted-chat-eval-test container's logs
giladravid16 Aug 21, 2025
b09ceaa
add coderabbit suggestions
giladravid16 Aug 21, 2025
9b262b6
debug
giladravid16 Aug 25, 2025
3ebf256
debug
giladravid16 Aug 25, 2025
7fc3502
debug
giladravid16 Aug 26, 2025
8d89a8f
update template
giladravid16 Aug 26, 2025
357bd93
debug
giladravid16 Aug 26, 2025
a3201ad
change provider_model_id
giladravid16 Aug 26, 2025
4e31882
change provider to vertex_ai
giladravid16 Aug 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions scripts/ci_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ oc process -p IMAGE_NAME="$ASSISTED_CHAT_TEST" -p SSL_CLIENT_SECRET_NAME=assiste

sleep 5
oc get pods -n "$NAMESPACE"
POD_NAME=$(oc get pods | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat-eval-tes)
POD_NAME=$(oc get pods -n "$NAMESPACE" | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat-eval-test)

TIMEOUT=600
ELAPSED=0
Expand All @@ -24,7 +24,7 @@ while [ $ELAPSED -lt $TIMEOUT ]; do
CURRENT_RESTARTS=$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].restartCount}')
if [[ $CURRENT_RESTARTS -gt 0 ]]; then
echo "Pod ${POD_NAME} was restarted, so the tests should run at least once, exiting"
oc logs -n "$NAMESPACE" "$POD_NAME"
oc logs -p -n "$NAMESPACE" "$POD_NAME" || true
exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')"
fi
if [[ "$CURRENT_STATUS" == "Succeeded" ]]; then
Expand Down
24 changes: 20 additions & 4 deletions template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,11 @@ objects:
- vector_io
providers:
inference:
- provider_id: gemini
provider_type: remote::gemini
- provider_id: vertex_ai
provider_type: remote::vertexai
config:
api_key: dummy-to-stop-llama-stack-from-complaining-even-though-we-use-vertex-and-not-gemini-directly
project: ""
location: us-central1
vector_io: []
Comment on lines +297 to 302
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Empty Vertex AI project config will likely break inference provider initialization

The remote::vertexai provider typically requires a non-empty GCP project. Leaving project: "" will cause provider init or first call to fail. Make project a required parameter and wire it here. Consider parameterizing location as well.

Apply this diff within this block:

         inference:
           providers:
-        - provider_id: vertex_ai
-          provider_type: remote::vertexai
-          config:
-            project: ""
-            location: us-central1
+        - provider_id: vertex_ai
+          provider_type: remote::vertexai
+          config:
+            project: ${VERTEX_PROJECT_ID}
+            location: ${VERTEX_LOCATION}

And add template parameters (outside this hunk):

 parameters:
+ - name: VERTEX_PROJECT_ID
+   value: ""
+   required: true
+   description: "GCP Project ID used by Vertex AI (from the service account's project)"
+ - name: VERTEX_LOCATION
+   value: "us-central1"
+   description: "Vertex AI location/region for model inference"

If you prefer to keep location hard-coded, only add VERTEX_PROJECT_ID and leave location: us-central1.

📝 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.

Suggested change
- provider_id: vertex_ai
provider_type: remote::vertexai
config:
api_key: dummy-to-stop-llama-stack-from-complaining-even-though-we-use-vertex-and-not-gemini-directly
project: ""
location: us-central1
vector_io: []
inference:
providers:
- - provider_id: vertex_ai
- provider_type: remote::vertexai
- config:
- project: ""
- provider_id: vertex_ai
provider_type: remote::vertexai
config:
project: ${VERTEX_PROJECT_ID}
location: ${VERTEX_LOCATION}
vector_io: []
Suggested change
- provider_id: vertex_ai
provider_type: remote::vertexai
config:
api_key: dummy-to-stop-llama-stack-from-complaining-even-though-we-use-vertex-and-not-gemini-directly
project: ""
location: us-central1
vector_io: []
parameters:
- name: VERTEX_PROJECT_ID
value: ""
required: true
description: "GCP Project ID used by Vertex AI (from the service account's project)"
- name: VERTEX_LOCATION
value: "us-central1"
description: "Vertex AI location/region for model inference"
🤖 Prompt for AI Agents 
In template.yaml around lines 297-302, the Vertex AI provider block uses an
empty project string which will break initialization; update the block to
reference a template parameter for the GCP project (e.g. VERTEX_PROJECT_ID)
instead of "" and wire location to a parameter if you want it configurable (or
leave as us-central1 if you prefer). Also add corresponding template parameters
near the top-level parameters section (outside this hunk): add VERTEX_PROJECT_ID
as required (string) and optionally VERTEX_LOCATION with a default of
us-central1 if you want it configurable. Ensure the provider config uses the
parameter names exactly and validate the template parameter types so deployment
fails early if VERTEX_PROJECT_ID is not supplied.

files: []
safety: []
Expand Down Expand Up @@ -358,7 +359,22 @@ objects:
password: ${env.ASSISTED_CHAT_POSTGRES_PASSWORD}
ssl_mode: ${LLAMA_STACK_POSTGRES_SSL_MODE}
ca_cert_path: /etc/tls/ca-bundle.pem
models: []
models:
- metadata: {}
model_id: vertex_ai/gemini-2.0-flash
provider_id: vertex_ai
provider_model_id: gemini-2.0-flash
model_type: llm
- metadata: {}
model_id: vertex_ai/gemini-2.5-pro
provider_id: vertex_ai
provider_model_id: gemini-2.5-pro
model_type: llm
- metadata: {}
model_id: vertex_ai/gemini-2.5-flash
provider_id: vertex_ai
provider_model_id: gemini-2.5-flash
model_type: llm
shields: []
vector_dbs: []
datasets: []
Expand Down
4 changes: 2 additions & 2 deletions test/prow/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ OCM_TOKEN=$(curl -X POST https://sso.redhat.com/auth/realms/redhat-external/prot
-H "Content-Type: application/x-www-form-urlencoded" \
-d "grant_type=client_credentials" \
-d "client_id=$CLIENT_ID" \
-d "client_secret=$CLIENT_SECRET" | jq '.access_token')
-d "client_secret=$CLIENT_SECRET" | jq -r '.access_token')
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Good fix: jq -r for raw token; add guard and curl hardening to avoid “null” tokens sneaking through

jq -r is correct. However, if the token request returns 4xx/5xx (or a JSON error without access_token), the pipeline can still succeed and you’ll proceed with OCM_TOKEN="null" or empty, causing confusing auth failures later.

Recommend: make curl fail on HTTP errors and validate the token right after assignment.

Add immediately after the assignment:

# Fail early if token is empty or 'null'
if [ -z "${OCM_TOKEN:-}" ] || [ "$OCM_TOKEN" = "null" ]; then
  echo "Failed to obtain OCM access token" >&2
  exit 1
fi

Optionally harden the curl call:

# Add --fail-with-body, retries, and timeout for robustness
OCM_TOKEN=$(curl -sS --fail-with-body --retry 3 --retry-delay 2 --max-time 15 \
  -X POST https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \
  -H "Content-Type: application/x-www-form-urlencoded" \
  -d "grant_type=client_credentials" \
  -d "client_id=$CLIENT_ID" \
  -d "client_secret=$CLIENT_SECRET" | jq -r '.access_token')
🤖 Prompt for AI Agents 
In test/prow/entrypoint.sh around line 12, the script currently parses the
access token with jq -r but doesn't make curl fail on HTTP errors nor validate
the token afterward; update the curl invocation to harden it (e.g. use
--fail-with-body, add --retry, --retry-delay and --max-time or equivalent flags)
so HTTP error responses cause failure, then immediately validate OCM_TOKEN after
assignment and exit non‑zero with an error message if it's empty or the literal
"null". Ensure the validation uses a POSIX-safe test like [ -z "${OCM_TOKEN:-}"
] || [ "$OCM_TOKEN" = "null" ] and prints to stderr before exiting.


echo "$OCM_TOKEN" > test/evals/ocm_token.txt

cd test/evals
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#python eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
python eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Re-enable eval: prefer python3/-u and ensure token is raw (jq -r) to avoid auth breakage

  • Using python may fail on images that only ship python3. Also, -u gives unbuffered logs in CI.
  • Critical: the token currently saved via jq '.access_token' includes quotes. Many clients expect a raw token; enabling eval.py now will likely read a quoted token and fail auth.

Apply this diff for the eval invocation:

-python eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
+python3 -u eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"

Additionally, update the token extraction earlier in the script to write the raw access token (no quotes):

Verification checklist:

  • Confirm AGENT_URL and AGENT_PORT are set in the job env (set -o nounset will fail fast if missing).
  • Confirm python3 exists in the image; otherwise switch to /usr/bin/env python3.

🏁 Script executed:

# Replace the existing assignment with the raw-output variant
OCM_TOKEN=$(curl -sS -X POST https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \
  -H "Content-Type: application/x-www-form-urlencoded" \
  -d "grant_type=client_credentials" \
  -d "client_id=$CLIENT_ID" \
  -d "client_secret=$CLIENT_SECRET" | jq -r '.access_token')

Length of output: 258

🏁 Script executed:

#!/bin/bash
set -e

echo "🔍 Checking OCM_TOKEN assignment in test/prow/entrypoint.sh..."
rg -n --context 3 "OCM_TOKEN" test/prow/entrypoint.sh || true

echo "🔍 Checking python invocation in test/prow/entrypoint.sh..."
rg -n --context 3 "python " test/prow/entrypoint.sh || true

echo "🔍 Checking for 'set -o nounset' in test/prow/entrypoint.sh..."
rg -n --context 3 "set -o nounset" test/prow/entrypoint.sh || true

Length of output: 1149

🏁 Script executed:

#!/bin/bash
set -e

echo "🔍 Locating test/evals/eval.py..."
if [ -f test/evals/eval.py ]; then
  echo "Found test/evals/eval.py, showing first 5 lines:"
  head -n 5 test/evals/eval.py
else
  echo "eval.py not found at test/evals/eval.py. Searching for any eval.py:"
  find test -type f -name eval.py
fi

Length of output: 327

Re-enable eval with unbuffered Python3 and raw token parsing

The current entrypoint.sh at lines 8–12 and 18 will break CI evaluation and auth:

  • The OCM_TOKEN assignment uses jq '.access_token', which preserves surrounding quotes. Downstream clients will see '"TOKEN"' instead of TOKEN, causing authentication failures.
  • The python eval.py … invocation uses the default python binary and buffered output. Many minimal CI images only include python3, and without -u, log streaming is delayed.

Suggested fixes:

  1. Update the token extraction to strip quotes:

    -OCM_TOKEN=$(curl -sS -X POST https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \
-  -H "Content-Type: application/x-www-form-urlencoded" \
-  -d "grant_type=client_credentials" \
-  -d "client_id=$CLIENT_ID" \
-  -d "client_secret=$CLIENT_SECRET" | jq '.access_token')
+OCM_TOKEN=$(curl -sS -X POST https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \
+  -H "Content-Type: application/x-www-form-urlencoded" \
+  -d "grant_type=client_credentials" \
+  -d "client_id=$CLIENT_ID" \
+  -d "client_secret=$CLIENT_SECRET" | jq -r '.access_token')

  2. Change the eval invocation to use unbuffered Python3, falling back to /usr/bin/env if necessary:

    -python eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
+python3 -u eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
# Or, if python3 isn’t guaranteed on PATH:
+# /usr/bin/env python3 -u eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"

  3. Ensure safety with strict shell options (set -o nounset is already present at line 3) and confirm that AGENT_URL and AGENT_PORT are exported in the CI environment so that nounset errors fail fast.

These changes are critical to avoid CI log buffering issues and authentication failures when re-enabling eval.py.

📝 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.

Suggested change
python eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
#!/bin/bash
set -o nounset
# Acquire OCM token
OCM_TOKEN=$(curl -sS -X POST https://sso.redhat.com/auth/realms/redhat-external/protocol/openid-connect/token \
-H "Content-Type: application/x-www-form-urlencoded" \
-d "grant_type=client_credentials" \
-d "client_id=$CLIENT_ID" \
-d "client_secret=$CLIENT_SECRET" | jq -r '.access_token')
# … other CI bootstrap steps …
# Run evaluation with unbuffered Python3 output
python3 -u eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
# Fallback if python3 isn’t on PATH:
# /usr/bin/env python3 -u eval.py --agent_endpoint "${AGENT_URL}:${AGENT_PORT}"
🤖 Prompt for AI Agents 
In test/prow/entrypoint.sh around line 18, the script currently extracts
OCM_TOKEN with jq that preserves surrounding quotes and invokes Python via the
default buffered "python" binary; this breaks CI auth and log streaming. Update
the token extraction to strip quotes (use jq -r or otherwise trim surrounding
quotes) so OCM_TOKEN is the raw token string, change the eval invocation to run
unbuffered Python3 (invoke python3 -u, preferably via /usr/bin/env python3 -u
for portability), and verify AGENT_URL and AGENT_PORT are exported in the CI
environment so set -o nounset will fail fast on missing values.