From 272b4b22e8db37017ee941d9a1c2cb00a3325278 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Thu, 6 Aug 2026 15:26:05 +1000 Subject: [PATCH] feat(ochre): add the vLLM serving AMI and give the daemon a route to it (#740) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(images): add a vllm-serving AMI as a stacked mkosi profile LaunchServingVM resolves its AMI by the tags spinifex:managed-by=bedrock and spinifex:bedrock-role=vllm-serving. No image carried either, so every Ensure failed at resolveServingAMI. The only GPU image ships the NVIDIA driver and Docker but no Python, no vLLM and no serving unit, so tagging it would have moved the failure from admission to first boot. Add a vllm profile that stacks onto gpu-nvidia and owns the whole guest side of the launcher contract: the runtime, vllm-serve.service, the weights mount and the tags. vLLM is baked into a uv-managed venv rather than pulled as a container at boot — a runtime docker pull of vllm/vllm-openai is known to stall this platform's write plane, and mkosi's unprivileged build has no daemon to bake an image from anyway. Also bring images/mkosi.profiles/ under make test-images. Its scripts have no .sh suffix, so the existing name glob skipped exactly the files worth linting; sources are found by shebang instead. * fix(images): make the vllm postinst survive the build sandbox Two failures found by actually running the build. uv installs to /usr/local/bin, which mkosi's postinst PATH need not carry, so `command -v uv` reported a good install as missing. Call it by absolute path, and check it the same way. `vllm --version` initialises device config and dies with "Failed to infer device type" on a builder with no GPU, so it could never have verified anything. Assert the pinned version through package metadata instead, which also catches a version mismatch rather than merely a broken entrypoint. * fix(images): pin the serving AMI datasource and mirror cloud-init to the console A serving VM boots through LaunchSystemInstance, so it comes up dual-NIC with a fw_cfg netcfg blob and takes its user-data solely from IMDS — the same shape as the EKS control plane. That path carries a cloud-init datasource pin; the vllm profile never got one, and the base config deliberately leaves datasource_list unpinned, so the serving image was the only system VM booting without it. Reuse the EKS pin rather than copying it. The 99- prefix is load-bearing: read_conf_d() applies cloud.cfg.d in REVERSE and mergemanydict() is first-source-wins, so a 90- name loses to cloud-init's own 90_dpkg.cfg ('_' sorts above '-') and the pin is discarded in silence. The postinst asserts the MERGED datasource_list via cloud-init's own read_conf_with_confd rather than checking the file exists, because it existed at 90- too and did nothing there. Also mirror every cloud-init stage to the serial console. A serving VM is headless, and a guest that wedges before handover never surrenders /var/log/cloud-init-output.log, so the console get-console-output reads is the only channel an operator has. The images_test.go case covers the two tags resolveServingAMI filters on; it matches on those alone and never on name, so a typo hides the image entirely. * fix(images): make the dhcpcd data-NIC lease genuinely one-shot dhcp_oneshot() promised to configure the link and exit "leaving no daemon to fight cloud-init", but the dhcpcd branch passed only -q, which is quiet rather than one-shot. Every Ubuntu image takes that branch, so a dhcpcd daemon survived on the data NIC holding the control socket. cloud-init's Ec2 datasource then runs its own ephemeral dhcpcd on the same NIC. Instead of spawning, dhcpcd forwards the invocation to the resident daemon as a control command and exits 0, and --dumplease returns the resident daemon's lease, so discovery looks successful. Cleanup then waits for a pid file at /run/dhcpcd/-4.pid that no process ever wrote, spinning its full 30000 x 10ms budget: init-local stalls for exactly 300s on every system-instance boot. Adding -1 exits once the interface is configured and -p keeps the address and routes across that exit. Both are needed; -1 alone tears the link back down. The test now models the real clients — the stub stays resident unless given the flag that makes its client exit, and the dhcpcd branch is forced by dropping udhcpc from PATH rather than left to whether the host happens to have /usr/share/udhcpc/default.script. * fix(images): serve without a CUDA toolkit and stop vllm-serve crash-looping vLLM picks FlashInfer for top-k/top-p sampling, and FlashInfer JIT-compiles that kernel on first use. The serving image ships the driver and the container toolkit but no nvcc, so the sampler warmup died with "Could not find nvcc and default cuda_home='/usr/local/cuda' doesn't exist" minutes into a GPU cold start, after the model had already loaded. VLLM_USE_FLASHINFER_SAMPLER=0 selects vLLM's native top-k/top-p sampler, which is the same operation with no CUDA toolchain at runtime. Everything else on the serving path is already precompiled in the wheels: the attention backend resolves to FLASH_ATTN and CUDA graph capture completes, both without nvcc. Baking FlashInfer's AOT modules is the alternative if the fused kernel is ever wanted, and it keeps the toolchain in the builder rather than the appliance. The unit had Restart=on-failure with no bound, so an unfixable start failure restarted forever and buried its own first traceback under identical copies on the serial console — the only view an operator has of a headless guest. StartLimitIntervalSec/StartLimitBurst make it report instead. The postinst asserts the installed unit carries the sampler setting, for the same reason the datasource pin is asserted there: the alternative is finding out at runtime, on a GPU, five minutes in. Also corrects the shared datasource pin's header, which still claimed it installs as 90-spinifex-ec2-imds.cfg. It is installed at 99- by the vllm profile precisely because a 90- name loses the reverse-sorted, first-source- wins cloud.cfg.d merge to cloud-init's own 90_dpkg.cfg. * fix(bedrock): raise the serving-endpoint readiness bound to 15 minutes The 5 minute bound was chosen before any cold start had been measured, and its own comment asked to be corrected once a model actually ran on wattle. It now has: Llama 3.2 1B on an RTX A1000 reached "Application startup complete" 257.6s after launch, leaving ~42s of margin on the smallest model this platform can serve, with a cold torch.compile accounting for 26.3s of it. A bigger card and a bigger model are unmeasured. The two failure modes are not symmetric. Too low tears down an endpoint that was about to come up and reports it as a failed launch; too high only makes a genuinely stuck launch take longer to give up. So the bound is set well clear of the single data point rather than close to it. The CLI's own wait moves to 20 minutes to stay above it. The daemon starts its clock when the VM is launched, up to a minute after the CLI starts its own, so leaving both at 15 would expire the client first and report a bare "timed out" in place of the daemon's abort, which carries the reason. * fix: give the Ochre daemon a routed port in the Bedrock system VPC The readiness probe and every InvokeModel call dial a serving VM's system-VPC address from inside the daemon, but that subnet lives in OVN on br-int and the host has no route to it, so a healthy VM never reached READY. Each daemon now owns an ENI in the system VPC's private subnet, bound to an OVS internal port on br-int by iface-id, exactly as ovn-controller binds a guest tap. The ENI address is installed at the subnet's prefix length, which is what supplies the connected route. BaseURL is unchanged: the address it already carried is now reachable. Describe-or-create keyed on a per-node description, since ENIs carry no tag filter, so a restart or a second endpoint reuses one address instead of leaking one per launch. The port is ensured before anything else the launch creates: a VM the daemon cannot dial holds a GPU for the whole readiness window before unwinding. --- Makefile | 27 +- cmd/spinifex/cmd/admin_ochre_endpoint.go | 10 +- docs/COMMANDS.md | 2 + images/mkosi.profiles/vllm/mkosi.conf | 22 ++ .../99-spinifex-serial-console.cfg | 7 + .../etc/systemd/system/vllm-serve.service | 41 +++ .../mkosi.extra/usr/local/sbin/vllm-serve | 293 +++++++++++++++ .../mkosi.profiles/vllm/mkosi.postinst.chroot | 126 +++++++ images/mkosi.profiles/vllm/vllm-serve_test.sh | 346 ++++++++++++++++++ scripts/images/common/mulga-mgmt-net.sh | 5 +- scripts/images/common/mulga-mgmt-net_test.sh | 58 ++- scripts/images/eks-node/cloud-init-ec2.cfg | 8 +- scripts/mkosi-build.sh | 6 +- spinifex/daemon/bedrock_deps.go | 5 + spinifex/handlers/bedrock/fakes_test.go | 107 +++++- spinifex/handlers/bedrock/hostport.go | 138 +++++++ spinifex/handlers/bedrock/hostport_test.go | 149 ++++++++ spinifex/handlers/bedrock/launch.go | 13 + spinifex/handlers/bedrock/readiness.go | 16 +- spinifex/handlers/bedrock/vpc.go | 6 + spinifex/network/host/vpc_hostport.go | 108 ++++++ spinifex/network/host/vpc_hostport_test.go | 148 ++++++++ spinifex/utils/images.go | 22 ++ spinifex/utils/images_test.go | 39 ++ spinifex/vm/lifecycle_test.go | 4 + spinifex/vm/network_helpers_test.go | 41 ++- spinifex/vm/network_plumber.go | 10 + 27 files changed, 1722 insertions(+), 35 deletions(-) create mode 100644 images/mkosi.profiles/vllm/mkosi.conf create mode 100644 images/mkosi.profiles/vllm/mkosi.extra/etc/cloud/cloud.cfg.d/99-spinifex-serial-console.cfg create mode 100644 images/mkosi.profiles/vllm/mkosi.extra/etc/systemd/system/vllm-serve.service create mode 100644 images/mkosi.profiles/vllm/mkosi.extra/usr/local/sbin/vllm-serve create mode 100755 images/mkosi.profiles/vllm/mkosi.postinst.chroot create mode 100644 images/mkosi.profiles/vllm/vllm-serve_test.sh create mode 100644 spinifex/handlers/bedrock/hostport.go create mode 100644 spinifex/handlers/bedrock/hostport_test.go create mode 100644 spinifex/network/host/vpc_hostport.go create mode 100644 spinifex/network/host/vpc_hostport_test.go create mode 100644 spinifex/utils/images_test.go diff --git a/Makefile b/Makefile index 70bcf696f..fc866e625 100644 --- a/Makefile +++ b/Makefile @@ -212,18 +212,37 @@ test-actions: @echo -e "\n....Running action tests...." LOG_IGNORE=1 go test -timeout 60s ./.github/actions/... -# Shell suites + shellcheck for scripts/images/ helpers baked into system -# images. Kept out of `preflight` (a dedicated CI job gates it on -# scripts/images/** changes instead) so image-asset churn doesn't run on -# every Go contributor's commit. +# Shell suites + shellcheck for scripts/images/ and images/mkosi.profiles/ +# helpers baked into system images. Kept out of `preflight` (a dedicated CI +# job gates it on scripts/images/**+images/mkosi.profiles/** changes instead) +# so image-asset churn doesn't run on every Go contributor's commit. +# +# images/mkosi.profiles/ ships scripts with no .sh suffix (mkosi's own +# mkosi.*.chroot lifecycle hooks, and wrapper binaries like vllm-serve under +# mkosi.extra/) so a plain `-name '*.sh'` glob silently skips exactly the +# files most worth linting. Sources there are found by shebang instead of +# name; this also means a profile that ships no scripts at all yields an +# empty match rather than an error. test-images: @echo -e "\n....Running scripts/images/**/*_test.sh...." @for t in $$(find scripts/images -name '*_test.sh' | sort); do \ echo "-- $$t"; \ bash "$$t" || exit 1; \ done + @echo -e "\n....Running images/mkosi.profiles/**/*_test.sh...." + @for t in $$(find images/mkosi.profiles -name '*_test.sh' | sort); do \ + echo "-- $$t"; \ + bash "$$t" || exit 1; \ + done @echo -e "\n....Running shellcheck over scripts/images/**/*.sh...." shellcheck -S warning $$(find scripts/images -name '*.sh' | sort) + @echo -e "\n....Running shellcheck over images/mkosi.profiles/** shell sources...." + @srcs="$$(grep -rlIE '^#!/bin/(bash|sh)' images/mkosi.profiles 2>/dev/null | sort)"; \ + if [ -n "$$srcs" ]; then \ + shellcheck -S warning $$srcs; \ + else \ + echo " (no shell sources found under images/mkosi.profiles)"; \ + fi @echo " test-images ok" # Check that new/changed code meets coverage threshold (runs tests first) diff --git a/cmd/spinifex/cmd/admin_ochre_endpoint.go b/cmd/spinifex/cmd/admin_ochre_endpoint.go index 2c2b42084..ed98ee8ca 100644 --- a/cmd/spinifex/cmd/admin_ochre_endpoint.go +++ b/cmd/spinifex/cmd/admin_ochre_endpoint.go @@ -78,10 +78,12 @@ func init() { } const ( - // defaultEndpointWaitTimeout is deliberately generous: the cold start this - // waits on has never been measured, so a tight default would report a - // timeout for what is really a slow first launch. - defaultEndpointWaitTimeout = 15 * time.Minute + // defaultEndpointWaitTimeout must stay comfortably above the daemon's own + // readiness bound. The daemon starts its clock once the VM is launched, + // which is up to a minute after this one starts, so an equal value expires + // here first and reports a bare client timeout instead of the daemon's + // abort, which says why the launch actually failed. + defaultEndpointWaitTimeout = 20 * time.Minute // endpointPollInterval trades responsiveness against KV read volume. A // cold start is minutes, so seconds of granularity is plenty. endpointPollInterval = 2 * time.Second diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 580a62d26..e29588e39 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -134,6 +134,8 @@ Operator surface over the daemon's `bedrock.endpoint.*` subjects. The gateway do | `spx admin ochre endpoint list` | — | Lists every endpoint record with its state, instance ID and base URL. | | `spx admin ochre endpoint delete` | `--model-id` (required) | Moves a `READY` endpoint to `DRAINING` and tears its VM down, releasing the GPU. Idempotent: an already-absent endpoint reports success. | +Self-hosted models are served from the `ubuntu-26.04-vllm-serving-x86_64` system image (`spx admin images import --name ubuntu-26.04-vllm-serving-x86_64`), built by `scripts/mkosi-build.sh --image ubuntu-vllm-serving` (`gpu-nvidia` + `vllm` profiles). It carries the tags `spinifex:managed-by=bedrock` and `spinifex:bedrock-role=vllm-serving`, which is what `LaunchServingVM` filters on rather than resolving by name. The image bakes vLLM's OpenAI-compatible server into a uv-managed venv — no Docker daemon, no runtime container pull — and boots `vllm-serve.service`, which waits for cloud-init's `/etc/conf.d/vllm-serve` handoff, resolves and mounts the endpoint's cloned weights volume read-only, then execs `vllm serve` on port 8000. + ### EKS Control-Plane Disaster Recovery | Command | Flags | Description | diff --git a/images/mkosi.profiles/vllm/mkosi.conf b/images/mkosi.profiles/vllm/mkosi.conf new file mode 100644 index 000000000..8267c1592 --- /dev/null +++ b/images/mkosi.profiles/vllm/mkosi.conf @@ -0,0 +1,22 @@ +# vllm profile — vLLM OpenAI-compatible serving runtime, layered on gpu-nvidia. +# +# Only the serving concern lives here: the GPU driver, DKMS build and +# nvidia-container-toolkit stay in gpu-nvidia, and this profile never +# assembles into an image without it (see image_profiles() in +# scripts/mkosi-build.sh). No Packages= of its own: curl and ca-certificates, +# which mkosi.postinst.chroot needs to fetch uv and CPython, already come from +# gpu-nvidia, and duplicating them here is exactly the drift this migration +# exists to delete. +[Content] +# A serving VM launches through LaunchSystemInstance, so it boots dual-NIC with +# a fw_cfg netcfg blob and takes its user-data solely from IMDS — the same shape +# as the EKS control plane, whose pin is reused here rather than copied. +# The 99- prefix is load-bearing: read_conf_d() applies cloud.cfg.d in REVERSE +# and mergemanydict() is first-source-wins, so a 90- name loses to cloud-init's +# own 90_dpkg.cfg ('_' sorts above '-') and the pin is silently discarded. +ExtraTrees=../../../scripts/images/eks-node/cloud-init-ec2.cfg:/etc/cloud/cloud.cfg.d/99-spinifex-ec2-imds.cfg + +[Build] +# mkosi.postinst.chroot fetches the uv installer, a standalone CPython build, +# and vLLM's own PyPI/CUDA wheels — all network, none of it in the archive. +WithNetwork=yes diff --git a/images/mkosi.profiles/vllm/mkosi.extra/etc/cloud/cloud.cfg.d/99-spinifex-serial-console.cfg b/images/mkosi.profiles/vllm/mkosi.extra/etc/cloud/cloud.cfg.d/99-spinifex-serial-console.cfg new file mode 100644 index 000000000..be3e9da44 --- /dev/null +++ b/images/mkosi.profiles/vllm/mkosi.extra/etc/cloud/cloud.cfg.d/99-spinifex-serial-console.cfg @@ -0,0 +1,7 @@ +# A serving VM is headless: the serial console get-console-output reads is the +# only channel an operator has, and a guest that wedges before handover never +# surrenders /var/log/cloud-init-output.log. Mirror every stage to both. +# The 99- prefix wins the REVERSE-sorted, first-source-wins cloud.cfg.d merge +# against cloud-init's own 05_logging.cfg, which sets output= too. +output: + all: '| tee -a /var/log/cloud-init-output.log > /dev/console' diff --git a/images/mkosi.profiles/vllm/mkosi.extra/etc/systemd/system/vllm-serve.service b/images/mkosi.profiles/vllm/mkosi.extra/etc/systemd/system/vllm-serve.service new file mode 100644 index 000000000..a65c41e45 --- /dev/null +++ b/images/mkosi.profiles/vllm/mkosi.extra/etc/systemd/system/vllm-serve.service @@ -0,0 +1,41 @@ +[Unit] +Description=vLLM OpenAI-compatible serving runtime (Bedrock self-host) +# mulga-ebs-byid's coldplug pass mints the by-id link the wrapper's fallback +# device resolution depends on. Ordering after it just avoids a guaranteed +# spurious wait on every boot — the wrapper's own bounded waits are what +# actually cover the race, since AttachVolume runs after this VM's launch +# already returned. +# +# cloud-init write_files seeds /etc/conf.d/vllm-serve during the cloud-config +# stage (same precedent as eks-token-webhook.service), so this too is not a +# correctness fix — await_env_file already covers a handoff that is not there +# yet — it just avoids burning that whole wait on every cold start. +After=mulga-ebs-byid.service cloud-config.service +# A start failure no retry can fix restarts forever otherwise, burying its own +# first traceback under identical copies on the only console an operator has. +StartLimitIntervalSec=300 +StartLimitBurst=3 + +[Service] +Type=exec +# ExecStart names the wrapper and takes no arguments, so this line contains no +# $VAR at all. That is deliberate: systemd expands $VAR/${VAR} in an Exec*= +# line from the unit's own environment before the command runs and +# substitutes empty for a name it does not know — the wrapper waits for and +# sources /etc/conf.d/vllm-serve itself, in a shell it owns, to sidestep the +# whole class of failure. +ExecStart=/usr/local/sbin/vllm-serve +# FlashInfer JIT-compiles its sampling module on first use and needs nvcc for +# it. vLLM's native top-k/top-p sampler is the same operation with no CUDA +# toolchain at runtime, which a serving appliance has no reason to carry. +Environment=VLLM_USE_FLASHINFER_SAMPLER=0 +Restart=on-failure +RestartSec=5 +# Headless appliance: the serial console is the only view of cold start +# (env-file wait, weights mount, vLLM's own load time) until /v1/models +# starts answering. +StandardOutput=journal+console +StandardError=journal+console + +[Install] +WantedBy=multi-user.target diff --git a/images/mkosi.profiles/vllm/mkosi.extra/usr/local/sbin/vllm-serve b/images/mkosi.profiles/vllm/mkosi.extra/usr/local/sbin/vllm-serve new file mode 100644 index 000000000..724b22cbd --- /dev/null +++ b/images/mkosi.profiles/vllm/mkosi.extra/usr/local/sbin/vllm-serve @@ -0,0 +1,293 @@ +#!/bin/sh +# vllm-serve — wrapper for vllm-serve.service. Waits for the cloud-init +# handoff, resolves and mounts the endpoint's cloned weights volume read-only, +# then execs vLLM's OpenAI-compatible server against it. +# +# Handoff contract, written by the Bedrock launcher as a cloud-init +# write_files entry (so it does not exist before cloud-init runs): +# +# /etc/conf.d/vllm-serve 0600 KEY=value shell fragment: +# VLLM_MODEL_ID= +# VLLM_ARGS= +# VLLM_WEIGHTS_DEVICE= +# VLLM_SERVE_PORT= +# +# ExecStart in vllm-serve.service takes no arguments and contains no $VAR — +# this script sources the handoff itself, in a shell it owns, rather than +# letting systemd expand anything from an Exec*= line. +set -eu + +ENV_FILE="${VLLM_ENV_FILE:-/etc/conf.d/vllm-serve}" +# The unit starts before cloud-init has necessarily delivered the handoff, so +# this bounds that race rather than assuming it is already there. +ENV_WAIT_SECS="${VLLM_ENV_WAIT:-60}" + +WEIGHTS_MOUNT="${VLLM_WEIGHTS_MOUNT:-/var/lib/vllm/weights}" +VENV_BIN="${VLLM_VENV_BIN:-/opt/vllm/venv/bin}" + +# AttachVolume runs after the launch already returned the instance, so the +# weights volume legitimately does not exist yet when this unit first starts. +DEVICE_WAIT_SECS="${VLLM_DEVICE_WAIT:-120}" + +SYS_BLOCK="${VLLM_SYS_BLOCK:-/sys/block}" +DEV_DIR="${VLLM_DEV_DIR:-/dev}" +BYID_DIR="${VLLM_BYID_DIR:-/dev/disk/by-id}" +MOUNTS_FILE="${VLLM_MOUNTS_FILE:-/proc/mounts}" + +log() { echo "[vllm-serve] $*"; } +fail() { echo "[vllm-serve] ERROR: $*" >&2; exit 1; } + +# Tries to read a *usable* handoff once: sets VLLM_MODEL_ID/VLLM_ARGS/ +# VLLM_WEIGHTS_DEVICE/VLLM_SERVE_PORT in the caller's shell (deliberately not +# run inside a `$(...)` — a subshell's assignments do not survive it) and +# returns 0 on success. Returns 1 when the handoff is not there yet or is +# still mid-write (worth retrying — cloud-init's write_files is not atomic +# from this reader's point of view: the file can exist, created but empty +# or half-flushed, for a brief window before its content lands), and 2 when +# a key IS present but malformed (never becomes valid by waiting longer, so +# not worth retrying — the same found/not-yet/bad three-way split +# resolve_weights_device uses below). +try_load_env() { + [ -f "${ENV_FILE}" ] || return 1 + + # VLLM_ARGS holds a space-separated list of flags, so this cannot be a + # plain `. "${ENV_FILE}"` the way rds-init sources its handoff: shell + # would parse "VLLM_ARGS=--foo --bar" as an assignment followed by a + # second command and fail on the second word. `read` with IFS='=' splits + # only the key off the front; the last variable absorbs the rest of the + # line verbatim, spaces and all, with no further word-splitting. + _raw_model="" _raw_args="" _raw_device="" _raw_port="" + while IFS='=' read -r _key _value || [ -n "${_key:-}" ]; do + case "${_key:-}" in + VLLM_MODEL_ID) _raw_model="${_value:-}" ;; + VLLM_ARGS) _raw_args="${_value:-}" ;; + VLLM_WEIGHTS_DEVICE) _raw_device="${_value:-}" ;; + VLLM_SERVE_PORT) _raw_port="${_value:-}" ;; + esac + done < "${ENV_FILE}" + + # Missing required keys means the write is still in flight, not that the + # handoff is broken — retry rather than fail. + [ -n "${_raw_model}" ] || return 1 + [ -n "${_raw_device}" ] || return 1 + + # Unlike a merely-missing key, a port that IS present but non-numeric + # will never self-correct by waiting: fail now, do not retry. + case "${_raw_port}" in + '') _raw_port=8000 ;; + *[!0-9]*) + echo "[vllm-serve] ERROR: bootstrap handoff has a non-numeric VLLM_SERVE_PORT: ${_raw_port}" >&2 + return 2 + ;; + esac + + VLLM_MODEL_ID="${_raw_model}" + VLLM_ARGS="${_raw_args}" + VLLM_WEIGHTS_DEVICE="${_raw_device}" + VLLM_SERVE_PORT="${_raw_port}" + return 0 +} + +await_env_file() { + _waited=0 + while :; do + # `if try_load_env; then` (not `try_load_env; _rc=$?`) for the same + # reason as await_weights_device below: a plain assignment-less + # command IS a checked context under set -e too, so calling + # try_load_env as a bare statement and letting a 1 ("not yet") + # propagate would abort the script on the very first empty-file read + # instead of looping. The if-condition is what set -e exempts. + if try_load_env; then + log "bootstrap handoff loaded (model=${VLLM_MODEL_ID}, port=${VLLM_SERVE_PORT})" + return 0 + else + _rc=$? + fi + [ "${_rc}" -eq 2 ] && fail "bootstrap handoff at ${ENV_FILE} is malformed (see above) — refusing to guess" + if [ "${_waited}" -ge "${ENV_WAIT_SECS}" ]; then + fail "no usable bootstrap handoff at ${ENV_FILE} after ${ENV_WAIT_SECS}s — cloud-init has not delivered it" + fi + sleep 1 + _waited=$((_waited + 1)) + done +} + +# An already-mounted weights point makes device resolution a no-op, so a +# Restart=on-failure re-run (vLLM itself crashed, not the mount) does not try +# to mount the same device twice. +weights_mounted() { + awk -v mnt="${WEIGHTS_MOUNT}" '$2 == mnt { found = 1 } END { exit !found }' "${MOUNTS_FILE}" +} + +# The device backing "/", stripped to its parent disk (vda2 -> vda, +# nvme0n1p2 -> nvme0n1), so the ext4 scan below never mistakes the root disk +# for the weights volume. Read from MOUNTS_FILE rather than shelling out to +# lsblk/findmnt against a real device node, which a test harness cannot fake. +root_disk() { + # || true: a plain assignment from a command substitution is a checked + # command under set -e — an awk failure here (MOUNTS_FILE unreadable, + # never expected against the real /proc/mounts) would otherwise abort + # this function's caller's subshell outright instead of leaving _root_src + # empty, which the case below already handles safely. + _root_src=$(awk '$2 == "/" { print $1; exit }' "${MOUNTS_FILE}") || true + _name="${_root_src#/dev/}" + case "${_name}" in + *n[0-9]*p[0-9]*) echo "${_name}" | sed -E 's/p[0-9]+$//' ;; + *) echo "${_name}" | sed -E 's/[0-9]+$//' ;; + esac + return 0 +} + +# Prints the filesystem type on dev, nothing if it holds none, and returns +# non-zero when it could not find out at all: a blkid that is missing, errored +# or pointed at an absent node prints exactly what a blank disk prints, so +# only the exit status tells the two apart. +fstype_of() { + if _out=$(blkid "$1" 2>/dev/null); then + _rc=0 + else + _rc=$? + fi + # 0 is a signature found, 2 a clean probe that found none. + [ "${_rc}" -eq 0 ] || [ "${_rc}" -eq 2 ] || return 1 + printf '%s' "${_out}" | sed -n 's/.*[[:space:]]TYPE="\([^"]*\)".*/\1/p' +} + +# The one by-id link mulga-ebs-byid mints for a hot-attached Viperblock +# volume. One line per candidate; the caller decides what more than one means. +# Explicit `return 0`: this is a pure enumerator, not a signal, and the last +# glob iteration's test can otherwise leave the function's own exit status +# nonzero (see resolve_weights_device's callers for why that matters). +byid_candidates() { + for _l in "${BYID_DIR}"/*Elastic_Block_Store*; do + [ -e "${_l}" ] || continue + printf '%s\n' "${_l}" + done + return 0 +} + +# The sole non-root block device carrying an ext4 filesystem — sound only +# because a serving VM carries exactly one weights volume by construction +# (LaunchServingVM attaches exactly one volume per launch). +ext4_candidates() { + # || true: root_disk's own last command is a pipeline over MOUNTS_FILE, + # and an enumerator's exit status must never leak an unrelated tool's + # failure into the caller as if it meant something. + _root="$(root_disk)" || true + for _p in "${SYS_BLOCK}"/*; do + [ -d "${_p}" ] || continue + _name=$(basename "${_p}") || continue + case "${_name}" in + loop*|sr*|zram*) continue ;; + esac + [ "${_name}" = "${_root}" ] && continue + _dev="${DEV_DIR}/${_name}" + [ -e "${_dev}" ] || continue + _fstype=$(fstype_of "${_dev}") || continue + # Deliberately an `if`, not `[ ... ] && printf`: the latter leaves + # this loop iteration's (and so the function's) own exit status equal + # to the test's, so a last-processed disk that is not ext4 would make + # the enumerator itself look like it failed to its caller. + if [ "${_fstype}" = "ext4" ]; then + printf '%s\n' "${_dev}" + fi + done + return 0 +} + +# Tries the fallback chain once. Prints the device and returns 0 on a single +# match, returns 1 when nothing is found yet (worth retrying — the volume may +# still be attaching) and 2 when the guest's own layout is ambiguous (never +# worth retrying: waiting longer cannot make two candidates become one). +resolve_weights_device() { + # 1. The device the launcher recorded, if the guest's own enumeration + # happens to agree with it. + if [ -e "${VLLM_WEIGHTS_DEVICE}" ]; then + printf '%s\n' "${VLLM_WEIGHTS_DEVICE}" + return 0 + fi + + # 2. mulga-ebs-byid's link — a virtio/nvme guest does not necessarily + # present the launcher's named device, but it always mints exactly one of + # these for the one weights volume a serving VM carries. + _byid="$(byid_candidates)" + # grep -c exits 1 when it counts zero matches, which set -e would treat as + # this whole function failing rather than "no candidates yet" — || true + # keeps a legitimate zero from aborting the script. + _byid_count=$(printf '%s\n' "${_byid}" | grep -c . || true) + if [ "${_byid_count}" -eq 1 ]; then + printf '%s\n' "${_byid}" + return 0 + elif [ "${_byid_count}" -gt 1 ]; then + echo "[vllm-serve] ERROR: found ${_byid_count} EBS by-id links; expected exactly one weights volume" >&2 + return 2 + fi + + # 3. The last resort: whichever non-root disk holds an ext4 filesystem. + _ext4="$(ext4_candidates)" + # Same zero-matches-exits-1 guard as the by-id count above. + _ext4_count=$(printf '%s\n' "${_ext4}" | grep -c . || true) + if [ "${_ext4_count}" -eq 1 ]; then + printf '%s\n' "${_ext4}" + return 0 + elif [ "${_ext4_count}" -gt 1 ]; then + echo "[vllm-serve] ERROR: found ${_ext4_count} non-root ext4 volumes; expected exactly one weights volume" >&2 + return 2 + fi + + return 1 +} + +await_weights_device() { + _waited=0 + while :; do + # A plain `_dev=$(resolve_weights_device); _rc=$?` is NOT a checked + # context under set -e: the assignment's own failure (resolve + # returning 1 while the volume is still attaching, the normal case on + # every cold start) would exit the whole script on the spot, and + # `_rc=$?` would never run to tell the difference from "ambiguous". + # The if/else form is the one context set -e exempts, so the "not + # found yet, keep waiting" path actually gets to retry. + if _dev=$(resolve_weights_device); then + printf '%s\n' "${_dev}" + return 0 + else + _rc=$? + fi + [ "${_rc}" -eq 2 ] && fail "ambiguous weights device candidates — refusing to guess (see above)" + if [ "${_waited}" -ge "${DEVICE_WAIT_SECS}" ]; then + fail "no weights volume resolved after ${DEVICE_WAIT_SECS}s" + fi + sleep 1 + _waited=$((_waited + 1)) + done +} + +await_env_file + +command -v blkid >/dev/null 2>&1 || + fail "blkid is not installed in this image; the by-id and ext4-scan fallbacks cannot run without it" + +if weights_mounted; then + log "${WEIGHTS_MOUNT} is already a mount point; reusing it" +else + _device=$(await_weights_device) + log "weights device resolved to ${_device}" + mkdir -p "${WEIGHTS_MOUNT}" + # Read-only: nothing in the serving path writes weights, and a COW clone + # that gets dirtied costs storage for no gain. + mount -o ro "${_device}" "${WEIGHTS_MOUNT}" || + fail "could not mount ${_device} read-only at ${WEIGHTS_MOUNT}" + log "weights mounted read-only at ${WEIGHTS_MOUNT} from ${_device}" +fi + +# --served-model-name reports the Bedrock model ID via /v1/models, not the +# mount path — the gateway sends the Bedrock ID as the OpenAI `model` field. +# VLLM_ARGS is deliberately unquoted: it is a space-separated list of extra +# flags the operator supplied, and each one must reach vllm serve as its own +# argument. +# shellcheck disable=SC2086 +exec "${VENV_BIN}/vllm" serve "${WEIGHTS_MOUNT}" \ + --port "${VLLM_SERVE_PORT}" \ + --served-model-name "${VLLM_MODEL_ID}" \ + ${VLLM_ARGS} diff --git a/images/mkosi.profiles/vllm/mkosi.postinst.chroot b/images/mkosi.profiles/vllm/mkosi.postinst.chroot new file mode 100755 index 000000000..125f8adfb --- /dev/null +++ b/images/mkosi.profiles/vllm/mkosi.postinst.chroot @@ -0,0 +1,126 @@ +#!/bin/bash +set -euo pipefail + +# vllm postinst — bakes vLLM's OpenAI-compatible server into a uv-managed venv +# and enables vllm-serve.service. A venv, not a container: a runtime `docker +# pull vllm/vllm-openai` inside a guest is known-fatal on this platform (see +# the viperblock write-plane-throughput investigation — it stalled indefinitely +# and drove host load to ~124), and mkosi's unprivileged user-namespace build +# has no daemon in the builder to pull into regardless. Baking the venv here +# instead makes first boot a pure local read. +# +# The .chroot suffix means mkosi has already chrooted into the image root +# before this runs, same as gpu-nvidia's postinst. + +VLLM_HOME=/opt/vllm + +# The exact CPython version vLLM's own wheels are built and tested against. +# Ubuntu 26.04's system interpreter is newer than that range, and pinning +# against a distro default that moves is exactly the drift the mkosi +# migration exists to delete — uv fetches this standalone build instead, so +# the interpreter is a build input, not a distro accident. Bumping this is a +# deliberate edit, done alongside checking vLLM's published wheel matrix. +readonly VLLM_PYTHON_VERSION="3.12.8" + +# uv's own version, not the Python or vllm it installs. Astral's installer +# accepts a versioned URL, so this pin is the same "build input, not a distro +# accident" guarantee as VLLM_PYTHON_VERSION above — an unpinned `install.sh` +# would let the installer itself drift between two builds of the same commit. +readonly UV_VERSION="0.12.1" + +# The vLLM release baked into the venv. Pinned for the same reason as the +# CPython and uv versions above: two builds of the same commit must produce +# the same image. Bumping this is a deliberate edit, done alongside checking +# vLLM's published wheel matrix against VLLM_PYTHON_VERSION. +readonly VLLM_VERSION="0.26.0" + +PYTHON_INSTALL_DIR="${VLLM_HOME}/python" +VENV_DIR="${VLLM_HOME}/venv" + +mkdir -p "${VLLM_HOME}" + +# uv itself is not in the Ubuntu archive, so its own installer is used rather +# than Packages=. UV_NO_MODIFY_PATH because there is no interactive shell +# profile in a chroot for it to edit, and UV_INSTALL_DIR pins the destination. +UV=/usr/local/bin/uv +curl -LsSf "https://astral.sh/uv/${UV_VERSION}/install.sh" | + env UV_INSTALL_DIR=/usr/local/bin UV_NO_MODIFY_PATH=1 sh +# Checked as a path, not via `command -v`: mkosi runs this postinst with a +# minimal PATH that need not carry /usr/local/bin, so a PATH lookup reports a +# perfectly good install as missing. Every call below names ${UV} for the +# same reason. +[ -x "${UV}" ] || { + echo "ERROR: uv installer completed but ${UV} is not executable" >&2 + exit 1 +} + +echo "[vllm] installing CPython ${VLLM_PYTHON_VERSION} via uv" +UV_PYTHON_INSTALL_DIR="${PYTHON_INSTALL_DIR}" "${UV}" python install "${VLLM_PYTHON_VERSION}" + +echo "[vllm] creating venv at ${VENV_DIR}" +UV_PYTHON_INSTALL_DIR="${PYTHON_INSTALL_DIR}" "${UV}" venv --python "${VLLM_PYTHON_VERSION}" "${VENV_DIR}" + +echo "[vllm] installing vllm ${VLLM_VERSION} into ${VENV_DIR}" +UV_PYTHON_INSTALL_DIR="${PYTHON_INSTALL_DIR}" "${UV}" pip install --python "${VENV_DIR}/bin/python" "vllm==${VLLM_VERSION}" + +# The one thing this script must not do silently: ship an image whose venv +# does not actually hold what was asked for. Checked via package metadata and +# not by running `vllm --version`, which initialises device config and dies +# with "Failed to infer device type" in a build sandbox that has no GPU — +# importing vllm itself is out for the same reason. +if [ ! -x "${VENV_DIR}/bin/vllm" ]; then + echo "ERROR: no vllm entrypoint in the baked venv at ${VENV_DIR}" >&2 + exit 1 +fi +installed_vllm="$("${VENV_DIR}/bin/python" -c \ + 'import importlib.metadata as m; print(m.version("vllm"))')" +if [ "${installed_vllm}" != "${VLLM_VERSION}" ]; then + echo "ERROR: venv holds vllm ${installed_vllm}, expected the pinned ${VLLM_VERSION}" >&2 + exit 1 +fi +echo "[vllm] verified vllm ${installed_vllm} in ${VENV_DIR}" + +# 0644 in git, and ExtraTrees= preserves source mode — without this the unit +# dies with EACCES on first boot and nowhere earlier. +chmod 0755 /usr/local/sbin/vllm-serve + +systemctl enable vllm-serve.service + +# Without this the failure is a runtime one, minutes into a GPU cold start: +# FlashInfer JIT-builds its sampling module, finds no nvcc, and the endpoint +# never reaches READY. +UNIT=/etc/systemd/system/vllm-serve.service +grep -q '^Environment=VLLM_USE_FLASHINFER_SAMPLER=0$' "${UNIT}" || { + echo "ERROR: ${UNIT} does not disable the FlashInfer sampler; this image ships no CUDA toolkit for its JIT" >&2 + exit 1 +} + +# Assert the MERGED result, not the file's presence: the pin already existed at +# a 90- prefix in the EKS image and did nothing there, shadowed by 90_dpkg.cfg. +# read_conf_with_confd is the same function cloud-init calls at boot. +command -v python3 >/dev/null || { + echo "ERROR: python3 missing — cannot verify the merged cloud-init datasource_list" >&2 + exit 1 +} +python3 - <<'PY' +import sys + +try: + from cloudinit import util +except ImportError as e: + sys.exit(f"ERROR: cloudinit python module unavailable ({e}) — cloud-init is expected to be installed by now") + +cfg = util.read_conf_with_confd("/etc/cloud/cloud.cfg") +got = cfg.get("datasource_list") +want = ["Ec2", "None"] +if got != want: + sys.exit( + "ERROR: merged cloud-init datasource_list is " + f"{got!r}, expected {want!r} — the 99-spinifex-ec2-imds.cfg pin is " + "being shadowed by another cloud.cfg.d drop-in (check sort order: " + "read_conf_d() applies the directory in REVERSE, first source wins)" + ) +print(f"[vllm] cloud-init datasource_list assertion passed: {got!r}") +PY + +echo "vllm postinst complete: CPython ${VLLM_PYTHON_VERSION} / uv ${UV_VERSION} / vllm ${VLLM_VERSION} venv baked at ${VENV_DIR}, vllm-serve.service enabled" diff --git a/images/mkosi.profiles/vllm/vllm-serve_test.sh b/images/mkosi.profiles/vllm/vllm-serve_test.sh new file mode 100644 index 000000000..d6ee4a23f --- /dev/null +++ b/images/mkosi.profiles/vllm/vllm-serve_test.sh @@ -0,0 +1,346 @@ +#!/bin/sh +# Self-contained POSIX test for vllm-serve (mkosi.extra/usr/local/sbin/vllm-serve): +# the env-file wait, the three-tier weights-device resolution (named device, +# by-id fallback, ext4-scan fallback), the bounded waits' timeouts, and the +# final read-only mount + exec. No root and no real block devices: blkid, +# mount and the venv's vllm binary are stubbed on PATH, and every path the +# script touches is redirected into a temp dir via its env knobs. Not shipped +# into the image — it sits beside the profile, not under mkosi.extra/. +# +# Run: sh images/mkosi.profiles/vllm/vllm-serve_test.sh +set -eu + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +# VLLM_TEST_SCRIPT: override to point at an alternate copy of the wrapper +# (e.g. a saved pre-fix version), so a regression case can be run against +# both and its before/after behaviour compared directly. +SCRIPT="${VLLM_TEST_SCRIPT:-${SCRIPT_DIR}/mkosi.extra/usr/local/sbin/vllm-serve}" +WORK=$(mktemp -d) +trap 'rm -rf "${WORK}"' EXIT + +STUBBIN="${WORK}/bin" +mkdir -p "${STUBBIN}" + +# blkid stub: prints a real blkid's line for any device listed in FS_TABLE as +# " ", nothing (rc=2) for one that holds none. +cat > "${STUBBIN}/blkid" <<'EOF' +#!/bin/sh +dev="$1" +fstype=$(awk -v d="${dev}" '$1 == d { print $2 }' "${FS_TABLE}" 2>/dev/null) +[ -n "${fstype}" ] || exit 2 +echo "${dev}: UUID=\"1234-5678\" TYPE=\"${fstype}\"" +EOF + +# mount stub: records the call and, on success, appends the mount to +# MOUNTS_FILE so weights_mounted() sees the effect on a later invocation +# (idempotent-restart case), the way a real kernel's /proc/mounts would. +cat > "${STUBBIN}/mount" <<'EOF' +#!/bin/sh +echo "mount $*" >> "${MOUNT_CALLS}" +[ "${MOUNT_FAIL:-0}" = "1" ] && exit 1 +dev=""; mnt="" +for a in "$@"; do + case "$a" in -*) continue ;; esac + if [ -z "$dev" ]; then dev="$a"; else mnt="$a"; fi +done +echo "${dev} ${mnt} ext4 ro 0 0" >> "${MOUNTS_FILE}" +exit 0 +EOF + +# vllm stub: the venv binary the wrapper execs into. Records its argv so the +# test can assert the mount point, port and served-model-name reached it. +cat > "${STUBBIN}/vllm" <<'EOF' +#!/bin/sh +echo "vllm $*" >> "${VLLM_CALLS}" +exit 0 +EOF + +chmod +x "${STUBBIN}"/* +PATH="${STUBBIN}:${PATH}" +export PATH + +FAILS=0 +fail() { echo "FAIL: $*"; FAILS=$((FAILS + 1)); } +pass() { echo "ok: $*"; } + +ENV_FILE="${WORK}/vllm-serve.env" +WEIGHTS_MOUNT="${WORK}/weights" +SYS_BLOCK="${WORK}/sys/block" +DEV_DIR="${WORK}/dev" +BYID_DIR="${WORK}/by-id" +MOUNTS_FILE="${WORK}/mounts" +FS_TABLE="${WORK}/fs.table" +MOUNT_CALLS="${WORK}/mount.calls" +VLLM_CALLS="${WORK}/vllm.calls" +export FS_TABLE MOUNT_CALLS VLLM_CALLS MOUNTS_FILE + +write_env() { + # write_env [extra-args]: lay down the bootstrap handoff + # cloud-init would have written from buildServeUserData. + cat > "${ENV_FILE}" <: materialise a whole disk in the fake sysfs + /dev. +add_disk() { + mkdir -p "${SYS_BLOCK}/$1" "${DEV_DIR}" + : > "${DEV_DIR}/$1" +} + +reset_state() { + rm -rf "${WORK}/sys" "${DEV_DIR}" "${BYID_DIR}" "${WEIGHTS_MOUNT}" "${ENV_FILE}" + mkdir -p "${SYS_BLOCK}" "${DEV_DIR}" "${BYID_DIR}" + # Root is vda2, so root_disk() must resolve to "vda" and never treat it as + # a weights candidate. + printf '/dev/vda2 / ext4 rw,relatime 0 0\n' > "${MOUNTS_FILE}" + : > "${FS_TABLE}" + : > "${MOUNT_CALLS}" + : > "${VLLM_CALLS}" + add_disk vda +} + +# run: invoke vllm-serve with every path knob pointed into the temp dir and +# short bounded waits, so the "nothing ever appears" case does not stall the +# suite. +run() { + env VLLM_ENV_FILE="${ENV_FILE}" \ + VLLM_ENV_WAIT="${VLLM_ENV_WAIT:-2}" \ + VLLM_WEIGHTS_MOUNT="${WEIGHTS_MOUNT}" \ + VLLM_VENV_BIN="${STUBBIN}" \ + VLLM_DEVICE_WAIT="${VLLM_DEVICE_WAIT:-2}" \ + VLLM_SYS_BLOCK="${SYS_BLOCK}" \ + VLLM_DEV_DIR="${DEV_DIR}" \ + VLLM_BYID_DIR="${BYID_DIR}" \ + VLLM_MOUNTS_FILE="${MOUNTS_FILE}" \ + sh "${SCRIPT}" "${WORK}/out" 2>&1 || { fail "$1: non-zero exit: $(cat "${WORK}/out")"; return 1; }; } +run_fails() { run > "${WORK}/out" 2>&1 && fail "$1: expected a non-zero exit" || pass "$1: refused"; } + +# --- Case 1: named device present --- +reset_state +add_disk sdf +write_env "${DEV_DIR}/sdf" +if run_ok "named-device"; then + grep -q "mount -o ro ${DEV_DIR}/sdf ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "named-device: mounted read-only from the named device" || fail "named-device: not mounted from named device" + grep -q "vllm serve ${WEIGHTS_MOUNT} --port 8000 --served-model-name test-model" "${VLLM_CALLS}" \ + && pass "named-device: vllm exec'd with mount point, port and model name" || fail "named-device: vllm not invoked correctly" +fi + +# --- Case 2: named device absent, mulga-ebs-byid's by-id link used instead --- +reset_state +add_disk vdb +: > "${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0123456789abcdef" +write_env "/dev/sdf" +if run_ok "byid-fallback"; then + grep -q "mount -o ro ${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0123456789abcdef ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "byid-fallback: mounted from the by-id link" || fail "byid-fallback: not mounted from by-id link" +fi + +# --- Case 3: named device and by-id both absent, ext4-scan fallback used --- +reset_state +add_disk vdb +echo "${DEV_DIR}/vdb ext4" > "${FS_TABLE}" +write_env "/dev/sdf" +if run_ok "ext4-fallback"; then + grep -q "mount -o ro ${DEV_DIR}/vdb ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "ext4-fallback: mounted from the sole non-root ext4 disk" || fail "ext4-fallback: not mounted from ext4 disk" +fi + +# --- Case 3b: the root disk is never an ext4-scan candidate --- +reset_state +echo "${DEV_DIR}/vda ext4" > "${FS_TABLE}" +write_env "/dev/sdf" +run_fails "ext4-fallback-root-excluded" +grep -q . "${MOUNT_CALLS}" \ + && fail "ext4-fallback-root-excluded: mounted the root disk as weights" || pass "ext4-fallback-root-excluded: root disk excluded" + +# --- Case 4: nothing ever appears --- +# Times the run to prove the wrapper actually spun in the retry loop for the +# full DEVICE_WAIT_SECS bound rather than dying on the first "not found yet" +# — an exit-status-only check cannot tell a real timeout from an instant +# death (that was exactly how the set -e regression below slipped through). +reset_state +write_env "/dev/sdf" +_t0=$(date +%s) +VLLM_DEVICE_WAIT=3 run_fails "nothing-appears" +_t1=$(date +%s) +_elapsed=$((_t1 - _t0)) +[ "${_elapsed}" -ge 3 ] \ + && pass "nothing-appears: waited the full ${_elapsed}s bound before giving up" \ + || fail "nothing-appears: returned after only ${_elapsed}s, expected >= 3s (instant death, not a real wait)" +grep -q . "${MOUNT_CALLS}" \ + && fail "nothing-appears: mounted something that should not exist" || pass "nothing-appears: refused, nothing mounted" + +# --- Case 4b/c/d: device appears part-way through the bounded wait, for each +# of the three resolution tiers in turn. This is the case the exit-status-only +# "nothing-appears" case above cannot distinguish from an instant set -e +# death: only a run that keeps retrying past the first failed probe and then +# succeeds proves the retry loop itself works, not just that it eventually +# gives up. + +# 4b: named device absent on the first probe, appears mid-wait. +reset_state +write_env "${DEV_DIR}/sdf" +( sleep 1; add_disk sdf ) & +BGPID=$! +if VLLM_DEVICE_WAIT=5 run_ok "named-device-appears-midwait"; then + grep -q "mount -o ro ${DEV_DIR}/sdf ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "named-device-appears-midwait: retried until the named device showed up, then mounted it" \ + || fail "named-device-appears-midwait: did not mount the named device once it appeared" +fi +wait "${BGPID}" 2>/dev/null || true + +# 4c: by-id link absent on the first probe, appears mid-wait. +reset_state +add_disk vdb +write_env "/dev/sdf" +( sleep 1; : > "${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0123456789abcdef" ) & +BGPID=$! +if VLLM_DEVICE_WAIT=5 run_ok "byid-appears-midwait"; then + grep -q "mount -o ro ${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0123456789abcdef ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "byid-appears-midwait: retried until the by-id link showed up, then mounted it" \ + || fail "byid-appears-midwait: did not mount the by-id link once it appeared" +fi +wait "${BGPID}" 2>/dev/null || true + +# 4d: ext4-scan candidate absent on the first probe (neither the disk nor its +# filesystem exists yet), appears mid-wait. +reset_state +write_env "/dev/sdf" +( sleep 1; add_disk vdb; echo "${DEV_DIR}/vdb ext4" >> "${FS_TABLE}" ) & +BGPID=$! +if VLLM_DEVICE_WAIT=5 run_ok "ext4-appears-midwait"; then + grep -q "mount -o ro ${DEV_DIR}/vdb ${WEIGHTS_MOUNT}" "${MOUNT_CALLS}" \ + && pass "ext4-appears-midwait: retried until the ext4 disk showed up, then mounted it" \ + || fail "ext4-appears-midwait: did not mount the ext4 disk once it appeared" +fi +wait "${BGPID}" 2>/dev/null || true + +# --- Case 5: two by-id links is ambiguous, refused rather than guessed --- +# Ambiguity is never worth retrying (more waiting cannot turn two candidates +# into one), so this must fail fast, well inside the wait bound — timed here +# to positively confirm the 2 (ambiguous) path is distinct from the 1 (not +# yet) retry path, not just that both eventually return non-zero. +reset_state +: > "${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0000000000000001" +: > "${BYID_DIR}/nvme-Amazon_Elastic_Block_Store_vol0000000000000002" +write_env "/dev/sdf" +_t0=$(date +%s) +VLLM_DEVICE_WAIT=30 run_fails "byid-ambiguous" +_t1=$(date +%s) +_elapsed=$((_t1 - _t0)) +[ "${_elapsed}" -lt 15 ] \ + && pass "byid-ambiguous: failed fast in ${_elapsed}s rather than waiting out the 30s bound" \ + || fail "byid-ambiguous: took ${_elapsed}s, expected a fast refusal instead of retrying an unresolvable ambiguity" +grep -q . "${MOUNT_CALLS}" \ + && fail "byid-ambiguous: mounted one of two candidates" || pass "byid-ambiguous: nothing mounted" + +# --- Case 6: two non-root ext4 disks is likewise ambiguous --- +reset_state +add_disk vdb +add_disk vdc +echo "${DEV_DIR}/vdb ext4" > "${FS_TABLE}" +echo "${DEV_DIR}/vdc ext4" >> "${FS_TABLE}" +write_env "/dev/sdf" +run_fails "ext4-ambiguous" +grep -q . "${MOUNT_CALLS}" \ + && fail "ext4-ambiguous: mounted one of two candidates" || pass "ext4-ambiguous: nothing mounted" + +# --- Case 7: env file absent, then appears mid-wait --- +reset_state +add_disk sdf +VLLM_ENV_WAIT=5 +( sleep 1; write_env "${DEV_DIR}/sdf" ) & +BGPID=$! +if VLLM_ENV_WAIT=5 run_ok "env-appears-midwait"; then + pass "env-appears-midwait: wrapper picked up the handoff once cloud-init wrote it" +fi +wait "${BGPID}" 2>/dev/null || true +unset VLLM_ENV_WAIT + +# --- Case 7b: the file exists but is empty/partial, then gets completed a +# moment later — deterministic reproduction (not dependent on catching a real +# `cat >file < "${ENV_FILE}" +( sleep 1; write_env "${DEV_DIR}/sdf" ) & +BGPID=$! +if VLLM_ENV_WAIT=5 run_ok "env-partial-write-midwait"; then + pass "env-partial-write-midwait: retried past an empty handoff file until it was fully written" +fi +wait "${BGPID}" 2>/dev/null || true + +# --- Case 8: env file never appears --- +# Timed for the same reason as the device-tier "nothing-appears" case: an +# exit-status-only check cannot tell a real ENV_WAIT_SECS wait from an +# instant death. +reset_state +_t0=$(date +%s) +VLLM_ENV_WAIT=3 run_fails "env-never-appears" +_t1=$(date +%s) +_elapsed=$((_t1 - _t0)) +[ "${_elapsed}" -ge 3 ] \ + && pass "env-never-appears: waited the full ${_elapsed}s bound before giving up" \ + || fail "env-never-appears: returned after only ${_elapsed}s, expected >= 3s (instant death, not a real wait)" + +# --- Case 9: already-mounted weights point is a no-op restart --- +reset_state +add_disk sdf +write_env "${DEV_DIR}/sdf" +printf '%s %s ext4 ro 0 0\n' "${DEV_DIR}/sdf" "${WEIGHTS_MOUNT}" >> "${MOUNTS_FILE}" +if run_ok "already-mounted"; then + grep -q . "${MOUNT_CALLS}" \ + && fail "already-mounted: mounted over an existing mount" || pass "already-mounted: no-op, straight to exec" + grep -q "vllm serve" "${VLLM_CALLS}" \ + && pass "already-mounted: still execs vllm" || fail "already-mounted: vllm never invoked" +fi + +# --- Case 10: VLLM_ARGS passes each flag through as its own argument --- +reset_state +add_disk sdf +write_env "${DEV_DIR}/sdf" "--dtype=bfloat16 --max-model-len=4096" +if run_ok "extra-args"; then + grep -q -- "--dtype=bfloat16 --max-model-len=4096" "${VLLM_CALLS}" \ + && pass "extra-args: VLLM_ARGS flags reached vllm serve" || fail "extra-args: flags missing" +fi + +# --- Case 11: a non-numeric port is refused rather than passed through --- +# A malformed-but-present value must fail fast rather than retry — timed here +# against a generous ENV_WAIT to positively distinguish it from the "keys not +# there yet" retry path above (case 7b), the same distinction +# resolve_weights_device's ambiguous-tier cases make for the device wait. +reset_state +add_disk sdf +cat > "${ENV_FILE}" <<'EOF' +VLLM_MODEL_ID=test-model +VLLM_ARGS= +VLLM_WEIGHTS_DEVICE=/dev/sdf +VLLM_SERVE_PORT=not-a-number +EOF +_t0=$(date +%s) +VLLM_ENV_WAIT=30 run_fails "bad-port" +_t1=$(date +%s) +_elapsed=$((_t1 - _t0)) +[ "${_elapsed}" -lt 15 ] \ + && pass "bad-port: failed fast in ${_elapsed}s rather than retrying an unfixable value" \ + || fail "bad-port: took ${_elapsed}s, expected a fast refusal instead of waiting out the 30s bound" +grep -q . "${VLLM_CALLS}" \ + && fail "bad-port: vllm invoked with a bad port" || pass "bad-port: refused before exec" + +if [ "${FAILS}" -eq 0 ]; then + echo "PASS: all vllm-serve cases" + exit 0 +fi +echo "FAILED: ${FAILS} case(s)" +exit 1 diff --git a/scripts/images/common/mulga-mgmt-net.sh b/scripts/images/common/mulga-mgmt-net.sh index c20f0e0fc..1193b7125 100644 --- a/scripts/images/common/mulga-mgmt-net.sh +++ b/scripts/images/common/mulga-mgmt-net.sh @@ -58,7 +58,10 @@ dhcp_oneshot() { if command -v udhcpc >/dev/null 2>&1 && [ -x /usr/share/udhcpc/default.script ]; then udhcpc -i "$iface" -f -q -n -t 8 -T 2 >/dev/null 2>&1 elif command -v dhcpcd >/dev/null 2>&1; then - dhcpcd -q -t 20 "$iface" >/dev/null 2>&1 + # -q is only quiet; -1 is what exits, and -p keeps the address on the + # way out. A surviving daemon owns the control socket, so cloud-init's + # own dhcpcd is forwarded to it and never writes the pid file it waits on. + dhcpcd -q -1 -p -t 20 "$iface" >/dev/null 2>&1 else echo "[mulga-mgmt-net] no DHCP client (udhcpc/dhcpcd) for $iface" >&2 return 1 diff --git a/scripts/images/common/mulga-mgmt-net_test.sh b/scripts/images/common/mulga-mgmt-net_test.sh index cfa80b288..014a0e95a 100644 --- a/scripts/images/common/mulga-mgmt-net_test.sh +++ b/scripts/images/common/mulga-mgmt-net_test.sh @@ -11,7 +11,15 @@ set -eu SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) SCRIPT="${SCRIPT_DIR}/mulga-mgmt-net.sh" WORK=$(mktemp -d) -trap 'rm -rf "${WORK}"' EXIT +# Reaps the stub's simulated daemons too, so a failing run does not strand +# background processes on the test host. +cleanup() { + if [ -s "${WORK}/daemons" ]; then + while read -r pid _; do kill "${pid}" 2>/dev/null || true; done < "${WORK}/daemons" + fi + rm -rf "${WORK}" +} +trap cleanup EXIT STUBBIN="${WORK}/bin" mkdir -p "${STUBBIN}" @@ -86,6 +94,26 @@ for a in "$@"; do prev="${a}" done [ -n "${iface}" ] || { for a in "$@"; do iface="${a}"; done; } + +# Both clients keep running unless told to exit, and the flag that does it is +# per-client: dhcpcd needs -1 (-q is only quiet), udhcpc needs -q. Leaving a +# live process behind is what the real client does, so the stub does it too. +oneshot=no +case "$(basename "$0")" in + dhcpcd) case " $* " in *" -1 "*) oneshot=yes ;; esac ;; + udhcpc) case " $* " in *" -q "*) oneshot=yes ;; esac ;; +esac +if [ "${oneshot}" = no ]; then + sleep 30 & + echo "$! $(basename "$0")" >> "${DAEMONS}" +fi + +# dhcpcd de-configures the interface on exit unless -p, so a one-shot without +# it hands back a link with no address at all. +if [ "$(basename "$0")" = dhcpcd ] && [ "${oneshot}" = yes ]; then + case " $* " in *" -p "*) ;; *) exit 0 ;; esac +fi + echo "${iface}" >> "${LEASED}" # What a real lease installs, and the reason the policy exists at all: OVN serves # the subnet's router IP as the gateway, which is what the return path needs. @@ -157,10 +185,11 @@ DEFAULTS="${WORK}/defaults"; export DEFAULTS LEASED="${WORK}/leased"; export LEASED TABLES="${WORK}/tables"; export TABLES RULES="${WORK}/rules"; export RULES +DAEMONS="${WORK}/daemons"; export DAEMONS reset_state() { : > "${IP_CALLS}"; : > "${DEFAULTS}"; : > "${LEASED}" - : > "${TABLES}"; : > "${RULES}" + : > "${TABLES}"; : > "${RULES}"; : > "${DAEMONS}" } called() { grep -qF "$1" "${IP_CALLS}"; } @@ -169,6 +198,9 @@ leased() { grep -q "^$1\$" "${LEASED}"; } # A route in policy table $1 for destination $2, and its full command in $3. has_route() { grep -q "^$1 $2 .*$3" "${TABLES}"; } rule_count() { grep -c "^$1 " "${RULES}"; } +# Reports which client was left running, not just that one was: the two +# branches fail for different reasons and the flag that fixes them differs. +survivors() { [ -s "${DAEMONS}" ] && { echo "surviving DHCP clients: $(cat "${DAEMONS}")"; return 0; }; return 1; } # --- The setup pass ------------------------------------------------------- @@ -194,6 +226,8 @@ want 'called "ip route replace 169.254.169.254/32 via 10.251.185.1 dev eth0"' \ # test escaped review. want 'grep -q "configured eth2" "${WORK}/setup.out"' "setup reaches every NIC in the blob" +wantnot 'survivors' "setup leaves no DHCP client running on any NIC" + # --- The customer ENI's return path --------------------------------------- # Deleting the customer ENI's default route is only half the policy: without a @@ -221,6 +255,26 @@ fi want '[ "$(rule_count 101)" -eq 1 ]' "a repeated setup pass leaves exactly one rule" want '[ "$(grep -c "^101 " "${TABLES}")" -eq 2 ]' "a repeated setup pass leaves exactly two routes in the table" +# --- The dhcpcd fallback -------------------------------------------------- + +# Forced by removing udhcpc from PATH rather than left to whether the host has +# /usr/share/udhcpc/default.script: every Ubuntu image takes this branch, and it +# is the one that stranded a daemon on the data NIC and cost cloud-init 300s. +FALLBACKBIN="${WORK}/fallback-bin" +mkdir -p "${FALLBACKBIN}" +cp "${STUBBIN}/ip" "${STUBBIN}/dhcpcd" "${FALLBACKBIN}/" +chmod +x "${FALLBACKBIN}"/* + +reset_state +if ! PATH="${FALLBACKBIN}:/usr/bin:/bin" sh "${SCRIPT}" > "${WORK}/fallback.out" 2>&1; then + fail "dhcpcd fallback pass exited non-zero" + cat "${WORK}/fallback.out" +fi + +wantnot 'survivors' "the dhcpcd fallback leaves no daemon on the data NIC" +want 'leased eth0' "the dhcpcd fallback still leaves the interface addressed after it exits" +want 'called "dhcp -q -1 -p -t 20 eth0"' "the dhcpcd fallback is invoked one-shot and persistent" + # --- The enforce pass ----------------------------------------------------- # cloud-init's network stage re-renders the ENIs from IMDS and re-DHCPs them, diff --git a/scripts/images/eks-node/cloud-init-ec2.cfg b/scripts/images/eks-node/cloud-init-ec2.cfg index 8861d944c..4b23d96f7 100644 --- a/scripts/images/eks-node/cloud-init-ec2.cfg +++ b/scripts/images/eks-node/cloud-init-ec2.cfg @@ -1,5 +1,9 @@ -# Spinifex eks-node cloud-init datasource pin (installed to -# /etc/cloud/cloud.cfg.d/90-spinifex-ec2-imds.cfg). +# Spinifex cloud-init datasource pin, shared by every system-VM image. +# The installed name differs by image and the prefix is load-bearing: +# read_conf_d() applies cloud.cfg.d in REVERSE and mergemanydict() is +# first-source-wins, so a 90- name loses to cloud-init's own 90_dpkg.cfg +# ('_' sorts above '-') and is silently discarded. The vllm profile installs +# it as 99-spinifex-ec2-imds.cfg for that reason. # # The system VM's user-data is delivered only via IMDS (the NoCloud seed # was retired in #411), and IMDS (169.254.169.254) is reachable only once diff --git a/scripts/mkosi-build.sh b/scripts/mkosi-build.sh index c61372d99..2757cdf03 100755 --- a/scripts/mkosi-build.sh +++ b/scripts/mkosi-build.sh @@ -54,6 +54,10 @@ image_profiles() { case "$1" in ubuntu-gpu-nvidia) echo "gpu-nvidia docker" ;; ubuntu-gpu-amd) echo "gpu-amd docker" ;; + # docker is deliberately absent: the serving VM runs vLLM as a systemd + # service and never touches a Docker daemon. vllm comes second because + # it stacks onto the GPU driver gpu-nvidia installs. + ubuntu-vllm-serving) echo "gpu-nvidia vllm" ;; spinifex-eks-node-gpu) echo "gpu-nvidia eks-common eks-agent" ;; spinifex-ecs-node-gpu) echo "gpu-nvidia ecs" ;; # The CPU variant is the GPU one minus gpu-nvidia. ecs pulls nothing @@ -236,7 +240,7 @@ if [[ -n "${IMAGE}" ]]; then fi if ! expanded="$(image_profiles "${IMAGE}")"; then echo "mkosi-build: unknown image: ${IMAGE}" >&2 - echo "mkosi-build: known images: ubuntu-gpu-nvidia ubuntu-gpu-amd spinifex-eks-node-gpu spinifex-ecs-node-gpu spinifex-ecs-node spinifex-eks-node" >&2 + echo "mkosi-build: known images: ubuntu-gpu-nvidia ubuntu-gpu-amd ubuntu-vllm-serving spinifex-eks-node-gpu spinifex-ecs-node-gpu spinifex-ecs-node spinifex-eks-node" >&2 exit 2 fi read -r -a PROFILES <<< "${expanded}" diff --git a/spinifex/daemon/bedrock_deps.go b/spinifex/daemon/bedrock_deps.go index 0ef2820ec..6199b9036 100644 --- a/spinifex/daemon/bedrock_deps.go +++ b/spinifex/daemon/bedrock_deps.go @@ -8,6 +8,7 @@ import ( handlers_bedrock "github.com/mulgadc/spinifex/spinifex/handlers/bedrock" handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds" handlers_systemvpc "github.com/mulgadc/spinifex/spinifex/handlers/systemvpc" + "github.com/mulgadc/spinifex/spinifex/network/host" "github.com/nats-io/nats.go/jetstream" ) @@ -45,6 +46,10 @@ func (d *Daemon) buildBedrockLaunchDeps() handlers_bedrock.LaunchDeps { Volume: d.volumeService, Attacher: handlers_rds.NewNATSVolumeAttacher(d.natsConn), Weights: weights, + // A fresh plumber rather than d.networkPlumber: it is stateless, and + // taking it here would order this call after startLocal. + HostPort: host.NewOVSPlumber(), + NodeID: d.node, } } diff --git a/spinifex/handlers/bedrock/fakes_test.go b/spinifex/handlers/bedrock/fakes_test.go index 0a7cbd331..43f48ba78 100644 --- a/spinifex/handlers/bedrock/fakes_test.go +++ b/spinifex/handlers/bedrock/fakes_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "slices" "sync" "sync/atomic" @@ -144,32 +145,90 @@ func (f *fakeSystemVPC) DescribeInternetGateways(_ context.Context, in *ec2.Desc const testModelID = "meta.llama3-2-1b-instruct-v1:0" // fakeVPC is an in-memory launchVPCProvisioner: every CreateNetworkInterface -// call returns a fresh ENI ID/IP so concurrent launches never collide. +// call returns a fresh ENI ID/IP so concurrent launches never collide. Records +// are retained so DescribeNetworkInterfaces can answer the describe-or-create +// lookup the daemon port depends on for idempotence. type fakeVPC struct { mu sync.Mutex nextID int created []string deleted []string detached []string + records map[string]*ec2.NetworkInterface } -func (f *fakeVPC) CreateNetworkInterface(_ context.Context, _ *ec2.CreateNetworkInterfaceInput, _ string) (*ec2.CreateNetworkInterfaceOutput, error) { +func (f *fakeVPC) CreateNetworkInterface(_ context.Context, in *ec2.CreateNetworkInterfaceInput, _ string) (*ec2.CreateNetworkInterfaceOutput, error) { f.mu.Lock() + defer f.mu.Unlock() f.nextID++ id := f.nextID eniID := fmt.Sprintf("eni-%d", id) f.created = append(f.created, eniID) - f.mu.Unlock() - return &ec2.CreateNetworkInterfaceOutput{NetworkInterface: &ec2.NetworkInterface{ - NetworkInterfaceId: aws.String(fmt.Sprintf("eni-%d", id)), + ni := &ec2.NetworkInterface{ + NetworkInterfaceId: aws.String(eniID), PrivateIpAddress: aws.String(fmt.Sprintf("10.244.1.%d", id%250+1)), MacAddress: aws.String("02:00:00:00:00:01"), - }}, nil + SubnetId: in.SubnetId, + Description: in.Description, + } + if f.records == nil { + f.records = map[string]*ec2.NetworkInterface{} + } + f.records[eniID] = ni + return &ec2.CreateNetworkInterfaceOutput{NetworkInterface: ni}, nil +} + +// putRecord seeds an ENI the fake did not mint, so a test can present the +// lookup with a record it would not otherwise produce. +func (f *fakeVPC) putRecord(eniID, subnetID, description, ip, mac string) { + f.mu.Lock() + defer f.mu.Unlock() + if f.records == nil { + f.records = map[string]*ec2.NetworkInterface{} + } + f.records[eniID] = &ec2.NetworkInterface{ + NetworkInterfaceId: aws.String(eniID), + SubnetId: aws.String(subnetID), + Description: aws.String(description), + PrivateIpAddress: aws.String(ip), + MacAddress: aws.String(mac), + } +} + +// DescribeNetworkInterfaces honours only the subnet-id and description filters, +// which are the two the daemon-port lookup uses (ENIs carry no tag filter). +func (f *fakeVPC) DescribeNetworkInterfaces(_ context.Context, in *ec2.DescribeNetworkInterfacesInput, _ string) (*ec2.DescribeNetworkInterfacesOutput, error) { + f.mu.Lock() + defer f.mu.Unlock() + var out []*ec2.NetworkInterface + for _, ni := range f.records { + match := true + for _, filter := range in.Filters { + var got string + switch aws.StringValue(filter.Name) { + case "subnet-id": + got = aws.StringValue(ni.SubnetId) + case "description": + got = aws.StringValue(ni.Description) + default: + return nil, fmt.Errorf("fakeVPC: unsupported ENI filter %q", aws.StringValue(filter.Name)) + } + if !slices.Contains(aws.StringValueSlice(filter.Values), got) { + match = false + break + } + } + if match { + out = append(out, ni) + } + } + return &ec2.DescribeNetworkInterfacesOutput{NetworkInterfaces: out}, nil } func (f *fakeVPC) DeleteNetworkInterface(_ context.Context, in *ec2.DeleteNetworkInterfaceInput, _ string) (*ec2.DeleteNetworkInterfaceOutput, error) { f.mu.Lock() f.deleted = append(f.deleted, aws.StringValue(in.NetworkInterfaceId)) + delete(f.records, aws.StringValue(in.NetworkInterfaceId)) f.mu.Unlock() return &ec2.DeleteNetworkInterfaceOutput{}, nil } @@ -314,6 +373,38 @@ func (s *stubRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) }, nil } +// fakeHostPort records the host-side ports a launch asked for, so tests can +// assert the daemon gets a routed presence before any VM is booted. +type fakeHostPort struct { + mu sync.Mutex + ensured []hostPortCall + failErr error +} + +// hostPortCall captures the args of one EnsureVPCHostPort invocation. +type hostPortCall struct { + eniID string + mac string + addr string +} + +func (f *fakeHostPort) EnsureVPCHostPort(eniID, mac, addr string) error { + f.mu.Lock() + f.ensured = append(f.ensured, hostPortCall{eniID: eniID, mac: mac, addr: addr}) + f.mu.Unlock() + return f.failErr +} + +func (f *fakeHostPort) calls() []hostPortCall { + f.mu.Lock() + defer f.mu.Unlock() + return slices.Clone(f.ensured) +} + +// testNodeID names the daemon replica in tests, since a launch refuses to run +// without one (each node owns a distinct system-VPC port). +const testNodeID = "node-a" + // launchHarness bundles every LaunchDeps fake so a test can reach into any of // them, mirroring handlers_rds's launchHarness. type launchHarness struct { @@ -324,6 +415,7 @@ type launchHarness struct { volumes *fakeVolume attacher *fakeAttacher weights fakeWeightsResolver + hostPort *fakeHostPort } func newLaunchHarness() *launchHarness { @@ -334,6 +426,7 @@ func newLaunchHarness() *launchHarness { volumes: &fakeVolume{}, attacher: &fakeAttacher{}, weights: fakeWeightsResolver{snapshotID: "snap-llama32-1b", resolvable: true}, + hostPort: &fakeHostPort{}, } } @@ -347,6 +440,8 @@ func (h *launchHarness) deps() LaunchDeps { Volume: h.volumes, Attacher: h.attacher, Weights: h.weights, + HostPort: h.hostPort, + NodeID: testNodeID, } } diff --git a/spinifex/handlers/bedrock/hostport.go b/spinifex/handlers/bedrock/hostport.go new file mode 100644 index 000000000..7ba78583c --- /dev/null +++ b/spinifex/handlers/bedrock/hostport.go @@ -0,0 +1,138 @@ +package handlers_bedrock + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/netip" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/mulgadc/spinifex/spinifex/tags" + "github.com/mulgadc/spinifex/spinifex/utils" +) + +// The system-VPC subnet lives inside OVN on br-int, so the daemon host has no +// route to it and cannot dial a serving VM at all. This is not only a readiness +// concern: InvokeModel is proxied from inside the daemon too, so the same hop +// carries real inference traffic. +const ( + bedrockDaemonPortRole = "daemon-port" + bedrockDaemonNodeTagKey = "spinifex:bedrock-node" +) + +// hostPortPlumber installs the host-side port backing the daemon's own ENI. +// Narrowed to what this package uses so it does not import the vm package. +type hostPortPlumber interface { + EnsureVPCHostPort(eniID, mac, addr string) error +} + +// daemonPortDescription is what idempotence keys off. ENIs carry no tag filter +// in DescribeNetworkInterfaces, but description is filterable, so the node's +// identity goes there and the tags are for operator visibility only. +func daemonPortDescription(nodeID string) string { + return "Bedrock daemon port for node " + nodeID +} + +// EnsureDaemonPort gives this daemon a routed presence in the Bedrock system +// VPC, so BaseURL's private address becomes reachable. One ENI per node, not +// per endpoint: describe-or-create means a restart or a second launch reuses +// the same address rather than leaking one per launch. +// +// Called after the system VPC is ensured and before the serving VM launches. +// Both halves are idempotent, so a host reboot is re-driven by the next launch +// without any separate recovery path. +func EnsureDaemonPort(ctx context.Context, deps LaunchDeps, subnetID, subnetCIDR string) error { + if deps.HostPort == nil { + return errors.New("bedrock: no host-port plumber configured; the daemon cannot reach a serving VM") + } + if deps.NodeID == "" { + return errors.New("bedrock: no node id; cannot identify this daemon's system-VPC port") + } + subnet, err := netip.ParsePrefix(subnetCIDR) + if err != nil { + return fmt.Errorf("bedrock: parse system subnet CIDR %q: %w", subnetCIDR, err) + } + + eni, err := ensureDaemonENI(ctx, deps.VPC, subnetID, deps.NodeID) + if err != nil { + return err + } + ip, err := netip.ParseAddr(eni.ip) + if err != nil { + return fmt.Errorf("bedrock: daemon ENI %s has unparseable address %q: %w", eni.id, eni.ip, err) + } + + // The ENI's own address at the SUBNET's prefix length. A /32 would address + // the port and still leave every serving VM unreachable; this prefix is what + // installs the connected route that makes them reachable. + addr := netip.PrefixFrom(ip, subnet.Bits()) + if err := deps.HostPort.EnsureVPCHostPort(eni.id, eni.mac, addr.String()); err != nil { + return fmt.Errorf("bedrock: install daemon host port for ENI %s: %w", eni.id, err) + } + slog.InfoContext(ctx, "bedrock: daemon system-VPC port ready", + "node", deps.NodeID, "eni", eni.id, "addr", addr) + return nil +} + +// ensureDaemonENI describe-or-creates this node's ENI in the system subnet. +// Unattached to any VM: it exists so ovn-controller has a logical switch port +// to bind the host's internal port to. +func ensureDaemonENI(ctx context.Context, vpcSvc launchVPCProvisioner, subnetID, nodeID string) (*launchENI, error) { + desc := daemonPortDescription(nodeID) + out, err := vpcSvc.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{ + {Name: aws.String("subnet-id"), Values: aws.StringSlice([]string{subnetID})}, + {Name: aws.String("description"), Values: aws.StringSlice([]string{desc})}, + }, + }, utils.GlobalAccountID) + if err != nil { + return nil, fmt.Errorf("bedrock: describe daemon ENI in subnet %s: %w", subnetID, err) + } + if out != nil { + for _, ni := range out.NetworkInterfaces { + eni := &launchENI{ + id: aws.StringValue(ni.NetworkInterfaceId), + ip: aws.StringValue(ni.PrivateIpAddress), + mac: aws.StringValue(ni.MacAddress), + } + // A record missing either field cannot back a host port, so fall + // through and mint a usable one rather than fail the launch. + if eni.id != "" && eni.ip != "" && eni.mac != "" { + return eni, nil + } + slog.WarnContext(ctx, "bedrock: ignoring incomplete daemon ENI record", + "node", nodeID, "eni", eni.id) + } + } + + created, err := vpcSvc.CreateNetworkInterface(ctx, &ec2.CreateNetworkInterfaceInput{ + SubnetId: aws.String(subnetID), + Description: aws.String(desc), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("network-interface"), + Tags: []*ec2.Tag{ + {Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByBedrock)}, + {Key: aws.String(bedrockSystemVPCRoleTagKey), Value: aws.String(bedrockDaemonPortRole)}, + {Key: aws.String(bedrockDaemonNodeTagKey), Value: aws.String(nodeID)}, + }, + }}, + }, utils.GlobalAccountID) + if err != nil { + return nil, fmt.Errorf("bedrock: create daemon ENI in subnet %s: %w", subnetID, err) + } + if created == nil || created.NetworkInterface == nil { + return nil, fmt.Errorf("bedrock: create daemon ENI in subnet %s: no interface returned", subnetID) + } + ni := created.NetworkInterface + eni := &launchENI{ + id: aws.StringValue(ni.NetworkInterfaceId), + ip: aws.StringValue(ni.PrivateIpAddress), + mac: aws.StringValue(ni.MacAddress), + } + if eni.id == "" || eni.ip == "" || eni.mac == "" { + return nil, fmt.Errorf("bedrock: create daemon ENI in subnet %s: incomplete interface returned", subnetID) + } + return eni, nil +} diff --git a/spinifex/handlers/bedrock/hostport_test.go b/spinifex/handlers/bedrock/hostport_test.go new file mode 100644 index 000000000..d647d4677 --- /dev/null +++ b/spinifex/handlers/bedrock/hostport_test.go @@ -0,0 +1,149 @@ +package handlers_bedrock + +import ( + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testSystemSubnet = "subnet-bedrocksys-0001" + +func TestEnsureDaemonPort_CreatesENIAndHostPort(t *testing.T) { + h := newLaunchHarness() + require.NoError(t, EnsureDaemonPort(t.Context(), h.deps(), testSystemSubnet, "10.244.1.0/24")) + + require.Len(t, h.vpc.created, 1, "one daemon ENI should be minted") + calls := h.hostPort.calls() + require.Len(t, calls, 1) + assert.Equal(t, h.vpc.created[0], calls[0].eniID) + assert.Equal(t, "02:00:00:00:00:01", calls[0].mac) +} + +// The port must carry the ENI address at the SUBNET's prefix length: a /32 +// addresses the port and still leaves every serving VM unreachable, which is +// the entire reason the port exists. +func TestEnsureDaemonPort_UsesSubnetPrefixLength(t *testing.T) { + h := newLaunchHarness() + require.NoError(t, EnsureDaemonPort(t.Context(), h.deps(), testSystemSubnet, "10.244.1.0/24")) + + calls := h.hostPort.calls() + require.Len(t, calls, 1) + assert.True(t, strings.HasSuffix(calls[0].addr, "/24"), + "host port address %q should carry the subnet's /24, not a host route", calls[0].addr) + assert.False(t, strings.HasPrefix(calls[0].addr, "10.244.1.0/"), + "host port address %q must be the ENI's own address, not the network address", calls[0].addr) +} + +// One ENI per node, not one per launch: without describe-or-create every +// endpoint would leak an address out of the system subnet. +func TestEnsureDaemonPort_ReusesTheNodesExistingENI(t *testing.T) { + h := newLaunchHarness() + deps := h.deps() + require.NoError(t, EnsureDaemonPort(t.Context(), deps, testSystemSubnet, "10.244.1.0/24")) + require.NoError(t, EnsureDaemonPort(t.Context(), deps, testSystemSubnet, "10.244.1.0/24")) + + assert.Len(t, h.vpc.created, 1, "the second ensure should reuse the first ENI") + calls := h.hostPort.calls() + require.Len(t, calls, 2, "the host port is still re-driven, since a reboot clears it") + assert.Equal(t, calls[0], calls[1], "both ensures should install the same port") +} + +// Two daemons sharing one address would fight over the same OVN logical port, +// so the node identity has to reach the ENI lookup. +func TestEnsureDaemonPort_DistinctENIPerNode(t *testing.T) { + h := newLaunchHarness() + a := h.deps() + a.NodeID = "node-a" + b := h.deps() + b.NodeID = "node-b" + + require.NoError(t, EnsureDaemonPort(t.Context(), a, testSystemSubnet, "10.244.1.0/24")) + require.NoError(t, EnsureDaemonPort(t.Context(), b, testSystemSubnet, "10.244.1.0/24")) + + assert.Len(t, h.vpc.created, 2, "each node should own its own ENI") + calls := h.hostPort.calls() + require.Len(t, calls, 2) + assert.NotEqual(t, calls[0].eniID, calls[1].eniID) +} + +func TestEnsureDaemonPort_RequiresPlumberAndNodeID(t *testing.T) { + h := newLaunchHarness() + + noPlumber := h.deps() + noPlumber.HostPort = nil + err := EnsureDaemonPort(t.Context(), noPlumber, testSystemSubnet, "10.244.1.0/24") + require.Error(t, err) + assert.Contains(t, err.Error(), "host-port plumber") + + noNode := h.deps() + noNode.NodeID = "" + err = EnsureDaemonPort(t.Context(), noNode, testSystemSubnet, "10.244.1.0/24") + require.Error(t, err) + assert.Contains(t, err.Error(), "node id") + + assert.Empty(t, h.vpc.created, "neither refusal should have minted an ENI") +} + +func TestEnsureDaemonPort_RejectsUnparseableSubnetCIDR(t *testing.T) { + h := newLaunchHarness() + err := EnsureDaemonPort(t.Context(), h.deps(), testSystemSubnet, "not-a-cidr") + require.Error(t, err) + assert.Empty(t, h.vpc.created, "the CIDR is validated before anything is created") +} + +func TestEnsureDaemonPort_PropagatesPlumberFailure(t *testing.T) { + h := newLaunchHarness() + h.hostPort.failErr = errors.New("ovs-vsctl exploded") + err := EnsureDaemonPort(t.Context(), h.deps(), testSystemSubnet, "10.244.1.0/24") + require.Error(t, err) + assert.Contains(t, err.Error(), "ovs-vsctl exploded") +} + +// An ENI record with no address or MAC cannot back a host port, so the lookup +// must fall through and mint a usable one rather than install a broken port. +func TestEnsureDaemonPort_SkipsIncompleteENIRecords(t *testing.T) { + h := newLaunchHarness() + h.vpc.putRecord("eni-broken", testSystemSubnet, daemonPortDescription(testNodeID), "", "") + + require.NoError(t, EnsureDaemonPort(t.Context(), h.deps(), testSystemSubnet, "10.244.1.0/24")) + calls := h.hostPort.calls() + require.Len(t, calls, 1) + assert.NotEqual(t, "eni-broken", calls[0].eniID) + assert.NotEmpty(t, calls[0].mac) +} + +// A serving VM the daemon cannot dial is worse than no VM: it holds a GPU for +// the whole readiness window before unwinding. So the port comes first, and a +// port that cannot be installed stops the launch before anything is created. +func TestLaunchServingVM_RefusesWhenTheDaemonPortFails(t *testing.T) { + h := newLaunchHarness() + h.hostPort.failErr = errors.New("br-int is not there") + + _, err := LaunchServingVM(t.Context(), h.deps(), testLaunchInput()) + require.Error(t, err) + assert.Empty(t, h.launcher.launches(), "no VM may be launched with no path to reach it") + assert.Empty(t, h.volumes.created, "no weights volume may be cloned either") +} + +// The daemon's own ENI must not be confused with a serving VM's: only the +// serving ENI is ever handed to the launcher. +func TestLaunchServingVM_DaemonPortENIIsNotTheServingENI(t *testing.T) { + h := newLaunchHarness() + out, err := LaunchServingVM(t.Context(), h.deps(), testLaunchInput()) + require.NoError(t, err) + + calls := h.hostPort.calls() + require.Len(t, calls, 1) + assert.NotEqual(t, out.ENIID, calls[0].eniID) + assert.Contains(t, out.BaseURL, out.PrivateIP, "BaseURL still names the serving VM's own address") +} + +// The description is what idempotence keys off, since DescribeNetworkInterfaces +// has no tag filter. A change to its shape silently orphans every existing port. +func TestDaemonPortDescriptionCarriesNodeID(t *testing.T) { + assert.Contains(t, daemonPortDescription("node-a"), "node-a") + assert.NotEqual(t, daemonPortDescription("node-a"), daemonPortDescription("node-b")) +} diff --git a/spinifex/handlers/bedrock/launch.go b/spinifex/handlers/bedrock/launch.go index 98a18e901..e85cf8195 100644 --- a/spinifex/handlers/bedrock/launch.go +++ b/spinifex/handlers/bedrock/launch.go @@ -47,6 +47,7 @@ var ErrNoWeightsStaged = errors.New("bedrock: no weights snapshot staged for thi type launchVPCProvisioner interface { CreateNetworkInterface(ctx context.Context, input *ec2.CreateNetworkInterfaceInput, accountID string) (*ec2.CreateNetworkInterfaceOutput, error) + DescribeNetworkInterfaces(ctx context.Context, input *ec2.DescribeNetworkInterfacesInput, accountID string) (*ec2.DescribeNetworkInterfacesOutput, error) DeleteNetworkInterface(ctx context.Context, input *ec2.DeleteNetworkInterfaceInput, accountID string) (*ec2.DeleteNetworkInterfaceOutput, error) DetachENI(ctx context.Context, accountID, eniID string) error } @@ -71,6 +72,12 @@ type LaunchDeps struct { Volume launchVolumeProvisioner Attacher volumeAttacher Weights gateway_bedrock.WeightsResolver + // HostPort installs this daemon's own port into the system VPC. Without it + // nothing the launch produces is reachable, so a launch refuses to run. + HostPort hostPortPlumber + // NodeID names the daemon replica, so each node's system-VPC port is a + // distinct ENI rather than one they contend for. + NodeID string } // LaunchInput describes the serving VM to launch. Everything here is already @@ -123,6 +130,12 @@ func LaunchServingVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (out } systemSubnetID := sysRefs.PrivateSubnetIDs[0] + // Before anything is created: a VM this daemon cannot dial is worse than no + // VM at all, since it holds a GPU for the whole readiness window first. + if err := EnsureDaemonPort(ctx, deps, systemSubnetID, sysRefs.PrivateSubnetCIDRs[0]); err != nil { + return nil, err + } + // Unwind in reverse creation order on any failure below. Each step appends // its own undo as soon as the resource exists. var rollback []func(context.Context) diff --git a/spinifex/handlers/bedrock/readiness.go b/spinifex/handlers/bedrock/readiness.go index d039162a9..7cc076390 100644 --- a/spinifex/handlers/bedrock/readiness.go +++ b/spinifex/handlers/bedrock/readiness.go @@ -9,11 +9,17 @@ import ( // defaultStartupTimeout bounds how long Ensure's launch goroutine polls a // freshly-launched serving VM's /v1/models endpoint before giving up and -// reverting the record to ABSENT. PROVISIONAL: chosen conservatively for a -// small model (Llama 3.2 1B) cold-booting a microVM plus vLLM's own weight -// load; needs correcting against a real measurement on wattle once the model -// actually runs there. -const defaultStartupTimeout = 5 * time.Minute +// reverting the record to ABSENT. +// +// Measured once, on the only GPU available to test on: Llama 3.2 1B on an +// RTX A1000 reached "Application startup complete" 257.6s after launch, of +// which 20.7s was guest boot and 26.3s a cold torch.compile. The former 5min +// left ~42s of margin on the smallest model this platform can serve, so the +// bound is set well clear of the one data point rather than close to it. A +// larger card and a larger model are unmeasured, and the failure mode of too +// low a value is a healthy endpoint torn down mid-start; too high only costs +// a stuck launch more time to give up. +const defaultStartupTimeout = 15 * time.Minute // readinessPollInterval is the spacing between /v1/models probes. Short // enough that a fast-booting VM isn't held back by the poll cadence, long diff --git a/spinifex/handlers/bedrock/vpc.go b/spinifex/handlers/bedrock/vpc.go index 5a469317d..bd7afe1b4 100644 --- a/spinifex/handlers/bedrock/vpc.go +++ b/spinifex/handlers/bedrock/vpc.go @@ -70,5 +70,11 @@ func EnsureSystemVPC(ctx context.Context, deps handlers_systemvpc.Deps, cfg *con if len(refs.PrivateSubnetIDs) == 0 { return nil, fmt.Errorf("bedrock: system VPC %s has no private subnet to place serving VMs in", refs.VpcID) } + // The two are built in lockstep, and callers index both by the same k to + // pair a subnet with its prefix length. + if len(refs.PrivateSubnetCIDRs) != len(refs.PrivateSubnetIDs) { + return nil, fmt.Errorf("bedrock: system VPC %s returned %d private subnets but %d CIDRs", + refs.VpcID, len(refs.PrivateSubnetIDs), len(refs.PrivateSubnetCIDRs)) + } return refs, nil } diff --git a/spinifex/network/host/vpc_hostport.go b/spinifex/network/host/vpc_hostport.go new file mode 100644 index 000000000..a6be40432 --- /dev/null +++ b/spinifex/network/host/vpc_hostport.go @@ -0,0 +1,108 @@ +package host + +import ( + "context" + "fmt" + "log/slog" + "net/netip" + + "github.com/mulgadc/spinifex/spinifex/vm" +) + +// vpcHostPortPrefix tags the daemon's own OVS internal ports on br-int. "vhp-" +// plus the 8-char short ENI is 12 chars, inside the 15-char IFNAMSIZ limit — +// the same naming discipline the per-tap IMDS ports follow. +const vpcHostPortPrefix = "vhp-" + +// VPCHostPortName returns the host-side port name for an ENI the daemon itself +// owns, rather than one belonging to a guest. +func VPCHostPortName(eniID string) string { return vpcHostPortPrefix + shortENIID(eniID) } + +// VPCHostPort describes one host-side port into a managed VPC subnet: an OVS +// internal port on br-int bound to a real ENI, giving the host a routed +// presence in a network that otherwise lives entirely inside OVN. Addr is the +// ENI's own address carried at the SUBNET's prefix length, which is what +// installs the connected route for the rest of the subnet. +type VPCHostPort struct { + Name string + IfaceID string + MAC string + Addr netip.Prefix +} + +func (d VPCHostPort) validate() error { + switch { + case d.Name == "": + return fmt.Errorf("VPCHostPort: Name required") + case d.IfaceID == "": + return fmt.Errorf("VPCHostPort: IfaceID required") + case d.MAC == "": + return fmt.Errorf("VPCHostPort: MAC required") + case !d.Addr.IsValid(): + return fmt.Errorf("VPCHostPort: Addr required") + } + return nil +} + +// EnsureVPCHostPort installs the daemon's own routed port into a managed VPC +// subnet. addr is CIDR notation carrying the ENI address at the subnet's +// prefix length. Idempotent, so it also re-establishes the port after a host +// reboot without the caller special-casing that. +func (p *OVSPlumber) EnsureVPCHostPort(eniID, mac, addr string) error { + prefix, err := netip.ParsePrefix(addr) + if err != nil { + return fmt.Errorf("VPC host port for %s: parse address %q: %w", eniID, addr, err) + } + return installVPCHostPort(context.Background(), NewExecRunner(), VPCHostPort{ + Name: VPCHostPortName(eniID), + IfaceID: vm.OVSIfaceID(eniID), + MAC: mac, + Addr: prefix, + }) +} + +// RemoveVPCHostPort tears down the port installed by EnsureVPCHostPort. +// Idempotent, so it is safe for an ENI that never had a port. +func (p *OVSPlumber) RemoveVPCHostPort(eniID string) error { + return removeVPCHostPort(context.Background(), NewExecRunner(), VPCHostPortName(eniID)) +} + +// installVPCHostPort creates the internal port, binds it to the ENI's logical +// switch port, and gives it the ENI's MAC and address. ovn-controller binds the +// LSP by iface-id exactly as it binds a guest tap, so the ENI (and therefore +// its LSP) must already exist when this runs. +func installVPCHostPort(ctx context.Context, r Runner, d VPCHostPort) error { + if err := d.validate(); err != nil { + return err + } + if _, err := r.Run(ctx, "ovs-vsctl", "--may-exist", "add-port", "br-int", d.Name, + "--", "set", "Interface", d.Name, "type=internal", + "external_ids:iface-id="+d.IfaceID, "external_ids:attached-mac="+d.MAC); err != nil { + return fmt.Errorf("create VPC host port %s on br-int: %w", d.Name, err) + } + if _, err := r.Run(ctx, "ip", "link", "set", d.Name, "address", d.MAC); err != nil { + return fmt.Errorf("set VPC host port %s MAC: %w", d.Name, err) + } + // `replace` rather than `add`: OVS conf.db survives a host reboot but the + // address on the netdev does not, so a re-run meets either state. + if _, err := r.Run(ctx, "ip", "addr", "replace", d.Addr.String(), "dev", d.Name); err != nil { + return fmt.Errorf("add %s to VPC host port %s: %w", d.Addr, d.Name, err) + } + if _, err := r.Run(ctx, "ip", "link", "set", d.Name, "up"); err != nil { + return fmt.Errorf("bring up VPC host port %s: %w", d.Name, err) + } + slog.Info("VPC host port ready", "port", d.Name, "iface_id", d.IfaceID, "addr", d.Addr) + return nil +} + +// removeVPCHostPort deletes the port, which takes its address with it. +func removeVPCHostPort(ctx context.Context, r Runner, name string) error { + if name == "" { + return fmt.Errorf("removeVPCHostPort: name required") + } + if _, err := r.Run(ctx, "ovs-vsctl", "--if-exists", "del-port", "br-int", name); err != nil { + return fmt.Errorf("delete VPC host port %s: %w", name, err) + } + slog.Info("VPC host port removed", "port", name) + return nil +} diff --git a/spinifex/network/host/vpc_hostport_test.go b/spinifex/network/host/vpc_hostport_test.go new file mode 100644 index 000000000..5c27d9635 --- /dev/null +++ b/spinifex/network/host/vpc_hostport_test.go @@ -0,0 +1,148 @@ +package host + +import ( + "context" + "net/netip" + "strings" + "testing" +) + +func testVPCHostPort() VPCHostPort { + return VPCHostPort{ + Name: VPCHostPortName("eni-0123456789abcdef0"), + IfaceID: "eni-0123456789abcdef0", + MAC: "02:0a:01:23:45:67", + Addr: netip.MustParsePrefix("10.246.29.9/24"), + } +} + +func TestInstallVPCHostPortValidate(t *testing.T) { + for _, tc := range []struct { + name string + want string + mut func(*VPCHostPort) + }{ + {"no name", "Name", func(d *VPCHostPort) { d.Name = "" }}, + {"no iface id", "IfaceID", func(d *VPCHostPort) { d.IfaceID = "" }}, + {"no mac", "MAC", func(d *VPCHostPort) { d.MAC = "" }}, + {"no addr", "Addr", func(d *VPCHostPort) { d.Addr = netip.Prefix{} }}, + } { + t.Run(tc.name, func(t *testing.T) { + s := newStubRunner() + d := testVPCHostPort() + tc.mut(&d) + if err := installVPCHostPort(context.Background(), s, d); err == nil || + !strings.Contains(err.Error(), tc.want) { + t.Fatalf("expected %s validation error, got %v", tc.want, err) + } + if len(s.calls) != 0 { + t.Errorf("validation must fail before issuing commands; calls: %v", s.calls) + } + }) + } +} + +func TestInstallVPCHostPort(t *testing.T) { + s := newStubRunner() + s.expect("ovs-vsctl", nil, nil) + s.expect("ip", nil, nil) + + d := testVPCHostPort() + if err := installVPCHostPort(context.Background(), s, d); err != nil { + t.Fatalf("installVPCHostPort: %v", err) + } + + want := []string{ + // An internal port on br-int carrying the OVN binding, so ovn-controller + // binds the ENI's LSP to it exactly as it would to a guest tap. + "ovs-vsctl --may-exist add-port br-int " + d.Name + " -- set Interface " + d.Name + + " type=internal external_ids:iface-id=" + d.IfaceID + " external_ids:attached-mac=" + d.MAC, + "ip link set " + d.Name + " address " + d.MAC, + "ip addr replace 10.246.29.9/24 dev " + d.Name, + "ip link set " + d.Name + " up", + } + for _, w := range want { + if !s.called(w) { + t.Errorf("missing command:\n %q\ncalls: %v", w, s.calls) + } + } +} + +// The subnet prefix length is what installs the connected route covering every +// serving VM. A /32 would address the port and still leave the daemon unable to +// reach anything else in the subnet, which is the whole point of the port. +func TestInstallVPCHostPortKeepsSubnetPrefixLength(t *testing.T) { + s := newStubRunner() + s.expect("ovs-vsctl", nil, nil) + s.expect("ip", nil, nil) + + d := testVPCHostPort() + if err := installVPCHostPort(context.Background(), s, d); err != nil { + t.Fatalf("installVPCHostPort: %v", err) + } + if s.called("ip addr replace 10.246.29.9/32") { + t.Errorf("host address installed as a /32; calls: %v", s.calls) + } + if !s.called("ip addr replace 10.246.29.9/24") { + t.Errorf("host address not installed at the subnet prefix length; calls: %v", s.calls) + } +} + +// The address must not be masked to the network address on the way through: +// 10.246.29.9/24 has to stay .9, not become .0. +func TestInstallVPCHostPortDoesNotMaskHostAddress(t *testing.T) { + s := newStubRunner() + s.expect("ovs-vsctl", nil, nil) + s.expect("ip", nil, nil) + + if err := installVPCHostPort(context.Background(), s, testVPCHostPort()); err != nil { + t.Fatalf("installVPCHostPort: %v", err) + } + if s.called("ip addr replace 10.246.29.0/24") { + t.Errorf("host address masked to the network address; calls: %v", s.calls) + } +} + +func TestRemoveVPCHostPort(t *testing.T) { + s := newStubRunner() + s.expect("ovs-vsctl", nil, nil) + + name := VPCHostPortName("eni-0123456789abcdef0") + if err := removeVPCHostPort(context.Background(), s, name); err != nil { + t.Fatalf("removeVPCHostPort: %v", err) + } + // --if-exists so a re-run, or an ENI that never had a port, is not an error. + if !s.called("ovs-vsctl --if-exists del-port br-int " + name) { + t.Errorf("port not deleted from br-int; calls: %v", s.calls) + } +} + +func TestRemoveVPCHostPortRejectsEmptyName(t *testing.T) { + s := newStubRunner() + if err := removeVPCHostPort(context.Background(), s, ""); err == nil { + t.Fatal("expected an error for an empty port name") + } + if len(s.calls) != 0 { + t.Errorf("empty name must issue no commands; calls: %v", s.calls) + } +} + +// IFNAMSIZ is 15 chars and the kernel rejects anything longer, so the name has +// to stay short no matter how long the ENI ID is. +func TestVPCHostPortNameWithinIFNAMSIZ(t *testing.T) { + for _, eni := range []string{"eni-1", "eni-0123456789abcdef0", strings.Repeat("e", 200)} { + if got := VPCHostPortName(eni); len(got) > 15 { + t.Errorf("VPCHostPortName(%q) = %q, %d chars, over the 15-char limit", eni, got, len(got)) + } + } +} + +// Distinct ENIs must not collide on one port name, or two daemons' ports would +// fight over the same netdev. +func TestVPCHostPortNameDistinctPerENI(t *testing.T) { + a := VPCHostPortName("eni-aaaaaaaaaaaaaaaaa") + b := VPCHostPortName("eni-bbbbbbbbbbbbbbbbb") + if a == b { + t.Errorf("distinct ENIs collided on port name %q", a) + } +} diff --git a/spinifex/utils/images.go b/spinifex/utils/images.go index 6c68b219b..dc0e69c10 100644 --- a/spinifex/utils/images.go +++ b/spinifex/utils/images.go @@ -405,6 +405,28 @@ var AvailableImages = map[string]Images{ Tags: map[string]string{"gpu-vendor": "nvidia"}, }, + // Bedrock self-host serving AMI. Resolved by spinifex:managed-by=bedrock + + // spinifex:bedrock-role=vllm-serving tags — LaunchServingVM's + // resolveServingAMI filters on both, never by name. + "ubuntu-26.04-vllm-serving-x86_64": { + Name: "ubuntu-26.04-vllm-serving-x86_64", + Description: "Ubuntu 26.04 vLLM self-host serving image — NVIDIA GPU base + vLLM OpenAI-compatible server baked into a uv-managed venv, served by vllm-serve.service against a read-only weights mount", + Distro: "ubuntu", + Version: "26.04", + Arch: "x86_64", + Platform: "Linux/UNIX", + CreatedAt: time.Date(2026, 8, 3, 0, 0, 0, 0, time.UTC), + URL: "https://iso.mulgadc.com/system-ami/ubuntu-26.04-vllm-serving-x86_64.qcow2", + Checksum: "https://iso.mulgadc.com/system-ami/ubuntu-26.04-vllm-serving-x86_64.qcow2.sha256", + ChecksumType: "sha256", + BootMode: "uefi", + Tags: map[string]string{ + "spinifex:managed-by": "bedrock", + "spinifex:bedrock-role": "vllm-serving", + "gpu-vendor": "nvidia", + }, + }, + "ubuntu-26.04-amd-gpu-x86_64": { Name: "ubuntu-26.04-amd-gpu-x86_64", Description: "Ubuntu 26.04 AMD GPU base image — linux-firmware, ROCm CLI, Python toolchain, Docker", diff --git a/spinifex/utils/images_test.go b/spinifex/utils/images_test.go new file mode 100644 index 000000000..21a8dfa8b --- /dev/null +++ b/spinifex/utils/images_test.go @@ -0,0 +1,39 @@ +package utils + +import "testing" + +// resolveServingAMI filters purely on these two tags and never on the image +// name, so a typo in either hides the serving image from Ochre and every +// endpoint launch dies at "bedrock: no vllm-serving AMI found". +func TestAvailableImages_VLLMServingCarriesBedrockTags(t *testing.T) { + const key = "ubuntu-26.04-vllm-serving-x86_64" + + img, ok := AvailableImages[key] + if !ok { + t.Fatalf("%s missing from AvailableImages", key) + } + + for tag, want := range map[string]string{ + "spinifex:managed-by": "bedrock", + "spinifex:bedrock-role": "vllm-serving", + } { + if got := img.Tags[tag]; got != want { + t.Errorf("tag %q = %q, want %q", tag, got, want) + } + } + + // The map key and Name are matched independently by different callers, so + // they drifting apart is a real failure mode rather than a tautology. + if img.Name != key { + t.Errorf("Name = %q, want %q", img.Name, key) + } + if img.BootMode != "uefi" { + t.Errorf("BootMode = %q, want uefi", img.BootMode) + } + if img.Arch != "x86_64" { + t.Errorf("Arch = %q, want x86_64", img.Arch) + } + if img.URL == "" || img.Checksum == "" { + t.Error("URL and Checksum must both be set or the import has nothing to fetch and verify") + } +} diff --git a/spinifex/vm/lifecycle_test.go b/spinifex/vm/lifecycle_test.go index 139da6e33..ebee224cf 100644 --- a/spinifex/vm/lifecycle_test.go +++ b/spinifex/vm/lifecycle_test.go @@ -1017,6 +1017,10 @@ func (p *scriptedNetworkPlumber) DetachIMDSDatapath(_ string) error { return nil func (p *scriptedNetworkPlumber) EnsureIMDSDatapathBridge() error { return nil } +func (p *scriptedNetworkPlumber) EnsureVPCHostPort(_, _, _ string) error { return nil } + +func (p *scriptedNetworkPlumber) RemoveVPCHostPort(_ string) error { return nil } + var _ NetworkPlumber = (*scriptedNetworkPlumber)(nil) func TestStartupTimeouts(t *testing.T) { diff --git a/spinifex/vm/network_helpers_test.go b/spinifex/vm/network_helpers_test.go index 6645ad76b..07bb08208 100644 --- a/spinifex/vm/network_helpers_test.go +++ b/spinifex/vm/network_helpers_test.go @@ -13,16 +13,20 @@ import ( // fakeNetworkPlumber records calls so tests can assert per-spec behaviour. type fakeNetworkPlumber struct { - setupCalls []TapSpec - cleanupCalls []string - imdsAttachCalls []imdsAttachCall - imdsDetachCalls []string - ensureBridgeCalls int - setupErr error - cleanupErr error - imdsAttachErr error - imdsDetachErr error - ensureBridgeErr error + setupCalls []TapSpec + cleanupCalls []string + imdsAttachCalls []imdsAttachCall + imdsDetachCalls []string + hostPortCalls []hostPortCall + hostPortRemoveCalls []string + ensureBridgeCalls int + setupErr error + cleanupErr error + imdsAttachErr error + imdsDetachErr error + hostPortErr error + hostPortRemoveErr error + ensureBridgeErr error } // imdsAttachCall captures the args of one AttachIMDSDatapath invocation. @@ -32,6 +36,13 @@ type imdsAttachCall struct { subnetID string } +// hostPortCall captures the args of one EnsureVPCHostPort invocation. +type hostPortCall struct { + eniID string + mac string + addr string +} + func (p *fakeNetworkPlumber) SetupTap(spec TapSpec) error { p.setupCalls = append(p.setupCalls, spec) return p.setupErr @@ -57,6 +68,16 @@ func (p *fakeNetworkPlumber) EnsureIMDSDatapathBridge() error { return p.ensureBridgeErr } +func (p *fakeNetworkPlumber) EnsureVPCHostPort(eniID, mac, addr string) error { + p.hostPortCalls = append(p.hostPortCalls, hostPortCall{eniID: eniID, mac: mac, addr: addr}) + return p.hostPortErr +} + +func (p *fakeNetworkPlumber) RemoveVPCHostPort(eniID string) error { + p.hostPortRemoveCalls = append(p.hostPortRemoveCalls, eniID) + return p.hostPortRemoveErr +} + var _ NetworkPlumber = (*fakeNetworkPlumber)(nil) func TestMgmtTapName(t *testing.T) { diff --git a/spinifex/vm/network_plumber.go b/spinifex/vm/network_plumber.go index 9700dcc40..fedeef21c 100644 --- a/spinifex/vm/network_plumber.go +++ b/spinifex/vm/network_plumber.go @@ -31,4 +31,14 @@ type NetworkPlumber interface { // patch pair, endpoint), leaving the shared br-imds bridge in place. Idempotent, // so safe for an ENI that never had a datapath installed. DetachIMDSDatapath(eniID string) error + + // EnsureVPCHostPort gives the host a routed presence in a managed VPC subnet + // by binding an ENI the daemon itself owns to an internal port on br-int. + // addr is CIDR notation carrying the ENI address at the SUBNET's prefix + // length, which is what installs the connected route for the rest of it. + EnsureVPCHostPort(eniID, mac, addr string) error + + // RemoveVPCHostPort tears down an EnsureVPCHostPort port. Idempotent, so it + // is safe for an ENI that never had one. + RemoveVPCHostPort(eniID string) error }