diff --git a/.github/workflows/gpu-e2e.yml b/.github/workflows/gpu-e2e.yml new file mode 100644 index 000000000..ae2d48de3 --- /dev/null +++ b/.github/workflows/gpu-e2e.yml @@ -0,0 +1,369 @@ +# GPU E2E — drives wattle's persistent, shared-host Spinifex stack over SSH to +# exercise GPU passthrough (raw EC2, ECS task GPU exposure, EKS pod GPU +# exposure). See docs/development/feature/wattle-deploy-and-ci.md (Part B). +# +# --------------------------------------------------------------------------- +# ONE-TIME HOST-SIDE SETUP — NOT part of this repo, a human must do this: +# +# 1. Register ONE self-hosted GitHub Actions runner on a banksia host with +# the dedicated label `ci-gpu` (in addition to `self-hosted`). Do +# NOT add it to the `ci-single` / `ci-multi` pool and do NOT give any +# other runner the `ci-gpu` label — a second labelled runner would +# let two GPU jobs race the single RTX A1000. With exactly one runner +# carrying the label, `runs-on: [self-hosted, ci-gpu]` alone forces +# serialization; the `concurrency:` block below queues same-group runs +# when that runner is already busy with a live job. +# 2. Drop the wattle SSH private key on that runner's filesystem and wire +# it into this repo as `secrets.WATTLE_SSH_KEY` (raw private key +# contents). Optionally override `vars.WATTLE_SSH_HOST` / +# `vars.WATTLE_SSH_USER` if they differ from the defaults below. +# 3. Confirm the runner user can drive wattle's control plane over SSH. The +# job calls `spx` (read-only checks) and the `aws` CLI with the already- +# configured `spinifex` profile directly — no wrapper scripts, no install +# on wattle (tf-user is already in the `spinifex` group there): +# ssh tf-user@192.168.1.13 'AWS_PROFILE=spinifex aws ec2 describe-regions' +# 4. Manual-work mutex: an agent doing hands-on GPU testing on wattle can +# block CI by touching a sentinel file in the RUNNER's home directory +# (NOT on wattle): +# touch ~/.ci-gpu-paused # blocks CI +# rm ~/.ci-gpu-paused # resumes CI +# or by setting the repo/environment variable `CI_GPU_PAUSED=true` +# (no runner filesystem access needed). Either one causes this workflow +# to skip cleanly (job succeeds, all work steps skipped) instead of +# queuing behind manual work indefinitely. +# +# Runbook: docs/development/feature/wattle-deploy-and-ci.md (Part B). +# --------------------------------------------------------------------------- + +name: GPU E2E + +on: + workflow_dispatch: + inputs: + test_run: + description: "Optional -test.run regex scoped to tests/e2e/gpu (empty = run all GPU tests)" + required: false + default: "" + push: + branches: + - main + - dev + paths: &gpu_paths + - "spinifex/gpu/**" + - "spinifex/instancetypes/**" + - "spinifex/daemon/gpu_adapter.go" + - "spinifex/daemon/gpu_pool_reconciler.go" + - "spinifex/handlers/ecs/gpu_report.go" + - "spinifex/handlers/eks/nodegroup.go" + - "spinifex/handlers/eks/nodegroup_userdata.go" + - "spinifex/vm/vm.go" + - "spinifex/vm/lifecycle.go" + - "cmd/ecs-agent/gpu_discovery.go" + - "cmd/ecs-agent/gpu_ledger.go" + - "cmd/spinifex/cmd/admin_gpu.go" + - "tests/e2e/gpu/**" + - "scripts/images/eks-node-gpu/**" + - "scripts/images/ecs-agent-gpu/**" + - ".github/workflows/gpu-e2e.yml" + pull_request: + branches: + - main + - dev + paths: *gpu_paths + +permissions: + contents: read + +# Single dedicated runner (see setup note 1 above) + this group together +# enforce single-flight on the one RTX A1000: a second dispatch has nowhere +# else to land (unique label) and, if it somehow overlaps in scheduling, +# queues here rather than cancelling/stomping the in-flight run. +concurrency: + group: ci-gpu + cancel-in-progress: false + +env: + # OTLP export to the central Elastic sink (wattle is the sink itself — see + # Part A.2). Repo variable overrides; carried for correlation even though + # the GPU suite's own daemon-side spans are emitted by wattle's already- + # configured obs-agent, not by this job. + SPINIFEX_OTLP_ENDPOINT: ${{ vars.SPINIFEX_OTLP_ENDPOINT || 'http://192.168.1.13:8200' }} + SPINIFEX_OTEL_ATTRS: ci.run_id=${{ github.run_id }},ci.workflow=${{ github.workflow }} + WATTLE_SSH_HOST: ${{ vars.WATTLE_SSH_HOST || '192.168.1.13' }} + WATTLE_SSH_USER: ${{ vars.WATTLE_SSH_USER || 'tf-user' }} + TEST_RUN: ${{ inputs.test_run || '' }} + +jobs: + gpu_e2e: + name: GPU E2E (wattle) + runs-on: [self-hosted, ci-gpu] + timeout-minutes: 75 + env: + ARTIFACT_DIR: /tmp/spinifex-gpu-e2e-artifacts-${{ github.run_id }} + steps: + # Manual-work mutex gate — see setup note 4 above. Checked BEFORE any + # SSH to wattle so a paused run touches the box zero times. + - name: Manual-work mutex gate + id: gate + env: + REPO_VAR_PAUSED: ${{ vars.CI_GPU_PAUSED }} + run: | + set -euo pipefail + PAUSED=false + if [ -f "$HOME/.ci-gpu-paused" ]; then + PAUSED=true + echo "::warning::ci-gpu runner paused via ~/.ci-gpu-paused sentinel on the runner — skipping this run (manual GPU work in progress on wattle). Remove the file to resume CI." + fi + if [ "$(echo "${REPO_VAR_PAUSED:-}" | tr '[:upper:]' '[:lower:]')" = "true" ]; then + PAUSED=true + echo "::warning::ci-gpu paused via repo/environment variable CI_GPU_PAUSED=true — skipping this run." + fi + echo "paused=${PAUSED}" >> "$GITHUB_OUTPUT" + + - name: Paused — nothing to do + if: steps.gate.outputs.paused == 'true' + run: echo "ci-gpu is paused; job exits cleanly without touching wattle. See gate step above for the reason." + + - name: Clean Workspace + if: steps.gate.outputs.paused != 'true' + run: sudo rm -rf --one-file-system "$GITHUB_WORKSPACE"/* + + - uses: actions/checkout@v7 + if: steps.gate.outputs.paused != 'true' + + - uses: actions/setup-go@v6 + if: steps.gate.outputs.paused != 'true' + with: + go-version-file: go.mod + cache: false + + - name: Install wattle SSH key + if: steps.gate.outputs.paused != 'true' + run: | + set -euo pipefail + install -m 0600 /dev/null "${RUNNER_TEMP}/wattle-ci" + echo "${{ secrets.WATTLE_SSH_KEY }}" > "${RUNNER_TEMP}/wattle-ci" + echo "ssh_opts=-o StrictHostKeyChecking=no -o ConnectTimeout=10 -i ${RUNNER_TEMP}/wattle-ci" >> "$GITHUB_ENV" + + # Healthcheck: confirm wattle's persistent AWS-compat control plane is + # actually up before spending time building/shipping the test binary. + - name: Healthcheck — describe-regions + if: steps.gate.outputs.paused != 'true' + run: | + set -euo pipefail + ssh ${{ env.ssh_opts }} "${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" \ + 'AWS_PROFILE=spinifex aws ec2 describe-regions --output text' + + # Ensure the base EC2 GPU AMI is present — the raw-EC2 GPU passthrough + # test is the always-on baseline, so CI self-provisions it rather than + # depending on a manual import. `--name` selects the catalog entry (pulls + # the prebuilt image from iso.mulgadc.com); `--ami-name` is REQUIRED to + # name the AMI ubuntu-26.04-nvidia-gpu-x86_64 — without it the import + # falls back to the generic ami---, which the + # tests/e2e/gpu filter would never match. Idempotent — skipped once + # wattle's persistent loop-fs already has the named AMI (the 4.4G source + # qcow2 is cached in the images dir, so a re-import wouldn't re-download). + # `spx admin images import` writes local spinifex storage, so it needs + # the `spinifex` group active (sg); aws-CLI calls hit the gateway over + # HTTPS and don't. + - name: Ensure base GPU AMI present (import-if-absent) + if: steps.gate.outputs.paused != 'true' + run: | + set -euo pipefail + REMOTE=' + set -e + BASE=ubuntu-26.04-nvidia-gpu-x86_64 + present() { + AWS_PROFILE=spinifex aws ec2 describe-images \ + --filters "Name=name,Values=$BASE" "Name=state,Values=available" \ + --query "Images[0].ImageId" --output text 2>/dev/null + } + ID=$(present) + if [ -n "$ID" ] && [ "$ID" != "None" ]; then + echo "base GPU AMI already present: $BASE = $ID"; exit 0 + fi + echo "base GPU AMI $BASE absent — importing prebuilt from catalog..." + sg spinifex -c "AWS_PROFILE=spinifex spx admin images import --name $BASE --ami-name $BASE" + for i in 1 2 3 4 5 6; do + ID=$(present) + [ -n "$ID" ] && [ "$ID" != "None" ] && break + sleep 5 + done + [ -n "$ID" ] && [ "$ID" != "None" ] || { echo "import ran but $BASE still not available" >&2; exit 1; } + echo "imported base GPU AMI: $BASE = $ID" + ' + ssh ${{ env.ssh_opts }} "${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" "$REMOTE" + + # Preflight — the base GPU AMI is REQUIRED: the setup step above just + # ensured it, so a miss here means the import silently failed. Hard-fail + # rather than run a false-green all-skipped suite. ECS/EKS node AMIs are + # opt-in tiers (heavier — node AMIs + cluster spin-up); their tests + # self-skip when absent, so only warn. + - name: Preflight — verify GPU AMIs + if: steps.gate.outputs.paused != 'true' + run: | + set -euo pipefail + REMOTE=' + set -e + present() { + AWS_PROFILE=spinifex aws ec2 describe-images \ + --filters "Name=name,Values=$1" "Name=state,Values=available" \ + --query "Images[0].ImageId" --output text 2>/dev/null + } + BASE=ubuntu-26.04-nvidia-gpu-x86_64 + ID=$(present "$BASE") + if [ -z "$ID" ] || [ "$ID" = "None" ]; then + echo "required base GPU AMI $BASE missing — cannot run the baseline EC2 GPU test" >&2 + exit 1 + fi + echo "required: $BASE = $ID" + for OPT in spinifex-ecs-node-gpu spinifex-eks-node spinifex-eks-node-gpu; do + OID=$(present "$OPT") + if [ -z "$OID" ] || [ "$OID" = "None" ]; then + echo "::warning::$OPT not imported — its GPU test(s) will skip (opt-in tier)" + else + echo "opt-in: $OPT = $OID" + fi + done + ' + ssh ${{ env.ssh_opts }} "${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" "$REMOTE" + + # Same build command CI's other e2e workflows use to cross-compile a + # suite into a shippable binary (tests/e2e/Makefile e2e-build target, + # SUITES_BUILD scoped to just gpu). Building on the runner keeps go + # toolchain / module cache off wattle entirely. + - name: Build GPU e2e test binary on runner + if: steps.gate.outputs.paused != 'true' + working-directory: tests/e2e + run: | + set -euo pipefail + make e2e-build SUITES_BUILD=gpu + ls -la _bin/gpu.test + + - name: Install go-junit-report on runner + if: steps.gate.outputs.paused != 'true' + run: | + go install github.com/jstemmer/go-junit-report/v2@latest + echo "$HOME/go/bin" >> "$GITHUB_PATH" + + - name: Ship GPU e2e test binary to wattle + if: steps.gate.outputs.paused != 'true' + run: | + set -euo pipefail + VM="${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" + ssh ${{ env.ssh_opts }} "$VM" "mkdir -p \$HOME/spinifex-gpu-e2e-bin" + scp ${{ env.ssh_opts }} tests/e2e/_bin/gpu.test "$VM":spinifex-gpu-e2e-bin/gpu.test + + # Same invocation shape e2e.yml uses to run a pre-built suite binary + # (SPINIFEX_E2E=1 SPINIFEX_MODE=single, ./.test -test.v + # -test.timeout=NNm), plus SPINIFEX_WAN_IP: wattle's ~24 bridges make + # net.InterfaceAddrs() enumeration pick a co-tenant bridge instead of + # the advertised gateway (mulga-7suds), so the override is required + # here (proper fix tracked separately: tests/e2e/harness/env.go should + # prefer spinifex.toml's advertised bridge). + - name: Run GPU e2e suite on wattle + id: e2e + if: steps.gate.outputs.paused != 'true' + run: | + set +e + VM="${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" + if [ -n "${TEST_RUN}" ]; then + TEST_RUN_FLAG="-test.run=${TEST_RUN}" + else + TEST_RUN_FLAG="" + fi + ssh -tt ${{ env.ssh_opts }} "$VM" \ + "set -u; mkdir -p '${ARTIFACT_DIR}'; cd \$HOME/spinifex-gpu-e2e-bin; \ + GITHUB_ACTIONS=true FORCE_COLOR=1 SPINIFEX_E2E=1 SPINIFEX_MODE=single \ + SPINIFEX_WAN_IP='${WATTLE_SSH_HOST}' \ + ARTIFACT_DIR='${ARTIFACT_DIR}' \ + ./gpu.test -test.v -test.timeout=60m ${TEST_RUN_FLAG} \ + 2>&1 | tee '${ARTIFACT_DIR}/test-gpu.log'; \ + exit \${PIPESTATUS[0]}" + E2E_RC=$? + echo "rc=${E2E_RC}" >> "$GITHUB_OUTPUT" + exit 0 + + - name: Collect artifacts from wattle + if: always() && steps.gate.outputs.paused != 'true' + run: | + set +e + mkdir -p "${ARTIFACT_DIR}" + VM="${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" + ssh ${{ env.ssh_opts }} "$VM" "tar -C '${ARTIFACT_DIR}' -cf - . 2>/dev/null || true" \ + | tar -C "${ARTIFACT_DIR}" -xf - 2>/dev/null + echo "collected: $(ls "${ARTIFACT_DIR}" 2>/dev/null | wc -l) entries" + + - name: Convert test log to JUnit XML + if: always() && steps.gate.outputs.paused != 'true' + run: | + shopt -s nullglob + for LOG in "${ARTIFACT_DIR}"/test-*.log; do + BASE=$(basename "$LOG" .log) + SUITE=${BASE#test-} + go-junit-report -in "$LOG" -out "${ARTIFACT_DIR}/junit-${SUITE}.xml" || true + done + + - name: Publish test summary + if: always() && steps.gate.outputs.paused != 'true' + uses: ./.github/actions/junit-report + with: + junit-glob: ${{ env.ARTIFACT_DIR }}/junit-*.xml + title: "GPU E2E results" + + - name: Analyze e2e failures + if: always() && steps.gate.outputs.paused != 'true' + uses: ./.github/actions/e2e-analyze + with: + junit-glob: ${{ env.ARTIFACT_DIR }}/junit-*.xml + log-dir: ${{ env.ARTIFACT_DIR }} + title: "GPU E2E failure analysis" + + - uses: actions/upload-artifact@v7 + if: always() && steps.gate.outputs.paused != 'true' + with: + name: gpu-e2e-artifacts-${{ github.run_id }} + path: ${{ env.ARTIFACT_DIR }} + if-no-files-found: warn + retention-days: 14 + + # Cleanup is NEVER a host teardown: it never runs setup-ovn.sh, + # dev-reset, or cluster-reset. It only reconciles EC2-API-level state — + # terminate any GPU-typed instance still pending/running (the Go + # suite's own t.Cleanup already does this on normal pass/fail exit; + # this is the backstop for a hard `-test.timeout` kill or a workflow + # cancellation, which skip deferred Go cleanups). Termination triggers + # gpuManager.Release() on wattle, returning the A1000 to the pool for + # the next queued run or manual test. + - name: Cleanup — terminate leftover GPU instances, release GPU + if: always() && steps.gate.outputs.paused != 'true' + run: | + set +e + VM="${WATTLE_SSH_USER}@${WATTLE_SSH_HOST}" + REMOTE=' + set +e + GPU_TYPES=$(AWS_PROFILE=spinifex aws ec2 describe-instance-types \ + --query "InstanceTypes[?GpuInfo!=null].InstanceType" --output text | tr "\t" ",") + if [ -z "$GPU_TYPES" ]; then + echo "no GPU instance types advertised; nothing to reconcile" + exit 0 + fi + IDS=$(AWS_PROFILE=spinifex aws ec2 describe-instances \ + --filters "Name=instance-type,Values=${GPU_TYPES}" "Name=instance-state-name,Values=pending,running" \ + --query "Reservations[].Instances[].InstanceId" --output text) + if [ -z "$IDS" ] || [ "$IDS" = "None" ]; then + echo "no leftover GPU instances — GPU already released" + exit 0 + fi + echo "terminating leftover GPU instance(s): $IDS" + AWS_PROFILE=spinifex aws ec2 terminate-instances --instance-ids $IDS + ' + ssh ${{ env.ssh_opts }} "$VM" "$REMOTE" + + - name: Fail job if GPU suite failed + if: always() && steps.gate.outputs.paused != 'true' + run: | + if [ "${{ steps.e2e.outputs.rc }}" != "0" ]; then + echo "::error::GPU e2e suite failed (rc=${{ steps.e2e.outputs.rc }}) — see analysis above; raw ES/APM traces at ${SPINIFEX_OTLP_ENDPOINT} (ci.run_id=${{ github.run_id }})" + exit 1 + fi diff --git a/scripts/images/ubuntu-gpu-nvidia.conf b/scripts/images/ubuntu-gpu-nvidia.conf index 652b0ab4d..9a8326d5f 100644 --- a/scripts/images/ubuntu-gpu-nvidia.conf +++ b/scripts/images/ubuntu-gpu-nvidia.conf @@ -10,6 +10,6 @@ IMAGE_SIZE="16G" SETUP_SCRIPT="scripts/images/ubuntu-gpu-nvidia-setup.sh" -AMI_NAME="ubuntu-nvidia-gpu" +AMI_NAME="ubuntu-26.04-nvidia-gpu-x86_64" SYSTEM_TAG="gpu-vendor=nvidia" diff --git a/scripts/launch-gpu.sh b/scripts/launch-gpu.sh index d9212f114..b5a1fca15 100755 --- a/scripts/launch-gpu.sh +++ b/scripts/launch-gpu.sh @@ -22,9 +22,9 @@ Usage: Options: --type TYPE EC2 instance type to launch (required) --disk GB Root disk size in GB (default: 50) - --ami NAME Override the AMI name to use (default: ubuntu-amd-gpu or ubuntu-nvidia-gpu) + --ami NAME Override the AMI name to use (default: ubuntu-amd-gpu or ubuntu-26.04-nvidia-gpu-x86_64) --amd Force AMD GPU AMI (ubuntu-amd-gpu) - --nvidia Force NVIDIA GPU AMI (ubuntu-nvidia-gpu) + --nvidia Force NVIDIA GPU AMI (ubuntu-26.04-nvidia-gpu-x86_64) Env overrides: SSH_KEY Path to SSH private key (default: ~/.ssh/spinifex-key) @@ -96,7 +96,7 @@ fi case "$GPU_BRAND" in amd) AMI_NAME="ubuntu-amd-gpu" ;; - nvidia) AMI_NAME="ubuntu-nvidia-gpu" ;; + nvidia) AMI_NAME="ubuntu-26.04-nvidia-gpu-x86_64" ;; esac # --- Resolve AMI --- diff --git a/spinifex/network/reconcile/apply.go b/spinifex/network/reconcile/apply.go index 01e9e9cb1..5a788ddc4 100644 --- a/spinifex/network/reconcile/apply.go +++ b/spinifex/network/reconcile/apply.go @@ -9,6 +9,7 @@ import ( "time" "github.com/mulgadc/spinifex/spinifex/network/ovn/nbdb" + "github.com/mulgadc/spinifex/spinifex/network/policy" "github.com/mulgadc/spinifex/spinifex/network/topology" "github.com/mulgadc/spinifex/spinifex/utils" ) @@ -468,13 +469,13 @@ func (r *reconciler) ensureGatewayClaimed(ctx context.Context, crPortName string } } -// applyEIPs runs every intent EIP through NATManager.AddEIP; idempotent. After the +// applyEIPs runs every floating IP through NATManager.AddEIP; idempotent. After the // DNAT row is in place it gates on the guest ENI's SB Port_Binding: AddEIP only // proves the gateway-chassis flow exists, not the gatewayLRP->guest hop, so a guest // whose port has not converged (e.g. just after a host reboot) stays dark while // every other signal is green. func (r *reconciler) applyEIPs(ctx context.Context, intent IntentState, _ ActualState) { - for _, spec := range intent.EIPs { + for _, spec := range r.floatingIPSpecs(intent) { if err := r.nat.AddEIP(ctx, spec); err != nil { slog.Error("reconcile/apply: AddEIP failed", "external_ip", spec.ExternalIP, "logical_ip", spec.LogicalIP, "err", err) } @@ -482,11 +483,46 @@ func (r *reconciler) applyEIPs(ctx context.Context, intent IntentState, _ Actual } } +// floatingIPSpecs is every dnat_and_snat the datapath must carry: user EIPs +// (intent.EIPs) plus auto-assigned/ELB public IPs recorded on ENIs (intent.Ports). +// Both need the same DNAT row and gatewayLRP->guest convergence, but only user EIPs +// were reconciled before — an auto-assigned IP's rule was created once at launch and +// never re-asserted, so a vpcd/OVN rebuild dropped it and its guest-port hop never +// converged (inbound DNATs at the gateway but never reaches the guest; ICMP is +// answered by the gateway LR, so ping works while TCP hangs). A user EIP on the same +// private IP wins; the auto-assigned entry is skipped to avoid a duplicate rule. +func (r *reconciler) floatingIPSpecs(intent IntentState) []policy.EIPSpec { + specs := make([]policy.EIPSpec, 0, len(intent.EIPs)+len(intent.Ports)) + for _, spec := range intent.EIPs { + specs = append(specs, spec) + } + for _, p := range intent.Ports { + if !p.PublicIP.IsValid() { + continue + } + logicalIP := p.PrivateIP.String() + if _, hasEIP := intent.EIPs[logicalIP]; hasEIP { + continue + } + specs = append(specs, policy.EIPSpec{ + VPCID: p.VPCID, + ExternalIP: p.PublicIP.String(), + LogicalIP: logicalIP, + PortName: topology.Port(p.PortID), + MAC: p.MAC.String(), + }) + } + return specs +} + // pruneOrphanEIPs sweeps dnat_and_snat rows whose stamped owning ENI is gone from // intent. vpc.delete-nat is fire-and-forget and can be lost, leaking rows across // dead VPCs; NATManager.PruneOrphanEIPs deletes any row whose spinifex:logical_port -// is absent from the live-port set. Runs only on the prune pass (drift), so an -// in-flight association whose port event has not converged is never swept. +// is absent from the live-port set. The live set is keyed the same way +// floatingIPSpecs derives PortName (topology.Port(p.PortID) for auto-assigned ports, +// e.PortName for user EIPs), so a currently-live auto-assigned EIP is never pruned. +// Runs only on the prune pass (drift), so an in-flight association whose port event +// has not converged is never swept. func (r *reconciler) pruneOrphanEIPs(ctx context.Context, intent IntentState) { live := make(map[string]struct{}, len(intent.Ports)+len(intent.EIPs)) for portID := range intent.Ports { diff --git a/spinifex/network/reconcile/apply_test.go b/spinifex/network/reconcile/apply_test.go index 11dba2b08..24bd36e25 100644 --- a/spinifex/network/reconcile/apply_test.go +++ b/spinifex/network/reconcile/apply_test.go @@ -620,3 +620,63 @@ func sortedCopy(in []string) []string { slices.Sort(out) return out } + +// TestFloatingIPSpecs covers the auto-assigned NAT reconcile gap: an ENI's +// auto-assigned public IP must be re-asserted as a dnat_and_snat (and run through +// guest-port convergence) alongside user EIPs, deduped when a user EIP already +// covers the same private IP. +func TestFloatingIPSpecs(t *testing.T) { + mac, _ := net.ParseMAC("02:00:00:00:00:01") + r := &reconciler{} + + intent := IntentState{ + EIPs: map[string]policy.EIPSpec{ + // User EIP on 172.31.0.9. + "172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")}, + // User EIP that also has an auto-assigned public IP recorded on its + // port: the EIP must win and the auto-assigned entry be skipped. + "172.31.0.4": {VPCID: "vpc-a", ExternalIP: "192.168.1.201", LogicalIP: "172.31.0.4", PortName: topology.Port("eni-dup")}, + }, + Ports: map[string]topology.PortSpec{ + // Auto-assigned public IP with no user EIP -> must produce a spec. + "eni-auto": {PortID: "eni-auto", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.5"), + PublicIP: netip.MustParseAddr("192.168.1.116"), MAC: mac}, + // Same private IP as the user EIP above -> deduped out. + "eni-dup": {PortID: "eni-dup", VPCID: "vpc-a", PrivateIP: netip.MustParseAddr("172.31.0.4"), + PublicIP: netip.MustParseAddr("192.168.1.117"), MAC: mac}, + // No public IP -> skipped. + "eni-nopub": {PortID: "eni-nopub", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.6")}, + }, + } + + specs := r.floatingIPSpecs(intent) + + byExternal := map[string]policy.EIPSpec{} + for _, s := range specs { + byExternal[s.ExternalIP] = s + } + + // Both user EIPs present. + if _, ok := byExternal["192.168.1.200"]; !ok { + t.Errorf("user EIP 192.168.1.200 missing from specs") + } + if _, ok := byExternal["192.168.1.201"]; !ok { + t.Errorf("user EIP 192.168.1.201 missing from specs") + } + // Auto-assigned public IP produced with the port's identity. + auto, ok := byExternal["192.168.1.116"] + if !ok { + t.Fatalf("auto-assigned 192.168.1.116 missing from specs") + } + if auto.LogicalIP != "172.31.0.5" || auto.PortName != topology.Port("eni-auto") || auto.MAC != "02:00:00:00:00:01" { + t.Errorf("auto-assigned spec malformed: %+v", auto) + } + // Deduped: the auto-assigned IP colliding with a user EIP's private IP is gone. + if _, ok := byExternal["192.168.1.117"]; ok { + t.Errorf("auto-assigned 192.168.1.117 should be deduped by the user EIP on 172.31.0.4") + } + // No public IP -> no spec for that port. + if len(specs) != 3 { + t.Errorf("want 3 specs (2 EIP + 1 auto), got %d: %+v", len(specs), specs) + } +} diff --git a/spinifex/vm/lifecycle.go b/spinifex/vm/lifecycle.go index e93626295..06916824f 100644 --- a/spinifex/vm/lifecycle.go +++ b/spinifex/vm/lifecycle.go @@ -661,20 +661,39 @@ const ( // seam overrides it to keep dial-failure tests off the wall clock. var qmpSocketWaitTimeout = 3 * time.Second -// qmpVFIOGreetingTimeout is the QMP greeting wait for a VFIO passthrough guest. -// QEMU must lock+DMA-map the entire guest RAM through the IOMMU before the -// monitor answers, which scales with RAM and far exceeds the plain-VM default -// (a 16GB EKS GPU worker takes >30s on wattle). Generous to cover larger guests -// and a loaded host. -const qmpVFIOGreetingTimeout = 180 * time.Second +const ( + // qmpVFIOGreetingBase is the fixed VFIO startup overhead before RAM pinning + // dominates (IOMMU domain setup, device realize). The RAM term is added on + // top; the result is floored so small guests keep the proven deadline. + qmpVFIOGreetingBase = 30 * time.Second + // qmpVFIOGreetingPerGiB is the wait added per GiB of guest RAM: the VFIO pin + // is synchronous and its cost grows ~linearly with RAM and host memory + // pressure, so a flat deadline under-waits large guests and SIGKILLs a QEMU + // that would have come up. Generous to cover a loaded host. + qmpVFIOGreetingPerGiB = 8 * time.Second + // qmpVFIOGreetingFloor is the minimum VFIO greeting wait. QEMU must + // lock+DMA-map the whole guest RAM before the monitor answers, which far + // exceeds the plain-VM default even for a small guest (a 16GB EKS GPU worker + // takes >30s on wattle). Small guests land here. + qmpVFIOGreetingFloor = 180 * time.Second + // qmpVFIOGreetingCap bounds the scaled wait so a mis-sized guest cannot + // wedge a launch indefinitely. + qmpVFIOGreetingCap = 600 * time.Second +) // qmpGreetingTimeout picks the QMP greeting deadline for a VM: the plain default -// unless the guest has GPU/VFIO passthrough, whose RAM pinning delays the monitor. +// unless the guest has GPU/VFIO passthrough, whose synchronous RAM pin delays the +// monitor. For VFIO the deadline is base + perGiB*RAM, floored so small guests +// keep the proven deadline and capped so a huge guest cannot wedge a launch. func qmpGreetingTimeout(v *VM) time.Duration { - if len(v.GPUAttachments) > 0 { - return qmpVFIOGreetingTimeout + if len(v.GPUAttachments) == 0 { + return qmp.DefaultGreetingTimeout } - return qmp.DefaultGreetingTimeout + memGiB := v.Config.Memory / 1024 + scaled := qmpVFIOGreetingBase + time.Duration(memGiB)*qmpVFIOGreetingPerGiB + scaled = max(scaled, qmpVFIOGreetingFloor) + scaled = min(scaled, qmpVFIOGreetingCap) + return scaled } // removeStaleQMPSocket unlinks a leftover QMP socket inode from a prior QEMU so @@ -726,7 +745,11 @@ func newQMPClientWithHandshake(ctx context.Context, v *VM) (*qmp.QMPClient, erro if err != nil { return nil, fmt.Errorf("connect QMP socket %s: %w", v.Config.QMPSocket, err) } - if _, err := sendQMPCommand(ctx, client, qmp.QMPCommand{Execute: "qmp_capabilities"}, v.ID); err != nil { + // A VFIO guest greets before its RAM pin completes, then cannot service + // qmp_capabilities until the pin finishes — which scales with guest RAM and + // host memory pressure. Give the handshake the same budget as the greeting so + // the reply is not decode-timed-out and the launch SIGKILLed mid-pin. + if _, err := sendQMPCommandWithTimeout(ctx, client, qmp.QMPCommand{Execute: "qmp_capabilities"}, v.ID, qmpGreetingTimeout(v)); err != nil { _ = client.Conn.Close() return nil, err } @@ -814,8 +837,19 @@ func (m *Manager) recordQMPSuccess(instance *VM) { }) } +// qmpCommandTimeout bounds a QMP command's response decode for a running guest. +// The handshake on a VFIO launch needs longer (the monitor greets before the RAM +// pin finishes, then stalls the qmp_capabilities reply) and passes its own value. +const qmpCommandTimeout = 30 * time.Second + // sendQMPCommand encodes cmd and decodes the response, skipping event messages. -func sendQMPCommand(ctx context.Context, q *qmp.QMPClient, cmd qmp.QMPCommand, instanceID string) (_ *qmp.QMPResponse, err error) { +func sendQMPCommand(ctx context.Context, q *qmp.QMPClient, cmd qmp.QMPCommand, instanceID string) (*qmp.QMPResponse, error) { + return sendQMPCommandWithTimeout(ctx, q, cmd, instanceID, qmpCommandTimeout) +} + +// sendQMPCommandWithTimeout is sendQMPCommand with an explicit response deadline, +// re-armed after each interleaved event. VFIO handshakes pass a RAM-scaled value. +func sendQMPCommandWithTimeout(ctx context.Context, q *qmp.QMPClient, cmd qmp.QMPCommand, instanceID string, timeout time.Duration) (_ *qmp.QMPResponse, err error) { if ctx.Value(noTraceKey{}) == nil { var span trace.Span _, span = otel.Tracer(vmTracerName).Start(ctx, "qmp "+cmd.Execute, @@ -842,7 +876,7 @@ func sendQMPCommand(ctx context.Context, q *qmp.QMPClient, cmd qmp.QMPCommand, i } } - if err := q.Conn.SetReadDeadline(time.Now().Add(30 * time.Second)); err != nil { + if err := q.Conn.SetReadDeadline(time.Now().Add(timeout)); err != nil { return nil, fmt.Errorf("set read deadline: %w", err) } defer func() { _ = q.Conn.SetReadDeadline(time.Time{}) }() @@ -861,7 +895,7 @@ func sendQMPCommand(ctx context.Context, q *qmp.QMPClient, cmd qmp.QMPCommand, i } if _, ok := msg["event"]; ok { slog.InfoContext(ctx, "QMP event", "event", msg["event"], "instanceId", instanceID) - if err := q.Conn.SetReadDeadline(time.Now().Add(30 * time.Second)); err != nil { + if err := q.Conn.SetReadDeadline(time.Now().Add(timeout)); err != nil { return nil, fmt.Errorf("set read deadline: %w", err) } continue diff --git a/spinifex/vm/lifecycle_test.go b/spinifex/vm/lifecycle_test.go index 06fac9617..8bc2f780e 100644 --- a/spinifex/vm/lifecycle_test.go +++ b/spinifex/vm/lifecycle_test.go @@ -1107,18 +1107,39 @@ func TestSendQMPCommand_ReconnectsAgainstSingleClientServer(t *testing.T) { } // TestQMPGreetingTimeout pins the VFIO scaling: a GPU passthrough guest must -// pin all its RAM through the IOMMU before the monitor answers, so it gets the -// larger greeting deadline; a plain VM keeps the default. +// pin all its RAM through the IOMMU before the monitor answers, so its greeting +// deadline sits at the floor for a small guest and grows with RAM up to the cap; +// a plain VM keeps the default. func TestQMPGreetingTimeout(t *testing.T) { plain := &VM{} assert.Equal(t, qmp.DefaultGreetingTimeout, qmpGreetingTimeout(plain), "a plain VM keeps the default greeting deadline") - gpuVM := &VM{GPUAttachments: []gpu.GPUAttachment{{PCIAddress: "0000:5e:00.0"}}} - assert.Equal(t, qmpVFIOGreetingTimeout, qmpGreetingTimeout(gpuVM), - "a VFIO passthrough VM gets the larger greeting deadline") - assert.Greater(t, qmpVFIOGreetingTimeout, qmp.DefaultGreetingTimeout, - "the VFIO deadline must exceed the plain default") + // A small GPU guest (<1 GiB scaling contribution) sits at the floor. + smallGPU := &VM{ + GPUAttachments: []gpu.GPUAttachment{{PCIAddress: "0000:5e:00.0"}}, + Config: Config{Memory: 4096}, + } + assert.Equal(t, qmpVFIOGreetingFloor, qmpGreetingTimeout(smallGPU), + "a small VFIO guest gets the floor greeting deadline") + assert.Greater(t, qmpVFIOGreetingFloor, qmp.DefaultGreetingTimeout, + "the VFIO floor must exceed the plain default") + + // A large GPU guest scales above the floor: 64 GiB -> floor + 64*8s. + largeGPU := &VM{ + GPUAttachments: []gpu.GPUAttachment{{PCIAddress: "0000:5e:00.0"}}, + Config: Config{Memory: 64 * 1024}, + } + assert.Equal(t, qmpVFIOGreetingBase+64*qmpVFIOGreetingPerGiB, qmpGreetingTimeout(largeGPU), + "a large VFIO guest scales its deadline with RAM") + + // An oversized guest is clamped to the cap, not left unbounded. + hugeGPU := &VM{ + GPUAttachments: []gpu.GPUAttachment{{PCIAddress: "0000:5e:00.0"}}, + Config: Config{Memory: 1024 * 1024}, + } + assert.Equal(t, qmpVFIOGreetingCap, qmpGreetingTimeout(hugeGPU), + "a huge VFIO guest is clamped to the cap") } // TestRemoveStaleQMPSocket covers the SIGKILL-leftover unlink: a stale socket diff --git a/tests/e2e/gpu/gpu_test.go b/tests/e2e/gpu/gpu_test.go index 44913108f..fe902d344 100644 --- a/tests/e2e/gpu/gpu_test.go +++ b/tests/e2e/gpu/gpu_test.go @@ -14,38 +14,23 @@ import ( "github.com/mulgadc/spinifex/tests/e2e/harness" ) -// TestGPUPassthrough_Launch verifies that RunInstances with a GPU instance -// type succeeds and reaches running state, confirming the VFIO bind path. -func TestGPUPassthrough_Launch(t *testing.T) { +// TestGPUPassthrough launches a single GPU instance and runs Launch, +// DeviceVisible, and DriverAccess as ordered subtests against it. Sharing one +// instance keeps its floating/private IP allocated for the whole test so no +// mid-suite terminate+relaunch recycles an IP into a stale OVN datapath. +func TestGPUPassthrough(t *testing.T) { fix := requireGPUFixture(t) requireBaseGPUAMI(t, fix) - harness.Phase(t, "GPU — Launch: RunInstances with %s", fix.GPUInstanceType) + harness.Phase(t, "GPU — Passthrough: launch, device visibility, driver access with %s", fix.GPUInstanceType) vpc := harness.EnsureDefaultVPC(t, fix.Harness) - keyName, _ := harness.EnsureKeyPair(t, fix.Harness, fix.ArtifactDir(t)) + keyName, keyPath := harness.EnsureKeyPair(t, fix.Harness, fix.ArtifactDir(t)) + harness.AuthorizeSSHIngress(t, fix.AWS, vpc.SGID) instanceID := launchGPUInstance(t, fix, vpc.SubnetID, []string{vpc.SGID}, keyName) harness.Detail(t, "instance", instanceID, "type", fix.GPUInstanceType, "ami", fix.AMIID) inst := harness.WaitForInstanceState(t, fix.AWS, instanceID, "running") - assert.Equal(t, "running", aws.StringValue(inst.State.Name)) -} - -// TestGPUPassthrough_DeviceVisible SSHes into the GPU VM and confirms the -// NVIDIA PCI device is visible via sysfs, proving the vfio-pci passthrough -// wired the device into the guest. -func TestGPUPassthrough_DeviceVisible(t *testing.T) { - fix := requireGPUFixture(t) - requireBaseGPUAMI(t, fix) - harness.Phase(t, "GPU — DeviceVisible: NVIDIA PCI device present in VM sysfs") - - vpc := harness.EnsureDefaultVPC(t, fix.Harness) - keyName, keyPath := harness.EnsureKeyPair(t, fix.Harness, fix.ArtifactDir(t)) - harness.AuthorizeSSHIngress(t, fix.AWS, vpc.SGID) - - instanceID := launchGPUInstance(t, fix, vpc.SubnetID, []string{vpc.SGID}, keyName) - inst := harness.WaitForInstanceState(t, fix.AWS, instanceID, "running") - harness.Detail(t, "instance", instanceID) host, port := harness.InstancePublicSSHHost(t, inst) tgt := harness.SSHTarget{User: "ubuntu", Host: host, Port: port, KeyPath: keyPath} @@ -53,36 +38,28 @@ func TestGPUPassthrough_DeviceVisible(t *testing.T) { harness.Step(t, "waiting for SSH on %s:%d", host, port) waitForSSH(t, tgt, 5*time.Minute) - harness.Step(t, "checking sysfs for NVIDIA vendor (0x10de)") - out := sshRun(t, tgt, "grep -l 0x10de /sys/bus/pci/devices/*/vendor 2>/dev/null | wc -l") - assert.NotEqual(t, "0", out, "no NVIDIA PCI device found in VM sysfs — GPU passthrough did not wire the device into the guest") -} + t.Run("Launch", func(t *testing.T) { + // RunInstances with a GPU instance type succeeds and reaches running + // state, confirming the VFIO bind path. + assert.Equal(t, "running", aws.StringValue(inst.State.Name)) + }) -// TestGPUPassthrough_DriverAccess confirms nvidia-smi succeeds inside the VM, -// proving the guest OS can communicate with the passed-through GPU. -func TestGPUPassthrough_DriverAccess(t *testing.T) { - fix := requireGPUFixture(t) - requireBaseGPUAMI(t, fix) - harness.Phase(t, "GPU — DriverAccess: nvidia-smi succeeds in VM") + t.Run("DeviceVisible", func(t *testing.T) { + // SSHes into the GPU VM and confirms the NVIDIA PCI device is visible + // via sysfs, proving vfio-pci passthrough wired the device into the guest. + harness.Step(t, "checking sysfs for NVIDIA vendor (0x10de)") + out := sshRun(t, tgt, "grep -l 0x10de /sys/bus/pci/devices/*/vendor 2>/dev/null | wc -l") + assert.NotEqual(t, "0", out, "no NVIDIA PCI device found in VM sysfs — GPU passthrough did not wire the device into the guest") + }) - vpc := harness.EnsureDefaultVPC(t, fix.Harness) - keyName, keyPath := harness.EnsureKeyPair(t, fix.Harness, fix.ArtifactDir(t)) - harness.AuthorizeSSHIngress(t, fix.AWS, vpc.SGID) - - instanceID := launchGPUInstance(t, fix, vpc.SubnetID, []string{vpc.SGID}, keyName) - inst := harness.WaitForInstanceState(t, fix.AWS, instanceID, "running") - harness.Detail(t, "instance", instanceID) - - host, port := harness.InstancePublicSSHHost(t, inst) - tgt := harness.SSHTarget{User: "ubuntu", Host: host, Port: port, KeyPath: keyPath} - - harness.Step(t, "waiting for SSH on %s:%d", host, port) - waitForSSH(t, tgt, 5*time.Minute) - - harness.Step(t, "running nvidia-smi") - out := sshRun(t, tgt, "nvidia-smi --query-gpu=name --format=csv,noheader") - assert.NotEmpty(t, out, "nvidia-smi returned no GPU name — driver not loaded or GPU not accessible") - harness.Detail(t, "gpu_name", out) + t.Run("DriverAccess", func(t *testing.T) { + // Confirms nvidia-smi succeeds inside the VM, proving the guest OS can + // communicate with the passed-through GPU. + harness.Step(t, "running nvidia-smi") + out := sshRun(t, tgt, "nvidia-smi --query-gpu=name --format=csv,noheader") + assert.NotEmpty(t, out, "nvidia-smi returned no GPU name — driver not loaded or GPU not accessible") + harness.Detail(t, "gpu_name", out) + }) } // TestGPUPassthrough_Release terminates a GPU instance and proves the GPU