From b4d1ba38e2e9a348b5af179230833f61e931ed7f Mon Sep 17 00:00:00 2001 From: Mlanawo MBECHEZI Date: Sat, 25 Jul 2026 12:17:26 +0300 Subject: [PATCH] test(e2e): fix the three firecracker tests that never ran --- tests/e2e/firecracker/ensure-job-image.sh | 26 ++++++++++++-- tests/e2e/firecracker/t2_ports.sh | 6 +++- tests/e2e/firecracker/t3_api_instances.sh | 44 ++++++++++++++++------- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/tests/e2e/firecracker/ensure-job-image.sh b/tests/e2e/firecracker/ensure-job-image.sh index f8d9188..2a33c4b 100755 --- a/tests/e2e/firecracker/ensure-job-image.sh +++ b/tests/e2e/firecracker/ensure-job-image.sh @@ -51,17 +51,27 @@ ensure_fc_job_image() { # Start from a private copy so the shared base rootfs stays pristine. cp "$RING_E2E_FC_ROOTFS" "$RING_E2E_FC_JOB_IMAGE" + # Replay the journal before touching the image with debugfs. The CI rootfs + # ships with `needs_recovery` set, and debugfs writes straight to the block + # device without consulting the journal — so the kernel replays it at mount + # and overwrites everything injected below. The symptom is remote from the + # cause: systemd logs "Wants dropin ... unreadable, ignoring: Structure needs + # cleaning", the unit never runs, the guest never reboots, and the job stays + # Running until the test times out. + e2fsck -fy "$RING_E2E_FC_JOB_IMAGE" >/dev/null 2>&1 || true + # NOTE: the guest must `reboot`, not `poweroff`. With `reboot=k` in the # kernel cmdline a guest reboot is a keyboard-controller reset that # Firecracker traps and exits the VMM on (exit_code=0). A `poweroff` only # halts the vCPU ("System halted") and leaves the firecracker process alive, # so Ring would never see the VM go away. This mirrors how `kind: job` # workloads are expected to signal completion on this runtime. + # No explicit ordering: the unit is pulled in by multi-user.target and only + # needs a working init to call `reboot`. Ordering it After that same target + # while being WantedBy it would be redundant at best. cat > "$service_file" <<'EOF' [Unit] Description=Ring kind:job E2E auto-reboot (signals job completion) -After=multi-user.target -Wants=multi-user.target [Service] Type=oneshot @@ -76,9 +86,19 @@ EOF debugfs -w -R "write $service_file /etc/systemd/system/ring-job-poweroff.service" "$RING_E2E_FC_JOB_IMAGE" 2>&1 \ | grep -v "^debugfs " >&2 || true # Enable via the standard *.wants symlink (idempotent: drop a stale one first). + # + # The destination MUST be relative. ext4 stores a symlink target of up to 59 + # bytes inline in the inode ("fast symlink") and needs a data block beyond + # that, but debugfs writes every target inline regardless — so an absolute + # `/etc/systemd/system/ring-job-poweroff.service` (45 bytes) produces an inode + # the kernel rejects with EUCLEAN. systemd then logs "Wants dropin ... + # unreadable, ignoring: Structure needs cleaning", the unit never runs, and + # the guest sits at the login prompt while Ring waits for a reboot that never + # comes. `../ring-job-poweroff.service` is 28 bytes, safely inline, and is the + # form systemd itself uses for units under /etc/systemd/system. debugfs -w -R "rm /etc/systemd/system/multi-user.target.wants/ring-job-poweroff.service" "$RING_E2E_FC_JOB_IMAGE" 2>&1 \ | grep -v "^debugfs " >&2 || true - debugfs -w -R "symlink /etc/systemd/system/multi-user.target.wants/ring-job-poweroff.service /etc/systemd/system/ring-job-poweroff.service" "$RING_E2E_FC_JOB_IMAGE" 2>&1 \ + debugfs -w -R "symlink /etc/systemd/system/multi-user.target.wants/ring-job-poweroff.service ../ring-job-poweroff.service" "$RING_E2E_FC_JOB_IMAGE" 2>&1 \ | grep -v "^debugfs " >&2 || true stat -c '%Y %s' "$RING_E2E_FC_ROOTFS" > "$RING_E2E_FC_JOB_STAMP" diff --git a/tests/e2e/firecracker/t2_ports.sh b/tests/e2e/firecracker/t2_ports.sh index a802a3c..02afb06 100755 --- a/tests/e2e/firecracker/t2_ports.sh +++ b/tests/e2e/firecracker/t2_ports.sh @@ -68,7 +68,11 @@ EOF # Record any pre-existing ring-* taps (orphans from earlier runs on a shared # host) so we can attribute the NEW tap to this deployment rather than picking # an unrelated one with `head -1`. -taps_now() { ip -o link show 2>/dev/null | grep -oE 'ring-[0-9a-f]+' | sort -u; } +# `|| true`: grep exits 1 when the host carries no ring-* tap at all (a clean +# machine, or the very first Firecracker test of a run), which under `set -e` +# would abort the test before it does any work. Same guard as setup.sh and +# t4_restart.sh. +taps_now() { (ip -o link show 2>/dev/null | grep -oE 'ring-[0-9a-f]+' || true) | sort -u; } TAPS_BEFORE=$(taps_now) "$RING_BIN" apply --file "$FIXTURE" diff --git a/tests/e2e/firecracker/t3_api_instances.sh b/tests/e2e/firecracker/t3_api_instances.sh index c06bc44..64a7dc8 100755 --- a/tests/e2e/firecracker/t3_api_instances.sh +++ b/tests/e2e/firecracker/t3_api_instances.sh @@ -5,11 +5,14 @@ # # 1. a deployment WITH ports → each instance carries its id AND a routable # guest address (the 10.42.x.y/30 allocated for its tap), -# 2. a deployment WITHOUT ports → each instance carries its id but NO address -# (no network was allocated, so there is nothing to route to). +# 2. a deployment WITHOUT ports → the same, because this runtime allocates a +# tap for every microVM; ports only decide whether inbound forwarders are +# spawned. # -# We assert against `deployment list --output json`, which serializes the same -# DTO the REST API returns — so this covers get.rs/list.rs + the dto shape. +# We assert against `GET /deployments?instances=true`, which serializes the +# DeploymentOutput DTO — so this covers list.rs + the dto shape. The query +# parameter is required: resolving an address costs a per-instance runtime +# inspect, so the endpoint skips it by default (and the CLI never asks for it). # # Requires: firecracker, /dev/kvm, jq, and CAP_NET_ADMIN for ring-server (tap # creation, needed by the with-ports case). SKIPs (exit 0) when the ring binary @@ -47,10 +50,19 @@ setup_fc start_ring ring_login +TOKEN=$(jq -r '.default.token' "$RING_TEST_DIR/auth.json") + # Returns the JSON `.instances` array for a deployment, by namespace/name. +# +# Hits the API directly with `?instances=true` rather than going through +# `deployment list`: resolving an instance address costs a per-instance runtime +# inspect, so the endpoint only does it when explicitly asked, and the CLI never +# passes the flag. Without it every `.address` is null on every runtime, and the +# assertions below would be testing the default listing rather than the DTO. instances_json() { local namespace="$1" name="$2" - "$RING_BIN" deployment list --output json 2>/dev/null \ + curl -fsS "$RING_URL/deployments?instances=true" \ + -H "Authorization: Bearer $TOKEN" 2>/dev/null \ | jq -c --arg ns "$namespace" --arg n "$name" \ '.[] | select(.namespace==$ns and .name==$n) | .instances' \ | head -n1 @@ -95,7 +107,13 @@ printf '%s' "$ADDR" | grep -qE '^10\.42\.[0-9]+\.[0-9]+$' \ || fail "instance address '$ADDR' is not a 10.42.x.y guest IP" log "with-ports instance address: $ADDR" -# === Case 2: deployment WITHOUT ports → instances carry id but NO address === +# === Case 2: deployment WITHOUT ports → instances still carry an address === +# +# Unlike Cloud Hypervisor, this runtime allocates a tap for every microVM, not +# only for those publishing ports: a VM needs outbound connectivity to be useful +# (fetching what it runs) whether or not anything is forwarded inbound. Ports +# only decide whether host-side forwarders are spawned. This assertion used to +# expect no address, matching the older ports-only behaviour. FIXTURE_NOPORTS="$RING_TEST_DIR/api-noports-vm.yaml" cat > "$FIXTURE_NOPORTS" </dev/null || tr [ -n "$ID_NP" ] || { printf '%s\n' "$INSTANCES_NP" >&2; fail "no-ports instance has no .id"; } log "no-ports instance id: $ID_NP" -# `address` must be absent/null — no network was allocated, so there is no -# reachable endpoint. `#[serde(skip_serializing_if = "Option::is_none")]` means -# the field should not even be present; `// empty` collapses both null + absent. +# `address` is present and routable, exactly as for a deployment with ports. ADDR_NP=$(printf '%s' "$INSTANCES_NP" | jq -r '.[0].address // empty' 2>/dev/null || true) -[ -z "$ADDR_NP" ] \ - || { printf '%s\n' "$INSTANCES_NP" >&2; fail "no-ports instance unexpectedly has address '$ADDR_NP'"; } -log "no-ports instance has no address (as expected)" +[ -n "$ADDR_NP" ] \ + || { printf '%s\n' "$INSTANCES_NP" >&2; fail "no-ports instance has no .address (tap should be allocated regardless of ports)"; } +printf '%s' "$ADDR_NP" | grep -qE '^10\.42\.[0-9]+\.[0-9]+$' \ + || { printf '%s\n' "$INSTANCES_NP" >&2; fail "no-ports instance address '$ADDR_NP' is not a 10.42.x.y guest IP"; } +log "no-ports instance address: $ADDR_NP" # === Cleanup === for name in api-ports-vm api-noports-vm; do @@ -129,4 +147,4 @@ for name in api-ports-vm api-noports-vm; do [ -n "$id" ] && { "$RING_BIN" deployment delete "$id" >/dev/null 2>&1 || true; } done -log "PASS — T3-FC: API reports instances as { id, address }; address present with ports, absent without." +log "PASS — T3-FC: API reports instances as { id, address }; address present with and without ports."