Merge pull request #239 from carbonin/fix-eval

Make eval tests more reliable

Author: openshift-merge-bot[bot]

Containerfile.add_llama_to_lightspeed

@@ -11,6 +11,13 @@ RUN cd /app-root/llama-stack && python3.12 -m pip install --editable .
 
 RUN cd /app-root/ && python3.12 -m pip install .
 
+RUN microdnf install -y patch
+
+# Patch created based on https://github.com/llamastack/llama-stack/pull/3957
+COPY  llama-stack-make-tool-call-args-fully-recursive.patch /tmp/
+RUN cat /tmp/llama-stack-make-tool-call-args-fully-recursive.patch \
+  | patch -p1 -d "$(dirname "$(dirname "$(python3.12 -c "import llama_stack; print(llama_stack.__file__)")")")"
+
 COPY migrate.py /app/migrate.py
 ENTRYPOINT ["/bin/sh", "-c", "python3.12 /app/migrate.py && python3.12 src/lightspeed_stack.py"]
 

Containerfile.assisted-chat

@@ -2,6 +2,15 @@
 # This is the digest of quay.io/lightspeed-core/lightspeed-stack:0.3.0
 FROM quay.io/lightspeed-core/lightspeed-stack@sha256:d1805df92f4de55d662e6274328830e2c1300f308c258abfcfad825a241cb50d
 
+USER root
+
+RUN microdnf install -y patch
+
+# Patch created based on https://github.com/llamastack/llama-stack/pull/3957
+COPY  llama-stack-make-tool-call-args-fully-recursive.patch /tmp/
+RUN cat /tmp/llama-stack-make-tool-call-args-fully-recursive.patch \
+  | patch -p1 -d "$(dirname "$(dirname "$(python3.12 -c "import llama_stack; print(llama_stack.__file__)")")")"
+
 COPY migrate.py /app/migrate.py
 ENTRYPOINT ["/bin/sh", "-c", "python3.12 /app/migrate.py && python3.12 src/lightspeed_stack.py"]
 

assisted-service-mcp

@@ -0,0 +1,13 @@
+diff --git a/llama_stack/models/llama/datatypes.py b/llama_stack/models/llama/datatypes.py
+index 7f1ebed5..f5e26ec3 100644
+--- a/llama_stack/models/llama/datatypes.py
++++ b/llama_stack/models/llama/datatypes.py
+@@ -31,7 +31,7 @@ class BuiltinTool(Enum):
+ 
+ 
+ Primitive = str | int | float | bool | None
+-RecursiveType = Primitive | list[Primitive] | dict[str, Primitive]
++type RecursiveType = Primitive | list[RecursiveType] | dict[str, RecursiveType]
+ 
+ 
+ class ToolCall(BaseModel):

llama-stack-make-tool-call-args-fully-recursive.patch

@@ -2,9 +2,8 @@
   conversation:
     - eval_id: basic_introduction
       eval_query: Hi!
-      eval_types: [response_eval:accuracy]
-      expected_response: Hi! I'm the Red Hat OpenShift Lightspeed Intelligent Assistant, and I'm here to guide you through installing OpenShift using the Assisted Installer.
-      description: Basic greeting test using keyword matching for reliability (avoids LLM judge flapping)
+      eval_types: [response_eval:intent]
+      expected_intent: A basic greeting that indicates willingess to help with installing OpenShift
 
 - conversation_group: basic_cluster_request_conv
   conversation:
@@ -56,18 +55,8 @@
   conversation:
     - eval_id: create_eval_test_sno
       eval_query: create a new single node cluster named eval-test-singlenode-uniq-cluster-name, running on version 4.19.7 with the x86_64 CPU architecture, configured under the base domain example.com, using the provided SSH key "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQCmeaBFhSJ/MLECmqUaKweRgo10ABpwdvJ7v76qLYfP0pzfzYsF3hGP/fH5OQfHi9pTbWynjaEcPHVfaTaFWHvyMtv8PEMUIDgQPWlBSYzb+3AgQ5AsChhzTJCYnRdmCdzENlV+azgtb3mVfXiyCfjxhyy3QAV4hRrMaVtJGuUQfQ== example@example.com".
-      eval_types: [tool_eval, response_eval:sub-string, response_eval:accuracy, action_eval]
-      expected_tool_calls:
-        - - tool_name: create_cluster
-            arguments:
-              name: "eval-test-singlenode-uniq-cluster-name"
-              version: "4\\.19\\.7"
-              base_domain: "example\\.com"
-              single_node: "(?i:true)"
-              cpu_architecture: "x86_64"
-              ssh_public_key: 'ssh-rsa\s+[A-Za-z0-9+/]+[=]{0,3}(\s+.+)?\s*'
-              platform: "none"
-      eval_verify_script: ../scripts/cluster_created_check.sh
+      eval_types: [response_eval:sub-string, response_eval:accuracy, action_eval]
+      eval_verify_script: ../scripts/verify_create_eval_test_sno.sh
       expected_keywords: ["eval-test-singlenode-uniq-cluster-name", "ID", "Discovery ISO", "download", "cluster"]
       expected_response: I have created a cluster with name eval-test-singlenode-uniq-cluster-name. Next, you'll need to download the Discovery ISO, then boot your hosts with it. Would you like me to get the Discovery ISO download URL?
     - eval_id: get_iso_eval_test_sno
@@ -84,19 +73,9 @@
   cleanup_script: ../scripts/delete_cluster.sh
   conversation:
     - eval_id: create_eval_test_multinode
-      eval_types: [tool_eval, response_eval:accuracy, response_eval:sub-string, action_eval]
+      eval_types: [response_eval:accuracy, response_eval:sub-string, action_eval]
       eval_query: Create a multi-node cluster named 'eval-test-multinode-uniq-cluster-name' with OpenShift 4.18.22 and domain test.local and with the x86_64 CPU architecture.
-      expected_tool_calls:
-        - - tool_name: create_cluster
-            arguments:
-              name: "eval-test-multinode-uniq-cluster-name"
-              version: "4\\.18\\.22"
-              base_domain: "test\\.local"
-              single_node: "(?i:false)"
-              cpu_architecture: "x86_64"
-              ssh_public_key: ""
-              platform: "baremetal"
-      eval_verify_script: ../scripts/cluster_created_check.sh
+      eval_verify_script: ../scripts/verify_create_eval_test_multinode.sh
       expected_keywords: ["eval-test-multinode-uniq-cluster-name", "ID", "Discovery ISO", "cluster"]
       expected_response: I have created a cluster with name eval-test-multinode-uniq-cluster-name. Before downloading the Discovery ISO, would you like to configure static network configuration for the hosts? If not, the cluster will use Dynamic Host Configuration Protocol (DHCP) for host networking configuration by default.
     - eval_id: set_ssh_key_eval_test_ssh
@@ -135,8 +114,8 @@
     - eval_id: cluster_info_tool_call
       eval_query: Give me details about cluster named 'abc123abc'
       description: Test handling for non-existent cluster name
-      eval_types: [tool_eval, response_eval:accuracy]
-      expected_response: Retrieval failed for cluster 'abc123abc' because the resource was not found.
+      eval_types: [tool_eval, response_eval:intent]
+      expected_intent: An explanation indicating that a cluster with the name abc123abc does not exist
       expected_tool_calls:
         - - tool_name: list_clusters
             arguments: {}
@@ -161,16 +140,16 @@
   conversation:
     - eval_id: refuse_azure_request
       eval_query: Can you create a cluster on Azure?
-      eval_types: [response_eval:accuracy]
-      expected_response: Sorry, I can't create clusters directly on public clouds like Azure. I am designed to assist with on-premise OpenShift installations using the Assisted Installer. For installing OpenShift on Azure, you can explore Red Hat OpenShift Service on Azure or use the standard OpenShift Installer.
+      eval_types: [response_eval:intent]
+      expected_intent: An explanation that indicates clusters cannot be created on public clouds and a reminder that the scope of this installer is on-premise installations
       description: Refuse out-of-scope request to create clusters on Azure; clarify on-prem scope and Assisted Installer
 
 - conversation_group: capabilities_scope_conv
   conversation:
     - eval_id: supported_platforms_summary
       eval_query: Which platforms can you help me install on?
-      eval_types: [response_eval:accuracy]
-      expected_response: I can help you install OpenShift on-premise using the Assisted Installer, either on bare metal servers or virtual machines (VMs) like vSphere, KVM or libvirt. I do not support public cloud platforms like Amazon Web Services (AWS), Azure, or Google Cloud Platform (GCP).
+      eval_types: [response_eval:intent]
+      expected_intent: An explanation that indicates on-premise platforms are supported, but public cloud platforms are not.
       description: Ensure the assistant concisely states supported on-prem baremetal/VM scope and references Assisted Installer
 
 - conversation_group: cluster_id_from_name
@@ -195,33 +174,14 @@
   conversation:
     - eval_id: create_eval_test_sno
       eval_query: create a new single node cluster named eval-test-static-net-cluster-uniq-cluster-name, running on version 4.19.7 with the x86_64 CPU architecture, configured under the base domain example.com, without an ssh key.
-      eval_types: [tool_eval, action_eval]
-      eval_verify_script: ../scripts/cluster_created_check.sh
-      expected_tool_calls:
-        - - tool_name: create_cluster
-            arguments:
-              name: "eval-test-static-net-cluster-uniq-cluster-name"
-              version: "4\\.19\\.7"
-              base_domain: "example\\.com"
-              single_node: "(?i:true)"
-              cpu_architecture: "x86_64"
-              ssh_public_key: ""
-              platform: "none"
+      eval_types: [action_eval]
+      eval_verify_script: ../scripts/verify_create_eval_test_sno_static_net.sh
     - eval_id: configure_hosts
       eval_query: |
         I want to configure static networking. Create configs with a single vlan interface backed by an ethernet interface. It should have an ethernet interface with mac address c5:d6:bc:f0:05:20, and the vlan interface has ip address 10.0.0.5/24. The vlan id is 400. Use the name eth0 for the ethernet interface and vlan0 as the name of the vlan interface. Also I want DNS config with a DNS server 8.8.8.8.
-      eval_types: [tool_eval]
-      expected_tool_calls:
-        - - tool_name: generate_nmstate_yaml
-            arguments: 
-              params: |-
-                (?s)^(?=.*"ethernet_ifaces":\s*\[\s*\{(?=.*"mac_address":\s*"c5:d6:bc:f0:05:20")(?=.*"name":\s*"eth0").*?\}\s*\])(?=.*"vlan_ifaces":\s*\[\s*\{(?=.*?"vlan_id":\s*400\b)(?=.*?"name":\s*"vlan0")(?=.*?"base_interface_name":\s*"eth0")(?=.*?"ipv4_address":\s*\{(?=.*?"address":\s*"10\.0\.0\.5")(?=.*?"cidr_length":\s*24\b).*?\}).*?\}\s*\])(?=.*"dns":\s*\{\s*"dns_servers":\s*\[\s*"8\.8\.8\.8"\s*\]\s*\}).*$
+      eval_types: [response_eval:intent]
+      expected_intent: Acknowledgement of the desired configuration and a request for the user to validate the configuration before applying it.
     - eval_id: apply_to_cluster
       eval_query: Yes, apply it to the cluster you just created.
-      eval_types: [tool_eval]
-      expected_tool_calls:
-        - - tool_name: alter_static_network_config_nmstate_for_host
-            arguments:
-              cluster_id: "[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}"
-              index: null
-              new_nmstate_yaml: "10.0.0.5"
+      eval_types: [action_eval]
+      eval_verify_script: ../scripts/verify_static_net_apply_to_cluster.sh

test/evals/eval_data.yaml

@@ -11,17 +11,12 @@ RUN python3.12 -m ensurepip
 RUN cd /app-root/llama-stack && python3.12 -m pip install --editable .
 RUN cd /app-root/ && python3.12 -m pip install .
 
-# Apply upstream patch required by current revision (make it optional if it fails)
+RUN microdnf install -y patch
 
-# Patch taken from https://github.com/meta-llama/llama-stack/commit/fd466b0459bfa7cc696ac80dba90b6e02d5869bd.patch
-COPY meta-llama.llama-stack.commit.fd466b0459bfa7cc696ac80dba90b6e02d5869bd.patch /tmp/
-
-RUN microdnf install -y patch && \
-    LLAMA_STACK_PATH=$(python3.12 -c "import llama_stack; import os; print(os.path.dirname(os.path.dirname(llama_stack.__file__)))") && \
-    echo "Applying patch to: $LLAMA_STACK_PATH" && \
-    cat /tmp/meta-llama.llama-stack.commit.fd466b0459bfa7cc696ac80dba90b6e02d5869bd.patch \
-      | sed '/^diff --git a\/tests\/unit\/server\/test_replace_env_vars.py b\/tests\/unit\/server\/test_replace_env_vars.py/,/^diff --git /{ /^diff --git /!d }' \
-      | patch -p1 -d "$LLAMA_STACK_PATH" || echo "Patch application failed, continuing anyway"
+# Patch created based on https://github.com/llamastack/llama-stack/pull/3957
+COPY  llama-stack-make-tool-call-args-fully-recursive.patch /tmp/
+RUN cat /tmp/llama-stack-make-tool-call-args-fully-recursive.patch \
+  | patch -p1 -d "$(dirname "$(dirname "$(python3.12 -c "import llama_stack; print(llama_stack.__file__)")")")"
 
 USER 1001
 EXPOSE 8080

test/prow/Dockerfile.plus-llama

@@ -1,13 +1,14 @@
 #!/bin/bash
 
-set -o nounset
-set -o errexit
-set -o pipefail
+# Source the common helper functions
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+source "${SCRIPT_DIR}/common.sh"
+
+setup_shell_options
+validate_environment
 
-: "${OCM_TOKEN:?OCM_TOKEN is required}"
-: "${UNIQUE_ID:?UNIQUE_ID is required}"
 OCM_BASE_URL=${OCM_BASE_URL:-https://api.stage.openshift.com}
-ASSISTED_SERVICE_URL="${OCM_BASE_URL}/api/assisted-install/v2"
+ASSISTED_SERVICE_URL=$(get_assisted_service_url)
 
 PULL_SECRET_RAW="$(
   curl -sSf -X POST \
@@ -55,7 +56,7 @@ curl -sSf -X POST \
 COUNTER=0
 while ! curl -sSf -H "Authorization: Bearer ${OCM_TOKEN}" "${ASSISTED_SERVICE_URL}/clusters/${CLUSTER_ID}"; do
     if [[ $COUNTER -gt 3 ]]; then
-        echo "Cluster creation timed out"
+        echo_err "Cluster creation timed out"
         exit 1
     fi
     ((COUNTER++))

test/scripts/cluster_create_test_setup.sh

@@ -0,0 +1,141 @@
+#!/bin/bash
+
+# Helper function to print to stderr
+echo_err() {
+    echo "$@" >&2
+}
+
+# Helper function to print to stdout (for clarity, though echo does this by default)
+echo_out() {
+    echo "$@"
+}
+
+# Setup common bash settings
+setup_shell_options() {
+    set -o nounset
+    set -o errexit
+    set -o pipefail
+}
+
+# Validate required environment variables
+validate_environment() {
+    : "${OCM_TOKEN:?OCM_TOKEN is required}"
+    : "${UNIQUE_ID:?UNIQUE_ID is required}"
+}
+
+# Get the assisted service URL
+get_assisted_service_url() {
+    echo "${OCM_BASE_URL:-https://api.stage.openshift.com}/api/assisted-install/v2"
+}
+
+# Fetch cluster data by name
+# Usage: fetch_cluster_data <cluster_name>
+fetch_cluster_data() {
+    local cluster_name="$1"
+    local service_url=$(get_assisted_service_url)
+    curl -sSf -H "Authorization: Bearer ${OCM_TOKEN}" "${service_url}/clusters" | \
+        jq -r ".[] | select(.name == \"${cluster_name}\")"
+}
+
+# Fetch infra env data by name
+# Usage: fetch_infra_env_data <infra_env_name>
+fetch_infra_env_data() {
+    local infra_env_name="$1"
+    local service_url=$(get_assisted_service_url)
+    curl -sSf -H "Authorization: Bearer ${OCM_TOKEN}" "${service_url}/infra-envs" | \
+        jq -r ".[] | select(.name == \"${infra_env_name}\")"
+}
+
+# Extract cluster properties from cluster data
+# Usage: extract_cluster_properties <cluster_data>
+# Outputs: ACTUAL_VERSION, ACTUAL_DOMAIN, ACTUAL_SINGLE_NODE, ACTUAL_CPU_ARCH, ACTUAL_SSH_KEY
+extract_cluster_properties() {
+    local cluster_data="$1"
+    ACTUAL_VERSION=$(echo "$cluster_data" | jq -r '.openshift_version')
+    ACTUAL_DOMAIN=$(echo "$cluster_data" | jq -r '.base_dns_domain')
+    ACTUAL_SINGLE_NODE=$(echo "$cluster_data" | jq -r '.high_availability_mode == "None"')
+    ACTUAL_CPU_ARCH=$(echo "$cluster_data" | jq -r '.cpu_architecture')
+    ACTUAL_SSH_KEY=$(echo "$cluster_data" | jq -r '.ssh_public_key')
+}
+
+# Validate cluster properties (version, domain, single_node, cpu_arch, ssh_key)
+# Usage: validate_cluster_properties <expected_version> <expected_domain> <expected_single_node> <expected_cpu_arch> [expected_ssh_key]
+validate_cluster_properties() {
+    local expected_version="$1"
+    local expected_domain="$2"
+    local expected_single_node="$3"
+    local expected_cpu_arch="$4"
+    local expected_ssh_key="${5:-}"
+
+    if [[ "$ACTUAL_VERSION" == "$expected_version" && \
+          "$ACTUAL_DOMAIN" == "$expected_domain" && \
+          "$ACTUAL_SINGLE_NODE" == "$expected_single_node" && \
+          "$ACTUAL_CPU_ARCH" == "$expected_cpu_arch" ]]; then
+        # If SSH key is provided, validate it too
+        if [[ -n "$expected_ssh_key" ]]; then
+            if [[ "$ACTUAL_SSH_KEY" == "$expected_ssh_key" ]]; then
+                return 0
+            else
+                return 1
+            fi
+        else
+            return 0
+        fi
+    else
+        return 1
+    fi
+}
+
+
+# Wait for cluster to exist and validate
+# Usage: wait_and_validate_cluster <cluster_name_prefix> <expected_version> <expected_domain> <expected_single_node> <expected_cpu_arch> <cluster_type> [expected_ssh_key]
+wait_and_validate_cluster() {
+    local cluster_name_prefix="$1"
+    local expected_version="$2"
+    local expected_domain="$3"
+    local expected_single_node="$4"
+    local expected_cpu_arch="$5"
+    local cluster_type="$6"
+    local expected_ssh_key="${7:-}"
+
+    local cluster_name="${cluster_name_prefix}-${UNIQUE_ID}"
+
+    local counter=0
+    while true; do
+        local cluster_data=$(fetch_cluster_data "$cluster_name")
+
+        if [[ -n "$cluster_data" && "$cluster_data" != "null" ]]; then
+            extract_cluster_properties "$cluster_data"
+            if validate_cluster_properties "$expected_version" "$expected_domain" "$expected_single_node" "$expected_cpu_arch" "$expected_ssh_key"; then
+                echo_out "The ${cluster_type} cluster was successfully created with correct configuration:"
+                echo_out "  Name: ${cluster_name}"
+                echo_out "  Version: ${ACTUAL_VERSION}"
+                echo_out "  Domain: ${ACTUAL_DOMAIN}"
+                echo_out "  Single Node: ${ACTUAL_SINGLE_NODE}"
+                echo_out "  CPU Architecture: ${ACTUAL_CPU_ARCH}"
+                if [[ -n "$expected_ssh_key" ]]; then
+                    echo_out "  SSH Key: ${ACTUAL_SSH_KEY}"
+                fi
+                exit 0
+            else
+                echo_err "Cluster found but configuration mismatch:"
+                echo_err "  Expected version: ${expected_version}, got: ${ACTUAL_VERSION}"
+                echo_err "  Expected domain: ${expected_domain}, got: ${ACTUAL_DOMAIN}"
+                echo_err "  Expected single node: ${expected_single_node}, got: ${ACTUAL_SINGLE_NODE}"
+                echo_err "  Expected CPU architecture: ${expected_cpu_arch}, got: ${ACTUAL_CPU_ARCH}"
+                if [[ -n "$expected_ssh_key" ]]; then
+                    echo_err "  Expected SSH key: ${expected_ssh_key}, got: ${ACTUAL_SSH_KEY}"
+                fi
+                exit 1
+            fi
+        fi
+
+        if [[ $counter -ge 3 ]]; then
+            echo_err "Cluster creation timed out"
+            exit 1
+        fi
+        ((counter++))
+        sleep 10
+    done
+}
+