[CI] Migrate tone_tests to CUDA 13: pull cu130 wheel, adapt sglang/vllm paths, add NCCL env & offline model cache #2811
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -231,7 +231,7 @@ get_whl(){ | |||||
|
|
||||||
| echo "get whl file from github action" | ||||||
| rm -f "$whls_path/mooncake.zip" | ||||||
| rm -f "$whls_path"/*.whl | ||||||
|
|
||||||
| local max_retries=5 | ||||||
| local base_delay=5 # seconds | ||||||
|
|
@@ -401,6 +401,81 @@ cleanup_test_env() { | |||||
| echo "Cleanup completed" | ||||||
| } | ||||||
|
|
||||||
| # Wait until GPU memory on the local host drains below a threshold. | ||||||
| # Returns 0 once drained, 1 if it times out. | ||||||
| wait_gpu_idle() { | ||||||
| local max_seconds=${1:-90} | ||||||
| local threshold_mb=${2:-1024} | ||||||
|
|
||||||
| if ! command -v nvidia-smi >/dev/null 2>&1; then | ||||||
| echo "nvidia-smi not available, skipping GPU drain wait" | ||||||
| return 0 | ||||||
| fi | ||||||
|
|
||||||
| echo "Waiting for GPU memory to drain (threshold ${threshold_mb}MB, timeout ${max_seconds}s)..." | ||||||
| local elapsed=0 | ||||||
| local max_used=0 | ||||||
| while [ $elapsed -lt $max_seconds ]; do | ||||||
| max_used=$(nvidia-smi --query-gpu=memory.used --format=csv,noheader,nounits 2>/dev/null | sort -n | tail -n 1) | ||||||
| [ -z "$max_used" ] && { echo "nvidia-smi query failed, skipping GPU drain wait"; return 0; } | ||||||
| if [ "$max_used" -le "$threshold_mb" ]; then | ||||||
| echo "GPU memory drained (max used ${max_used}MB)" | ||||||
| return 0 | ||||||
| fi | ||||||
| sleep 3 | ||||||
| elapsed=$((elapsed + 3)) | ||||||
| done | ||||||
| echo "GPU memory not drained within ${max_seconds}s (max used ${max_used}MB)" | ||||||
| return 1 | ||||||
| } | ||||||
|
|
||||||
| # Force-kill every host process still holding GPU memory. This catches leftovers | ||||||
| # that an in-container pkill cannot reach: processes reparented to the host | ||||||
| # (orphans) or in a different PID namespace. Only GPU-holding PIDs are targeted. | ||||||
| force_kill_gpu_procs() { | ||||||
| command -v nvidia-smi >/dev/null 2>&1 || return 0 | ||||||
| local pids | ||||||
| pids=$(nvidia-smi --query-compute-apps=pid --format=csv,noheader 2>/dev/null | tr -cd '0-9\n' | grep -E '^[0-9]+$' | sort -u) | ||||||
|
Contributor
There was a problem hiding this comment. Using Using
Suggested change
|
||||||
| [ -z "$pids" ] && return 0 | ||||||
| echo "Force-killing GPU-holding PIDs: $(echo $pids | tr '\n' ' ')" | ||||||
| for pid in $pids; do | ||||||
| kill -9 "$pid" 2>/dev/null || true | ||||||
| done | ||||||
| sleep 3 | ||||||
| } | ||||||
|
|
||||||
| # Fully clear GPU memory on the current node: graceful in-container kill first, | ||||||
| # then host-level force-kill of any process still holding GPU memory. | ||||||
| # Restart the reused container to reset all in-container state (processes, GPU | ||||||
| # memory, ERDMA queue-pairs / RDMA contexts). 'docker restart' keeps the | ||||||
| # writable layer, so the mooncake wheel and ERDMA drivers are NOT reinstalled. | ||||||
| # force_kill_gpu_procs stays as a fallback for leftovers restart cannot reclaim. | ||||||
| drain_gpu_local() { | ||||||
| echo "Restarting container ${CONTAINER_NAME} to reset GPU/ERDMA state..." | ||||||
| docker restart ${CONTAINER_NAME} >/dev/null 2>&1 || true | ||||||
| if ! wait_gpu_idle 60; then | ||||||
| force_kill_gpu_procs | ||||||
| wait_gpu_idle 45 || echo "WARNING: GPU still occupied after restart+force-kill (possible stuck/zombie process or GPU fault)" | ||||||
| fi | ||||||
| } | ||||||
|
|
||||||
| # Between test cases in run-all the container is reused; reset in-container | ||||||
| # state on both the local and (for double-machine runs) remote nodes via a | ||||||
| # lightweight container restart (no wheel / ERDMA driver reinstall). | ||||||
| drain_gpu_between_tests() { | ||||||
| echo "===== Resetting environment between test cases =====" | ||||||
| drain_gpu_local | ||||||
|
|
||||||
| if [ -n "$REMOTE_IP" ]; then | ||||||
| echo "Resetting environment on remote node $REMOTE_IP..." | ||||||
| ${SSH_CMD} "$REMOTE_IP" " | ||||||
| source ${REMOTE_TEST_DIR}/run/.shrc && \ | ||||||
| source ${REMOTE_TEST_DIR}/scripts/common.sh && \ | ||||||
| drain_gpu_local | ||||||
| " 2>/dev/null || true | ||||||
| fi | ||||||
| } | ||||||
|
|
||||||
| setup_node_env() { | ||||||
| local registry_addr=$1 | ||||||
| echo "===== Setting up docker environment =====" | ||||||
|
|
@@ -416,6 +491,7 @@ setup_node_env() { | |||||
| fi | ||||||
|
|
||||||
| local extra_args="" | ||||||
| extra_args="$extra_args -e NCCL_GIN_TYPE=0 " | ||||||
| extra_args="$extra_args --device=/dev/infiniband/uverbs0 --device=/dev/infiniband/uverbs1 --device=/dev/infiniband/rdma_cm " | ||||||
| if [ "${USE_HUGGINGFACE_MIRROR}" = "true" ]; then | ||||||
| extra_args="$extra_args -e HF_ENDPOINT=${HUGGINGFACE_MIRROR} -e HF_HUB_ENABLE_HF_TRANSFER=1" | ||||||
|
|
@@ -840,6 +916,19 @@ collect_and_validate_model_results() { | |||||
| fi | ||||||
| } | ||||||
|
|
||||||
| # Echo an offline env prefix when the given model already exists in the | ||||||
|
There was a problem hiding this comment. The HuggingFace offline helper and its call sites look generally safe, but there are a couple of behavioral edge cases worth tightening or at least documenting. hf_offline_prefix() in scripts/tone_tests/scripts/common.sh builds a cache_dir name of
This is good in that offline mode is only enabled when a cache snapshot is detected, but it assumes that:
If the cache structure changes, or if a partial snapshot exists (config.json present but other files missing), we might flip into offline mode and end up with non-obvious startup failures that are hard to debug from CI logs. On the flip side, if the snapshot is valid but located differently (or config.json is absent for some reason), the tests will unexpectedly go online again and hit the hub/mirror, reintroducing the 429 risk this PR is trying to avoid. Would it make sense to either (a) harden the detection (e.g., also checking for weights or a known file) and log when offline mode is enabled, or (b) treat missing cache as a more explicit failure for these tone tests so we don’t silently flip back to online behavior? 🤖 Generated by Qoder • Fix in Qoder |
||||||
| # container's HuggingFace cache, so servers use the local snapshot instead of | ||||||
| # querying the hub (skips downloads and avoids hf-mirror 429 rate limiting). | ||||||
| # Models are pre-cached under MODEL_CACHE (mounted at /root/.cache) on both nodes. | ||||||
| hf_offline_prefix() { | ||||||
| local model_name=$1 | ||||||
| [ -z "$model_name" ] && return 0 | ||||||
| local cache_dir="models--$(echo "$model_name" | sed 's#/#--#g')" | ||||||
| if ${docker_exec} "ls /root/.cache/huggingface/hub/${cache_dir}/snapshots/*/config.json >/dev/null 2>&1"; then | ||||||
| echo "HF_HUB_OFFLINE=1 TRANSFORMERS_OFFLINE=1 " | ||||||
| fi | ||||||
| } | ||||||
|
|
||||||
| launch_sglang_server() { | ||||||
| local model_path=$1 | ||||||
| local host=$2 | ||||||
|
|
@@ -855,7 +944,8 @@ launch_sglang_server() { | |||||
| return 1 | ||||||
| fi | ||||||
|
|
||||||
| local offline_prefix=$(hf_offline_prefix "$model_path") | ||||||
| local sglang_cmd="${docker_exec} \"${offline_prefix}python -m sglang.launch_server --model-path ${model_path} --host ${host} --port ${port}" | ||||||
| if [ -n "$extra_args" ]; then | ||||||
| sglang_cmd="${sglang_cmd} ${extra_args}" | ||||||
| fi | ||||||
|
|
@@ -905,6 +995,7 @@ launch_vllm_server() { | |||||
| if [ -n "$env_vars" ]; then | ||||||
| env_prefix="${env_vars} " | ||||||
| fi | ||||||
| env_prefix="${env_prefix}$(hf_offline_prefix "$model_path")" | ||||||
|
|
||||||
| local vllm_cmd="${docker_exec} \"${env_prefix}python3 -m vllm.entrypoints.openai.api_server --model '${model_path}' --host '${host}' --port ${port}" | ||||||
|
|
||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new GPU drain helpers added around wait_gpu_idle(), force_kill_gpu_procs(), and drain_gpu_local()/drain_gpu_between_tests in scripts/tone_tests/scripts/common.sh look like they assume the CI node is fully dedicated to Mooncake tests, but the host-wide kill behavior is pretty aggressive.
In drain_gpu_local(), if GPU memory has not dropped below the threshold after a container restart, the code calls force_kill_gpu_procs(), which uses
nvidia-smi --query-compute-apps=pidto enumerate all GPU-holding PIDs on the host and then sendskill -9to each of them. drain_gpu_between_tests() calls drain_gpu_local for every case in run_all_tests(), and also invokes it on the remote node via SSH.On a shared CI host or any environment where other GPU workloads run alongside tone_tests, this will repeatedly and silently terminate unrelated jobs (training runs, other services, etc.), which is a pretty big safety/availability risk. There’s no scoping to the Mooncake container, user, or any job-specific tag; the only filter is "uses GPU compute".
If the operational assumption really is "these nodes are single-tenant and reserved for Mooncake CI", it would be helpful to either codify that constraint or tighten the kill logic so it only targets processes launched by this test harness (e.g., via container PID namespace, cgroups, or a known user). Otherwise, it might be safer to treat "GPU did not drain within timeout" as a hard failure for the suite rather than attempting to clear arbitrary GPU workloads on the host.
🤖 Generated by Qoder • Fix in Qoder