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 }