diff --git a/.github/actions/smoke-test/action.yml b/.github/actions/smoke-test/action.yml index e9eae929..dd1b7a39 100644 --- a/.github/actions/smoke-test/action.yml +++ b/.github/actions/smoke-test/action.yml @@ -223,6 +223,18 @@ inputs: flips the safer default off. required: false default: "fail" + registry-auth-name: + description: | + Display name of the registry credential in the RunPod account + (the `name` shown by `GET /v2/registries`) to attach to every pod. + + Empty (default) pulls anonymously — fine for our public images, + but the datacenter IP pool shares Docker Hub's rate limit. A name + that the account doesn't have is fatal: the run stops before any + pod is created rather than pulling with credentials nobody asked + for. + required: false + default: "" runs: using: composite @@ -423,6 +435,9 @@ runs: # deadline for slow pulls (mainly multi-GB ROCm base images). CREATE_TIMEOUT: ${{ inputs.create-timeout }} SAVE_COMFYUI_IMAGES: ${{ inputs.save-comfyui-images }} + # Empty → anonymous pulls. Non-empty and unknown to the account + # → test_images.py exits before creating pods. + REGISTRY_AUTH_NAME: ${{ inputs.registry-auth-name }} # test_images.py appends the matrix to $GITHUB_STEP_SUMMARY (always) # and writes this JSON, so results are readable without log access. SMOKE_RESULTS_JSON: ${{ runner.temp }}/smoke-results/results.json diff --git a/.github/workflows/manual-release.yml b/.github/workflows/manual-release.yml index 60999a6d..a0052956 100644 --- a/.github/workflows/manual-release.yml +++ b/.github/workflows/manual-release.yml @@ -204,12 +204,24 @@ jobs: exit 1 fi + # Same as release.yml: the squash commit we are tagging carries the body, + # and GitHub pre-pends it to its own generated notes. + - name: Release body from the squash commit + id: notes + run: | + set -euo pipefail + BODY="${RUNNER_TEMP}/release-body.md" + git log -1 --format=%b HEAD \ + | grep -viE '^(co-authored-by|signed-off-by):' > "${BODY}" || true + echo "path=${BODY}" >> "$GITHUB_OUTPUT" + - name: Create tag and GitHub Release uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 with: tag_name: ${{ inputs.tag }} name: ${{ inputs.tag }} target_commitish: ${{ steps.source.outputs.head_sha }} + body_path: ${{ steps.notes.outputs.path }} generate_release_notes: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 090c08d8..c8d4a5f9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -170,12 +170,25 @@ jobs: fetch-depth: 0 fetch-tags: true + # A releasable push to main is one squash commit, so its extended + # description is the release body. GitHub pre-pends it to the notes it + # generates, keeping the PR line and the Full Changelog link. + - name: Release body from the squash commit + id: notes + run: | + set -euo pipefail + BODY="${RUNNER_TEMP}/release-body.md" + git log -1 --format=%b HEAD \ + | grep -viE '^(co-authored-by|signed-off-by):' > "${BODY}" || true + echo "path=${BODY}" >> "$GITHUB_OUTPUT" + - name: Create tag and GitHub Release uses: softprops/action-gh-release@3d0d9888cb7fd7b750713d6e236d1fcb99157228 with: tag_name: ${{ needs.version.outputs.tag }} name: ${{ needs.version.outputs.tag }} target_commitish: ${{ github.sha }} + body_path: ${{ steps.notes.outputs.path }} generate_release_notes: true env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/official-templates/comfyui/Dockerfile b/official-templates/comfyui/Dockerfile index 79855226..2b56f8d2 100644 --- a/official-templates/comfyui/Dockerfile +++ b/official-templates/comfyui/Dockerfile @@ -64,6 +64,13 @@ RUN curl -fSL "https://github.com/ltdrdata/ComfyUI-Manager/archive/${MANAGER_SHA curl -fSL "https://github.com/MadiatorLabs/ComfyUI-RunpodDirect/archive/${RUNPODDIRECT_SHA}.tar.gz" -o runpoddirect.tar.gz && \ mkdir -p ComfyUI-RunpodDirect && tar xzf runpoddirect.tar.gz --strip-components=1 -C ComfyUI-RunpodDirect && rm runpoddirect.tar.gz +# Manager's get_pip_cmd() probes `python -m pip --version` with a 5s timeout and +# treats a timeout as "pip is missing" — on a slow network volume a working pip +# reads as absent. Patched before the git commit below so the tree stays clean. +# The count assertion fails the build if the upstream pattern moves. +RUN test "$(grep -c 'timeout=5)' ComfyUI-Manager/glob/manager_util.py)" = 2 && \ + sed -i 's/timeout=5)/timeout=30)/g' ComfyUI-Manager/glob/manager_util.py + # Init git repos with upstream remotes so ComfyUI-Manager can detect versions # and users can update via Manager at their own risk WORKDIR /tmp/build/ComfyUI @@ -143,6 +150,7 @@ ENV FILEBROWSER_CONFIG=/workspace/runpod-slim/.filebrowser.json # ---- CUDA variant (re-declared for runtime stage) ---- ARG CUDA_VERSION_DASH=12-8 +ARG TORCH_INDEX_SUFFIX=cu128 ARG TORCH_VERSION ARG TORCHVISION_VERSION ARG TORCHAUDIO_VERSION @@ -152,6 +160,9 @@ ARG FILEBROWSER_VERSION ARG FILEBROWSER_SHA256 # Keep runtime pip installs aligned with the baked CUDA-specific PyTorch stack. +# The pins carry a local version (+cu128), which PyPI does not serve — without +# this index every runtime install that touches torch fails to resolve. +ENV PIP_EXTRA_INDEX_URL="https://download.pytorch.org/whl/${TORCH_INDEX_SUFFIX}" RUN printf "torch==%s\ntorchvision==%s\ntorchaudio==%s\n" \ "$TORCH_VERSION" "$TORCHVISION_VERSION" "$TORCHAUDIO_VERSION" \ > /opt/comfyui-runtime-constraints.txt diff --git a/official-templates/comfyui/docker-bake.hcl b/official-templates/comfyui/docker-bake.hcl index 5301ddac..200294e0 100644 --- a/official-templates/comfyui/docker-bake.hcl +++ b/official-templates/comfyui/docker-bake.hcl @@ -12,7 +12,7 @@ variable "CIVICOMFY_SHA" { default = "555e984bbcb0" } variable "RUNPODDIRECT_SHA" { - default = "809065c9d2f3" + default = "9e32b1a09577" } variable "FILEBROWSER_VERSION" { default = "v2.59.0" diff --git a/official-templates/comfyui/scripts/start.sh b/official-templates/comfyui/scripts/start.sh index fb87c0a3..93517fc4 100755 --- a/official-templates/comfyui/scripts/start.sh +++ b/official-templates/comfyui/scripts/start.sh @@ -56,11 +56,15 @@ export_env_vars() { # Clear files : > "$ENV_FILE" : > "$PAM_ENV_FILE" + : > /etc/rp_environment mkdir -p /root/.ssh : > "$SSH_ENV_DIR" # Export to multiple locations for maximum compatibility - printenv | grep -E '^RUNPOD_|^PATH=|^_=|^CUDA|^LD_LIBRARY_PATH|^PYTHONPATH|^PIP_CONSTRAINT=' | while read -r line; do + # PIP_EXTRA_INDEX_URL travels with PIP_CONSTRAINT: the constraint pins a + # +cuXXX build that only the PyTorch index serves, so one without the other + # makes every install in an SSH session unresolvable. + printenv | grep -E '^RUNPOD_|^PATH=|^_=|^CUDA|^LD_LIBRARY_PATH|^PYTHONPATH|^PIP_CONSTRAINT=|^PIP_EXTRA_INDEX_URL=' | while read -r line; do # Get variable name and value name=$(echo "$line" | cut -d= -f1) value=$(echo "$line" | cut -d= -f2-) @@ -90,6 +94,12 @@ export_env_vars() { # Start Jupyter Lab server for remote access start_jupyter() { mkdir -p /workspace + + if [ -z "${JUPYTER_PASSWORD:-}" ]; then + JUPYTER_PASSWORD=$(openssl rand -hex 16) + echo "JUPYTER_PASSWORD was not set; generated one for this pod: ${JUPYTER_PASSWORD}" + fi + echo "Starting Jupyter Lab on port 8888..." nohup jupyter lab \ --allow-root \ @@ -100,7 +110,7 @@ start_jupyter() { --FileContentsManager.preferred_dir=/workspace \ --ServerApp.root_dir=/workspace \ --ServerApp.terminado_settings='{"shell_command":["/bin/bash"]}' \ - --IdentityProvider.token="${JUPYTER_PASSWORD:-}" \ + --IdentityProvider.token="${JUPYTER_PASSWORD}" \ --ServerApp.allow_origin=* &> /jupyter.log & echo "Jupyter Lab started" } @@ -170,6 +180,21 @@ upgrade_comfyui_if_needed() { echo "ComfyUI workspace upgraded successfully" } +# The venv has no pip of its own, so a bare `pip` would resolve to +# /usr/local/bin/pip and install against the base interpreter. Custom-node +# install scripts do call it that way. Existing venvs keep their real pip. +create_pip_shim() { + if [ -e "$VENV_DIR/bin/pip" ]; then + return + fi + if printf '#!/bin/sh\nexec "%s/bin/python" -m pip "$@"\n' "$VENV_DIR" \ + > "$VENV_DIR/bin/pip" 2>/dev/null; then + chmod +x "$VENV_DIR/bin/pip" + else + echo "WARNING: could not write $VENV_DIR/bin/pip — is the volume full?" + fi +} + log_cuda_venv_diagnostics() { local expected_build local_packages status expected_build=$(sed -n 's/^torch==.*+\(cu[0-9][0-9]*\).*$/\1/p' \ @@ -301,13 +326,21 @@ if [ -d "$OLD_VENV_DIR" ] && [ ! -d "$VENV_DIR" ]; then echo " Reinstalling deps for $NODE_COUNT custom nodes" echo " This may take several minutes" echo "=============================================" - mv "$OLD_VENV_DIR" "${OLD_VENV_DIR}.bak" + # Timestamped, and failure must not abort the boot: with a plain `.bak` + # target left over from an earlier migration, `mv` moves the venv *inside* + # it, fails under `set -e`, and the pod restarts in a loop. + VENV_BACKUP="${OLD_VENV_DIR}.bak.$(date +%Y%m%d%H%M%S)" + if mv "$OLD_VENV_DIR" "$VENV_BACKUP"; then + BACKED_UP=1 + else + BACKED_UP=0 + echo "WARNING: could not move $OLD_VENV_DIR aside; continuing with a fresh venv" + fi cd "$COMFYUI_DIR" - python3.12 -m venv --system-site-packages "$VENV_DIR" + python3.12 -m venv --system-site-packages --without-pip "$VENV_DIR" # The venv is created at runtime, so there is nothing for shellcheck to follow. # shellcheck source=/dev/null source "$VENV_DIR/bin/activate" - python -m ensurepip # Skip nodes baked into the image — their deps are in system site-packages CURRENT=0 INSTALLED=0 @@ -319,15 +352,17 @@ if [ -d "$OLD_VENV_DIR" ] && [ ! -d "$VENV_DIR" ]; then esac CURRENT=$((CURRENT + 1)) echo "[$CURRENT] $NODE_NAME" - pip install -r "$req" 2>&1 | grep -E "^(Successfully|ERROR)" || true + python -m pip install -r "$req" 2>&1 | grep -E "^(Successfully|ERROR)" || true INSTALLED=$((INSTALLED + 1)) fi done echo "Ensuring ComfyUI requirements are present..." - pip install -r "$COMFYUI_DIR/requirements.txt" 2>&1 | grep -E "^(Successfully|ERROR)" || true + python -m pip install -r "$COMFYUI_DIR/requirements.txt" 2>&1 | grep -E "^(Successfully|ERROR)" || true echo "Migration complete — $INSTALLED user nodes processed (${NODE_COUNT} total, baked nodes skipped)" - echo "Old venv backed up at ${OLD_VENV_DIR}.bak — delete it to free space:" - echo " rm -rf ${OLD_VENV_DIR}.bak" + if [ "$BACKED_UP" = "1" ]; then + echo "Old venv backed up at $VENV_BACKUP — delete it to free space:" + echo " rm -rf $VENV_BACKUP" + fi fi # Setup ComfyUI if needed @@ -343,13 +378,14 @@ if [ ! -d "$COMFYUI_DIR" ] || [ ! -d "$VENV_DIR" ]; then # Create venv with access to system packages (torch, numpy, etc. pre-installed in image) if [ ! -d "$VENV_DIR" ]; then cd "$COMFYUI_DIR" - python3.12 -m venv --system-site-packages "$VENV_DIR" + # --without-pip: pip stays in the image (local disk, bytecode compiled + # at build) instead of on the network volume, where importing it can + # exceed ComfyUI-Manager's probe timeout. --system-site-packages keeps + # it importable, and installs still land in this venv via sys.prefix. + python3.12 -m venv --system-site-packages --without-pip "$VENV_DIR" # shellcheck source=/dev/null source "$VENV_DIR/bin/activate" - # Ensure pip is available in the venv (needed for ComfyUI-Manager) - python -m ensurepip - echo "Base packages (torch, numpy, etc.) available from system site-packages" echo "ComfyUI ready — all dependencies pre-installed in image" fi @@ -360,9 +396,17 @@ else echo "Using existing ComfyUI installation" fi -# Warm up pip so ComfyUI-Manager's 5s timeout check doesn't fail on cold start. -# Log wall time — Manager fails if `python -m pip --version` takes >5s. -echo "Warming up pip (Manager timeout is 5s)..." +create_pip_shim + +# Interactive sessions are started by sshd, not by this script, and the PATH +# copied into the login files above was captured before the venv existed — +# `python` was then missing entirely and `pip` resolved to the base interpreter. +printf 'if [ -f "%s/bin/activate" ]; then . "%s/bin/activate"; fi\n' \ + "$VENV_DIR" "$VENV_DIR" >> /etc/rp_environment + +# Warm up pip before Manager probes it. Log wall time — the Dockerfile raises +# Manager's timeout to 30s. +echo "Warming up pip (Manager timeout is 30s)..." time python -m pip --version log_cuda_venv_diagnostics diff --git a/tests/README.md b/tests/README.md index 54177fd4..b41e3ac8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -59,13 +59,18 @@ tests/ group/world-readable keys, and the SSH probe will fail every pod with no obvious reason. -4. **(Recommended)** A Docker Hub registry credential on the account. +4. **(Recommended)** A Docker Hub registry credential on the account, + named with `REGISTRY_AUTH_NAME` (or pinned by `REGISTRY_AUTH_ID`). RunPod datacenters share an anonymous Hub IP pool that hits the `toomanyrequests` rate limit fast — without auth, parallel runs in particular produce a wave of "image pull backoff" failures that look - like image bugs but aren't. The script auto-discovers the first entry - from `GET /v2/registries`; pin a specific one with `REGISTRY_AUTH_ID` - or `REGISTRY_AUTH_NAME`. + like image bugs but aren't. + + Nothing is picked implicitly. Unset means anonymous pulls; a name the + account doesn't have stops the run before the first pod. Handing a + pod the wrong login is worse than handing it none — Docker Hub + answers `unauthorized: incorrect username or password` and never + falls back to an anonymous pull, so even a public image fails. ## Quick start @@ -88,7 +93,8 @@ Run it: You should see, in order: 1. `loaded N GPU types from GET /v2/catalog/gpus` — startup catalog query -2. `using registry auth: …` — Docker Hub auth resolved (or a warning) +2. `using registry auth: …` — the named credential resolved (or `no + registry auth requested` when none was asked for) 3. `==================== running 1 job(s) with MAX_PARALLEL=1 ===` 4. `attempt: CPU pod …` → `pod p-xxx created, waiting for RUNNING` 5. `t+Ns endpoint=root@…:NNNN ssh_probe=OK` — pod is up @@ -284,7 +290,7 @@ DWELL_SEC=0 ./test_images.py images.yaml base_cpu # Use a non-default SSH key RUNPOD_SSH_KEY=~/.ssh/my_runpod_key ./test_images.py images.yaml -# Pin to a specific registry auth (avoid auto-pick when you have several) +# Attach a registry credential by name (unset = anonymous pulls) REGISTRY_AUTH_NAME='dockerhub-prod' ./test_images.py images.yaml # …or by id REGISTRY_AUTH_ID='clxxxxxxxxxx' ./test_images.py images.yaml @@ -586,8 +592,8 @@ pytorch: | `CPU_VCPU_COUNT` | `4` | vCPUs requested for CPU pods. Must be a power of two inside the chosen flavor's `vcpu.min..max`. | | `CPU_FLAVOR_ID` | _(empty)_ | Pin a CPU flavor (e.g. `cpu3c`) instead of auto-picking the cheapest fitting one from `GET /v2/catalog/cpus`. | | `SMOKE_RESULTS_JSON` | _(empty)_ | Path to write the machine-readable result report to. Empty = don't write it. The markdown step summary is written regardless. | -| `REGISTRY_AUTH_ID` | _(empty)_ | Explicit Docker Hub registry auth id to pass as `--registry-auth-id`. Overrides auto-discovery. | -| `REGISTRY_AUTH_NAME` | _(empty)_ | Display name to look up via `GET /v2/registries` when `REGISTRY_AUTH_ID` is not set. Falls back to the first entry. | +| `REGISTRY_AUTH_ID` | _(empty)_ | Registry credential id to attach to every pod. Skips the name lookup. | +| `REGISTRY_AUTH_NAME` | _(empty)_ | Display name to look up via `GET /v2/registries`. Empty = anonymous pulls; a name the account doesn't have exits 1 before any pod is created. | | `DWELL_SEC` | `60` | Extra seconds to wait after SSH becomes reachable, then re-probe SSH to catch containers that boot, accept SSH, then crash. Set 0 to skip the re-probe. | | `CREATE_TIMEOUT` | `600` | Max seconds to wait for SSH to become reachable. Raise for ROCm workflows (`create-timeout: "1200"` on the action) — the official `rocm/pytorch:*` base images are 30-50GB and routinely take 8-15 minutes to pull. | | `POLL_INTERVAL` | `10` | Poll cadence for SSH probes. | @@ -683,7 +689,9 @@ wraps everything in this script needs for a clean CI run: fails the generator instead of silently ignoring one. 4. Invokes `python3 tests/test_images.py ` with `MAX_PARALLEL=`, `CLOUD_TYPE=`, - `ON_SKIP=` and `CREATE_TIMEOUT=`. A failed + `ON_SKIP=`, `CREATE_TIMEOUT=` and + `REGISTRY_AUTH_NAME=` (empty by default, i.e. + anonymous pulls). A failed image makes the smoke-test action fail, which prevents a release from being created. @@ -723,7 +731,8 @@ fields. | `no RunPod API key — set RUNPOD_API_KEY…` | key missing from env and `~/.runpod/config.toml` | `export RUNPOD_API_KEY=` | | `RunPod API rejected the key (HTTP 401…)` | key expired or lacks pod-management permission | regenerate at | | `warn: no GPU catalog` | `GET /v2/catalog/gpus` failed — usually a bad/absent key | fix the key; budget and `check_all_gpu` selection are disabled without it | -| `warn: no registry auth configured` | no Docker Hub credential on the account | add one in the RunPod console (paid Hub account strongly recommended for parallel runs) | +| `registry auth '' not found in this RunPod account` | `REGISTRY_AUTH_NAME` doesn't match any credential — typo, or a different account than the one that has it | the error lists the names the account does have; fix the name or add the credential in the RunPod console | +| `unauthorized: incorrect username or password` in the pod's system log | the attached credential's Docker Hub login is stale (common after switching accounts) | replace the token on that credential in the RunPod console, or drop `REGISTRY_AUTH_NAME` to pull our public images anonymously | | every pod SKIPs with an SSH failure | private key not mode `600`, or its public half isn't registered | `chmod 600 `; verify the fingerprint appears in `GET /v2/account/ssh-keys` | | `no ssh endpoint assigned yet` for the whole timeout | the pod never got a machine, so neither `ssh.direct` nor `ssh.proxy` exists | genuine provisioning failure — retry, or check the pod in the console. A missing *direct* port alone no longer causes this: the proxy is used instead | | `cuda_versions is set but none of the N candidate GPUs reports any CUDA version in the SECURE cloud` | CUDA axis on a ROCm/AMD sweep, or every candidate lives in the other cloud tier | drop `cuda_versions` for ROCm — the axis is NVIDIA-only; otherwise rerun with the other `CLOUD_TYPE` | diff --git a/tests/runpod_smoke/config.py b/tests/runpod_smoke/config.py index ad6c0fd0..bedab5b1 100644 --- a/tests/runpod_smoke/config.py +++ b/tests/runpod_smoke/config.py @@ -95,15 +95,16 @@ def _coerce_on_skip(raw: str) -> str: STALL_HINT_AFTER = int(os.environ.get("STALL_HINT_AFTER", "180")) -# Docker Hub authenticated pulls — without this, RunPod datacenters share -# an anonymous IP pool that hits Docker Hub's `toomanyrequests` rate limit -# fast. Either set REGISTRY_AUTH_ID explicitly, or REGISTRY_AUTH_NAME to -# pick by display name, or the script auto-picks the first entry from -# `GET /v2/registries`. +# Docker Hub authenticated pulls. Unset means anonymous pulls: fine for +# public images, but RunPod datacenters share an IP pool that hits Docker +# Hub's `toomanyrequests` rate limit fast. Name the credential with +# REGISTRY_AUTH_NAME (looked up in `GET /v2/registries`) or pass its +# REGISTRY_AUTH_ID directly. A name that doesn't resolve is fatal — +# nothing is ever picked implicitly. # -# REGISTRY_AUTH_ID is reassigned by main() after auto-discovery — access +# REGISTRY_AUTH_ID is reassigned by main() after the name lookup — access # it via `config.REGISTRY_AUTH_ID` (not a bare `from config import`) to -# pick up the post-discovery value. +# pick up the resolved value. REGISTRY_AUTH_ID = os.environ.get("REGISTRY_AUTH_ID", "") REGISTRY_AUTH_NAME = os.environ.get("REGISTRY_AUTH_NAME", "") diff --git a/tests/runpod_smoke/pod.py b/tests/runpod_smoke/pod.py index eb838aa1..3ba34538 100644 --- a/tests/runpod_smoke/pod.py +++ b/tests/runpod_smoke/pod.py @@ -108,19 +108,18 @@ def _signal_handler(signum: int, _frame) -> None: # --------------------------------------------------------------------------- -def discover_registry_auth(prefer_name: str = "") -> Optional[str]: - """Find a registry credential id from `GET /v2/registries`.""" +def list_registries() -> Optional[list[dict]]: + """Registry credentials on the account, or None if the call failed. + + Resolution by name is the caller's job: a credential is never picked + implicitly, because the wrong Docker Hub login turns a public pull + into `unauthorized: incorrect username or password` inside the pod. + """ status, data = api.request_with_retries("GET", "/registries", timeout=30) if not (200 <= status < 300) or not isinstance(data, dict): return None registries = data.get("registries") - if not isinstance(registries, list) or not registries: - return None - if prefer_name: - for item in registries: - if (item.get("name") or "").lower() == prefer_name.lower(): - return item.get("id") - return registries[0].get("id") + return registries if isinstance(registries, list) else None # --------------------------------------------------------------------------- diff --git a/tests/test_images.py b/tests/test_images.py index 54a765db..d917afa2 100755 --- a/tests/test_images.py +++ b/tests/test_images.py @@ -45,7 +45,7 @@ _normalize_cuda_version, parse_manifest, ) -from runpod_smoke.pod import discover_registry_auth +from runpod_smoke.pod import list_registries from runpod_smoke.runner import test_image @@ -81,8 +81,8 @@ def _check_prereqs(manifest_path: Path) -> Optional[int]: """Return None on success, or an exit-code int on failure. Verifies: 1. the manifest file actually exists 2. the REST API v2 accepts our key - Anything else (GPU catalog, registry auth) is best-effort — the script - degrades gracefully if those are missing.""" + The GPU catalog is best-effort — the script degrades gracefully + without it. Registry auth is checked separately, in main().""" if not manifest_path.is_file(): log(f"Images manifest not found: {manifest_path}") return 1 @@ -119,18 +119,42 @@ def _init_gpu_catalog() -> None: ) -def _init_registry_auth() -> None: - if not config.REGISTRY_AUTH_ID: - config.REGISTRY_AUTH_ID = ( - discover_registry_auth(config.REGISTRY_AUTH_NAME) or "" - ) +def _init_registry_auth() -> Optional[int]: + """Resolve the credential to attach to every pod. Exit code on failure. + + Asking for a credential that the account doesn't have is fatal: we + would otherwise fall back to a pull the caller didn't ask for and + only find out from a rate-limit or auth error inside the pod. + """ if config.REGISTRY_AUTH_ID: - log(f"using registry auth: {config.REGISTRY_AUTH_ID}") - else: + log(f"using registry auth: {config.REGISTRY_AUTH_ID} (REGISTRY_AUTH_ID)") + return None + if not config.REGISTRY_AUTH_NAME: log( - "warn: no registry auth configured — Docker Hub pulls will be " - "anonymous and likely hit the toomanyrequests rate limit" + "no registry auth requested — pulls will be anonymous, which " + "works for public images but shares Docker Hub's rate limit" ) + return None + + registries = list_registries() + if registries is None: + print("::error::GET /v2/registries failed — cannot resolve " + f"registry auth '{config.REGISTRY_AUTH_NAME}'") + return 1 + for item in registries: + if (item.get("name") or "").lower() == config.REGISTRY_AUTH_NAME.lower(): + config.REGISTRY_AUTH_ID = item.get("id") or "" + break + if not config.REGISTRY_AUTH_ID: + have = ", ".join(sorted(r.get("name") or "?" for r in registries)) + print(f"::error::registry auth '{config.REGISTRY_AUTH_NAME}' not found " + f"in this RunPod account (has: {have or 'none'})") + return 1 + log( + f"using registry auth: {config.REGISTRY_AUTH_ID} " + f"('{config.REGISTRY_AUTH_NAME}')" + ) + return None # --------------------------------------------------------------------------- @@ -877,7 +901,9 @@ def main() -> int: if rc is not None: return rc - _init_registry_auth() + rc = _init_registry_auth() + if rc is not None: + return rc manifest = parse_manifest(manifest_path) try: diff --git a/tests/unit/test_registry_auth.py b/tests/unit/test_registry_auth.py new file mode 100644 index 00000000..a0da2c2e --- /dev/null +++ b/tests/unit/test_registry_auth.py @@ -0,0 +1,90 @@ +"""A credential must never be attached unless it was asked for by name. + +Auto-picking `registries[0]` made the run depend on whatever the RunPod +account happened to list first. After an account switch that entry held a +stale Docker Hub login, and every pod died in ~12s with `unauthorized: +incorrect username or password` — on a public image that needs no login +at all, because Docker Hub rejects bad credentials instead of falling +back to an anonymous pull. +""" + +import unittest +from unittest import mock + +import test_images as T +from runpod_smoke import config + + +REGISTRIES = [ + {"id": "clstale0000", "name": "dockerhub-old"}, + {"id": "clgood11111", "name": "dockerhub-ci"}, +] + + +class RegistryAuthResolution(unittest.TestCase): + def setUp(self) -> None: + self.addCleanup( + setattr, config, "REGISTRY_AUTH_ID", config.REGISTRY_AUTH_ID + ) + self.addCleanup( + setattr, config, "REGISTRY_AUTH_NAME", config.REGISTRY_AUTH_NAME + ) + config.REGISTRY_AUTH_ID = "" + config.REGISTRY_AUTH_NAME = "" + + def test_no_name_pulls_anonymously_without_listing(self): + with mock.patch.object(T, "list_registries") as listed: + self.assertIsNone(T._init_registry_auth()) + self.assertEqual(config.REGISTRY_AUTH_ID, "") + listed.assert_not_called() + + def test_name_resolves_to_its_own_id(self): + config.REGISTRY_AUTH_NAME = "dockerhub-ci" + with mock.patch.object(T, "list_registries", return_value=REGISTRIES): + self.assertIsNone(T._init_registry_auth()) + self.assertEqual(config.REGISTRY_AUTH_ID, "clgood11111") + + def test_name_match_ignores_case(self): + config.REGISTRY_AUTH_NAME = "DockerHub-CI" + with mock.patch.object(T, "list_registries", return_value=REGISTRIES): + T._init_registry_auth() + self.assertEqual(config.REGISTRY_AUTH_ID, "clgood11111") + + def test_unknown_name_is_fatal_and_attaches_nothing(self): + config.REGISTRY_AUTH_NAME = "dockerhub-typo" + with mock.patch.object(T, "list_registries", return_value=REGISTRIES): + self.assertEqual(T._init_registry_auth(), 1) + self.assertEqual(config.REGISTRY_AUTH_ID, "") + + def test_empty_account_is_fatal_when_a_name_was_asked_for(self): + config.REGISTRY_AUTH_NAME = "dockerhub-ci" + with mock.patch.object(T, "list_registries", return_value=[]): + self.assertEqual(T._init_registry_auth(), 1) + + def test_failed_listing_is_fatal(self): + config.REGISTRY_AUTH_NAME = "dockerhub-ci" + with mock.patch.object(T, "list_registries", return_value=None): + self.assertEqual(T._init_registry_auth(), 1) + + def test_explicit_id_skips_the_lookup(self): + config.REGISTRY_AUTH_ID = "clpinned0000" + with mock.patch.object(T, "list_registries") as listed: + self.assertIsNone(T._init_registry_auth()) + self.assertEqual(config.REGISTRY_AUTH_ID, "clpinned0000") + listed.assert_not_called() + + +class MainStopsBeforeSpendingMoney(unittest.TestCase): + def test_unresolved_name_returns_before_any_pod_is_planned(self): + with mock.patch.object(T, "_parse_args", return_value=("images", None)), \ + mock.patch.object(T, "_check_prereqs", return_value=None), \ + mock.patch.object(T, "_init_registry_auth", return_value=1), \ + mock.patch.object(T, "parse_manifest") as parsed, \ + mock.patch.object(T, "_run_jobs") as ran: + self.assertEqual(T.main(), 1) + parsed.assert_not_called() + ran.assert_not_called() + + +if __name__ == "__main__": + unittest.main()