From f4e378e8b508f1f061d9fb7bd957d3c6d82d61ec Mon Sep 17 00:00:00 2001 From: danielhanchen Date: Fri, 12 Jun 2026 05:31:24 +0000 Subject: [PATCH] docker: review fixes from the 8-reviewer pass and staging CI entrypoint.sh: a container started without a GPU request has no nvidia-smi at all (the toolkit injects it), so the old check 1 reported 'CUDA runtime in this image is broken, re-pull' for the most common user error. Fold the missing-binary case into the actionable 'No GPU visible' message and document the CPU-only option (UNSLOTH_ALLOW_CPU=1). run.sh / test_locally.sh: guard empty-array expansions with the ${arr[@]+...} form; bash 3.2 (macOS /bin/bash) treats "${empty[@]}" as unbound under set -u, which broke the documented macOS CPU path. studio_launch.sh: exclude *_TOKEN, *_API_KEY, *_PASSWORD, *_SECRET, *_LICENSE from the env snapshot written for SSH sessions; secrets stay in process env only, never on disk. supervisord.conf / Dockerfile.studio: pin HOME=/root for the studio and jupyter programs (jupyter would silently fall back to token auth if HOME were unset), default JUPYTER_PORT and UNSLOTH_ENABLE_SSHD at the image level so a direct supervisord invocation cannot hit a bad %(ENV_*)s expansion, and document the root-services decision (non-root parity with the previous production image is a tracked follow-up). docker_confirm.ps1: mirror the bash script's GPU selector translation so GPUS=0 / 0,1 select devices instead of silently using all GPUs. docker-publish.yml: studio cache scope moves to mode=min; a mode=max cache of a ~24GB image would evict everything else in the 10GB GHA quota for no hit-rate gain. --- .github/workflows/docker-publish.yml | 5 ++++- docker/Dockerfile.studio | 11 +++++++++++ docker/docker_confirm.ps1 | 13 ++++++++++--- docker/entrypoint.sh | 21 ++++++++++++--------- docker/run.sh | 8 +++++--- docker/studio_launch.sh | 5 ++++- docker/supervisord.conf | 5 +++++ docker/test_locally.sh | 2 +- 8 files changed, 52 insertions(+), 18 deletions(-) diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index b4b174e468..710017df6e 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -286,8 +286,11 @@ jobs: file: ./docker/Dockerfile.studio platforms: ${{ matrix.platform }} labels: ${{ steps.meta.outputs.labels }} + # mode=min (final layers only): a mode=max cache of this ~24GB + # image would blow straight through the 10GB per-repo GHA cache + # quota and evict the base build's cache for zero hit-rate gain. cache-from: type=gha,scope=studio-${{ matrix.platform }} - cache-to: type=gha,scope=studio-${{ matrix.platform }},mode=max + cache-to: type=gha,scope=studio-${{ matrix.platform }},mode=min outputs: type=image,name=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }},push-by-digest=true,name-canonical=true,push=true build-args: | BASE_IMAGE=${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}@${{ needs.merge.outputs.digest }} diff --git a/docker/Dockerfile.studio b/docker/Dockerfile.studio index 8afd9e9819..eafe4ea95e 100644 --- a/docker/Dockerfile.studio +++ b/docker/Dockerfile.studio @@ -35,8 +35,19 @@ FROM ${BASE_IMAGE} ARG UNSLOTH_STUDIO_REF=main ARG TARGETARCH +# Services run as root in this revision (the base image is root-only by +# design); the previous production image ran them as a dedicated uid-1001 +# user. Non-root parity is a tracked follow-up. sshd is key-only and stays +# disabled unless a PUBLIC_KEY/SSH_KEY is provided, and no secrets are +# persisted to disk (see studio_launch.sh). +# +# The JUPYTER_PORT / UNSLOTH_ENABLE_SSHD defaults exist so supervisord's +# %(ENV_*)s expansions still resolve when someone bypasses the launcher +# and runs supervisord directly. USER root ENV UNSLOTH_STUDIO_HOME=/opt/unsloth-studio \ + JUPYTER_PORT=8888 \ + UNSLOTH_ENABLE_SSHD=false \ DEBIAN_FRONTEND=noninteractive # install.sh needs curl + git; supervisor + openssh-server run the service diff --git a/docker/docker_confirm.ps1 b/docker/docker_confirm.ps1 index 5ef1f71892..9eeb5f13f8 100644 --- a/docker/docker_confirm.ps1 +++ b/docker/docker_confirm.ps1 @@ -96,9 +96,16 @@ Hr # 3) Container runtime check -------------------------------------------------- Bold "3) Container runtime check" +# Mirror docker_confirm.sh's GPU selector translation: bare indices and +# comma lists become device= selectors (Docker reads a bare integer for +# --gpus as a COUNT, not an index). +$GPU_SELECTOR = "all" +if ($GPUS -notin @("auto", "all", "none")) { + $GPU_SELECTOR = if ($GPUS -like "device=*") { $GPUS } else { "`"device=$GPUS`"" } +} if ($GPU_MODE) { $log = Join-Path $WORK "gpu_check.log" - docker run --rm --gpus all $BASE_IMAGE python -c "import torch; assert torch.cuda.is_available(); print('torch', torch.__version__, '-', torch.cuda.get_device_name(0))" *> $log + docker run --rm --gpus $GPU_SELECTOR $BASE_IMAGE python -c "import torch; assert torch.cuda.is_available(); print('torch', torch.__version__, '-', torch.cuda.get_device_name(0))" *> $log if ($LASTEXITCODE -eq 0) { Ok ("torch.cuda available in-container: " + (Get-Content $log -Tail 1)) } else { @@ -124,7 +131,7 @@ Bold "4) Training smoke" if ($GPU_MODE -and -not $SKIP_TRAIN) { $log = Join-Path $WORK "train_smoke.log" $hfArgs = @(); if ($env:HF_TOKEN) { $hfArgs = @("-e", "HF_TOKEN") } - docker run --rm --gpus all --ipc=host @hfArgs $BASE_IMAGE python /workspace/smoke_test.py *> $log + docker run --rm --gpus $GPU_SELECTOR --ipc=host @hfArgs $BASE_IMAGE python /workspace/smoke_test.py *> $log if ($LASTEXITCODE -eq 0) { Ok "smoke_test.py: 5 LoRA steps completed" Select-String -Path $log -Pattern "^step|loss" | Select-Object -Last 5 | ForEach-Object { Info $_.Line } @@ -153,7 +160,7 @@ Hr # 6) Studio + JupyterLab ------------------------------------------------------ Bold "6) Studio + JupyterLab (full image)" $runArgs = @("-d", "-p", "${PORT_STUDIO}:8000", "-p", "${PORT_JUPYTER}:8888") -if ($GPU_MODE) { $runArgs += @("--gpus", "all") } else { $runArgs += @("-e", "UNSLOTH_ALLOW_CPU=1") } +if ($GPU_MODE) { $runArgs += @("--gpus", $GPU_SELECTOR) } else { $runArgs += @("-e", "UNSLOTH_ALLOW_CPU=1") } $script:STUDIO_CID = (docker run @runArgs $IMAGE 2>(Join-Path $WORK "studio_run.err")) if (-not $script:STUDIO_CID) { Bad ("full image failed to start (see " + (Join-Path $WORK "studio_run.err") + ")") diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 07cc503d90..c365d86ad2 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -49,14 +49,12 @@ if [[ "${UNSLOTH_ALLOW_CPU:-0}" == "1" ]]; then fi # --- Check 1: nvidia-smi present and can enumerate at least one GPU --------- -if ! command -v nvidia-smi >/dev/null 2>&1; then - err "nvidia-smi not found inside the container." - err "The CUDA runtime in this image is broken. Re-pull the image." - exit 1 -fi - -if ! nvidia-smi -L 2>/dev/null | grep -q '^GPU'; then - err "No GPU visible to nvidia-smi from inside the container." +# nvidia-smi is injected by nvidia-container-toolkit when the container is +# started with a GPU request; it is NOT baked into the image. A missing +# binary therefore means "no GPU was attached", the same failure class as +# an empty -L listing, not a broken image. +if ! command -v nvidia-smi >/dev/null 2>&1 || ! nvidia-smi -L 2>/dev/null | grep -q '^GPU'; then + err "No GPU visible inside the container." cat >&2 <<'MSG' Likely causes (in order of frequency): @@ -80,7 +78,12 @@ Likely causes (in order of frequency): podman: --device nvidia.com/gpu=all k8s: nvidia.com/gpu resource request + GPU operator -To bypass this check (e.g. offline tooling), set UNSLOTH_SKIP_GPU_CHECK=1. + 5. This host has no NVIDIA GPU at all (Docker Desktop on macOS, Windows + without WSL2 GPU support, CPU-only Linux). Training needs a GPU, but + Jupyter, GGUF tooling and Studio chat work on CPU: + docker run -e UNSLOTH_ALLOW_CPU=1 ... + +To bypass this check entirely (e.g. offline tooling), set UNSLOTH_SKIP_GPU_CHECK=1. MSG exit 1 fi diff --git a/docker/run.sh b/docker/run.sh index 191837a923..aa285cb805 100755 --- a/docker/run.sh +++ b/docker/run.sh @@ -109,8 +109,10 @@ fi # Avoid `set -x` here so the literal HF_TOKEN / WANDB_API_KEY / UNSLOTH_LICENSE # values do not get echoed to stdout/CI logs. The forwarded env vars are # already in ENV_FORWARD; printing them again was a secret leak. -exec docker run --rm "${TTY_FLAG[@]}" \ - "${GPU_FLAG[@]}" \ +# The ${arr[@]+"${arr[@]}"} form keeps empty arrays nounset-safe on +# bash 3.2 (macOS /bin/bash), where a bare "${empty[@]}" trips set -u. +exec docker run --rm ${TTY_FLAG[@]+"${TTY_FLAG[@]}"} \ + ${GPU_FLAG[@]+"${GPU_FLAG[@]}"} \ --ipc=host \ --ulimit memlock=-1 \ --ulimit stack=67108864 \ @@ -118,5 +120,5 @@ exec docker run --rm "${TTY_FLAG[@]}" \ -v "$TRITON_CACHE":/workspace/.cache/triton \ -v "$WORK_DIR":/workspace/host \ "${ENV_FORWARD[@]}" \ - "${PORT_FLAGS[@]}" \ + ${PORT_FLAGS[@]+"${PORT_FLAGS[@]}"} \ "$IMAGE" "$@" diff --git a/docker/studio_launch.sh b/docker/studio_launch.sh index 143b00fe3b..7441d77d2e 100644 --- a/docker/studio_launch.sh +++ b/docker/studio_launch.sh @@ -18,8 +18,11 @@ export JUPYTER_PORT="${JUPYTER_PORT:-8888}" export UNSLOTH_STUDIO_HOME="${UNSLOTH_STUDIO_HOME:-/opt/unsloth-studio}" # Make the runtime env visible to SSH sessions, which get a fresh login shell -# without the `docker run -e` vars. Same pattern as the production image. +# without the `docker run -e` vars. Secrets are excluded on purpose: tokens, +# API keys and passwords stay in process env only, never on disk where an +# SSH session (or anything reading /etc/profile.d) could pick them up. printenv | grep -E '^(HF_|CUDA_|NCCL_|JUPYTER_|UNSLOTH_|WANDB_|PATH=|TRITON_)' | \ + grep -vE '^[^=]*(_TOKEN|_API_KEY|_PASSWORD|_SECRET|_LICENSE)=' | \ sed 's/^\([^=]*\)=\(.*\)$/export \1="\2"/' > /etc/profile.d/unsloth_env.sh || true # --- Jupyter ----------------------------------------------------------------- diff --git a/docker/supervisord.conf b/docker/supervisord.conf index f24b59d557..943bbd62b5 100644 --- a/docker/supervisord.conf +++ b/docker/supervisord.conf @@ -33,6 +33,7 @@ autostart=true autorestart=true startretries=3 startsecs=5 +environment=HOME="/root",USER="root" stdout_logfile=/dev/stdout stdout_logfile_maxbytes=0 stderr_logfile=/dev/stderr @@ -43,6 +44,10 @@ command=jupyter lab --no-browser --ip=0.0.0.0 --port=%(ENV_JUPYTER_PORT)s --allo directory=/workspace autostart=true autorestart=true +; HOME pins the config lookup to /root/.jupyter, where the launcher wrote +; the password config; without it an unset HOME would silently fall back +; to token auth. +environment=HOME="/root",USER="root" stdout_logfile=/dev/stdout stdout_logfile_maxbytes=0 stderr_logfile=/dev/stderr diff --git a/docker/test_locally.sh b/docker/test_locally.sh index 449cae307b..19a94cf021 100755 --- a/docker/test_locally.sh +++ b/docker/test_locally.sh @@ -385,7 +385,7 @@ INNER --ulimit stack=67108864 \ -v "$HOST_RUN_DIR:/workspace/host" \ -v "$HOME/.cache/huggingface:/workspace/.cache/huggingface" \ - "${HF_ARGS[@]}" \ + ${HF_ARGS[@]+"${HF_ARGS[@]}"} \ -e HF_HUB_ENABLE_HF_TRANSFER=1 \ "$TAG" \ bash /workspace/host/run_notebook.sh 2>&1 | tee "$GPT_LOG"