Skip to content

ci: aikit test #1395

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.

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

Merged
merged 9 commits into from
Aug 15, 2025
Merged

ci: aikit test #1395

merged 9 commits into from
Aug 15, 2025

Conversation

sozercan
Copy link
Collaborator

@sozercan sozercan commented Aug 15, 2025

Reason for Change:

  • add aikit test with llama3.2:1b image
  • runs on a kind cluster
  • this does not require any gpus, and can be pulled and run fast on default github runners (takes ~30 seconds from workspace deployment to LLM response)

Requirements

  • added unit tests and e2e tests (if applicable).

Issue Fixed:

Fixes #1375

Notes for Reviewers:

Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Copy link

Title

Add AIKit Integration Test Workflow


Description

  • Added AIKit integration test workflow

  • Configured to use llama3.2:1b image

  • Validates model response and completion


Changes walkthrough 📝

Relevant files
Tests
aikit.yaml
AIKit Integration Test Workflow                                                   

.github/workflows/aikit.yaml

  • Created new workflow for AIKit integration testing
  • Configured environment variables for KIND and KUBECTL versions
  • Added steps to install dependencies, create a KIND cluster, and
    build/deploy workspace controller
  • Included steps to create AIKit workspace, validate inference
    readiness, and test model responses
  • Added debug info step for troubleshooting
  • +225/-0 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Timeout Handling

    The timeout handling in the Wait for inference to be ready step could be improved. The script waits for a maximum of 300 seconds, but it does not handle cases where the inference might take longer than expected or where the Kubernetes cluster might be slow to respond. Consider adding more robust error handling and logging to understand why the timeout might occur.

    echo "Waiting for inference to be ready..."
    timeout=300
    while [ $timeout -gt 0 ]; do
      inference_status=$(kubectl get workspace workspace-aikit-test -o jsonpath='{.status.conditions[?(@.type=="InferenceReady")].status}' 2>/dev/null || echo "")
      if [ "$inference_status" = "True" ]; then
        echo "InferenceReady condition is True"
        break
      fi
      echo "Waiting for InferenceReady condition... (status: $inference_status, $timeout seconds remaining)"
      sleep 10
      timeout=$((timeout - 10))
    done
    
    if [ $timeout -le 0 ]; then
      echo "Timeout waiting for InferenceReady condition"
      exit 1
    fi
    
    Port Forwarding Cleanup

    The port forwarding process started in the Validate response content step is killed using kill $PORT_FORWARD_PID. This approach might not be reliable if the process ID changes or if the process does not terminate cleanly. Consider using pkill or killall with a more specific identifier to ensure the port forwarding process is terminated properly.

    kubectl port-forward service/workspace-aikit-test 8080:80 &
    PORT_FORWARD_PID=$!
    
    # Wait for port-forward to be ready
    sleep 5
    
    # Test that we get expected model information
    echo 'Validating model response contains expected data...'
    MODELS_RESPONSE=$(curl -s http://localhost:8080/v1/models)
    echo "Models response: $MODELS_RESPONSE"
    
    # Extract the first model name from the response
    MODEL_NAME=$(echo "$MODELS_RESPONSE" | grep -o '"id":[[:space:]]*"[^"]*"' | head -1 | sed 's/"id":[[:space:]]*"\([^"]*\)"/\1/')
    echo "Extracted model name: $MODEL_NAME"
    
    # Check if response contains 'llama'
    if echo "$MODELS_RESPONSE" | grep -i 'llama'; then
      echo 'Model response validation passed!'
    else
      echo 'Model response validation failed - no llama model found'
      kill $PORT_FORWARD_PID
      exit 1
    fi
    
    echo 'Testing completion response...'
    COMPLETION_RESPONSE=$(curl -s -X POST \
      -H 'Content-Type: application/json' \
      -d "{
        \"model\": \"$MODEL_NAME\",
        \"messages\": [{\"role\": \"user\", \"content\": \"hello\"}]
      }" \
      http://localhost:8080/v1/chat/completions)
    echo "Completion response: $COMPLETION_RESPONSE"
    
    # Check if we get some response with expected fields
    if echo "$COMPLETION_RESPONSE" | grep -E '(choices|text|usage)'; then
      echo 'Completion response validation passed!'
    else
      echo 'Completion response validation failed - missing expected fields'
      kill $PORT_FORWARD_PID
      exit 1
    fi
    
    # Clean up port forwarding
    kill $PORT_FORWARD_PID

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add error handling

    Validate the JSON payload structure and ensure that the API endpoint supports the
    provided request format. Consider adding error handling for the API response.

    .github/workflows/aikit.yaml [187]

     COMPLETION_RESPONSE=$(curl -s -X POST \
       -H 'Content-Type: application/json' \
       -d "{
         \"model\": \"$MODEL_NAME\",
         \"messages\": [{\"role\": \"user\", \"content\": \"hello\"}]
       }" \
       http://localhost:8080/v1/chat/completions)
    +if [ $? -ne 0 ]; then
    +  echo 'Failed to get completion response'
    +  kill $PORT_FORWARD_PID
    +  exit 1
    +fi
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion adds basic error handling for the API response, which is a good practice but not critical for the functionality.

    Low
    Limit log output

    Limit the log output to a reasonable number of lines to avoid excessive logging and
    potential performance issues.

    .github/workflows/aikit.yaml [218]

    -kubectl logs -l kaito.sh/workspace=workspace-aikit-test --tail=-1
    +kubectl logs -l kaito.sh/workspace=workspace-aikit-test --tail=100
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion limits the log output to a reasonable number of lines, which is a minor improvement for performance and readability.

    Low
    Possible issue
    Specify namespace for pods

    Ensure that the label selector matches the actual pods created by the workspace.
    Verify the pod labels using kubectl get pods -l
    kaito.sh/workspace=workspace-aikit-test -n ${{ env.KAITO_NAMESPACE }}.

    .github/workflows/aikit.yaml [146]

    -kubectl wait --for=condition=Ready --timeout=300s pods -l kaito.sh/workspace=workspace-aikit-test
    +kubectl wait --for=condition=Ready --timeout=300s pods -l kaito.sh/workspace=workspace-aikit-test -n ${{ env.KAITO_NAMESPACE }}
    Suggestion importance[1-10]: 3

    __

    Why: The suggestion adds the namespace to the pod label selector, which is a minor improvement for clarity and correctness.

    Low

    Co-authored-by: Ernest Wong <chwong719@gmail.com>
    Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
    Co-authored-by: Ernest Wong <chwong719@gmail.com>
    Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
    Co-authored-by: Ernest Wong <chwong719@gmail.com>
    Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
    Signed-off-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
    Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
    @chewong chewong merged commit 3182f25 into kaito-project:main Aug 15, 2025
    14 of 16 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    test: add e2e regression test for AIKit image
    2 participants