31 Commits

Author	SHA1	Message	Date
Ben Duncan	8ab96b29a1	fix: replicate every JetStream KV bucket across the cluster (#1117) ... * fix: replicate every JetStream KV bucket across the cluster A KV bucket created without a replica count gets one replica, and one replica lives on one JetStream-chosen server recorded in no config. Losing, restarting or partitioning that server makes the bucket return "no responders available" from every node at once, so a single-node event becomes a cluster-wide outage. The reconciler leader-lease buckets were the worst case: acquiring a lease is a write, so a lease bucket without a quorum cannot be acquired by anyone, and every reconciler sharing it stops cluster-wide at exactly the moment a node has failed. The cluster's node count is now declared once, in config.LoadConfig, and read by the one helper every bucket creation goes through. Every process that creates a bucket loads the cluster config first, so no service can forget to pass it and no signature has to carry it. An undeclared size is an error at creation rather than a silent fall back to one replica. Existing buckets are raised rather than left behind: a service opening one raises it, the daemon sweeps them all after formation, and "spx admin kv replicas --repair" does it on demand. Raising reads the live stream config and changes only the replica count, and never lowers one. Removes the ~30 replicas/clusterSize parameters that only forwarded a number, and JetStreamManager.UpdateReplicas, which set len(nodes) with no clamp and would have failed outright on a cluster of six or more. Proved by a repo-wide invariant test pinning bucket creation to the one helper, a whole-cluster sweep in every multi-node nightly cell, and a new kvquorum suite (cell 33) that takes away the node leading the most buckets and requires the survivors to keep answering for all of them. * chore: name cell 33 in the nightly dispatch input * fix: a KV bucket that cannot be raised must still open The first version of the replica fix treated a raise it could not place as a hard error, and on a real three-node cluster that turned the fix into the outage it prevents. A node whose config named a fourth server wanted four replicas, NATS answered "no suitable peers for placement" for three, and the daemon retried for its full five-minute budget without finishing bootstrap. The bucket was present, quorate and serving on three nodes throughout. A config naming a node before that node serves is the ordinary middle of growing a cluster, so that is a state to tolerate rather than refuse. The line is now drawn between the two cases instead of around both. Creating a bucket below the cluster's replica count still fails, because that is the defect and a fallback there reintroduces it where it is hardest to notice. Failing to raise an existing bucket warns and hands the bucket back, and the raise is retried on every open besides the daemon sweep and --repair. This matters most in kvlease, which runs on every reconcile tick: refusing there would stop every reconciler sharing a lease bucket that was working. getOrCreateBucket now attaches before it creates. A create asking for more replicas than the cluster can place is refused before the server looks at the name, so with create-first "this bucket already exists" was an inference from which error came back, and the wrong one comes back on a single-node server. RaiseAllBucketReplicas attempts every bucket and joins the failures, and --repair keeps going too. Stopping at the first would leave the rest of a cluster short because one bucket could not be placed. Also adds nats.ServerName/NodeFromServerName beside the template that renders server_name, with a test tying them together. The kvquorum suite had mapped stream peers to nodes by comparing them to node names, which they are not: they are the node name behind a "spinifex-nats-" prefix, so nothing matched and the node-loss test could not choose a victim. * test: wait for the right NATS peer count after a node rejoins /routez reports a node's routes to its peers, not its own membership, so a fully formed three-node cluster is two peers per node. The kvquorum suite waited for three and spent its whole five-minute rejoin budget on a cluster that had already reformed. Every other caller passes len(Nodes)-1; the helper's doc comment now says so. The assertion the test exists for had already passed: node1 was leading 18 of 47 buckets when it was taken away, and node2 and node3 each still answered for all 47. * fix: replicate KV buckets across the largest odd count a cluster can place A bucket was replicated across min(nodes, 5) servers, which stores a fourth full copy on a four-node cluster and still survives exactly one loss — the same tolerance R3 gives for three quarters of the write amplification. A Raft majority needs an odd count, so the rule becomes the largest odd count the cluster can place, capped at five: 1, 3, 3, 5, 5. It is one expression with two entry points, so no caller can hold a stale copy of the policy and keep passing after the policy changes. Three retry loops could not tell a configuration fault from a cluster that had yet to form. An undeclared cluster size never becomes declared by waiting, so each of them spent its whole budget on one and then reported quorum as the cause. They now stop on a permanent error and say what is actually wrong. This is what the integration suite was hitting: it builds its gateway by hand and never loads a cluster config, so the size was never declared and the IAM retry sat in its sleep until the test timed out. Both ends are fixed. The audit reported configuration alone. A bucket at three replicas with one peer offline and one lagging is one failure from unavailable and read as ok. BucketReport now carries the leader and a count of the peers JetStream itself calls current and reachable, quorum is measured against that count, and spx admin kv replicas exits 0, 1 or 2 so a deployment gate can tell a problem --repair fixes from one needing a human. Declaration is write-once: daemon.Start refuses a conflicting redeclaration because it is about to create buckets, and it declares before JetStream init rather than after it. config.LoadConfig warns instead, because it also reads config files that are not this process's own cluster. spx admin init --nodes 2 is refused — no replica count on two nodes both survives a loss and accepts writes — and single-node replication is asserted in the single e2e suite, so it runs in every nightly host permutation. * fix: withhold the vLLM serving image until its artifact is published The catalog offers ubuntu-26.04-vllm-serving-x86_64, and the qcow2 it names 404s on iso.mulgadc.com, so spx admin images import would fail for every operator who tried it. Its GPU siblings are published; this one never was. The entry moves to WithheldImages rather than being deleted, because the entry is the definition — the URL, the checksum and the two tags resolveServingAMI filters on. Publishing the artifact is then a move back, not a retype. Bedrock already handles the image not being registered: resolveServingAMI returns ErrServingAMINotFound, which is the honest answer while there is nothing to launch. The bedrock tag contract is still pinned, now against the map the definition lives in, and it additionally asserts the entry is not offered for import. The e2e reachability walk covers AvailableImages alone, so a withheld entry is out of it by construction. * fix: drop the cluster size from the call sites dev added Two constructors on dev still pass a replica count that no longer exists as a parameter: the cluster size is a declared process fact, so EKSServiceDeps has no ClusterSize field and NewIAMServiceImpl takes three arguments. The merge is textually clean and breaks the build, which is the shape of collision a signature removal always has. * docs: explain what an operator sees when a cluster server fails The replication section said how many copies a bucket has and stopped there, which leaves the question an operator actually has during an incident: one server is gone, what still works. It now covers the majority side carrying on, an isolated server refusing changes on purpose and why that is the behaviour you want, guests continuing to run while anything that has to be recorded does not, the risk of reading state from a server that is cut off, and a returning server catching up by itself. The surrounding prose reads from the operator's side rather than the implementation's: what you see and what to do, not which consensus protocol is underneath.	2026-09-28 12:34:55 -05:00
Ben Duncan	1afc7fa8aa	fix: retry a recovery relaunch that lost a race with a starting dependency (#1020) ... * fix: retry a recovery relaunch that lost a race with a starting dependency A guest drain-stopped by a coordinated cluster shutdown could be lost for good. The relaunch ran before JetStream was serving, the volume lease claim timed out, and the failure was recorded as permanent. The backoff this needed already existed. relaunchWithRetry retries five times, but only on ErrMountRetryable, and mountErrRetryable recognised exactly two viperblock state-load sentinels. A lease claim returning context deadline exceeded matched neither, so the mount was reported as permanently failed and the backoff never engaged once. - Classify a lease store that cannot answer as ErrLeaseStoreUnavailable, and include it in mountErrRetryable. A lease held by another node is deliberately excluded: that is an answer, not an outage. - Probe readiness rather than connectedness. checkViperblockReady tested natsConn.IsConnected(), which is true against a server whose JetStream is still starting; it now calls AccountInfo with a timeout. - Reconsider a latched failure. RetryRecoveryFailed relaunches instances parked in error by a dependency-unavailable reason, once the backing store is confirmed ready, for a bounded number of passes, then leaves them alone. Only mount-state and launch failures qualify; a failed reconnect or pre-relaunch hook can mean the guest itself needs looking at. Closes the e2e gap that made this feel covered. The node-failure tests stop the service units directly so guests keep running and recovery is a QMP re-attach to a live process. Nothing stopped a guest, so nothing relaunched one from a sealed volume against dependencies that were still starting. TestMultinodeClusterRestartResumesGuests does, including the wait for every spx process to exit that makes it a real restart rather than a stop that returned early. Beads mulga-v7kp4, mulga-kyb1d. * fix: stop every spx process by unit in the cluster-restart helper The first CI run of TestMultinodeClusterRestartResumesGuests failed at the process wait with "node1 still has 2 spx process(es)", and the same thing happened by hand on a prod node. Every service is the same binary, and the stop was driven from a hard-coded list. StopNode names seven units and omits spinifex-northstar and spinifex-qmp-collector, which is exactly the two survivors. The unit name now comes from the surviving process's cgroup, so the set that stops is whoever is still running the binary and the list cannot go stale when a service is added. StopNode itself is left alone: its list is deliberate for the hard outage it simulates, where the target's drain must not fire. Also marks recovery_retry_test.go as //test:in-package — the retry policy is unexported and the tests drive it through the seams restore_test.go uses. * fix: stop spinifex.target in the cluster-restart helper, not the units The second CI run reformed NATS to 2 peers of 3: node3's NATS was stopped and never started again. StopNode stops the service units directly and leaves spinifex.target active. That is right for the hard outage it simulates and wrong for a real shutdown — the target then reads as satisfied while its units are dead, and the later start raced a stop job that was still queued. The helper now runs what the operator procedure and update-nodes.sh run: stop spinifex.target, wait for every spx process to exit, and additionally wait for systemd to have no spinifex job left queued before starting. That last wait is the one that closes this race. * fix: wait for the right NATS peer count in the cluster-restart helper WaitNATSPeers counts remote peers, so a healthy three-node cluster reports two. Asking for three could never be satisfied, and the run that hit it had in fact restarted all three NATS correctly.	2026-09-14 10:39:06 -05:00
Ben Duncan	18853cef31	fix: close the four defects keeping the e2e nightly red (#1007) ... * fix: stop the e2e analyser inventing failures and let a detach outlast its seal go-junit-report emits <error message="No test result found"> for a === RUN with no matching result line, and attaches the test's own captured log as the body. The analyser counted those as failures, so the baremetal cell reported 25 failures against a junit file recording 1, with log output reading as assertions. Separate the marker from a real error in both the parse and the roll-up, count the unresolved cases and say so in the summary. The gateway gave a detach 30s while the daemon budgets 180s for the ebs.unmount seal and 180s more for the QMP unplug chain, so any detach whose seal replayed a WAL was reported as ServerInternal while it went on to succeed. Also switch four nightly job gates from always() to !cancelled(): always() overrides cancellation, so a cancelled run kept starting jobs. * fix: serialise the e2e workflows on one group and retry a dropped libvirt e2e.yml had no concurrency block at all, so every push to dev could provision alongside every other push and alongside the nightly. All four workflows that build on these hypervisors now share e2e-hypervisors, queued rather than cancelled. A per-environment group is not expressible: GitHub picks the group before it picks a runner, and the environment is a property of the runner. tofu apply also failed outright on a transient libvirt connect, which fails a 45-minute job in 90 seconds and reports it as a test failure. retry_run retries only when that attempt's own output names a connection failure, so a real failure still fails once. Also collect the unfiltered journal from the matrix cells and the node journal from cell-21: the unit filter drops the kernel and OOM lines that explain why one node behaved unlike its siblings, and a passing run's journal is what makes a failing one readable. * fix: build the baremetal junit from the unfiltered suite log run-baremetal-e2e.sh streams the full suite output to suite-output.log and a grep-filtered summary to its stdout, which the workflow tees into test-baremetal.log. That filter anchors --- PASS at column 0 and Go indents every subtest result by four spaces, so the junit was generated from a log with no subtest results in it: go-junit-report saw each === RUN with no outcome and emitted <error message="No test result found">. 17 of cell-28's 39 testcases and 93 of cell-20's 120 were that. Regenerating cell-28 from suite-output.log gives 39 tests, 0 failures, 0 errors. * fix: converge released EIPs instead of trusting a fire-and-forget teardown A disassociate leaves the ENI running, so the orphan sweep's owning-port test never fires for it. The teardown then rests entirely on vpc.delete-nat, which is core NATS and is lost whenever vpcd is restarting — and the intent KV watch is UpdatesOnly, so the same window hides the write that would have driven a repair pass. The dnat_and_snat row and its host /32 route to a released public address survived until something else happened to touch them, which for a live ENI was nothing at all. Give the sweep the second test it was missing: AddEIP is the only writer of dnat_and_snat, so the floating IPs intent asks for are the complete set of addresses that may hold one, and a row for any other address is stale however alive its port. Ports and external IPs travel together in LiveEIPs, both unioned from the start-of-pass snapshot and the fresh re-read, so a mid-pass launch is still spared. The host unbind now runs even when the row had already gone, which is the state a crash between the OVN delete and the unbind leaves. Run that sweep on the startup pass too. Startup is the pass that follows the blind window, so skipping it deferred repair of exactly the changes that were missed. The port-group and ENI-port sweeps stay gated there for the in-flight create they were written for; the EIP sweep re-reads intent immediately before deciding and abandons the whole sweep if that read fails, which is the same protection by a stronger route. * fix: keep a release verify off the hypervisors an e2e run is using release.yml is the fifth workflow that provisions on the ci-single pool and had no concurrency block at all, so a tag push could land a tofu apply on the same libvirt host a nightly cell was mid-provision on. The group goes on the verify job rather than the workflow: the build jobs touch no hypervisor and should not queue behind a nightly. * fix: stop cell-27 handing a 50-minute suite a 50-minute job The rds suite is budgeted 50m in suite-sets.sh and cell-27 took the matrix default of 50 minutes for the whole job, so provisioning, install, the suite and teardown had to fit inside the suite's allowance alone. It never could: the last measured run spent 6 minutes reaching the suite and was killed 44 minutes into it, mid-TestAutomatedBackups. A job timeout reports "The operation was canceled" and nothing else, which is why this cell has no verdict in any nightly rather than a failure anyone could read. cell-27 gets 75 minutes, and the select job now refuses to start a run whose job budget does not clear its suite budget plus a setup allowance. The inner timeout firing first is the property that matters: go test says where it stopped and what failed, and the job timeout says nothing at all. cell-29 already had the same fix applied by hand; nothing stopped the next suite from repeating it. * fix: converge host EIP plumbing against intent, not against the NAT row The released-EIP sweep landed and did not fire. Its premise was that a stranded dnat_and_snat row is what a stranded host route can be found by, and on the path that actually strands one there is no row left. AddEIP scrubs the predecessor row sharing the private IP before installing the new one, so when reconcile re-added the instance's auto-assigned address it silently deleted the released EIP's row — and applyEIPs runs ahead of the sweep, which then walked a table with nothing stale in it while the /32 route and its proxy-ARP stayed up. Give the sweep the host's own inventory. ListEIPIngress reads the EIPs back from the FORWARD accepts EnsureEIPIngress stamps with its own comment, so a binding is found by what installed it rather than by a row some other path is free to delete. Anything plumbed that intent no longer asks for is torn down, which also covers a crash between the OVN delete and the unbind. A failed inventory fails the pass: an empty list and an unreadable one are indistinguishable, and reading the second as the first is a sweep that reports success having done nothing. Close the window inside the pass too. The predecessor scrub now reads the row it is about to drop and releases that address's host state, so the strand does not exist between applyEIPs and the sweep rather than merely being repaired a step later. * fix: retry the release verify's libvirt calls like every other job does The verify job drives the same libvirt provider on the same hypervisors as the e2e cells, where a dropped connection under concurrent load is the steady state and is retried. Here it was a hard failure that reads as a broken release. It was skipped earlier only because the job checks out mulga and not spinifex, so the shared helpers were not on disk — a sparse checkout of .github/scripts costs nothing and makes apply and destroy behave like their e2e counterparts. * fix: do not report a rebooting DB instance available on the pre-reboot agent's health RegisterDBInstance stamped a fresh LastSeen while leaving Agent.EngineHealth at the value the previous agent process reported. After a reboot the new agent registers before it has probed anything, so the record read as a healthy engine with a beat newer than the transition, and the reconciler moved the instance to available while PostgreSQL was still starting up. A client that connects the moment the status turns green then gets "the database system is starting up". A register now discards the previous process's verdict and records when this agent process started. The restart reconciler requires that registration to be newer than the transition, which also closes the window where a beat lands between the engine being stopped and the VM going down. * test: record the credential kubectl sent when an EKS call comes back Unauthorized The IRSA pod subtest's first kubectl is the gateway-CA configmap, which lands before the pod dumps are registered, so a 401 there left no evidence at all. A verbose read-only probe reports whether an Authorization header was sent, which separates a token the apiserver rejected from one the exec plugin never made. * fix: report a starting engine promptly, and budget CI jobs from the suite The agent waited a full heartbeat interval before its first beat, so a restart could not be observed as finished for up to 30 seconds after the engine was already serving. Beat at startup instead, and report every 5 seconds until the engine has answered once — the control plane is actively waiting on that beat, and the steady-state cadence it hands back is for a database already serving. The rds suite is the one most sensitive to what else the hypervisor is doing: its DB VMs serialise behind a 4 GiB semaphore, so its wall clock is set by how long it waits for the budget. The same pass ran 45m beside four nightly cells and overran 50m beside nine, panicking with two tests still running. Raise its budget to 60m. That budget was then larger than the jobs meant to contain it. A job cancelled on timeout reports only "The operation was canceled" and takes the artifacts with it, so the inner `go test` timeout must always fire first — and it could not for rds on e2e.yml, nor for storagefault or diskperf, which have asked for 90m and 180m inside a 60m job since they were added. Both workflows now resolve their job budget from suite-sets.sh rather than carrying a literal: e2e.yml from the largest requested suite, e2e-suites.yml per matrix leg, so a leg is bounded by its own suite and not by the longest one beside it. The default push set resolves to exactly the timeouts both workflows had before. * fix: check a permutation cell's job budget against the suites it runs The guard only saw cells that name a suite, so the nine that run a topology's nightly subset were never checked against it — the same class of mistake the guard exists to catch, in the majority of the cells. Bound each by the longest suite in its subset: max() and not sum(), because whichever suite hangs must panic on its own timeout first, and a sum would demand a budget the set has never come close to using. Every cell clears it today. * fix: report why a PostgreSQL engine would not start, and stop failing a stop that landed Three defects, all found by the first rds pass that ever ran deep enough to reach them. A wait that saw an instance reach `failed` called t.Fatalf from inside the polling condition, and testify runs that condition on its own goroutine. The Goexit killed the tick and left the collector empty, which reads as the condition being satisfied — so the wait returned successfully on an instance it had just declared broken, logged "reached status available" about one that was failed, and let the test carry on against it. Six conditions across four suites call Fatal or require the same way. EventuallyErr now runs its condition on the test's goroutine, which makes all six behave as written. A DB VM stop has two drivers: the API call and the reconciler resuming one whose caller it assumes has died. When they overlap the loser is told the VM is not in a state to be stopped — which is the state the stop was heading for — and the customer's StopDBInstance returned InvalidDBInstanceState for a stop that had just completed. Both paths now treat a refused command the way they already treat an unanswered one: it says nothing about the VM, so the fleet view settles it. PostgreSQL had no equivalent of the engine log tail MariaDB has carried since it was added, so a postmaster that refuses a setting produced "engine did not respond on /run/postgresql:5432" and nothing else — on a guest with no SSH, whose console records only that pg_ctl could not start the server. The probe now quotes the tail of the log the image already directs the postmaster to, and the reboot and modify paths carry the agent's reason into the failure the way the create path always has. The reason reaches the customer through StatusInfos. The rds suite's budget goes to 90m and cell-27's job to 100m. Its wall clock is what the DB-VM queue costs rather than what the tests do — single tests logged 11m, 22m and 26m waiting for budget — and the pass above still had three of seventeen tests running at 60m. * fix: keep a DB VM recoverable when its agent cannot start, and flush the volume before a snapshot An rds-agent that failed to construct exited, so a transient IMDS credential resolution took the guest down for good: rds-datadir and rds-init cascade on the missing handoff, the engine never starts, and the control plane sees only silence until it declares the instance failed ten minutes later. Construction is retried like every other boot step instead. OpenRC skips start_post when the start itself fails, which is the one case with no diagnostic at all — the console carried nothing but "died". The log mirror moves out of the handoff wait so a failed start uses it too. A snapshot quiesced the engine and nothing else. The engine repairs its own files from the WAL, but postgresql.conf sits beside them and it does not: a restore came up on a torn tail and the postmaster stopped on a syntax error at the unterminated include hook. The quiesce now flushes the whole data mount, and the hook is renamed into place rather than appended, so a boot that finds a half written one replaces it instead of appending after its open quote. * fix: flush the config cloud-init wrote before a reset can take it A DB instance rebooted seconds after it went available came back with no RDS_GATEWAY_URL, so rds-agent could not reach the control plane and the engine never started. cloud-init writes agent.env once, per-instance, and never again; a reboot is a QMP system_reset with no guest shutdown, so the file was still dirty page cache when the machine reset under it and the next boot read nothing. The handoff wait now syncs, which is the point at which the agent has proved it read that file. * fix: stop dropping an EIP teardown published while vpcd is restarting DisassociateAddress publishes vpc.delete-nat fire-and-forget. On run 34046316024 vpcd disconnected from NATS at 03:51:26.73 and its replacement subscribed at 03:51:27.09; the publish landed at 03:51:26.92, in the gap, and went nowhere. ReleaseAddress then returned the address to the pool two minutes later with its /32 host route still delivering to the guest that had held it. vpc.delete-nat now uses request-reply like vpc.add-nat, and retries only on no-responders — which returns immediately, so the cost is paid solely when nothing is listening. A teardown that still fails is logged as an error naming the address rather than lost silently. Neither is fatal to the caller: the reconciler's orphan sweep remains the backstop. vpc.add-nat on the EIP path stays fire-and-forget. The shared helper blocks it on the flows barrier for up to 45s, and associating an address should not inherit that. Two reconcile passes in that run had the address absent from intent and did not sweep its host route, and the journal cannot say why: the sweep logged only when it acted, so a route it kept was indistinguishable from one it never saw. It now logs the bindings it found, kept and wanted. * ci: render one run-level table for the e2e nightly Reading a nightly meant opening nineteen jobs, and the Actions UI does not separate a cell that failed from one cancelled out from under it — a distinction every investigation of this suite has needed, and one the run 34046316024 analysis had to reach for the jobs API to get. A final job, needs: every cell and if: always(), renders cell, topology, result, duration, runner and per-cell test counts into the step summary, with run-level totals, the three slowest cells and the failing test names. Result, duration and runner come from the jobs API so a cell that died before writing junit still gets a row. Counts come from a new junit-only artifact per cell rather than the full cell artifact, so the roll-up does not pull nineteen journals it has no use for. Verified against run 34046316024: 14 success, 2 cancelled, 1 failure, with the two cancellations shown as cancelled rather than folded into failure. * fix: assert EC2's actual EIP semantics, and probe EKS auth on an authenticated path natuplink asserted that an instance's auto-assigned public IP is still plumbed after an EIP associated to the same instance is torn down. EC2 does the opposite: associating an EIP releases the auto-assigned address back into the pool, and it cannot be reused. Our datapath already does that — run 34059879666 shows 192.168.0.72's ingress removed at 07:09:44.000, 36ms before .73's was installed — so the test was asserting behaviour neither AWS nor we have. It now asserts the release at the point it happens, and that no public delivery remains after the teardown. This surfaced only once the teardown itself started working: the run's other assertions passed, including .73's removal, which was the failure before. Our records still disagree with our datapath — the ENI keeps the released address, which is why termination tears the old one down twice and never the current one. That is the remaining half of mulga-7zc9w and is not fixed here; it needs an IPAM release on the associate path. Separately, AuthDiagnostics read /version, which is anonymously readable, so its 200 was consistent with both branches it claimed to distinguish. It now runs auth whoami, reads kube-system, and invokes the credential plugin directly, reporting exit code, stderr and whether a token was produced — never the token, since CI logs outlive it. * test: capture per-guest state when a spread guest never answers SSH TestMultinodeSpread's only failure dump is dumpPlacementNATDiag, which collects OVN, OVS, ARP and route state aimed at the NAT and EIP half of the test. The failure it actually hits is a guest that never accepted SSH — run 34040324968, the guest on node3 at 10.100.2.5 — and for that it recorded no console, no port binding and no tap state. There was no way to tell from the artifact whether the guest booted at all. Now dumps, per guest: the console, the NB logical switch port, the SB port binding, the OVS interface backing its tap, and br-int's port list. Probed on every node rather than the expected one, because a port bound on the wrong chassis is exactly what a hosting-node-only dump cannot show. Covers all three guests, not just the failing one, so a working guest is available as the control. * ci: sample hypervisor load either side of every cell Eight runner names map onto three hypervisors, so cells that never share a runner still share a host: when cell 10's SSH wait expired on run 34040324968, cells 22, 17, 18 and 28 were concurrently live, two of them multi-node. Nothing in the artifacts measured host load, which left co-tenancy as an explanation that could be neither confirmed nor ruled out. Records loadavg, MemAvailable, SwapFree, the cpu line carrying steal, and the QEMU process count before and after each cell, into the cell artifact. * ci: note that main's missing concurrency block is why the nightly runs ungrouped Three refs carry three different groupings: main has no concurrency block at all, dev uses e2e-nightly, this branch uses e2e-hypervisors. Scheduled runs load the workflow from the default branch, so every cron nightly runs ungrouped and cannot queue against anything — which is how a dispatch of 34050630627 ran alongside the scheduled 34046316024 and cost it cells 24 and 27. e2e-hypervisors is the right group and deliberately broader than dev's, since e2e.yml, e2e-suites.yml and e2e-ddil.yml draw on the same runner pools. Nothing on a feature branch can fix this; the block has to reach main.	2026-09-06 16:53:32 -05:00
Ben Duncan	828b5df4aa	fix: pause a guest on a backend I/O error instead of handing it EIO (#977) ... * fix: fail DRAIN when a volume seal failed shutdownAndUnmount logged the volume unmount error and returned nothing, so StopAll returned nil and the DRAIN phase ACKed success with vms_stopped set. The coordinator then advanced to STORAGE and stopped viperblock while the seal was still in flight, tearing the VBState envelope and leaving the guest unable to mount its root volume on the next boot. The deploy reported success. shutdownAndUnmount now returns ErrVolumeSealFailed when the unmount did not seal; QMP, PID-file, fw_cfg and telemetry cleanup stay best-effort. stopCleanup and stopOne carry it without aborting the rest of the teardown, since QEMU is already down by then, and StopAll aggregates the per-VM failures so handleShutdownDrain ACKs with Error set and never reports vms_stopped. volumeMounterAdapter.Unmount was the second swallow site: it logged each failed seal and returned nil unconditionally. It now attempts every volume and returns the aggregate, which is what the VolumeMounter contract already described. Terminate still tolerates a failed seal — the volumes are deleted next, so an unsealed block map loses nothing a caller can ask for. This does not fix the seal itself. It converts silent data loss into a refused upgrade that an operator can act on. * test: add the storagefault e2e suite for backend-outage guest survival Freezes predastore with SIGSTOP to hold connections open while answering nothing, which is the fault a timeout-free path cannot distinguish from a slow peer. On one node that is a total outage; on three it freezes every node but the guest's own, breaking the RS(2,1) shard floor. The suite writes and flushes a verifiable region before the fault and churns a separate unverified region during it, so an interrupted write is never mistaken for lost data. The root-filesystem load runs from the user's home rather than /tmp, which is tmpfs on this image and would move no bytes on the root device. * test: verify the root device is under write pressure, and size the outage The previous guard matched /^ *vda / against /proc/diskstats, where the device name is the third field, and read $7 which is ms_reading rather than sectors_written. It printed empty and asserted nothing, so a green run could not distinguish a surviving guest from an idle disk. It now samples sectors_written twice and fails unless it advances. The outage is sized from viperblock's absorption rather than guessed: a workload must fill DefaultMaxPendingBytes before backpressure engages at all, so a short freeze measures the retry budget rather than the fault. * test: add nbdkit fault injection to the storage-fault e2e suite The predastore freeze takes the whole storage cluster away from every guest on it, which is too broad to run beside anything else. Removing a single volume's NBD server is the same fault at one-volume scope: one host, one guest, nothing cluster-wide stopped. Both signals are here because they sit on opposite sides of the defect. SIGSTOP holds the socket open and answers nothing, so the guest blocks and no error reaches ext4. SIGKILL closes it, and QEMU's nbd driver at the default reconnect-delay of 0 fails requests at once, which under werror=report aborts the journal. Signalling is guarded: a pid is matched on comm, our plugin and the volume argument together, and re-confirmed immediately before the signal, because these hosts run nbdkit for other volumes and pids are reused. * fix: pause a guest on a backend I/O error instead of handing it EIO Every virtio-blk device was created with werror=report,rerror=report, so a backend outage was delivered to the guest as EIO. ext4 aborts its journal on that and remounts read-only, and the write that failed is simply gone. Under werror=stop QEMU pauses the VM and holds the failed request, so a backend that comes back resumes the guest with nothing lost. The boot drive, the cold-boot data volume and the hot-plugged data volume all move to stop. Alongside it the NBD transport gets reconnect-delay=30, which pauses requests rather than failing them while a storage process restarts on the same socket; on the legacy -drive form it has to be file.reconnect-delay, because the bare name reaches the raw format driver, which has no such option. The QMP heartbeat now recognises run state io-error and issues cont. After ten attempts the instance is reported impaired and the cadence drops to one attempt per ten heartbeats, but it never stops: QEMU holds the request for as long as the guest exists, so a backend that returns an hour later can still be resumed. The storagefault suite gains the fault this was written for — kill the volume's own nbdkit, hold, then put it back on the same socket — and it now verifies a sha256 region across the outage rather than trusting the guest to be reachable. Three harness defects went with it: the socket-liveness poll ran unprivileged against a 0770 directory and could never succeed, the relaunched socket was left 0755 where QEMU needs group write, and the two variants shared one guest so a fault in the first was diagnosed in the second. * test: cover the io-error resume policy * test: stop two parallel tests racing on the terminate backoff variable * fix: report a guest paused on a backend I/O error as impaired A guest held by werror=stop kept reporting ok on both status checks. QMP stays responsive while the guest is paused -- cont is being sent and answered -- so the QMP failure gate that DescribeInstanceStatus keys on never trips, and the one state a customer most needs to see looked healthy. Instance status now reports impaired/failed while the resume budget is non-zero, timestamped from IOErrorSince so impairedSince is when the guest paused rather than when it was escalated. System status reports impaired too: a backend I/O error is an infrastructure fault, not a guest one, and that distinction is what the two checks exist to draw. The counter is cleared by the first poll that sees the guest running, so both clear on recovery. * test: reap the relaunched nbdkit when the fault test ends relaunchNbdkit detaches the server with setsid so it outlives the SSH session that started it, and viperblockd never learned it exists. Nothing reaped it, so every run left an nbdkit holding a socket and a cache for a volume that was later detached or deleted -- four were found running on dev-prod against two detached and two deleted volumes. The PID is resolved from the socket path rather than the volume id, so a legitimate viperblockd server for the same volume is never the one killed. * test: prove the backend is serving before a fault is called cleared Thawing the process a fault stopped is not the same as restoring the service. The outage test treated SIGCONT as the end of the fault and went straight to judging the guest, so it passed while the cluster it ran on stayed unable to serve a single object for the next 35 minutes, and the next test would have inherited that. thawPredastore now puts and gets an object through every gate, with a budget, and fails the test if the backend does not come back. * fix: do not hand back a mount entry whose export is gone A volume whose unmount failed kept its entry in MountedVolumes while its nbdkit was gone. Every later ebs.mount then matched that entry and returned its socket path, so vmMgr waited for a socket nothing would ever create and the instance could not be started again — on dev-prod a guest stopped against a frozen backend was left permanently stopped, four start attempts failing identically on a socket the node had already removed. Drop the entry when there is positive evidence the export is dead: the socket is absent, or the recorded pid is not running. Neither can be true of a live export, so the double-writer hazard the guard exists to prevent is unaffected. Also make the durability test's pattern helpers report failures. The read dd sat in a pipeline, so its exit status was discarded and the harness hashed nothing and returned the empty-string sha256 as a checksum. * test: report the guest console when a restart never answers SSH A restart that reaches running but never comes back is indistinguishable from a dead root filesystem without the serial console, and the log is gone once the fixture terminates the instance. * fix: converge guest port datapaths concurrently, not one deadline after another Each guest port that misses its SB binding burns the full convergence deadline, and applyEIPs spent them in series. Three ports belonging to long-stopped instances therefore held a newly launched guest's EIP dark for the sum of them before its own probe started. AddEIP stays serial since it mutates NB. Only the waits fan out, bounded, and sb-cluster-state-reset is now serialised with a minimum interval so concurrent probes cannot each fire a cluster-wide resync. * fix: refuse a stale cross-node start after the owning node is gone The volume lease already refused a cross-node mount, but it is a liveness claim: 45s TTL, renewed every 15s. A node that dies holding un-uploaded writes stops renewing, and 45 seconds later nothing refuses a start elsewhere that loads a stale backend checkpoint. A failed seal now writes a durable marker naming the node holding the only current copy, cleared on a seal that reaches the backend and on delete. The lease covers a live holder, the marker a dead one. The refusal also reaches the caller instead of being flattened to ServerInternal, which told an operator to retry something that cannot succeed while the holder is down. * fix: keep the daemon's message when a start is refused Rebuilding the error from the code alone rendered the generic ErrorLookup text, so a refusal naming the node holding the volume reached the caller as boilerplate about attaching volumes. * fix(ec2): terminate detaches every DeleteOnTermination=false volume TerminateStoppedInstance skipped the detach for a non-Boot DoT=false volume on the assumption that Stop's Unmount had already cleared its attachment. Unmount only clears it when the seal succeeded, so a volume whose seal failed stayed in-use pointing at an instance that terminate then deleted, leaving it unreachable with no owner to release it. The running-instance path already detaches every DoT=false volume; the stopped path now matches it, and tolerates an already-gone volume the same way. DeleteVolume also keeps its provider error on the way out. The terminate reaper retries the delete every two minutes and logged a bare ServerInternal, which made a blob node being down indistinguishable from a transient fault. * fix(ec2): DetachVolume --force releases a volume whose instance is gone The ordinary detach routes to ec2.cmd.{instanceID} for the guest unplug, so a volume left attached by a part-way teardown could never be detached: the instance no longer runs anywhere and nothing answers. That also made it undeletable, since DeleteVolume refuses an attached volume. ForceDetachVolume already existed for this deadlock but was reachable only from account teardown. Force now falls back to it when the instance has no responder, which is the one case where clearing the attachment cannot strand a live guest. * fix(viperblock): unsealed writes steer placement instead of pinning a volume The dirty marker refused any mount from a node other than the one holding unsealed writes. Instance start already forwards to the node that last ran the instance and falls back after a 30s window, so that refusal turned a working fallback into a volume that could run nowhere once its owner was gone. It traded a rare data-loss risk for a permanent availability failure. The marker is now a record and a warning, not a gate. A takeover logs the previous holder and why it could not seal, then adopts the marker so the old node is no longer treated as the better source. Live exclusion is unchanged and still enforced by the lease. It is also written on open rather than on seal failure. A node killed mid-write never reaches its seal, so the old trigger left no trace in exactly the case the marker exists to describe. Adds 'spx admin volumes unsealed' to see which volumes would cost something if started elsewhere. * fix(viperblock): fence a volume whose lease moved instead of logging it The lease was checked once, at mount, and never again. A node that lost it kept its export up and kept writing into a volume another node owned, so both copies advanced and the backend settled it by arrival order. Losing a renewal now decides: another holder means tear the export down, no holder means the entry merely aged out, so reclaim it and leave the guest running. Fencing never seals — this node's copy is the stale one — and leaves the dirty marker for the winner to take over. This is not an epoch fence. It narrows the window from the 45s TTL to one renewal interval at no write-path cost; the backend still cannot refuse a stale writer. * fix(network): advertise EIPs so a recycled address is reachable The gateway switch port only asked ovn-northd to publish the router's NAT addresses in centralised and routed modes, so on a distributed cluster nothing ever announced an elastic IP. OVN still answers ARP for it, which is why a first-time allocation works and hid this — but no gratuitous ARP is sent when the address moves to a new ENI, so every host on the segment keeps resolving it to the previous holder's MAC until its cache ages out. Reusing an address from the pool then lands on a dead MAC and the instance is unreachable from outside. Distributed mode needs the option as much as the others: northd scopes each entry with is_chassis_resident(<vif port>), so the announcement leaves from the chassis running the instance rather than the gateway, which is what distributed NAT wants. Converge on the already-attached path too. AttachIGW returns early once the gateway port exists and nothing else rewrites it, so without that an existing VPC would stay silent for its whole life. Confirmed on dev-prod: setting the option moved three EIPs from unadvertised to carrying their correct external_mac, each scoped to its own ENI's port. * fix(viperblock): make the volume fence and its dirty marker ownership-safe Four corrections to the lease-loss fence, all found in review. Drop the reacquire path. Losing a lease and finding no successor was treated as a lapsed entry worth reclaiming, but leases.acquire returns the cached entry — already flagged lost, with a dead renewal goroutine — so the "reclaimed" lease renewed nothing and the node went on writing a volume it could not prove it owned. The two errors are not symmetric: a false fence costs one guest a restart, a false reclaim leaves two nodes writing one volume. Fence in both cases, and report which happened through the outcome attribute. Surrender a lease this node cannot confirm. Renewal only noticed a loss when the server rejected an update, so a node partitioned from NATS kept its export up while the TTL expired and another node took the volume. Track the last confirmed renewal and give the volume up once that ages past the point where the server may admit another owner. Bound the renewal call too, so a hung JetStream request cannot hold the goroutine past that point. Do not act on a kill that did not take. Both the fence and ebs.unmount removed the mount entry, released the lease and announced the result before knowing nbdkit had exited — the fence then reported a writer stopped that was still running, and the unmount sealed into a directory a live process was writing. Wait for the exit, and on failure leave the volume mounted and say so. An already-exited process now counts as success, which is what the caller asked. Give the dirty marker a generation. It recorded an owner and nothing to order two nodes' claims, so a node returning after a takeover could clear or overwrite the winner's marker and point placement back at the stale copy. Stamp the lease generation, refuse a write from an older one, and only clear a marker this node still owns. A marker that cannot be written now fails the open rather than leaving writes nothing records. * test(viperblock): cover the stall surrender and the unmount kill guard The two ownership cases the fence work left untested. A holder partitioned from NATS while the winner keeps it never gets an update rejected, so revision checking cannot see it — it has to give the volume up on its own clock. Pins that validity sits below the server TTL with room for a check in between, that a lapsed holder reports the loss as a stall rather than as a takeover, and that releasing a surrendered lease leaves the entry for whoever holds it now. The unmount side of the kill guard: sealing under a live nbdkit writes a state torn between two writers, so a kill that did not take has to fail the unmount and leave the entry mounted for a retry. * fix(e2e): keep the guest private key for as long as the key pair EnsureKeyPair computed the PEM path from the caller's directory but wrote the file inside the ensureOnce closure, which runs only on the first call. Callers passed a per-test artifact dir, and ArtifactDir removes that dir when the test passes — so the first test's success deleted the only copy of the private key while the key pair stayed registered, and every later call returned a path nothing had ever been written to. Guests after the first were then launched against a public key whose private half no longer existed, so the harness could never authenticate and gave up after the full five-minute budget. The guests were healthy and reachable on tcp/22 throughout, which is why this read as a storage or network fault for so long. The directory parameter was the defect, so it is gone: the PEM now lives in a directory the fixture owns and removes with the key pair, and no caller can supply one that is shorter-lived. Also reorder the killed-nbdkit teardown. The reaper registered its own cleanup after the detach registered one, so LIFO killed the block server before the platform detached the disk and DetachVolume then failed with "Node ... is in use". The detach cleanup now reaps once it is done. * fix: stop leaking an nbdkit export when recovery cannot adopt it Restart recovery adopts a guest's boot volume, then routinely fails the -efi volume with "claim lease: context deadline exceeded" -- JetStream is still catching up at daemon start and the KV claim runs on the client's 5s default. Recovery gave up on the first error and never retried, so the export stayed live with no registry entry. At terminate the unmount found nothing to seal, returned NotFound and killed nothing, and the process outlived its guest holding the volume and socket open. EFI is always the second volume adopted, which is why it was always the one that leaked. Two changes, because either alone leaves a hole: - rebuildMountedVolume retries the build (4 attempts, 3s apart) so a JetStream that is merely slow no longer orphans an export for the life of the process. - unmountVolume reaps a corroborated export for the volume it was asked to unmount when the registry has no entry for it, so an export orphaned by any means is still cleaned up. The reap goes through reapOrphanedNbdkit, which declines anything a live process references: at unmount the guest's blockdev may not be torn down yet, and killing the server underneath it strands QEMU. * test: hold the backend-serving probe and assert predastore readiness The backend-outage test declared the backend restored on one successful round trip through each gate. A gate can answer the request that arrives just after a thaw and then lose its blob peers under the load that follows, which is a backend that never recovered reported as one that did -- and that is what happened: the probe reported all three gates serving minutes before the guest was still taking BLOCK_IO_ERROR on every resume. The round trip has to hold across three rounds now, and the count restarts rather than decays when one fails, so a gate that serves and then stops is the failure the hold is looking for rather than noise it averages out. It also checks /readyz on each host and reports any failed check by name. The round trip only proves this workstation can write through a gate; it says nothing about which blob peers that gate can reach, and a gate one node short of the write floor serves a small object and fails a guest. A host that serves no admin listener is logged and skipped, since the port is optional configuration. * fix: scope predastore's admin port to the cluster plane in the host policy predastore's admin listener binds the host's cluster bind_addr and serves /healthz and /readyz unauthenticated. Readiness now names which blob peers a gate cannot reach, which is exactly the kind of topology detail 4432 is peer-scoped for, so 8660 joins it in the peer-scoped rule rather than the public one. * fix: keep resuming a paused guest at heartbeat rate after a long outage Past ten attempts the cont dropped to one per ten heartbeats, so a guest whose backend had come back could sit paused for another five minutes before anything retried it. Measured on dev-prod: the backend was proved serving through all three gates while the guest was still waiting on attempt 60, five minutes after attempt 50. The slowdown was there to avoid paying a failed I/O every thirty seconds on a backend still down. That is nearly free, and five minutes of downtime after recovery is not, so only the log line is throttled now. * test: fail the run when a key pair name resolves to more than one pair A create that lands twice leaves two key pairs with one name and different material. RunInstances resolves the name to one of them while the harness holds the private key of the other, and the only symptom is every guest in the run refusing the key at the SSH gate — five minutes of waiting per guest, four guests, and nothing anywhere naming the cause. Observed on dev-prod: two pairs created a millisecond apart, both named e2e-key-30de10ba. One DescribeKeyPairs after the create says so outright instead. * test: tolerate an already-released volume in the detach teardown Teardown runs DetachVolumeWait after a terminate that releases the volume itself, so the detach can arrive at a volume already on its way to available. That is the outcome it wanted, and treating the IncorrectState refusal as fatal failed a backend-outage run whose assertions had all passed — the guest came back, every acknowledged write verified, and the test still went red on the way out. VolumeInUse is still fatal: that one means the volume is attached somewhere it should not be. * test: carry the merged suites onto the reworked key-pair and account-scoped seams The diskperf suite arrived on dev calling EnsureKeyPair with the artifact dir this branch removed, and DeleteVolume's metadata lookup is account-scoped now, so the provider-failure test has to seed and delete under the same account. * refactor: make the fence outcome a type in one const block Four separate exported string constants with a paragraph of comment each, passed to RecordVolumeFence as a bare string. Group them, give them a FenceOutcome type, and take that type at both call sites so an arbitrary value can no longer reach the metric attribute. * refactor: move viperblockd's wire contract into a leaf vbwire package The daemon imported viperblockd for exactly two symbols -- the fenced volume subject and its event -- which put the storage service back on the daemon's dependency graph to name something it only listens for. vbwire holds the contract instead: the subject and event, the dirty bucket name and record shape, and the read-only listing an operator tool needs. It imports nothing from spinifex, so both the daemon and the admin CLI drop their edge to viperblockd entirely. cmd/service.go keeps its one, which is genuine: it constructs the service. * docs: drop a specific host name from the start-refusal comment Named a real dev-prod host in a comment and in test fixtures. Say what the name is instead. * ci: give storagefault a dedicated nightly cell The suite is eligible for both topologies and runs on the PR gate, but no nightly cell ran it: the permutation cells run a deliberately narrow subset and none of the dedicated suite cells named it. Cell 29, single-node, following eks/ecs/rds. It cannot join a permutation cell -- it SIGSTOPs a cluster-wide service, so every suite sharing that environment would report the outage as its own failure. * ci: run storagefault nightly on three nodes, not one The previous commit put its dedicated cell on a single-node runner. That was the wrong topology and the wrong justification: single-node is RS(1,0), where the shard floor cannot be broken and a cross-node takeover after a failed seal does not exist, so the two assertions worth having a dedicated cell for were the two it could not make. Cell 29 is now real-multi on ci-multi. It also corrects the claim that the PR gate already covers the multi-node case. The suite is eligible for both topologies, but e2e.yml's default suite list never names it, so it has never run there. A full pass is ~63 minutes on dev-prod bare metal, almost all of it deliberate waiting -- one test holds predastore SIGSTOPped for seven minutes -- which is too long to add to a gate that currently finishes well inside its budget. Nightly is where it belongs. Consequences: a per-cell timeout_minutes, defaulting to the previous 50, since 90 minutes of suite cannot run in a 50-minute job; the permutation steps gated on matrix.suite == '' so the dedicated cells do not also run them; and a Run Multi Suite E2E step that builds NODES_CSV from the tofu inventory and asserts the suite against E2E_SUITES_MULTI before running it. * fix: stop a startup KV race leaving vpcd serving keyless guests vpcd opens five KV buckets while NATS is still coming up, each attempt carrying a ~5s JetStream API deadline, and applied three different policies to the same failure: spinifex-vpc-enis was fatal, while instance-state, security-groups, subnets and vpcs each degraded silently for the lifetime of the process. A five-second blip during a cold start therefore decided how a node served metadata until someone restarted it. On dev-prod all three nodes hit this after a deploy. Two were saved by accident: their ENI bucket also timed out, so they exited 1 and systemd restarted them cleanly. The third's ENI bucket opened in time, so it never crashed and ran for hours with no instance record loader. With the local state file unreadable as well, every instance-backed IMDS leaf 404'd, cloud-init's crawl died on ami-id and fell back to DataSourceNone, and every guest launched on that node booted with no authorized_keys. kvstore.OpenWithRetry retries for a bounded window before any policy applies, logs the cause per attempt and the wait on a late success, wraps the last error with the bucket name, and returns at once on a cancelled context. All five opens use it. Failing to open the instance record space is now fatal, matching the ENI bucket: the dependency whose loss stops guests logging in should not be treated more leniently than the one whose loss stops ENI resolution. Also fixes the reason the fallback was load bearing at all. The local state file was written 0600 and vpcd reads it as a different user in the shared spinifex group, so the local-first path had never once worked and its mtime+size cache had never fired. It is 0640 now, set with an explicit Chmod so an inherited umask cannot undo it. Closes mulga-gtpzz, mulga-dhqxr. Still open: mulga-mm2oc, IMDS serving an unresolvable instance as "no key pair" with nothing logged. * fix: scope the ime-* firewall accept to IMDS and VPC DNS only The per-tap OVS demux delivers any guest packet addressed to 169.254.169.254 or .253 onto an ime-* host port regardless of L4 port, and iifname "ime-*" accept then admitted all of it. Every wildcard-bound host service — sshd, the AWS gateway, northstar, the console — answered guest packets on those addresses; only asymmetric reply routing kept the handshake from completing, and a guest could still drive northstar to recurse unthrottled. Scope the accept to IMDS (.254:80) and VPC DNS (.253:53), letting everything else fall through to policy drop. * docs: document the instance-to-host plane and how to protect it without the ISO The metadata/VPC-DNS link-local plane (169.254.169.254/.253) is served by host services and is invisible to a NIC-level or cloud firewall, since the traffic is intra-host on the OVS bridge. Explain that the ISO ships the scoping rule armed, that the binary installer ships the firewall off, and that a non-ISO install must enable it with setup.sh --firewall=on or firewall_enabled=true. Include verification from an instance and the host. * fix: withhold the IMDS meta-data listing when the instance does not resolve Every omission from the meta-data root is a positive claim about the instance — public-keys/ absent means "launched without a key pair", and cloud-init believes it, writes no authorized_keys and boots the guest keyless. So resolving the instance was best-effort for a listing whose whole meaning depends on it: a backend error or a miss shortened the listing instead of failing it, and nothing anywhere recorded that the answer was a guess. Withhold the listing with a 500 instead. cloud-init retries a 5xx and cannot retry a 200 that is merely wrong, so this converts a permanent silent failure into a transient loud one. servePublicKeys already had this policy for the leaf; the root is what contradicted it. A profile lookup that fails still serves the listing minus iam/, and now logs. The key pair is the field a guest cannot recover from losing, while a missing role surfaces through the credentials leaf, which reports the miss itself. Also stops the storagefault cross-node test failing on its own access path. It tolerates the pattern changing across a takeover, but readPattern needs sudo, and a volume opened from the backend checkpoint may predate cloud-init writing its sudoers drop-in — indistinguishable from an unreadable device at the dd call site. It now checks sudo first, and reads cloud-init's recorded datasource to tell a rolled-back root filesystem from IMDS never having answered, failing loudly on the latter rather than accepting it as storage loss. --------- Co-authored-by: brynmailer-mdc <bdm@mulgadc.com>	2026-09-03 19:45:33 -05:00
Julian Sommer	4b4542a4c5	test: retry the multinode same-SG cross-host ping and capture OVN state at first failure ... The cross-host ICMP probe fired once, with no convergence budget, the moment sshd first answered on the source guest. The default SG self-ingress rule matches on the northd-derived $<pg>_ip4 address set, which only reaches the remote chassis after northd regenerates it from port-group membership and ovn-controller installs the flows there, so a first-packet burst can be swallowed by the priority-900 default drop with no log line anywhere. Give the probe the same 45s/2s budget the single-node suite already uses, and snapshot NB/SB address sets, port groups, ACLs, port bindings and br-int flows from every node plus the source guest on the first failed burst rather than at teardown, so the state is captured while the drop is still observable. Emit attempt count and convergence time so a pass that needed most of the budget still surfaces as a propagation delay instead of a silent green.	2026-09-02 11:37:22 +10:00
Julian Sommer	ce9b6e5b41	fix: gate cluster-wide IPsec on every chassis being configured (#953)	2026-08-31 15:31:59 +10:00
Ben Duncan	42d87d7a5a	feat: EBS export tunables, plus blob-read fault isolation for ebsmetadata (#799) ... * feat(ebs): make nbdkit thread count and per-volume read cache configurable Both numbers that set a volume's export concurrency and memory were hard-coded: nbdkit ran on its own default of 16 threads per connection because no -t was ever passed, and the read cache was a 128 MiB literal in the mount path. Neither could be changed without a recompile, and neither was visible in the logs or the process argv. Add default_threads and cache_size_mb to [nodes.<node>.ebs]. Defaults are unchanged, so a config that does not set them behaves exactly as before. CacheSizeMB is a pointer so an explicit 0 (cache disabled) stays distinguishable from an unset key, matching how ViperblockConfig treats its own toggles. -efi volumes stay uncached regardless. Validation is a range check only. Both are operator tunables applied per nbdkit process, and there is one of those per volume, so the host cost is the value times the volume count -- left for the operator to weigh rather than second-guessed here. * feat(security): arm the firewall by install path and sync the clock at firstboot The host firewall armed itself from a code default, so a curl-to-bash install onto an existing Debian or Ubuntu machine applied a default-deny INPUT policy seconds after bootstrap. Anything the operator was already running that is not on the allow list stopped accepting connections, with nothing logged and no config key to set beforehand — no template writes firewall_enabled, so every install fell through to the default. Arming is now the install path's decision. setup.sh takes --firewall=on|off and records it in /etc/spinifex/firewall/mode, defaulting on under ISO_BUILD and off everywhere else: the ISO is an appliance on hardware we define, a package install is a guest on a machine that was already doing something. firewall_enabled becomes a three-state *bool so "unset" and "explicitly false" stay distinguishable. Flipping the default to false instead would have torn down a working policy on every existing node, because false is an instruction to disable rather than an absence. Resolution is config, then mode file, then on, so a node installed before any of this keeps what it has. The ISO had the opposite problem. setup.sh is the only place that enables spinifex-firewall.service and it returns before that line under ISO_BUILD, with no systemd in the chroot to talk to. Nothing else enabled it, so after a reboot the ruleset — runtime state — was gone, and ReconcileFirewall short-circuited on an unchanged peer file and never put it back. An ISO node was firewalled until its first reboot and open permanently after. firstboot now enables the unit, and the reconcile checks the table is loaded rather than treating an unchanged peer set as proof the policy is applied. Two policy changes: 443 is held for the nginx TLS edge so that rollout needs no per-node firewall change, and the SSH port is read from sshd_config and its Include drop-ins instead of being hardcoded to 22 — a host on Port 2222 was allowed exactly one port that was not the one it uses, and ct state established kept the operator's session alive until they disconnected. 53 stays open on every node, now with a comment saying so: northstar serves public authoritative DNS. Clock: firstboot steps and waits for sync before spx admin init mints credentials, since a certificate issued against a wrong RTC carries a NotBefore that outlives the correction. Bounded to 60s and non-fatal so an air-gapped install still completes, and once is enough because rtcsync writes the corrected time back. A chrony drop-in replaces Debian's makestep 1 3, which stops stepping after three updates and then slews at roughly 7 seconds a day — a node that burned those before the network came up never visibly converged. * fix(security): let a node join a cluster whose firewall is already armed Expanding a single-node ISO install into a cluster could not succeed. Two independent blockers, both from the same root: the peer set is derived from converged cluster state, but the ports needed to reach convergence are gated on that same set. The formation port is peer-scoped, and a node dialling in to join is not a peer yet, so the handshake was dropped rather than refused and the joiner retried in silence until it timed out. It cannot simply move to the public rule: outside formation the same port carries the daemon cluster manager, whose /health and /local/* routes report node topology, service inventory and running instances unauthenticated. So it is opened for the formation window only, by spx admin init, and closed again on every exit path. The reconcile then deadlocked. Holding out for the full chassis list is right on a node with no policy loaded, but on one armed for a smaller cluster the ports a joining node needs in order to register its chassis are exactly the ones the stale set blocks, so the list can never complete. Widen the peer set on the tunnel set already loaded, and report the partial state so the caller retries sooner than the steady-state interval. A converged cluster whose Southbound DB is briefly unreadable is still left untouched. * docs(install): explain the firewall around cluster formation An ISO-installed server is a cluster of one, and its firewall admits internal cluster traffic only from members it recognises. Forming a cluster out of such servers therefore needs the firewall off first: step 3 of the multi-node guide has each server dial node 1's OVN database on the raft ports, and those are peer-scoped like everything else on the cluster plane. Neither guide said so, and the failure is a silent drop that looks like a network fault. Both guides now walk the off / form / on sequence, and the multi-node guide gains a reference section covering which ports are public, how arming differs between the ISO and the binary installer, and how to verify membership landed. Re-enable the boot-time unit whenever the daemon arms the policy. Disabling the firewall to form a cluster also disables that unit, so re-arming used to look complete while leaving the node bare after its next reboot. * fix: make the ISO-to-cluster conversion path work end to end Forming a cluster from four ISO-imaged servers surfaced a run of bugs that share a starting state CI never exercises: the ISO delivers an initialized, armed, running node, while the conversion path was written against the binary installer's unconfigured one. - install-node.sh never disarmed the host firewall, so nodes 2 and 3 could not dial the first node on the OVN raft ports and formation could not proceed. It now owns the whole lifecycle: disarm, form, re-arm, then verify the loaded policy against the expected peer set. - spx admin init left the admin access key and secret empty on the preserve path. The pair cannot be read back, and the IAM store is rebuilt by formation, so the empty strings reached every joiner's bootstrap.json and crash-looped awsgw. It is reissued instead, and IAM seeding now skips an empty admin block rather than taking the gateway down on every restart. - node-reset.sh left /etc/default/ovn-central behind. Raft membership lives in that file, not in the databases the reset deletes, so ovn-central recreated the database as a cluster member dialling a peer that had itself been wiped. - node-reset.sh also destroyed the installed firewall policy, which nothing regenerates. set-peers then failed on every attempt and the node stayed open permanently and silently. The arming mode is read before the wipe and the stage regenerated after it. - spinifex-firewall-apply rejected every verb when the policy file was missing, including disable. Turning the firewall off is the recovery path, so that left a node that could be neither armed nor disarmed. - viperblock exited 0 on a fatal startup error, so Restart=on-failure never fired. During formation its JetStream bucket has no leader yet, so the EBS provider died for good and every volume request failed with no responders. predastore and qemunbd had the same bug. - spx admin images import created image directories at 0700, so the first sudo import locked group members out of a tree deliberately made group-writable. Three of these hid behind checks that cannot succeed: ovn-sbctl show against a clustered database, and the OVN northbound and southbound waits in setup-ovn.sh. All three failed to stderr and let the run continue, so a check that could never pass was indistinguishable from one that was merely slow. The USB install guide claimed the ISO leaves Spinifex unconfigured and told the operator to run spx admin init. firstboot has always done that itself, so the guide now describes what the ISO actually produces and how to start using it. * fix: let spread placement launch across every node A spread placement group reserved four distinct nodes correctly and then failed the launch with ServerInternal, having started only two or three of them. The scheduler was never at fault; two faults downstream stopped the instances following the reservation. vpcd subscribes to vpc.add-nat with QueueSubscribe, which delivers one message at a time, and the handler holds its reply across the OVN flows barrier. The 45s caller deadline was sized against a single 30s barrier, which is right for one instance and wrong for the N a spread launch issues at once — the third request waits behind two full barriers and times out. That one subscription now overlaps its handlers under a bounded semaphore. Each barrier still syncs after its own write, so the guarantee is unchanged while N barriers collapse into roughly one. With the requests overlapping, every barrier still ran to its alarm. install-node.sh computed OVN_REMOTE after building the database nodes and gave it only to compute nodes, so the database nodes were configured without --ovn-remote. setup-ovn.sh writes ovn-northd's NB/SB connections from that list and skips it on the localhost default, and it already documents what follows: a northd dialling only its local member stops advancing nb_cfg once raft leadership moves, wedging ovn-nbctl --wait=hv. The list is knowable before any node is touched, so it is computed first and passed to the database nodes too. Nothing reported an OVN fault while this was happening. Every chassis was registered, ovn-controller and IPsec were up, and the raft cluster was healthy. The only signal was the barrier logging that it had overrun and was continuing without confirmation — which reads like slowness and was a permanent wedge. * feat: sweep block sizes in the disk benchmark and add an iperf3 network benchmark * fix: use iperf 2 with CSV output and report why a remote start failed * fix: stop pkill matching its own shell and verify the iperf server is listening * ci: run the predastore throughput benchmark at the end of the nightly * ci: boot the ISO in the nightly, single-node and three-node * ci: use the existing CI node password secret in the ISO cells * ci: report free space before the benchmark instead of failing inside warp * ci: keep the benchmarks and node disks off tmpfs * ci: export the work directories from RUNNER_TEMP rather than job env * ci: run the ISO and benchmark workloads from the nvme scratch area * perf: run guest NICs on vhost-net with multiqueue Guest-to-guest VPC throughput measured 1.68 Gbit/s single-stream against 21.8 Gbit/s host-to-host on the same underlay, and did not improve with more streams. QEMU's -netdev tap defaults vhost to off, so the whole datapath ran in the main loop thread in userspace: that thread sat at 59-65% CPU while all four vCPU threads idled at ~4%. Every tap netdev now carries vhost=on, and VPC ENIs additionally run multiqueue at min(vCPUs, 8) queue pairs. Multiqueue needs all three of the tap, the netdev and the device to agree, so TapNetDev and NetDevice derive the netdev options and the 2N+2 MSI-X vector count from one queue figure rather than leaving callers to keep them in step. The QMP hot-plug path takes the same options as cold boot, with a test comparing it against the rendered cold-boot strings so the two cannot drift. MMIO machines stay single-queue: they have no MSI-X. * ci: check out the sibling repos the go workspace needs in the ISO cells * ci: skip cell-21 workload cleanup when the workload never ran * ci: size the cell-21 node at 8 GiB and keep its serial log * ci: keep the cell-21 node diagnostics alongside the serial log * fix: let the daemon open /dev/vhost-net and delete multi_queue taps Enabling vhost=on broke instance launch two ways. DevicePolicy=closed gates the cgroup device list before capabilities are consulted, so QEMU's open of /dev/vhost-net returned EPERM despite CAP_DAC_OVERRIDE; add the device to the allow list and bump the unit version. Separately, `ip tuntap del` re-opens the device with TUNSETIFF and fails with EINVAL on a multi_queue tap, leaking it on every teardown; route deletion through `ip link del`, which is flag-agnostic. * perf: default guest NICs to a single queue Multiqueue measured a net loss. Spreading one flow across queue pairs reorders packets and TCP reads that as loss: a 4-vCPU guest fell from 1.76 to 1.50 Gbit/s, with cwnd collapsing on spurious retransmits. The host overlay funnels a node pair onto one softirq core, so the extra queues have no headroom to win back what the reordering costs. Keep the plumbing and its tests behind MultiqueueNICs so re-enabling is one line once that funnel is fixed. vhost-net stays on unconditionally; it moves the datapath off the QEMU main loop, measured at 60% of a core under load against 3% with it. * fix: raise the UDP receive buffer for predastore's QUIC transport Predastore carries blob, meta and raft over QUIC and quic-go asks for a 7MiB receive buffer. Left at the kernel default of 208KiB it silently dropped datagrams under load: the four-node cluster had shed 80k-125k each, which failed shard writes, put raft into repeated elections, and ultimately handed guests an I/O error that cost two of them their root filesystems. Adds a sysctl stage writing /etc/sysctl.d/99-spinifex-net.conf, so the setting lands in the ISO image and on every node push rather than living in a runbook. * ci: give the cell-21 workload an identity the node accepts * feat: tie the guest MTU and NIC multiqueue to ipsec_enabled ESP costs 34 bytes on top of geneve's 58, so a cluster running the overlay in plaintext can advertise 1442 instead of 1408 — worth roughly 10% of guest throughput. Derive it from network.ipsec_enabled rather than hardcoding the conservative figure. Multiqueue moves with the same flag, because its sign depends on it. Behind IPsec it costs ~12%: ESP has no L4 ports, so RSS funnels a node pair onto one core and extra queues only reorder packets, which TCP reads as loss. Without IPsec the sending host's single vhost thread saturates first and queues are worth +54%. IPsec on with multiqueue on is the worst of the four combinations, so it stops being a standalone knob and becomes vm.Deps.MultiqueueNICs. The reconciler only ever created a subnet's DHCPOptions row and never revisited it, so the create-time MTU stuck for the life of the subnet and this change would have applied to new subnets only. Worse in the other direction: turning IPsec back on would have left subnets advertising 1442 over ESP, which drops large segments with no ICMP to recover from. applySubnets now diffs and updates in place. reconcile.Config takes IPSecDisabled rather than IPSecEnabled so the zero value is the conservative MTU — a caller that forgets it is merely slower. * docs: add an install-time performance tuning guide Measured on the four-node cluster rather than inferred: the UDP receive buffer sysctl, what IPsec actually costs and why, the multiqueue sign flip, the overlay MTU budget, and the jumbo-frame check that must precede enabling it. A starter doc — several sections end in an open question rather than a recommendation. * ci: keep the cell-21 node alive for the workload step * ci: collect the cell-21 qemu log and widen its timeout * feat: derive the guest MTU from the fabric, and deepen the virtio NIC config The overlay overhead is passed on to the guest rather than hidden from it. Hiding it means running the fabric far enough above 1500 that a standard guest MTU fits in the headroom, which is what the large public clouds do — but they own the switches. On a fabric we do not control, the honest figure is the one the path can carry, and a jumbo underlay should reach the guest: 9000 gives 8942, or 8908 with IPsec. network.underlay_mtu is the lever; SubnetMTU spends it. Configured rather than probed, because a node's own NIC MTU says nothing about the switch between it and its peers. host_mtu sets VIRTIO_NET_F_MTU so a guest that never takes a DHCP lease — statically addressed, or IPv6-only — still learns the ceiling. rx/tx_queue_size raise the virtio ring from QEMU's default 256. That default is a packet-rate ceiling rather than a memory saving: a vCPU preempted for a fraction of a millisecond fills all 256 slots and the overflow is dropped. ethtool is the only way to read NIC offload, ring and channel state, so a node without it cannot be diagnosed for throughput at all. * ci: give the cell-21 workload its own guest key * fix: set the guest MTU on the tap, not just in the guest The tap was created and brought up without an MTU, so it stayed at the kernel default of 1500 while DHCP and host_mtu told the guest something else. Harmless at 1442; it silently blackholed every large frame on a jumbo underlay, with no ICMP back because the drop is at the first hop. TapSpec.MTU comes from the same guest overlay MTU the DHCP option and host_mtu already use, and is applied in the bring-up call so a size the fabric cannot carry fails the launch instead of degrading quietly. * fix: stop setting tx_queue_size, which QEMU caps at 256 for tap QEMU only honours a tx_queue_size above 256 for vhost-user and vhost-vdpa backends. On a tap it rejects the value outright rather than clamping it, so every launch died with "must be a power of 2 between 256 and 256" and the instance went straight to terminated. rx_queue_size is unaffected and stays at 1024. A test pins the absence of tx_queue_size across machine types and queue counts. * docs: lead the tuning guide with the overlay segmentation finding The guide's headline was a 5.4x that came almost entirely from turning IPsec off, and it treated MTU and the fabric as the main levers. The host reaches 92.6 Gbit/s at MTU 1500 on the same NIC, so the fabric is not the constraint and the encrypted default is not the thing to tune away from. Adds the host-versus-guest comparison up front, documents network.underlay_mtu, and makes the missing segmentation offload the first open question. * ci: stop deselected matrix cells failing in teardown * docs: quantify what disabling overlay IPsec is worth The guide described the trade without the numbers an operator needs to make it. Adds the measured 4.9x, notes that the MTU and multiqueue changes follow ipsec_enabled rather than being separate knobs, and records two transition hazards: a stale wide MTU on running guests, and RunInstances failing while the OVN flows barrier catches up with the new SAs. * ci: collect and retain the e2e benchmark and cluster logs Cells 21 and 22 both copied two paths that only ever exist on the nodes, so the benchmark JSON was never uploaded and cell-22 collected nothing at all. Both now gather what they actually produce, and keep it for 90 days. * fix: quieten spx by installing an explicit CLI logger default spx never set a process logger, so it inherited whatever a linked package left behind — until now a JSON handler at Info on stdout, interleaving library log records with command output and breaking piped use. Execute() claims the default before any command runs: text on stderr, LevelError, so commands report failures through their own stderr writes and libraries stay quiet. --verbose/-v raises to info and SPX_LOG_LEVEL sets an arbitrary level, the flag winning over the environment. Service subcommands still call initTelemetry, which replaces the handler outright. No root --debug flag: viperblock, qemunbd, nats and awsgw each define their own --debug with a service-specific meaning, and a root persistent flag would shadow inconsistently depending on the command. The guard test samples the handler at package-init time rather than inside the test, because tests in this package start services that install their own logger legitimately. * feat: private /admin/CreateAccount for self-service tenant signup Add POST /admin/<Method> to the AWS gateway: SigV4-signed, JSON in and JSON out, callable only by an IAM user in the super-admin account holding spinifex:<Method>. CreateAccount is the one method, so a signup form can provision a tenant without an operator shell. No new service, port, unit or certificate — it mounts inside the gateway's existing authenticated group and inherits its auth, throttling and tracing. Provisioning is resumable rather than transactional, which fits stores that have no transaction. Two CAS creates carry the correctness: the client token admits one in-flight attempt and makes a lost response replayable for 24 hours, and the name reservation makes duplicate account names impossible. Every step after the account exists is create-if-absent, so a retry with the same token finishes the job instead of failing. The CLI and the endpoint share one provisioning helper, which closes a gap on both at once: CreateAccount never checked names, so running `spx admin account create` twice produced two accounts that differed only by ID. The response waits for the account's default VPC over a new ec2.EnsureDefaultVpc request/reply, rather than trusting the existing fire-and-forget iam.account.created publish — credentials for an account that cannot launch anything are worse than a slower call. [signup] max_accounts bounds how many accounts may exist; absent means uncapped, which is what every cluster does today. * fix: stop the awsgw template hardcoding one deployment's console URL console_url shipped as https://console.spx3.com, so every fresh install — including an on-premise cluster that will never run signup — was configured to point new accounts at our console. It is the only deployment-specific domain in any template. It now ships empty, and the gateway warns when it provisions an account without one, since a blank sign-in link is otherwise visible only to the customer who receives it. Extract the remote client from the command body while here. The signing, request shape and error decoding are the part a Cloudflare Worker mirrors, and as a function they are testable against a TLS test server rather than only by hand against a live cluster. * refactor: drop console_url from signup and default max_accounts to 128 The gateway never read console_url — it passed the value straight through to the response, and the idempotency record froze it for 24 hours so a corrected typo still replayed stale. The client that made the request already knows where it wants to send the new tenant. max_accounts is now a pointer in the parsed config so an absent [signup] section takes the 128 default while an explicit 0 still means uncapped. * feat: run the predastore fault-injection harness as cell-24 Cell-23 asks how fast predastore is when every node is healthy; cell-24 asks whether it survives one that is not. It runs after cell-23 rather than beside it: both want a quiet single runner, and a benchmark sharing a machine with a fault injection run measures the wrong thing. The port offset keeps the two clear of each other even so, and the job clears stale nodes on entry and exit, since a run killed before its cleanup trap leaves one holding ports that fail the next run somewhere unrelated. * ci: keep the throughput benchmark's output with its results The run log only existed in the job log, so the artifact it uploaded could not be read without going back to the run that produced it. * ci: keep the fault-injection run log with its results events.txt carries the timeline and the FAIL line, but not the build output or anything Warp wrote before the harness took over, so a failed cell could only be read from the job log it was printed into. * fix: serve the cluster region to the console instead of hardcoding it The console hardcoded ap-southeast-2 as its SigV4 region in seven files, so on any other cluster the credential scope was wrong and the gateway rejected every signed request. The SPA is embedded in the binary, so a build-time value would pin one binary to one region. The UI server now serves GET /api/config with the region, no-store so a proxy fronting several clusters cannot serve one the other's region, and the SPA loads it once before render. The region comes from the serving node's config because awsgw pins signature verification to that same field. Reading the cluster-wide [aws] region instead would let a cluster whose two keys disagree pass review and still fail to authenticate. Also relabels the login card as Spinifex credentials, since the fields are not AWS ones, and hides the CA install banner unless the console is reached directly: behind a proxy with a publicly trusted certificate there is nothing for the cluster CA to fix. * fix: say once when the host firewall will not be maintained Both quiet outcomes of the reconcile — no cluster config, and the policy switched off — return nil without logging, so a node that never arms is indistinguishable in the journal from one that armed on its first pass. * test: arm the host firewall before asserting on its peer set Only the ISO install arms the policy; every other path writes off to the mode file, so the daemon disables the firewall and never writes a peer set. The test read that file as its baseline, which it could never have. It now switches the mode on for its own duration and puts it back, waits for the peer set the way install-node.sh does, and on failure reports the mode, the node count and what the daemon said rather than just the missing file. * test: move the console region test to the external cmd package It was in package cmd, which test-package-check rejects, so preflight failed on the branch. The hook follows the pattern already in export_test.go. * test: make the reboot EIP diagnostic capable of naming the drop The two ofproto/trace probes injected packets with no TTL, so OVN's router killed them at lr_in_ip_input and both traces ended early for a reason that had nothing to do with the failure under investigation. A keyword filter then stripped the table-match lines, hiding where each trace stopped. Traces now carry a TTL and are kept whole, the real datapath megaflows for the guest reply are dumped alongside them, and any drop matching the guest source is resolved from its OpenFlow cookie back to the Southbound logical flow that installed it. * fix: exempt an EIP instance from the subnet drop gate with allow, not reroute A subnet with no route of its own carries a priority-1100 drop policy, and an instance with an external IP is lifted above it by a priority-1200 policy. That policy rerouted to the pool gateway, which in distributed NAT is outside every logical router port subnet, because the gateway LRP is the link-local /30. The static default route survives that by naming an output port; a routing policy has no such field, so ovn-northd discarded all three of them and the drop gate below swallowed everything the instances sent. The effect was that every external IP answered inbound traffic and none of it came back: the SYN reached the guest, the guest replied, and the reply died in the router's policy stage. Guest-initiated egress was lost the same way. The policy only ever needed to lift the gate. The router's default route already points at the gateway, so allow leaves the routing decision intact and gives northd nothing to reject. * test: match the log the firewall quorum gate actually emits The assertion waited for "chassis encap addresses; not writing a partial peer set", which no code has ever logged. The reconcile reports a short chassis list as "OVN reports N of M chassis encap addresses" and the maintain loop carries that into its retry warning, so the wait could only ever time out. Arming the node in the previous commit got the test as far as this assertion for the first time, which is why it surfaced now. The failure also prints the journal tail and the same inputs the earlier wait reports. * fix: stop one unreadable volume document failing every volume listing A single volume document whose shards no longer reconstruct made DescribeVolumes, DescribeVolumeStatus and DescribeVolumesModifications fail for every account, because the listing returned the first fetch error rather than skipping the document. ListVolumes now skips a document it cannot read or decode and logs the skip, mirroring ListAMIs. Callers whose answer would be wrong rather than merely partial take ListVolumesStrict: the orphan scan and the snapshot in-use check both gate destructive work on a complete list. The tolerance covers fetch failures as well as decode failures. An object whose shards no longer join is as unusable as one whose bytes will not parse, and it was the fetch path that took the cluster down.	2026-08-15 14:22:49 -05:00
Josh Siviour	f7a2d85691	fix(e2e): stop flagging a Cluster bind to a single-homed node's own address (#789) ... checkSocketScope compared a socket's bind address to node.Addr and called it the node's WAN address, but node.Addr is the cluster peer address (harness.Node's own doc). On CI's single-homed tofu nodes this is the node's only address, so every Cluster-scope service failed no matter what it bound. Compare against a positively-known SPINIFEX_WAN_IP instead, distinct from the peer address, and record via harness.Detail when no such address is available so the skipped check is visible rather than silently passing.	2026-08-13 15:19:23 +10:00
Josh Siviour	b93f6c258a	fix(e2e): resolve the listener inventory without a Go toolchain on the target node (#787) ... * fix(e2e): resolve the listener inventory without a Go toolchain on the target node tests/e2e/multinode/listener_invariant_test.go resolved docs/security/network-connections/README.md at runtime via `go env GOMOD`, which works under `go test` but not inside a compiled .test binary shipped to a node with no Go toolchain. Embed the doc in a new docs/security/network-connections package instead, so the content is compiled into the binary and every caller parses bytes rather than a resolved path. RepoRoot and DocPath are now unused and removed. * docs: trim the embed package comment to the three-line standard	2026-08-13 14:18:02 +10:00
Josh Siviour	57bfbc20df	test: listener invariant static guard + e2e runtime check (#782) ... * test: add the listener invariant static guard and e2e runtime check Adds both halves the plan called for: a static test in spinifex/network/invariants that scans install scripts and config templates for a bare OVSDB ptcp remote, a wildcard bind on a Cluster/Encap port, or a bind port absent from the inventory, and an e2e test in tests/e2e/multinode that asserts the same on live ss -tulnp output. Both read docs/security/network-connections/README.md "## 1. Inbound Listeners" as their fixture through a shared, non-test parser package (spinifex/network/listenerinventory), so a wildcard exception has to be declared in the doc's own prose rather than hardcoded in either test. * fix: make the listener wildcard exception fail closed on an explicit marker The prior heuristic granted an exception for any unnegated "wildcard" or "0.0.0.0" mention within 40 characters of the port's row. That fails open: a future row phrased as "avoids the wildcard" or with a negation further upstream than the window would silently exempt itself. Replace it with a single exact literal phrase, "binds the wildcard by design", that a row must contain to grant the exception — no other wording counts, so an unrecognized mention denies rather than grants. Update the two rows that need it (5300, 500/4500) and document the marker's role in the inventory's own prose. * docs: record that the Geneve tunnel socket binds the wildcard by design	2026-08-13 12:29:26 +10:00
Josh Siviour	91d22d9f12	feat(security): add an nftables host firewall scoping cluster ports to peers (#769) ... * feat(security): add an nftables host firewall scoping cluster ports to peers Nodes shipped no host firewall, so every cluster listener — OVN NB/SB, the formation port, NATS, predastore — was reachable from anything that could route to the node. On a single-NIC node the planes collapse onto the public address, which made that the default rather than the exception. setup.sh installs a policy and an apply helper but does not enable them: the peer set is unknown until the cluster's planes resolve, and a drop policy with an empty set would cut the first node that applied it out of its own formation. install-node.sh writes the peers and enables the unit after formation. The policy owns only table inet spinifex_filter and replaces it in one transaction. vpcd writes MASQUERADE and per-EIP FORWARD rules into the ip nat/filter tables and reinstalls them only on start, so a whole-ruleset flush — what Debian's nftables.service does on both start and stop — would break elastic IPs silently. INPUT only, for the same reason: a forward-hook drop here could not be rescued by an accept in the other table. Guest traffic keeps working: IMDS and VPC DNS terminate on ime-* ports in the host netns, and every guest agent reaches its control plane over the AWS gateway rather than NATS, so no cluster port needs to face the guests. * feat: reconcile the host firewall peer set from the daemon The peer set was written by install-node.sh and by cluster-bootstrap.yml. That missed two of the four formation paths (bm-bootstrap.sh and the ISO firstboot), duplicated the same logic in each, and went stale as soon as a node was added or replaced. host.ReconcileFirewall now owns it: every formation path starts the daemon, and it re-runs on each startup. A node's own config names its peers by their wan address only, so a set built from config alone drops every peer's NATS, OVSDB and formation traffic. The lan plane comes from the generated NATS cluster routes and the OVN remote lists, and the vpc plane from the Southbound Encap table; an empty encap answer is an error rather than a narrower set. ProtectSystem=full applies to the sudo child too, so the root helper could not write /etc/spinifex/firewall either — it is now in the daemon's ReadWritePaths, optional so a node installed before the firewall stage still starts. The apply helper gains set-peers and disable verbs, re-validates every address as a dotted quad with in-range octets, and rolls the peer file back if the ruleset will not load. * fix: retry the host firewall reconcile until every chassis registers The peer set is only safe to write once OVN reports an encap address for every node. On a fresh bootstrap the daemon routinely wins the race against ovn-controller, so a single startup attempt saw a partial chassis list and either wrote a set that dropped Geneve from the nodes missing from it, or failed outright and left the node with no policy until a restart. Gate on the full chassis count and schedule the reconcile instead of running it once: back off from 15s to 5m while it fails, then re-run every 5m so a node added or replaced after formation is picked up without a daemon restart. An unchanged peer set short-circuits before the helper runs. * test: cover the firewall chassis quorum gate in the multinode e2e suite A partial chassis list is the normal state early in a bootstrap, and writing a peer set from one produces a firewall that drops Geneve from every node missing from it. The gate that refuses that write had no live coverage: a 5-node bootstrap converges too fast for any daemon to observe a partial list, so the verification run only ever exercised the OVN-unreachable path. Reach the branch by inflating the expected count instead of breaking OVN: a phantom node in one node's config makes the cluster one chassis short, with nothing in the Southbound DB disturbed. Asserts the gate logs, that no peer file is written while it holds, that the loaded ruleset survives, and that the node recovers to a byte-identical peer set once the phantom is removed.	2026-08-12 22:37:27 +10:00
Josh Siviour	5fc6a137ec	fix(e2e): fail TestMultinodeIPSec when a cluster that requested IPsec has none (#612) ... * fix(e2e): fail TestMultinodeIPSec when a cluster that requested IPsec has none An empty ipsec_encapsulation was treated as "bootstrapped without IPsec" and skipped. On ubuntu-24.04 openvswitch-ipsec.service is inactive at daemon start, so the enable guard correctly refuses to flip the flag and the cluster runs plaintext intra-AZ Geneve — but the test then skipped and the cell reported green, masking the gap on 2 of 3 multinode cells. Cross-check network.ipsec_enabled from the node config: an explicit false still skips, anything else fails with the plaintext condition named. * fix(e2e): read only the ipsec_enabled value, not the whole config line Detail() rendered "ipsec_enabled=ipsec_enabled = true" because the raw grep line was logged as the value. Take the first token after "=" so a trailing comment cannot be read as the value either, and compare exactly rather than by substring.	2026-07-30 23:28:42 +10:00
Julian Sommer	7e356b5b9b	test: cover DescribeInstanceStatus and the SDK instance-status-ok waiter (#588)	2026-07-27 13:46:16 +10:00
Josh Siviour	cae76ba1b6	test: complete the e2e live-vs-mock rebalance (epic mulga-8oyg3) (#566) ... * test(volume): assert chunk GC stays wired on in CreateVolume GCEnabled was zero-valued at every viperblock construction site for a long time and no test noticed. It is not part of viperblock.VBState, so it never round-trips through a saved config.json and cannot be observed by reading volume state back. Extract the viperblock.VB assembly out of CreateVolume into buildVBConfig so the GC wiring is assertable directly against the returned config, without a live viperblock or S3 backend. The config is returned by pointer because viperblock.VB carries atomic.Uint64 fields that vet's copylocks check rightly refuses to see copied. * test(e2e): snapshot backend byte accounting before volume purge DeleteVolume purges a volume's entire backend prefix, including any chunks a leak left behind, before anything measures it. That is the same masking that hid the storage-growth leak in production: a passing E2E cannot fail on backend growth when the evidence is deleted first. Snapshot the volume's backend chunk footprint in EnsureVolume's cleanup closure, immediately before DeleteVolume, so every volume the shared teardown path purges is measured rather than silently discarded. The snapshot is always taken and logged; the hard ratio check additionally runs for callers that registered a guest-written-bytes baseline, since most callers never attach or write to the volume they create and so have no baseline to compare against. The tolerance is the RS(2,1) erasure floor widened for per-chunk header and tail-chunk overhead. It is sized to distinguish unbounded growth from a footprint settling at the floor, not to catch bounded residual garbage. The check is non-fatal inside the cleanup callback: t.Fatalf's runtime.Goexit would abort the DeleteVolume call queued behind it and turn a reported leak into a real one. * test(e2e): add memory-constrained concurrent-write durability regression The existing durability tests write one 4MiB sentinel, sequentially, on a comfortably sized guest, which is the exact inverse of the known-open data-loss bug: acked writes read back zero-filled after a cold reread when a guest is memory-starved during concurrent writeback. No happy-path test can reach it, because the fault lives in the intersection of memory pressure, write concurrency and cache-cold reread. Add a suite that encodes those three conditions together: a guest discovered at or below the working set, N parallel large writers launched as background jobs in a single command so they genuinely overlap, swap disabled, then sync and drop_caches before rereading. Assertions are per stream rather than aggregate so a failure names the losing writer instead of leaving whoever reads it to re-derive which of N writers lost data from one combined digest. GuestExec gains a timeout-parameterised variant. Its fixed two-minute default is sized for sub-second payloads, and a multi-gigabyte write to a network-backed volume would exceed it, so the suite would have failed by timeout while looking like it had caught the corruption. The budget scales with the working set, and a deadline is reported distinctly from a data mismatch so the two red states are never confused. The suite is dispatchable but deliberately absent from the default SUITES: the bug it reproduces is still open, so it is expected to fail and must not sit in a merge-gating path. It is registered in the workflow allow-list so it stays discoverable rather than being buried behind a skip. Also record on the live IAM instance-profile test why its assertions were not moved down to the integration tier, so the trace is not repeated. * test(e2e): drop backend byte accounting, assert a node-wide delta instead The accounting helper listed the internal predastore bucket to attribute bytes per volume. That listing is refused unconditionally: predastore authorises bucket access by same-account ownership, and the bucket belongs to the service account, so a test caller gets 403 no matter what it holds. Granting the test a service credential would invert the trust boundary to buy a measurement, which is the wrong trade. Replace it with what the S3 surface actually permits: sample node-wide allocated bytes either side of the write and bound the delta against the bytes the guest wrote. Coarser, but it measures something real rather than depending on a call that cannot succeed. The tolerance moves into the suite that uses it. It encodes RS(2,1)'s 1.5x expansion floor plus headroom; a cluster running the recommended RS(3,2) has a 1.667x floor and only ~12% of margin left, so the constant needs revisiting if the encoding changes. * revert: drop memdurability e2e suite The suite asserted the guest read back what it wrote after a memory-constrained concurrent write, but the assertion could not fail for the reason it claimed: it compared the guest's own read against the guest's own write with the page cache still warm, so it passed whether or not the data reached the backend. A test that is green on a build with a known-open durability bug is worse than no test — it advertises coverage that does not exist. Removes the suite and its Makefile/workflow wiring, plus DiscoverInstanceTypeAtLeastMemory, which had no other caller. GuestExecTimeout stays: GuestExec now delegates to it, and the old two-minute hard cap it replaced was a latent trap for any long guest command. * test(e2e): add partial-block concurrent-write durability suite Replaces the removed memdurability suite, which passed on a build that still carried the bug it claimed to test. The engine reaches its read-modify-write path only for a partial block; a fully covered block takes a full-overwrite branch that reads nothing and can lose nothing. The old workload wrote through ext4 with 1 MiB buffers, which the filesystem coalesced into aligned full-block requests: block-layer tracing recorded zero unaligned and zero sub-4096 requests, so the path under test never ran, and writers 4 MiB apart could not collide on a block even in principle. This suite writes the shape the defect needs. Two writers issue concurrent O_DIRECT writes into disjoint halves of the same 4096-byte blocks on a raw, unformatted device, stamping every half with the round number, then drop the page cache and reread with O_DIRECT asserting every half carries the final round. One half reading back at an earlier round is the signature of the lost update. The old suite also could not fail even had it reached the path: its write phase ended in 'wait; sync; sha256sum' and reported sha256sum's exit status, while bare wait returns 0 unconditionally, so writer errors were invisible and a backend ENOSPC presented as a pass. Here each writer's exit status and exact byte count are asserted before any durability claim is made. Working set is 8 MiB rather than 2048 MiB, and the volume is deleted on teardown; the predecessor left roughly 3 GiB of backend per run and twice drove a node under the nearfull watermark. Dispatchable but not in the default suite list until a green run and a demonstrated fail-first against a pre-fix engine exist. * test(e2e): let the fixture own the working-set volume delete The suite deleted the volume in its own cleanup and the fixture deleted it again on close, so every run ended with an InvalidVolume.NotFound from the second attempt. Detach stays — DeleteVolume against a still attached volume is what leaks volumes on teardown — and the delete is left to the fixture so the volume is released exactly once. * test(e2e): gate merges on the partial-block durability suite Verified on one node with the same test binary and workload, varying only the engine: 1758 of 8192 halves reverted a generation against the pre-fix build, 0 of 8192 against the fixed one. That is the fail-first evidence the suite needed before it could be trusted to gate anything. Adding it to the default suite list rather than leaving it dispatchable is the point. Its predecessor sat outside every path that runs, which is how it survived long enough to be discovered green on a build that still lost data. * test(integration): cover the ECR data plane in the front gate The live ecr/ suite drove docker, crane and skopeo against a real registry, yet none of it boots a guest: it needs a registry, an object store and an HTTP client. The in-process gateway already serves the OCI Distribution v2 surface on an ephemeral port with a memory-backed object store, so these run here in 0.18s instead of behind full provisioning. Covers the three claims the live tests made beyond authorization, which is already down here: a push/pull round trip asserting blobs and manifests read back byte-identical, refusal to auto-create a repository on push, and retagging an existing manifest without disturbing the original tag. Content is compared rather than status codes alone. A registry that stores a blob under the wrong digest, truncates it, or serves a different one answers 200 to every request in the sequence. The capacity/load leg deliberately stays live: it exists to put real parallel pressure on a real predastore, and a memory-backed object store cannot make that claim. * fix(e2e): make volume teardown release the volume before deleting it Fixture cleanups drain LIFO, so a suite that creates its instance before its volume tears the volume down first, while it is still attached. DeleteVolume answered VolumeInUse, nothing retried, and the volume was stranded on the node. Reordering the chain cannot fix this — registration order has no way to express the instance/volume dependency. Instead teardown is now self-sufficient for any creation order: it force-detaches, waits for the volume to reach available, and only then deletes. Instance teardown waits for terminated for the same reason, since TerminateInstances returns as soon as the request is accepted. Two things kept the leak invisible. A failing cleanup was logged and forgotten, so a run stayed green with resources left behind; failures are now reported as leaks, failing the bound test or surfacing through Close() for TestMain to fail the run. And a panic in one cleanup aborted the whole chain, stranding every resource behind it; each callback is now isolated. RegisterVolumeTeardown and the fixture had grown separate teardown paths, only one of which detached. Both now share it.	2026-07-22 17:02:20 +10:00
Josh Siviour	48749576c3	test: rebalance E2E into in-process integration tier + shared-guest live scenarios (#556) ... * test: add in-process integration test tier Roughly half of the behavioural tests under tests/e2e/ assert control-plane API shapes, authz and error codes that need no live environment, yet they can only run against a tofu-provisioned environment on a scarce self-hosted runner. This adds the tier they can move down to. The real gateway router runs in-process: SetupRoutes() is a pure function of GatewayConfig, so it serves from an httptest server over embedded NATS JetStream with real IAM/STS services and genuine SigV4 verification. Only the daemon side of the wire is faked -- the ec2.*/ebs.* NATS subjects a live node would answer are stubbed per test. Nothing is provisioned and no fixed ports are bound, so the tier runs concurrently on a shared build host and is safe as a CI front gate ahead of the live suites. It is build-tagged and kept out of preflight, which stays unit-only. * test(integration): add IAM/STS client accessors and port the zero-stub authz tests Adds IAMClient/STSClient plus ClientsWithCreds/ClientsWithSessionCreds so a test can act as a principal other than the seeded root user, and stubs the node-discovery subject in StartGateway so call paths that trigger discovery resolve immediately instead of riding out the fan-out timeout. Ports the four enforcement tests that guard on ec2:DescribeRegions, a gateway-local action, so they need no daemon-side stub at all. * test(integration): port remaining IAM/STS authz tests from e2e Ports TestIAMUserCRUD, TestIAMAccessKeyLifecycle, TestIAMUserAuthentication, TestIAMPolicyCRUD, TestIAMPolicyAttachmentEnforcement, TestIAMPolicyLifecycle, TestIAMGroupsLifecycle, TestIAMRolesAndProfiles and TestSTSAssumeRoleAndGetCallerIdentity from tests/e2e/iam/ into the in-process integration tier. Each test now self-bootstraps its own users/policies/roles instead of relying on shared suite-level state, since every integration test gets its own gateway and embedded NATS. TestIAMRolesAndProfiles stubs ec2.IamProfileAssociation.describe because DeleteInstanceProfile always counts live associations over NATS regardless of which guard actually fires. TestIAMCleanup is not ported: it only swept a shared long-lived E2E environment, and every ordering/dependency assertion it made is already covered by TestIAMPolicyLifecycle, TestIAMGroupsLifecycle and TestIAMRolesAndProfiles's own delete-conflict guards. * test(e2e): remove IAM/STS authz tests migrated to the integration tier All 13 signed-request authz tests plus TestIAMCleanup now have faithful, mutation-proven equivalents in tests/integration/ — the gateway dispatches IAM/STS actions directly with no daemon hop, so the in-process tier is a faithful and far cheaper substitute. The iam/ e2e suite now runs only TestIAMInstanceProfileAssociation and TestIMDS. Drop the now fully-dead iam_helpers_test.go alongside the six deleted test files, and remove harness.IAMGroupARN/IAMDeleteGroupBestEffort, which had no remaining caller anywhere in the repo once iam_groups_test.go was gone. * feat(integration): daemon-lite harness for key/tags/routetable/vpc Adds DaemonLite, an in-process stand-in for a live spinifex daemon that subscribes the real key, tags, route table, and VPC/subnet/SG/IGW service implementations directly to their NATS subjects, backed by memory and embedded-JetStream stores. Ports TestKeyPairs, TestTagManagement, TestRouteTableValidation, and TestReplaceRouteConvergence from tests/e2e/single so the integration tier drives genuine daemon-side business logic instead of StubSubject canned replies. vpcd (OVN) is out of scope, so its "vpc.create-sg"/"vpc.delete-sg" acknowledgements are stubbed with a fixed success reply — the SG record itself is still written by the real service impl before that event fires, so the stub only unblocks the synchronous round-trip rather than replacing tested behavior. * ci(e2e): gate live E2E jobs behind the in-process integration tier Runs the in-process integration suite first, on the ci-single runner host rather than inside a provisioned VM, so a broken control plane is caught before either live job pays the Spinifex install. Both e2e_single and e2e_multi now require it, and e2e_status treats a skipped integration job as a failure so the gating status cannot go green on a gate that never ran. Drops the shadow-mode suite selector: targeted per-diff suite selection is not the direction being pursued, and leaving a non-functional shadow job in the gating workflow only invited it to be mistaken for a live gate. * test(e2e): remove single/ tests superseded by daemon-lite ports TestKeyPairs, TestTagManagement, TestRouteTableValidation, and TestReplaceRouteConvergence now have proven-equivalent ports in tests/integration/ (mutation testing confirmed parity). Delete the e2e originals and their runX helpers, along with keypair_test.go's and tags_test.go's file-local helpers left with no caller. generateImportPubKey is shared by other single/ tests so it moves to helpers_test.go instead of being deleted. Slims the manifest-lint baseline for the removed create-call entries. * ci: run the lb e2e suite on multinode only Both e2e jobs filter SUITES by exclusion, and neither excluded lb, so the suite ran twice per e2e run: 354s on single and 654s on multi. Balancing across instances on one node is not a deployment mode we support, and ELBv2 proper requires two AZs, so the single-node run was redundant. Drops ~354s from the single-node job, which is the gate critical path. * test(integration): port 3 of 7 ECR tests to the in-process tier Wires the gateway's ECR control plane (repository/policy/lifecycle metadata over an in-process daemon-lite NATS subscriber, blob and manifest bytes via a memory object store, JWT issuer/verifier keyed off the existing IAM master key) into the integration harness, and ports TestECRControlPlane, TestECRGetLoginPassword, and TestECRIAMAuthorization_AssumedRolePullOnly from tests/e2e/ecr — each proven equivalent by mutation (evaluatePrincipalPolicy forced to allow, and the repo-describe path forced to not-found) before its E2E original was removed. The other 4 authorization tests (PullOnly, ReadOnly, DetachPolicyDeniesImmediately, DeactivatedKeyReturns401) stay on the E2E tier: AttachUserPolicy lacks the AWS-managed-ARN existence-check bypass that AttachRolePolicy/AttachGroupPolicy already have, so attaching AmazonEC2ContainerRegistryPullOnly/ReadOnly straight to a user fails NoSuchEntity in both tiers alike. Left untouched pending a fix to that shared IAM code. * fix(iam): accept AWS-managed policy ARNs on AttachUserPolicy AttachUserPolicy called getPolicyByARN unconditionally, so attaching an AWS-managed ARN to a user returned NoSuchEntity. AttachRolePolicy and AttachGroupPolicy already skip that lookup for AWS-managed ARNs and store them opaquely; the user path was missed when that guard was introduced. ListAttachedUserPolicies had the mirror of the same gap: it resolved every ARN through getPolicyByARN and silently dropped the ones it could not find, so a managed ARN would disappear from the listing rather than round-trip. Evaluation already handled these correctly via resolveAttachedPolicy, so only the write and list paths needed the guard. * test(integration): port remaining 4 ECR authorization tests Ports TestECRIAMAuthorization_PullOnly, _ReadOnly, _DetachPolicyDeniesImmediately, and _DeactivatedKeyReturns401 from tests/e2e/ecr now that AttachUserPolicy accepts AWS-managed policy ARNs. Each was proven equivalent by mutation before its E2E original was removed: PullOnly/ReadOnly/DetachPolicyDeniesImmediately fail when evaluatePrincipalPolicy is forced to always allow, and DeactivatedKeyReturns401 fails when the access-key-active check in resolveECRLongLivedPrincipal is bypassed. All mutants were reverted and a clean run confirmed before committing. tests/e2e/ecr/iam_authorization_test.go is now fully removed — every test it held has an in-process equivalent, and its helpers had no other callers in the package. * ci: select e2e suites by allow-list instead of deny-list Both jobs filtered SUITES by exclusion, so any suite named in neither deny-list ran in both jobs. That is how lb came to run twice per e2e run, and it makes adding a suite silently fan it out everywhere. Each job now names the suites it is eligible to run. Selection is still driven by SUITES, so this is behaviour-preserving for the current default: single/iam/cert on the single-node job, multinode/lb/cert on the multi-node job, exactly as before. eks, ecs and storagegrowth are listed as eligible on the single-node job but are not yet in the default SUITES. * test(e2e): delete seven tests with no coverage TestFinalClusterStats and TestInstanceLaunch assert nothing beyond what other callers of their shared helpers already imply. The multinode baseline, volume, guest-SSH, and preflight tests duplicate single-node coverage or exercise per-guest behaviour that isn't cluster-specific. TestMultinodeInstanceDistribution can't fail: three independent RunInstances calls with no placement group make a strict spread assertion flaky and a loose one meaningless. Preflight checks (/dev/kvm writability, peer SSH reachability) move into requireMultiNodeFixture so the package fixture singleton still fails fast with a clear message instead of disappearing along with the dedicated test. The remaining callers of needInstanceTrio (CrossNodeOps, CrossNodeGateway, NodeFailure, VolumeDurability) still trigger the shared trio launch on their own, so removing the distribution and volume-lifecycle tests doesn't leave it unbuilt. Also slims the manifest-lint baseline entry for the deleted multinode volume test. * fix(e2e): require full 3-node spread in TestMultinodeSpread SpreadPlacement passed at unique>=3, warned via t.Logf at unique==2, and only failed below that. A 33% spread failure (two instances colocated) passed with a log line. The 243.6s NAT-egress scenario that follows only proves what it claims to if the private trio actually landed on three distinct chassis, so that premise needs to be a hard assertion, not a warning. * refactor(e2e): drop the unused spread-assertion harness helpers AssertSpreadAcrossNodes and CountInstancesPerNode lost their only caller when the instance-distribution test was removed. Beyond being dead, the assertion helper documented minNodes=1 as an acceptable logging-only mode inherited from the bash driver, which is the pattern that let a placement test pass while every instance landed on one node. InstanceHostingNode and its ps helpers stay; five multinode tests use them. * fix(ec2): stop leaked goroutines on prepareEFIVolume error paths newViperblock starts the chunk uploader and WAL syncer unconditionally, but prepareEFIVolume only released them on the happy path via Close(). Six error returns between acquisition and Close left both goroutines running. Add a defer immediately after acquisition, matching the sibling prepareRootVolume; Close() remains the durability boundary. * test(predastore): add real predastore fixture for integration tests Extract predastore_integration_test.go's inline daemon startup into a reusable testutil/predastore package so tests/integration can point DaemonLite-style wiring at a real S3-compatible backend instead of an unreachable host. Kept as its own leaf package rather than folding into the shared testutil package: it transitively imports predastore's quicclient, whose package-level connection pool starts a permanent goroutine at init that would trip goleak checks in any of the ~80 other packages that import testutil for unrelated NATS helpers. * test(integration): migrate CreateVolume to assert real predastore persistence Add StartVolumeDaemonLite, an opt-in DaemonLite variant wired to the real predastore fixture, and migrate TestCreateVolume_PersistsToRealPredastore off memory-backed DaemonLite so CreateVolume/DescribeVolumes exercise viperblock's real S3 backend end to end instead of stopping at input validation. * test(integration): add real storage-state oracle for CreateVolume DescribeVolumes only proves the handler's own read path agrees with its own write path, both via the same deserialization; it cannot catch a bug in a field it never re-derives (e.g. a unit-conversion error in the bytes handed to viperblock, since VolumeMetadata.SizeGiB is copied from the request directly). vbscan reads a volume's persisted config.json and chunk objects straight over S3, independent of any spinifex handler code, so TestCreateVolume_PersistsToRealPredastore can assert on what viperblock actually wrote rather than only on what the handler claims. vbscan is a local reimplementation of scripts/vbrefscan's read-only scan rather than an import: spinifex cannot depend on mulga, the module that contains it. It only reimplements the config/chunk-listing half of vbrefscan — the checkpoint-based reference-integrity half needs viperblock.ParseBlockCheckpointBytes, which does not exist in the viperblock version spinifex currently depends on (v1.12.1) and is also moot for this volume: CreateVolume never calls SaveLiveCheckpoint, so a freshly created volume has no checkpoint to scan regardless of version. * refactor(test): move the predastore daemon fixture out of the coverage scope test-cover runs go test ./spinifex/..., so anything under spinifex/ is compiled and measured. The daemon fixture cannot be exercised by unit tests, so it reported a coverage gap no unit test could close. The e2e harness already avoids this by living under tests/; the fixture now does the same. vbscan stays put, being ordinary library code with its own unit tests. * test(integration): migrate RunInstances multi-count expansion off live guests RunInstances with MinCount/MaxCount=2 expands into a single per-node targeted launch (distributeInstances in gateway/ec2/instance/placement.go) before the daemon-facing NATS hop, so the real gateway router plus a stubbed daemon response is sufficient to prove it — no throwaway VM is needed to exercise the expansion logic itself. The live single-node E2E test's WaitForInstanceState/TerminateInstances sequence was teardown hygiene for the real guests it booted, not an assertion of the expansion behavior, so it is dropped rather than emulated. The migrated test additionally asserts the per-node launch request itself carries MinCount=MaxCount=2, proving the count actually propagates rather than the launch succeeding by coincidence or via the zero-daemon degenerate path in placement.go. * docs: drop a doc reference to the deleted live test * test(integration): migrate launch-template RunInstances expansion off live guests RunInstances --launch-template resolves ImageId/InstanceType from the stored template entirely gateway-side (expandLaunchTemplate, before validation/placement/the daemon-facing NATS hop), so proving it forwards the resolved fields never needed a real guest boot. Ports the assertion to tests/integration via a new opt-in StartLaunchTemplateDaemonLite (real LaunchTemplateServiceImpl over embedded JetStream) plus a direct subscription on the per-node launch subject to capture what the gateway actually dispatched, rather than only the client-visible response. The live test keeps its CRUD/versioning lifecycle (create/modify/delete version, tag-filtered describe) — none of it touches a guest — and drops only the RunInstances-from-template + wait-for-running + terminate tail, whose only job was waiting out a real boot to read back fields already proven resolved. * test(integration): migrate KV-only account-scoping checks off live tenants Ports the KeyPair/VPC-Subnet/IGW-EIGW/AccountSettings cross-account isolation assertions from tests/e2e/single/account_test.go to the in-process integration tier. Each of these resources resolves ownership from a plain account-scoped KV key (utils.AccountKey(accountID, id), or an account-keyed settings record) with no vm.Manager or real guest involved, so DaemonLite now also wires the real EgressOnlyIGWServiceImpl and AccountSettingsServiceImpl alongside the existing VPC/IGW/Key services. Two independently minted tenant accounts (via IAMService.CreateAccount/CreateUser/CreateAccessKey/CreatePolicy/ AttachUserPolicy, in-process — the same calls runAccountCreate makes without the CLI/stdout round-trip) replace the live tier's carousel so isolation is checked between real, separately owned accounts. Left live: instance scoping (Step2) and volume scoping (Step3) — checkInstanceOwnership, the actual cross-account gate for stop/start/ reboot/terminate/attach-volume, runs daemon-side only after a real vm.Manager lookup, so it cannot be exercised without a live guest. Snapshot scoping (Step5) needs a real backing volume for CreateSnapshot (viperblock, out of DaemonLite's scope), so it stays live entirely rather than split off only its two KV-only assertions. Global-resources (Step9) carries no account branching in the handlers at all, so asserting its equality across two accounts wouldn't exercise anything a fake node responder didn't already trivially satisfy — not migrated. * test(integration): migrate launch-template CRUD lifecycle off live guests TestLaunchTemplates booted zero guests: its RunInstances coverage had already moved to TestRunInstances_LaunchTemplateExpansion, leaving only template CRUD/versioning assertions occupying a full live-cluster slot. Port the remaining create/describe/version/modify/delete lifecycle, including the SourceVersion merge and a nested timestamp round-trip, into TestLaunchTemplates_CRUDLifecycle against the in-process integration gateway. * refactor(tests): move spot instance lifecycle coverage to integration tier RequestSpotInstances is a mock over on-demand RunInstances (no bidding, interruption, or reclamation), so booting real VMs in the e2e tier proved nothing spot-specific. Migrate the SIR CRUD lifecycle (request, describe, cancel, close-on-terminate, insufficient-capacity error) to a NATS-level integration test that exercises the real gateway orchestration and daemon service impl without a live guest. Real-VM-boot assertions are already covered by TestRunInstances_MultiCountExpansion and the e2e baseline tests. * fix(tests): share one embedded NATS server across the integration package ClientTokenStore in spinifex/gateway/ec2/instance is a process-wide sync.Once singleton bound to whichever *nats.Conn first initialises it. Every integration test booted its own embedded NATS server, so the first RunInstances-calling test captured the singleton, and any later test inherited a KV handle bound to that server's now-closed connection. TestMain now boots one embedded NATS+JetStream server for the whole package; every StartGateway call connects into its own dynamically provisioned NATS account (accountAuthenticator in shared_nats.go), giving each test a private JetStream namespace despite the shared server, and none of the per-test connections are closed until every test has finished - whichever one the singleton latches onto stays live for the run. Also wire the suite into the required PR gate: it was previously only invoked from e2e.yml's push-triggered (main/dev), self-hosted "Integration (front gate)" job, so a regression here never blocked a PR. The whole package runs in single-digit seconds, so it now runs inline in `make preflight` and as its own step in spinifex.yml's build_and_test job. * test(integration): migrate serial-console access checks off live guests Enable/Disable/GetSerialConsoleAccessEnabled is pure gateway+account-settings logic with no guest involved, so it moves to the in-process integration tier against the real AccountSettingsServiceImpl wired into DaemonLite. * test(integration): migrate DescribeImages round-trip off live guests runImage's AMI describe round-trip needs no real cluster: it registers an AMI against a hand-seeded snapshot via the real ImageServiceImpl, backed by a new StartImageDaemonLite helper wiring RegisterImage/DescribeImages onto an in-memory object store. * test(integration): migrate predastore object lifecycle off live guests runPredastoreObjectLifecycle only needs a real predastore daemon, not a tofu-provisioned cluster: it now runs against tests/fixtures/predastore's in-process fixture, covering the same create/put/get/delete/list contract. * test(integration): migrate ListFoundationModels catalog check off live cluster ListFoundationModels is pure gateway-side static catalog logic with no NATS/daemon hop, so it moves to the integration tier via a new BedrockClient harness accessor; this tier configures no credential resolver, so the Anthropic entry's absence is asserted explicitly rather than left optional. * test(integration): cover RunInstances client-token idempotency Proves replaying the same ClientToken through the real gateway returns the original reservation without a second daemon dispatch, and a different token launches independently. * test(integration): cover RunInstances launch-template expansion Proves version selection (omitted Version resolves to the template default, explicit Version selects that version) and field precedence (explicit RunInstances parameters override the template, unset fields inherit it) through the real gateway. * test(integration): cover RunInstances malformed-input rejection Proves a malformed ImageId and a malformed targeted-launch CapacityReservationId are rejected before any NATS dispatch, with no daemon stub registered so a regression that let either through would fail fast instead of hanging. * test(integration): cover RunInstances iam:PassRole enforcement Proves a caller granted ec2:RunInstances but not iam:PassRole on an instance profile's role is denied, and the identical session succeeds once PassRole is granted for that exact role ARN. * test(integration): cover RunInstances placement-group strategy routing Adds a placement-group daemon-lite wiring a real PlacementGroupServiceImpl over NATS, then proves spread strategy assigns exactly one instance per node while cluster strategy pins the whole launch to the single highest-capacity node. * test(integration): cover RunInstances capacity distribution across nodes Proves the default (non-placement-group) path packs a multi-instance launch across nodes proportionally to spare capacity rather than exhausting one node first. * test(e2e): merge SameSGComms and SGToSGDatapath around one client/target pair Both booted their own instance pair to check SG-to-SG datapath enforcement and same-default-SG east-west connectivity. Merging them around one shared client/target pair cuts four boots to two, and converting the inline harness.Step sequence into named t.Run stages (PortGroupMembership, AllowedTraffic, DeniedTraffic, RevokeAndVerifyBlocked, ReauthorizeAndVerifyRestored, SameSGComms) gives each its own pass/fail attribution instead of one opaque test result. SameSGComms now runs last and joins the default SG onto each ENI via ModifyNetworkInterfaceAttribute only after every SG-to-SG allow/deny assertion has already recorded its own result, since the default SG's self-reference rule admits all traffic between co-members and would otherwise defeat the denied-traffic check. * test(e2e): merge default-SG reachability, guest DNS, and egress enforcement DefaultSGReachabilityBaseline, GuestDNSResolution, and SecurityGroupEgress each booted their own instance to check one facet of "a guest is reachable only when policy says so": ingress deny/authorize, resolver + WAN egress, and egress ACL enforcement. Merging them around one shared guest cuts three boots to one, with named t.Run stages (Deny, Authorize, Resolver, WANEgress, RevokeEgress/RestoreEgress) giving each its own attribution. The egress revoke/restore round-trip now mutates this scenario's own dedicated SG instead of the shared default SG the rest of the suite depends on, isolating its egress-rule mutation from every other test. * fix(ci): single-quote e2e test_run regex across the ssh boundary * fix(e2e): anchor SG egress-enforcement probe to a WAN IP, not the gateway * test(e2e): merge single-node guest churn tests into one durability scenario Combines TestSSHProbe, TestENIHotplug, TestENIHotplugReconcile, TestVolumeDurability, TestModifyInstanceAttribute, and TestRebootInstance into TestGuestChurnDurability, a single shared-guest scenario that re-verifies a data-integrity sentinel sha256 after every perturbation stage (ENI hotplug, daemon restart, ENI detach, instance-type resize, stop/start, reboot). This proves the sentinel survives the full churn sequence rather than just one isolated stop/start. * fix(vpc): clear subnet drop gate when installing egress reroute A subnet with an attached IGW but no 0.0.0.0/0 route carries a priority-1100 drop gate on the VPC router. When a route later lands (NATGW reroute at 900, IGW reroute at 1000) the reroute is installed but the drop gate was never removed, so the higher-priority gate wins and black-holes the subnet's egress in lr_in_policy before it reaches routing or SNAT. This left NAT-gateway private-subnet egress with 100% packet loss: the SNAT stage saw zero packets and no conntrack was committed. Remove the subnet's drop gate for the same prefix once a reroute is installed. Idempotent and prefix-scoped, so a specific-prefix reroute is a no-op and leaves the default gate intact. * test(e2e): merge VPC subnet, NAT gateway, and EIP single-node tests Merges the former TestVPCSubnetE2E, TestNewVPCEgressBaseline, TestNATGateway, and TestInstanceEIP into one staged scenario, TestVPCEgressPaths, around one scenario-owned VPC and its two guests, cutting five guest boots to three. The public guest serves PublicSubnetEgress, then the NAT Gateway bastion hop, then (last, since association rewrites its public IP) the EIP flip. The private guest serves the NAT Gateway path, launched directly into the same VPC's already-existing private subnet instead of a dedicated one. RouteBeforeSubnet keeps its own second VPC and instance since it exercises the implicit main-route-table code path, a different one than the explicit route table this scenario's own VPC uses for its public subnet — collapsing the two would drop coverage of whichever path got merged away. The NAT Gateway up/down flip now repeats for an env-tunable round count (SPINIFEX_VPCEGRESS_NAT_ROUNDS, default 1), matching every other merged single-node scenario. * test: TEMPORARY mutation-check — revert a795c2b2 drop-gate clear Reverts the RemoveSubnetEgressDrop call in ensureSubnetEgressAtPriority to confirm TestVPCEgressPaths/Round1/NATGatewayUp actually detects the regression it guards against. This commit will be reverted immediately after the live run confirms a failure. * test: TEMPORARY mutation-check v2 — neutralize RemoveSubnetEgressDrop Restores the real a795c2b2 fix in ensureSubnetEgressAtPriority (the first mutation attempt only reverted this call site and was masked by the independent gate-recompute path in routetable service_impl.go's CreateRoute, which also clears the drop gate on every 0.0.0.0/0 route add and predates a795c2b2). Neutralizing RemoveSubnetEgressDrop itself disables gate-clearing regardless of caller, reproducing the literal regression. Will be reverted immediately after live verification. * Revert "test: TEMPORARY mutation-check v2 — neutralize RemoveSubnetEgressDrop" Mutation-check confirmed: neutralizing RemoveSubnetEgressDrop reproduced a795c2b2's regression exactly (stale priority-1100 drop gate outranking the priority-1000 IGW reroute, confirmed via OVN flow dump; PublicSubnetEgress failed with an SSH handshake timeout to the public instance). Restoring the real fix; igw.go is now byte-identical to a795c2b2. * fix(e2e): stop pruning shared multinode SSH key pem mid-run The multinode package fixture cached Fixture.Artifacts once, computed from harness.ArtifactDir(t, env) against whichever top-level Test* happened to win the pkgFixOnce race. That helper both names the directory after that one test and registers a t.Cleanup that removes it once that test passes. needKeyPair wrote the shared EC2 key pair's pem into that same directory, so as soon as the winning test finished, its own artifact-dir cleanup deleted the pem every later test in the package still needed to SSH into fresh guests, even though the underlying EC2 key pair itself was untouched (its DeleteKeyPair cleanup already correctly drains at process Close() in TestMain). Fix: give Fixture an ArtifactDir(t) method — the same per-call pattern already used by the gpu/single/storagegrowth/iam fixtures — so each diagnostic dump site recomputes its own dir against the CURRENT test. The shared key pair pem now lives under Fixture.TmpDir, which is genuinely package-scoped and only removed once in TestMain after m.Run() completes. * refactor(e2e): parallelize LB e2e scenarios on multinode clusters Each of the 8 TestLoadBalancer scenarios already stands up its own dedicated LB + target group (ModifyListener/ListenerRules mutate their LB's listener in place, so it can never be shared). The bottleneck was running them serially: each teardown made createActiveLB back off waiting for the previous LB's sys.micro slot to be reclaimed before the next one could stand up. On multinode clusters (peer != "", i.e. the 3-node cluster this suite targets) there's spare capacity to stand up all 8 dedicated LBs at once, so scenarios now run as t.Parallel() subtests instead of serial create/teardown/backoff. Since each scenario owns an independent LB, one never reaching active only fails that scenario — it no longer cascades into five sequential 5-minute timeouts, so the serial fail-fast gate is skipped in this mode; concurrency itself bounds the worst case to a single timeout window. Single-node dev boxes (no peer) keep the original serial path, including the fail-fast gate, since they lack the spare capacity for concurrent LB standup. No subtest body, assertion, or call signature changed — only the dispatch/gating logic in TestLoadBalancer. * fix(ci): gate e2e jobs on the suites they actually run * fix(e2e): serialize internet-facing LB subtests among themselves Live multinode verification showed InternetFacing_ALB and InternetFacing_NLB running concurrently each clobber the other's public-LB DNS record mid-registration, so neither ever resolves within the 90s wait. The old serial suite never had two internet-facing LBs alive at once, so this never surfaced. Drop t.Parallel() from the three internet-facing subtests so their register+resolve windows never overlap each other, while leaving the internal subtests parallel as before. They may still run alongside the internal group, which resolves internal (not public) DNS and isn't affected. * fix(e2e): give each ALB LB scenario its own security group Concurrent internal subtests all created ALBs with no explicit SecurityGroups, so each ALB's ENI fell back to the one shared VPC default SG. One scenario authorizing/reconfiguring its own listener racing another mutating the same SG made Internal_ALB_ModifyListener intermittently fail to serve on the modified port under parallel load. Each ALB scenario now creates a dedicated SG (createScenarioSG), authorized only for the ports that scenario's listener(s) use, and passes it to CreateLoadBalancer. The shared app-instance/probe-client SG is untouched, since SG ingress is enforced at the destination ENI regardless of the source ENI's SG. NLBs are unaffected: they already get an auto-managed per-LB SG when none is supplied. * test(e2e): annotate intentional create-path calls for manifest-lint The vpcegress and per-scenario LB security-group tests create raw AWS resources on purpose — the egress datapath, the route-before-subnet ordering, and per-scenario SG isolation are the subjects under test, so they cannot route through a shared Ensure* fixture. Mark each with // e2e:allow-create and slim the baseline of entries for the removed single-node network tests. * test(e2e): restore live spot terminate-close and cancel-running coverage The integration-tier spot test calls CloseForInstance directly and infers cancel-keeps-running from the SIR's InstanceId, because DaemonLite has no vm.Manager to drive the real TerminateInstances -> daemon teardown cleaner -> RemoveFromSpotRequest -> CloseForInstance chain or to observe a real VM's running state. Add a thin live e2e test that drives both terminal transitions through a real backing VM: cancel one spot-backed instance and observe it stays running, terminate the other and observe its SIR reach closed through the genuine async delegation. * fix(network): refresh orphan-EIP prune liveness to spare mid-pass launches pruneOrphanEIPs listed OVN NAT rows live but matched them against the intent snapshot captured at the start of the reconcile pass. Because the apply phase can block for tens of seconds on guest-port datapath waits, a guest launched during the pass had a live dnat_and_snat row absent from the stale snapshot, so the prune swept it and blackholed the guest's auto-assigned public IP until the next pass re-added it — an intermittent multinode SSH-dark flake. Add an optional FreshIntent re-read callback (wired in vpcd to LoadIntentFromKV) and union its ports into the live set before pruning, so a mid-pass launch counts as live. Skip the prune entirely when the re-read fails rather than risk a false sweep against a snapshot known to be stale. Self-healing and idempotent: a genuine orphan absent from both snapshot and fresh read is still pruned next drift. Single-node behaviour is unchanged (callback nil in tests).	2026-07-21 16:46:18 +10:00
Ben Duncan	28a876d17d	feat(dns): resolvable ALB/NLB + EKS DNS, forwarder hardening, cross-tenant reconcile (#532)	2026-07-17 12:02:59 +10:00
Julian Sommer	635915b5b0	feat(spx): surface OVN NB/SB Raft roles in get nodes (#493)	2026-07-10 09:48:05 +10:00
Josh Siviour	0823bf8c16	feat: OVN OVSDB RAFT clustering for multinode Spinifex (epic mulga-siv-486) (#458) ... * feat: add OVSDB RAFT cluster primitives to setup-ovn.sh Add --db-cluster-local-addr / --db-cluster-remote-addr flags so a management node can run the OVN NB/SB databases as a native OVSDB RAFT cluster instead of a single non-clustered ovn-central. When a local addr is given, write a shared OVN_CTL_OPTS to /etc/default/ovn-central (NB RAFT 6643 / SB RAFT 6644) and restart the per-DB units directly, since the ovn-central aggregator is ExecStart=/bin/true and won't restart its children. Empty remote addr creates the cluster (init node); a set remote addr joins it. NB/SB client set-connection runs only on the create node (RAFT replicates it cluster-wide). The single-node --management path is unchanged when no cluster addrs are supplied, so dev boxes and existing single-node clusters keep working. --ovn-remote already passes through verbatim, so a comma-separated SB endpoint list works for compute nodes; documented in usage. * feat: OVN client multi-endpoint failover for RAFT clusters Accept a comma-separated OVN NB/SB endpoint list so vpcd can target a clustered DB. config.ParseEndpoints splits the list (trimming whitespace, dropping empties); live.go Connect passes one libovsdb WithEndpoint per endpoint, so the client tries them in order, fails over on disconnect, and follows RAFT leader redirects. Single-endpoint strings still yield one endpoint, so single-node dev is unchanged. ovn-sbctl --db= already accepts the raw comma list, so the SB CLI consumers (discoverChassis, gateway_claim) need no change. * feat: write clustered OVN NB/SB endpoints across the RAFT quorum spx admin init/join now emit comma-separated OVN NB/SB endpoint lists for the first three quorum nodes (formation.BuildOVNDBAddrs) instead of loopback or a single leader address, so vpcd fails over across the cluster. firstboot defers OVN bring-up to the provisioning controller when it owns formation (SkipFormation): a standalone single-node .db would otherwise block the controller's clustered create/join. * test: add multinode OVN RAFT cluster + failover e2e; document clustered topology ovn_raft_test.go asserts NB/SB cluster/status reports a 3-server quorum with one leader on every DB node, then stops the NB leader's ovn-central and proves an NB write still lands via a surviving endpoint. Docs: vpc-networking gains the multi-endpoint vpcd config and a RAFT cluster-status debugging section; network-connections lists the 6643/6644 RAFT ports.	2026-07-09 09:43:29 +10:00
Julian Sommer	8a94886b13	feat: cloud-init EC2 datasource (#411)	2026-06-25 13:48:08 +10:00
joshsiv-mulga	5fa12ec358	fix(e2e): skip stopped node in multinode NATS peer poll ... StopNode now stops spinifex-nats.service, so WaitNATSPeers polling that node's dead 127.0.0.1:8222 monitor port fails with curl exit 7 and never converges. Add WithSkipNodes to exclude the stopped node from the poll; TestMultinodeNodeFailure skips node2.	2026-06-25 08:05:44 +10:00
Julian Sommer	fa066a64d7	fix	2026-06-24 16:37:58 +10:00
Bryn Mailer	15dede64ad	feat: predastore compaction E2E (#407) ... * feat: add predastore compaction E2E Inject a short compaction interval into test-cluster predastore config via a new --predastore-compaction-interval flag on admin init/join, gated so production output stays byte-identical when unset. Add a harness helper that drives write-then-delete churn against predastore S3, brackets it with du-over-SSH usage, and asserts end-of-churn usage settles below 70% of the churn peak; wire it into the multinode suite as TestPredastoreCompaction. * fix: compaction E2E gate writes churn to its own bucket The reclaim gate signed PutObject against the config-defined system 'predastore' bucket, which belongs to account 000000000000 with only a static [[auth]] grant. The harness admin identity is a different IAM account, so predastore returned 403 AccessDenied cross-account. Create a per-run bucket the admin identity owns (AdministratorAccess) and churn into it. Objects still erasure-code into distributed/nodes/ node-* so the du-based reclaim measurement is unchanged. The gateway (:9999) does not proxy S3, so the client stays predastore-direct. * test: run predastore compaction gate before destructive node tests The compaction reclaim gate was declared last in the multinode suite, so it measured shard-store usage after NodeFailure/NodeRecovery had torn down and restarted node2. NodeRecovery only gates on NATS/gateway/EC2 readiness, not predastore, so node2's shard store was still settling and the du baseline came back empty. Move the test before the destructive node tests so it runs against a pristine, fully-converged cluster primed by the volume tests. * fix: race-free SSH output capture + empty-tolerant du probe The compaction reclaim gate crashed with ParseInt("") when node1's du probe returned empty combined output. Harden the probe and the SSH layer: - ssh.Run used one bytes.Buffer for both session.Stdout and session.Stderr, which x/crypto/ssh copies in two concurrent goroutines — a data race on a non-concurrent-safe type. Switch to session.CombinedOutput, which guards the combined stream with a mutex-wrapped singleWriter. - shardStoreUsageBytes now terminates the remote command with a sentinel echo. A missing sentinel is reported as output lost in SSH retrieval; a present sentinel with no number is treated as a 0 measurement (anomalous, logged) instead of crashing the gate. The sentinel lets the next run distinguish a transport-layer drop from a genuinely empty remote measurement. * test(e2e): replace predastore du-reclaim gate with object lifecycle contract The multinode compaction gate measured shard-store disk usage over SSH (du) and asserted reclaim after churn. It could never pass: segments only seal at the 4 GiB roll threshold, so test-scale churn produces no compactable segment, and the du probe read 0 bytes before and after. Reclaim correctness is already covered deterministically by predastore's store-level unit tests. Delete AssertPredastoreCompaction and its du/churn/poll/sentinel machinery, the harness unit tests that exercised them, and the multinode TestPredastore Compaction entry. Add a single-node TestPredastoreObjectLifecycle that checks the user-visible contract: an object is readable after PutObject, gone after DeleteObject (GET returns NoSuchKey and ListObjectsV2 is empty), and the store keeps serving fresh writes. newPredastoreS3 is retained, refactored to take a host so it works outside the multinode cluster type. * test(e2e): drop orphaned multinode TestPredastoreCompaction Complete the du-reclaim -> object-lifecycle refactor: remove the multinode TestPredastoreCompaction caller (and its now-unused context and harness imports) that still referenced the deleted harness.AssertPredastoreCompaction, which broke e2e-build. Wire the single-node TestPredastoreObjectLifecycle entry that replaces it.	2026-06-23 16:19:10 +10:00
Julian Sommer	35eaca5dc4	fix: add detailed logging to e2e	2026-06-23 09:59:54 +10:00
Julian Sommer	058da3eff4	fix(viperblockd): seal volume block-map to predastore on detach + durability e2es (#371)	2026-06-17 11:24:03 +10:00
Julian Sommer	2e9501524b	feat(e2e): more e2e coverage (#365)	2026-06-17 09:52:50 +10:00
Julian Sommer	50b5495d26	chore: more comment cleanup	2026-06-12 12:38:31 +10:00
joshsiv-mulga	a5b6a74a0b	fix(e2e): serialise TestMultinodeGuestSSH after cluster restabilises ... TestMultinodeGuestSSH ran with t.Parallel(), so it resumed immediately after the sequential bucket and probed trio[0] — which CrossNodeOps stop/starts but only waits to reach state "running", not for sshd to answer. The guest was often still booting, producing kex_exchange_identification resets and 2m timeouts that flaked CI red. Drop t.Parallel() and declare the test last in the sequential bucket so it runs after CrossNodeOps + NodeFailure/Recovery + Spread have restabilised the cluster, giving every trio member time to settle and removing concurrent VM churn.	2026-06-11 13:00:35 +10:00
joshsiv-mulga	c9a4e904a8	test(e2e): fresh-install reachability baselines + drop qemu hostfwd fallback ... Add baseline tests that confirm two fresh-install regressions the existing suite masks (it authorizes tcp/22 on the default SG before every SSH probe): - default SG default-deny gates external reach of instances in the default subnet (SG, not routing) - a newly-created public subnet never gets its OVN IGW egress LRP, so it has no route to the VPC IGW despite MapPublicIpOnLaunch Plus a positive test: instances sharing the default SG can communicate via ICMP (same-host single-node, cross-host multinode). Single-node baselines run after TestClusterStatsCLI (0-VM gate); multinode after TestMultinodePreflight (fixture init). Instances are self-cleaning so they don't perturb the singleton-VM ordering. Remove the qemu hostfwd SSH fallback from the harness (InstancePublicSSHHost, GuestSSHEndpoint) - it bypasses the OVN datapath and masks these regressions. Endpoints now t.Fatal when no public IP is assigned.	2026-06-04 19:56:02 +10:00
Josh Siviour	fd3dc6b680	feat(network): ADR-0006 layer redesign + IPsec/OVN hardening (#255) ... * refactor(vpcd): remove macvlan bridge mode Macvlan was a single-NIC SSH-safe hack that predated veth mode and was never selected by any active config path. Deletion is Phase 0 item 1 of the network layer redesign (mulga-siv-125.1). Removed: - BridgeModeMacvlan const + setMacvlanMAC + ifaceIsMacvlan probe - Macvlan MAC alignment block in launchService (only fired on dead path) - Macvlan branch in detectBridgeMode + matching tests - Macvlan duplicates of IGW/AddNAT centralized-NAT tests (veth versions already cover the same path) - --macvlan flag + macvlan case branch in setup-ovn.sh Refactored: - useCentralizedNAT() now == BridgeModeVeth (was != BridgeModeDirect); verifyBridgeMode already rejects empty/unknown at startup - Three IGW-pool tests now explicitly WithBridgeMode(BridgeModeVeth) (previously relied on empty-mode defaulting to centralized) Kept intentionally: - Deprecation notes in docs/compute/vpc-networking/README.md - Legacy macvlan-interface cleanup in reset-dev-env.sh + ansible teardown role (migration for operator systems with stale spx-ext-* interfaces from pre-veth installs) * feat(vpcd): resolve external CIDR at startup with bounded retry Block service start until the WAN bridge has an IPv4 address, with a 30s retry window. Phase 0 item 2 of the network layer redesign (mulga-siv-125.1). Guards the boot race where vpcd starts before systemd-networkd finishes DHCP on br-wan, or before netplan applies a static config. Skipped when ExternalMode is empty (overlay-only deployments). Forward-compatible with ADR-0006: the new resolveExternalCIDR / externalCIDRFromBridge helpers become the backing implementation of HostWiring.ExternalCIDR() once the network/host package lands in Phase 1. Until then the resolved value is only used to fail-start on missing uplink. * fix(vpcd): cover external CIDR startup paths Extract launchService's CIDR-resolve block to ensureExternalCIDRReady helper so it's unit-testable, and add direct tests for externalCIDRFromBridge (loopback + unknown-interface) to exercise the original literal body. Closes the diff-coverage gap on Phase 0 commit 15b564d which left these paths unreachable from unit tests. * refactor(network/ovn): extract L1 OVN client to network/ovn package Move OVNClient interface, LiveOVNClient, ACLSpec, ErrNATNotFound and namedUUID helper from services/vpcd to a new spinifex/network/ovn package. The mock relocates to spinifex/network/ovn/mock as a sub-package. Storage fields on the mock become exported so vpcd tests can continue to seed/inspect state across the new package boundary. services/vpcd retains thin type-alias and var-alias shims so existing callers compile unchanged; Phase 2 removes those shims as vpcd is torn down. Adds -coverpkg=./spinifex/... to test-cover so cross-package test exercise of mock methods continues to count toward coverage now that the mock lives outside vpcd. First sub-step of Phase 1 of docs/development/feature/spinifex-network-redesign.md (bead mulga-siv-125.2). Pure refactor; no functional change. All existing tests pass; preflight green. * feat(network/topology): TopologyManager interface + vpcd adapter Add spinifex/network/topology with Manager interface, VPC/Subnet/Port specs, and OVN object naming helpers covering the L2 stable contract (§7 of the redesign plan). Phase 1 keeps existing in-tree OVN names (rtr-, ext-port-, sg_) and notes the §7.3 targets for a later reconciliation migration. TopologyHandler in services/vpcd grows EnsureVPC / DeleteVPC / EnsureSubnet / DeleteSubnet / EnsurePort / DeletePort / SetPortSecurityGroups methods that mirror the corresponding NATS handlers' OVN ops with Spec inputs instead of nats.Msg envelopes, satisfying topology.Manager via a compile-time check. Existing handlers are unchanged; the duplication is short-lived (Phase 2 collapses them). Smoke tests in topology_manager_test.go cover the idempotent create path, delete cascade, and declarative SG membership move. Deliverable: TopologyManager.EnsureVPC/Subnet/Port callable; all existing tests green. Second sub-step of Phase 1 of docs/development/feature/spinifex-network-redesign.md (bead mulga-siv-125.2). * feat(network/host): L0 HostWiring interface + physical/veth impls Phase 1 deliverable for mulga-siv-125.2. Introduces network/host (L0 of ADR-0006): UplinkMode enum, Wiring interface, and Physical/Veth implementations with injectable Runner + InterfaceReader for testability. Not yet wired into daemon/vpcd — Phase 2 replaces daemon.SetupComputeNode and vpcd's resolveExternalCIDR with calls through this package. * feat(network/policy): L3 SecurityGroup/NAT/Route managers Introduce network/policy as the data-plane policy layer (ADR-0006 L3). Three managers, all backed by network/ovn: - SecurityGroupManager: clear+add ACL replacement on an existing port group; port group lifecycle stays with L2 (topology.Manager). - NATManager: bound to one NATMode at construction (derived from host.UplinkMode). Distributed sets logical_port/external_mac on EIP dnat_and_snat rules; centralised leaves them unset. Per-EIP, NAT GW, and IGW-default SNAT entrypoints; all deletes idempotent. - RouteManager: AddStaticRoute is read-then-decide so retries no longer duplicate rows (closing the long-standing non-idempotency bug in vpcd/topology.go's IGW default-route path). ACL priority constants and infrastructure ACL builders move from vpcd/{security,acl}.go into policy/acl.go with their semantics preserved (default-deny 900/800 logged, DHCP-allow 1050, tenant 1000). No vpcd wiring changes here — additive only; the reconciler (bead .3.2) and network/external (bead .3.4) will consume these managers. bead: mulga-siv-125.3.1 * feat(network/external): L5 IGW/EIP/NATGW managers Pure additive — new package, no topology.Manager changes, no rewiring of existing vpcd handlers. IGWManager builds the external switch + localnet + gateway LRP + default route + chassis HA, with a flows barrier callers can inject (production: ovn-nbctl --wait=hv sync). EIP/NATGW are thin L5 wrappers over policy.NATManager. GatewayIPAllocator interface lets the DHCP-backed allocator stay in vpcd until the DHCP-mgr removal bead lands; LinkLocalAllocator + StaticRangeAllocator ship here. * feat(network/reconcile): L6 intent-actual reconciler replaces 5-pass startup Build spinifex/spinifex/network/reconcile/ package per Phase 2.2 of the network layer redesign. Reads desired state from NATS KV filtered by vpc.AZ (with legacy empty-AZ matches-all rule), scans actual state from OVN NB DB, diffs and applies with VPC → Subnet → Port → SG → IGW → EIP/NATGW topological ordering. Atomic port-group join via CreateLogicalSwitchPortInGroups closes the racy gap the legacy scanENIPortMembership pass used to revive 30s later. A 5-minute drift goroutine (leader-gated via JetStream KV CAS) replaces the old 30s ReconcileSGsLoop. Adds AZ field to VPCRecord stamped at create time. Empty AZ on pre-Phase 2.2 records matches every local AZ, so no migration pass is required. Replaces and removes the 5-pass startup sequence in services/vpcd: - Reconcile + ReconcileFromKV - ReconcileSGsOnce + ReconcileSGsLoop - RetrofitAllExternalLocalnetOptions - RetrofitAllGatewayPortNetworks IGW fresh-create continues to flow through the legacy NATS handler path (handleIGWAttach) so the dhcpManager wiring stays intact until Phase 2.3; the reconciler only rebinds gateway-chassis on existing IGW topology. Closes mulga-siv-125.3.2. * refactor(network/topology): promote live Manager impl to network/topology Move the topology.Manager implementation out of services/vpcd/topology_manager.go into network/topology/manager_live.go as a standalone liveManager constructed via NewLiveManager(ovn.Client, opts...). Inputs that varied per-handler (DNS server resolution from the external pool) are now passed as functional options (WithDNSServer). TopologyHandler still satisfies topology.Manager but its methods delegate to a cached liveManager. The shim stays until the rest of mulga-siv-129 routes NATS subscribers through topology.Manager directly. Also exports SubnetGatewayCIDR + FormatDNSServerList from network/topology so the reconciler in network/reconcile can share helpers with the live manager (audit pass to follow per plan §6). Test cases ported one-for-one from services/vpcd/topology_manager_test.go to network/topology/manager_live_test.go; the vpcd-side test file is deleted. New WithDNSServer test asserts the option propagates into DHCPOptions rows. Refs mulga-siv-129 (Phase 2.6 vpcd subscriber migration, Slice 1 of N). * refactor(vpcd): collapse SG subscribers onto network/{topology,policy} managers Phase 2.6 / mulga-siv-129 slice 2. Rewrites the vpc.create-sg / vpc.update-sg / vpc.delete-sg NATS subscribers in services/vpcd as thin wrappers over topology.Manager (port group lifecycle) and policy.SecurityGroupManager (ACL programming). Deletes the ~1000 lines of duplicated ACL builder and provisioning logic — its authoritative copy lives in network/policy. Surface additions: - ovn.ErrPortGroupNotFound sentinel; mock + live GetPortGroup wrap not-found via fmt.Errorf("%w: ...") so callers can use errors.Is. - topology.Manager grows EnsureSGPortGroup / DeleteSGPortGroup (resolves the apply.go TODO from §2.1 of the redesign plan). liveManager implements them with GetPortGroup-then-Create / GetPortGroup-then-Clear+Delete idempotency. Behaviour delta: the old provisionSG rolled back the OVN port group on AddACLs failure ("all-or-nothing"). The new path lets the port group survive and relies on network/reconcile to retry EnsureSG on its next sweep. The all-or-nothing fail-fast test cases are dropped; ACL emission semantics are covered by network/policy/sg_test.go. * refactor(vpcd): migrate VPC/Subnet/ENI subscribers to topology.Manager Phase 2.6 / mulga-siv-129 slice 3. Rewrites vpc.create / vpc.delete / vpc.create-subnet / vpc.delete-subnet / vpc.create-port / vpc.delete-port / vpc.update-port-sgs NATS subscribers as thin wrappers (≤30 lines each) over topology.Manager. Subscriber bodies now: parse event → translate to spec → delegate to manager. Deletes: - vpcd.subnetGateway (and helpers_test.go + topology_test.go's TestSubnetGateway block); parity coverage lives in network/topology.TestSubnetGatewayCIDR. - vpcd.reconcilePortSGs (the live manager already exposes the same diff-then-transact via reconcilePortSGs on the L2 side). - vpcd.TopologyHandler.dnsServer; lmDNSServer (passed via topology.WithDNSServer to the live manager) is the live path. Behaviour delta: handlers now strict-parse PrivateIpAddress / MacAddress / CidrBlock at the NATS boundary and reject malformed payloads instead of degrading to "no DHCP options". The reconciler guarantees that field validity is upheld upstream of the publish. * refactor(network/reconcile): drive SG port groups via topology.Manager applySGs now calls topology.Manager.EnsureSGPortGroup / DeleteSGPortGroupByName instead of reaching directly into the OVN client. Reconcile.Config gains a Topology field (required); the production caller in services/vpcd passes the TopologyHandler shim, the reconcile package tests construct a fresh topology.NewLiveManager bound to the mock OVN client. DeleteSGPortGroupByName is added to the topology.Manager interface for the reconciler's orphan-removal path, which iterates raw OVN port-group names without a groupID round-trip. The existing DeleteSGPortGroup(groupID) delegates to it. Resolves the TODO at apply.go:137 carried over from slice 2 of mulga-siv-129. * feat(network/policy): add FlowsBarrier option to NATManager NATManager.AddEIP needs to block on `ovn-nbctl --wait=hv sync` after writing the dnat_and_snat row so the NATS reply only returns once every chassis has the rewrite flow. The legacy vpc.add-nat handler does this at the tail of handleAddNAT; the manager has lacked it since slice 2.1. NewNATManager grows a functional-options tail (Option / WithFlowsBarrier). Defaults to a no-op for tests; production wiring in services/vpcd passes waitForFlowsHV. AddEIP now invokes the barrier after AddNAT, logging at warn level on failure to match the legacy handler's _ = waitForFlowsHV() swallowing semantics. Pre-work for slice 4 of mulga-siv-129 — without the barrier the NAT EIP subscriber migration would lose the post-write flow-install gate. * refactor(vpcd): migrate NAT GW subscribers to policy.NATManager handleAddNATGateway / handleDeleteNATGateway now decode the NATS event and delegate to policy.NATManager.{AddNATGateway,DeleteNATGateway}. The manager carries identical OVN logic — same snat type, same ExternalIDs (spinifex:vpc_id + spinifex:nat_gateway_id) — so existing TestTopologyHandler_AddNATGateway / DeleteNATGateway suites stay green. Lazy-constructed NATManager added on TopologyHandler (natmOnce). Mode is derived from the bridge mode (veth → centralized, direct → distributed) and the FlowsBarrier is bound to waitForFlowsHV for parity with the legacy EIP path's tail barrier (used by the next slice, vpc.add-nat). Slice 4 of mulga-siv-129, sub-step b. * refactor(vpcd): migrate EIP NAT subscribers to policy.NATManager handleAddNAT / handleDeleteNAT now decode the NATS event and delegate to policy.NATManager.{AddEIP,DeleteEIP}. The behavioural contract is preserved end-to-end: - Idempotency-skip on a matching dnat_and_snat row (same VPC + LogicalIP, plus MAC + LogicalPort under distributed NAT) moves into AddEIP so duplicate vpc.add-nat publishes never tear down the rule the previous invocation just installed (mulga-siv-124). - Stale-rule cleanup across every router stays in AddEIP. - Post-write waitForFlowsHV barrier is now driven by the manager via the FlowsBarrier option wired in services/vpcd (slice 4a). - DeleteEIP returns nil on ErrNATNotFound, matching the handler's fire-and-forget "already absent = success" semantic. The vpcd "errors" import drops out with the inline error-comparison. Slice 4 of mulga-siv-129, sub-step c. * refactor(vpcd): migrate IGW subscribers to external.IGWManager (partial) handleIGWAttach now branches at decode time: - Centralised NAT + DHCP-sourced pool keeps using attachIGWLegacy because the gateway-LRP IP comes from vpc.dhcp.acquire, and that DHCP client is the vpcd-local manager that bead mulga-siv-125.3.3 will retire. - All other shapes (distributed NAT, static-range pool, no pool) delegate to external.IGWManager.AttachIGW. The manager mirrors every step the legacy handler ran — external switch + localnet port + gateway LRP + switch-side router peer + default route + chassis HA + waitForFlowsHV tail barrier — so observable behaviour is unchanged. handleIGWDetach drops its inline OVN walk entirely; both paths exit through external.IGWManager.DetachIGW followed by releaseGatewayLRPLease. The lease release stays in vpcd because the upstream client lives in services/vpcd/dhcp; calling it after every detach is safe because the upstream server expires unknown leases on its own. Lazy-constructed IGWManager added on TopologyHandler (igwmOnce). Pool, allocator (StaticRangeAllocator), chassis, NAT mode, NAT manager, route manager, and FlowsBarrier are all derived from the existing TopologyHandler state. The DHCP-coupled legacy branch retains its full inline OVN logic; it will disappear in slice-after-125.3.3 follow-on. Slice 4 of mulga-siv-129, sub-step d. * feat(ec2/instance): honor AssociatePublicIpAddress per-NIC override RunInstances --network-interfaces "AssociatePublicIpAddress=true|false" was silently ignored, only subnet.MapPublicIpOnLaunch gated public IP assignment. AWS contract is per-NIC override wins. Extend TestPrepareRunInstances_PublicIPAutoAssigned to table-driven 3 cases: subnet true no-override, subnet false override true, subnet true override false. * refactor(config): drop upstream-DHCP config surface (static WAN pool only) Hard-error in LoadConfig on legacy keys: [network] external_dhcp, [nodes.<n>.vpcd] dhcp_bind_bridge, and [[network.external_pools]] source = "dhcp". Add pool range validation (must fit inside the gateway/prefix_len CIDR; per-cluster non-overlap). Strip ExternalDHCP from config.NetworkConfig + formation.NetworkConfig + admin.ConfigSettings, and DhcpBindBridge from config.VPCDConfig + admin.ConfigSettings + vpcd.Config. vpcd.ExternalPoolConfig.IsDHCP() short-circuits to false; the field and downstream DHCP branches in external_ipam / topology.go remain for Slice C to delete together with dhcp_manager. cmd/spinifex/cmd/admin.go: drop useExternalDHCP path and the detectedDhcpBindBridge helper; require --external-pool when --external-mode=pool; auto-detect default no longer infers source=dhcp. service.go's wan_bridge legacy-key rejection now advises removing the key entirely (vpcd auto-detects br-wan), not renaming to dhcp_bind_bridge. Two topology_test cases that exercise the DHCP gateway-LRP path are skipped — Slice C deletes attachIGWLegacy and these tests together. * refactor(vpcd): delete dhcp_manager + DHCP gateway-LRP path (Slice C) Removes upstream-DHCP-client gateway-LRP allocation in favor of static WAN-pool allocation (StaticRangeAllocator). Part of bead mulga-siv-125.3.3. - delete services/vpcd/dhcp_manager.go + dhcp/ subpackage (~865 lines) - delete allocateGatewayLRPIPViaDHCP + releaseGatewayLRPLease paths - drop ExternalPoolConfig.Source + DhcpBindBridge + IsDHCP() fields - drop ExternalIPAllocation lease fields (Expiry/ServerID/HWAddr) and ExternalIPAMRecord.Source - drop NewExternalIPAM nc *nats.Conn argument; update all 4 call sites - rename verifyBridgeMode local dhcpBindBridge -> wanBridge; update error strings and tests Slice D (delete attachIGWLegacy + dispatcher) intentionally deferred. * refactor(vpcd): delete attachIGWLegacy + IsDHCP stub (Slice D) Closes mulga-siv-129 slice 4d. Part of bead mulga-siv-125.3.3. handleIGWAttach now unconditionally delegates to external.IGWManager. attachIGWLegacy (~207 lines, ~5 OVN write steps + chassis scheduling) deleted along with its dispatcher branch in handleIGWAttach. The ExternalPoolConfig.IsDHCP() dead stub is gone. No code paths or NATS subjects regress: the static-pool / distributed branches were already going through external.IGWManager since Slice 4d (6719dc3). * feat(ipam): Purpose field on every IPAM record (Slice B) Part of bead mulga-siv-125.3.3. Adds a Purpose enum (handlers/ec2/vpc/purpose.go) tagging every IP allocation with its semantic role (eni-primary, eni-public, eip, igw-lrp, natgw-external, …) so multi-VPC clusters can reclaim/audit by (owner, purpose). External IPAM (spinifex-external-ipam, v1→v2): - ExternalIPAllocation.Type → .Purpose; legacy values mapped via LegacyExternalTypeToPurpose (gateway→igw-lrp, auto_assign→eni-public, elastic_ip→eip). Unknown legacy values warn and become "unknown". - AllocateIP / AllocateFromPool / allocateFromPool param rename allocType→purpose. All call sites pass Purpose* constants. - migrate/002_external_ipam_purpose.go: idempotent CAS-retry migration with v1/v2-union record shape so partial migrations resume cleanly. Internal IPAM (spinifex-vpc-ipam, v1→v2): - IPAMRecord.Allocated []string → []IPEntry{IP, Purpose, OwnerID}. - AllocateIP gains purpose+ownerID args; ReleaseIP unchanged (still keyed on IP). - AllocatedIPs returns []IPEntry. - eni.go reorders eniId generation before AllocateIP so the owning resource ID lands on the new entry. - migrate/003_ipam_purpose.go: cross-bucket backfill that reads the ENI bucket via the new RunKVWithJetStream path. Orphaned IPs (no owning ENI) warn and become Purpose=unknown. Migrate framework: - KVContext gains an optional JetStream field; new Registry.RunKVWithJetStream method threads it through. RunKV signature unchanged — all other callers are unaffected. Tests: - ipam_test.go: AllocateIP signature updates; AllocatedIPs now asserts IP / Purpose / OwnerID on the returned entries. - external_ipam_test.go: replaces "auto_assign"/"elastic_ip" literals with PurposeENIPublic/PurposeEIP and asserts alloc.Purpose. - migrate/002 + 003 tests: schema-detect, idempotent rerun, no-JetStream fallback, empty bucket. * refactor(daemon): tombstone obsolete spinifex-dhcp-leases KV bucket The dhcp_manager removed in Phase 2.3 (bead mulga-siv-125.3.3) owned the spinifex-dhcp-leases KV bucket. The migrate framework only handles per-bucket schema version bumps, not bucket deletion, so the obsolete bucket would linger on upgraded clusters. Add a one-shot tombstone after initJetStream that deletes the bucket if present. DeleteKVBucketIfExists treats ErrBucketNotFound and ErrStreamNotFound as success, so the call is idempotent across restarts. * feat(config): add network.ipsec_enabled cluster flag (default true) Add IPSecEnabled to NetworkConfig in both the cluster TOML schema and the formation handshake payload. Default true so AWS-parity intra-AZ encryption is on by default. Operators can flip false in [network] for trusted single-rack lab topologies. Phase 2.5 (bead mulga-siv-125.3.5) consumes the flag from admin init (strongSwan provisioning), formation server (propagates to joiners), and daemon startup (ovs-vsctl ipsec_encapsulation toggle). * feat(admin): IPsec peer cert generator signed by cluster CA Add GenerateIPSecPeerCert + IPSecCertPaths in spinifex/admin so admin init (and the future admin add-node path) can issue per-node IKEv2 peer credentials signed by the existing cluster CA. The cert carries id-kp-ipsecIKE (RFC 4945) so strongSwan accepts it as an IKEv2 peer credential when ovs-monitor-ipsec materialises tunnel configs. Cluster CA is reused directly — no intermediate strongSwan PKI today; when intermediate CAs land that becomes a separate workstream. The cert/key pair lives under {configDir}/ipsec/peer.{pem,key}; the daemon points OVS Open_vSwitch.other_config (certificate, private_key, ca_cert) at these paths when network.ipsec_enabled is true. * feat(admin): wire IPsec peer cert into admin init + join paths Add --ipsec bool flag (default true) on adminInitCmd. Single-node init and the multi-node leader path both emit an IPsec peer cert after the existing CA+server-cert generation. The multi-node formation server propagates the IPSecEnabled flag to joining nodes via NetworkConfig; admin join honours it and emits its own peer cert during the same cert-issuance step that already produces the server cert. spinifex.toml template gains an unconditional [network] block carrying ipsec_enabled so the value is visible to operators on every install regardless of whether external networking is configured. The daemon reads it via cluster config (NetworkConfig.IPSecEnabled). Bead mulga-siv-125.3.5. * build: install openvswitch-ipsec for OVN native IPsec openvswitch-ipsec provides ovs-monitor-ipsec, which reads the Open_vSwitch.other_config cert/key/CA pointers and materialises strongSwan configs for every Geneve tunnel managed by ovn-controller. Pulls in strongswan + strongswan-pki as transitive deps. Required by bead mulga-siv-125.3.5. * feat(daemon): enable OVN native IPsec on intra-AZ Geneve at startup When network.ipsec_enabled is true (default), the daemon points the local OVS DB other_config at the per-node IPsec peer cert that admin init / admin join provisioned (configDir/ipsec/peer.{pem,key} + the cluster CA at configDir/ca.pem), then sets other_config:ipsec_encapsulation=true. ovs-monitor-ipsec (shipped by the openvswitch-ipsec package) consumes those pointers and materialises strongSwan configs for every Geneve tunnel ovn-controller programs — including OVN-IC inter-AZ tunnels when federation lands in Phase 3a. Missing cert files log a warning and leave Geneve plaintext; operators upgrading an older cluster reissue certs via spx admin to close the gap. Bead mulga-siv-125.3.5. * test(e2e): multinode IPsec wiring + xfrm SAs + ESP capture TestMultinodeIPSec asserts: - OVS DB on every node carries certificate/private_key/ca_cert pointers and other_config:ipsec_encapsulation=true (what daemon.enableOVNIPSec writes). - ip xfrm state on every node has at least one AEAD SA in AES-GCM mode (rfc4106(gcm(aes)) or gcm(aes)) within 90s. - tcpdump catches ESP (ip proto 50) on the underlay (best-effort — logged only; geneve may be idle during the 5s capture window). Skips cleanly when the cluster was bootstrapped with --ipsec=false (no ipsec_encapsulation key on first node). Read-only over SSH; runs in the parallel bucket alongside the other non-mutating multinode tests. * fix(igw): write attachment State="attached" for AWS parity Reconciler intent loader (network/reconcile/intent.go::loadIGWs) filters IGW records with `State == "attached"`, but AttachInternetGateway was writing `State = "available"`. The reconciler therefore never picked up any attached IGW — `intent_igws=0` on every pass — so the OVN external switch + gateway LRP + WAN bridge wiring built by IGWManager.AttachIGW never ran for default-VPC IGWs (whose live `vpc.igw-attach` publish at daemon bootstrap is lost — vpcd subscriber not yet wired). Net effect on single-node e2e: every SSH probe to 192.168.x public IPs timed out (TestSSHProbe, TestModifyInstanceAttribute, TestRebootInstance, TestNATGateway, TestSGToSGDatapath) because SNAT/DNAT rules on the VPC router had no path to the external network. Match AWS InternetGatewayAttachment.State semantics: - Attach: State="attached" - Detach: State="detached" Detach value is internal — recordToEC2 gates Attachments on VpcId != "", so the detached State is never surfaced to API consumers. Bead: mulga-siv-131 * fix(reconcile): call AttachIGW when IGW topology missing applyIGWs previously deferred to the NATS handler when an intent IGW had no external switch in OVN. When the vpc.igw-attach event is lost (bootstrap-time default-VPC IGW published before vpcd subscriber is ready), deferral never resolves and VM public-IP traffic dies. Call IGWManager.AttachIGW directly — it is idempotent (short-circuits when the external switch already exists), so the reconciler converges default-VPC IGWs on first pass without duplicating work on subsequent passes. Closes mulga-siv-132. * fix(test): align NewExternalIPAM call with 2-arg signature Rebase onto dev pulled in daemon_system_instance_test.go which used the legacy 3-arg NewExternalIPAM(jsNC, js, pools). The feature branch already narrowed the signature to (js, pools) via the upstream-DHCP config drop; align the test with the current signature. * fix(vpcd): EnsureSubnet must EnsureVPC first to avoid NATS race vpc.create-vpc and vpc.create-subnet are independent NATS events with no inter-event ordering guarantee. Tenant bootstrap on iam.account.created publishes both within <1ms, so the subnet handler's CreateLogicalRouterPort can race the parent VPC's CreateLogicalRouter and fail with "logical router not found". The cascade fails CreateNetworkInterface which fails RunInstances which surfaces as InsufficientInstanceCapacity to the SDK — masking the underlying topology race. handleSubnetCreate now calls EnsureVPC(VPCID-only) before EnsureSubnet. EnsureVPC short-circuits on existing LR (idempotent), so the race-winning vpc.create-vpc handler's full CIDR/VNI ExternalIDs are preserved; if subnet wins the race the LR is created with bare ExternalIDs (cosmetic) and the follow-on vpc.create-vpc short-circuits. Adds TestIntegration_SubnetBeforeVPC regression covering both orderings. Closes mulga-siv-133. * feat(ansible): require --external-pool, drop NAT, forward EXTRA_VARS spx admin init now rejects --external-mode=nat and demands --external-pool when mode=pool. The ansible init role silently skipped pool flags when the range was unset, so installs fell over inside the spx validator. Make targets also swallowed `-e` (make's env-override flag), so operators could not override pool start/end without editing defaults.yml. - vars/defaults.yml: rename nat_gateway_ip → external_gateway_ip; add spinifex_external_no_external opt-out; document the spx flag contract. - roles/init/tasks/main.yml: drop NAT branch; assert pool range upfront with a fail_msg pointing at EXTRA_VARS / defaults.yml; emit gateway, prefix-len, gateway-ip only when explicitly set (let spx auto-detect); honour --no-external. - playbooks/dev-reset.yml: rename captured var; coerce legacy external_mode=nat to pool with a warn; tighten gateway regex with \b. - Makefile: ansible-dev-* targets now forward EXTRA_VARS="key=val ..." through --extra-vars so `make` callers can override without editing defaults.yml. - scripts/ansible/README.md: document pool requirement, both invocation styles (direct -e vs make EXTRA_VARS), NAT removal. * fix(ipsec): guard ipsec_encapsulation on ovs-monitor-ipsec liveness enableOVNIPSec set OVS cert pointers + ipsec_encapsulation=true without ensuring ovs-monitor-ipsec.service was running. Without that daemon, OVS holds the cert pointers unused, strongSwan never gets IKE config, and tunnel traffic ovn-controller marks for encryption gets silently dropped. TestMultinodeIPSec fails on `node1 xfrm has no AEAD SAs`. - systemd: spinifex-daemon Wants=/After= ovs-monitor-ipsec.service - daemon: ensureOVSMonitorIPSecActive — systemctl enable --now + is-active poll; refuses to flip ipsec_encapsulation if daemon is dead - daemon: single-node short-circuit — no peers means no tunnels means the flag is at best inert and at worst load-bearing-silent-drop - ci: capture-net-diag.sh + workflow steps to snapshot OVS/OVN/xfrm state on failure, fan out across all 3 multinode nodes Refs: mulga-siv-136 * test(e2e): sticky-skip downstream SSH tests when probe fails When TestSSHProbe times out (3 min, typically because of mulga-siv-134 datapath bug), four downstream tests each re-probe SSH on the same broken datapath and time out identically — single-node job runs ~24 min instead of ~12 min, with the same error logged five times. First waitForSSHReady timeout now sets a package-level atomic flag; subsequent waitForSSHReady / trySSHReady / requireSSHHealthy calls skip immediately. Preserves the original red signal on TestSSHProbe and on any test that genuinely needed SSH (Skip ≠ Pass). Bool variant (trySSHReady) consults but does not set the flag — its caller already treats False as advisory warning, not fatal. TestSGToSGDatapath has its own EventuallyErr SSH wait (not via waitForSSHReady) and needs an explicit requireSSHHealthy(t) gate at the top of runSGToSGDatapath. TestNATGateway already calls waitForSSHReady first so the gate fires naturally. Refs: mulga-siv-137 * fix(ipsec): use Debian unit name openvswitch-ipsec.service CI artifact shows `Unit ovs-monitor-ipsec.service could not be found` on every node. Debian's openvswitch-ipsec package ships the unit as openvswitch-ipsec.service (the binary inside is ovs-monitor-ipsec, but systemd's name follows the package). The previous commit guarded on the wrong unit name, so: - spinifex-daemon.service Wants=/After= silently no-op'd (missing unit) - enableOVNIPSec's `systemctl enable --now ovs-monitor-ipsec.service` failed, daemon logged a warning and continued without flipping ipsec_encapsulation=true - TestMultinodeIPSec skipped with `node1 reports ""` because OVS DB never received the cert pointers Rename across all 5 spots (daemon, systemd unit, test, CI diag script, workflow journal capture) to match the Debian package layout. Note: the package itself still must be present on the CI VM image — that's tracked in the mulga repo (image-builder provision.sh + bootstrap-install.sh shim). Refs: mulga-siv-136 * fix(e2e): authorize default SG tcp/22 + icmp in single-node fixture admin init leaves the default SG closed (AWS parity); the Go single suite never ported the Phase-5 SG-ingress block that run-e2e.sh has had since day one, so every test runner SSH/ping probe hits the OVN port-group ACL drop. - ec2helpers: add AuthorizeICMPIngress + EnsureDefaultSGOpen helpers. - single/main_test: open default SG once per process in pkgFixOnce.Do. - witness: extend ensureDefaultSGSSHIngress to authorize ICMP too. - run-e2e.sh: append matching ICMP authorize next to tcp/22 block. mulga-siv-134 * fix(network/reconcile): key DHCP_Options lookup by subnet_id not CIDR Multi-tenant default VPCs share the same subnet CIDR (172.31.0.0/20) per account. The existing FindDHCPOptionsByCIDR gate at apply.go:107 found the first account's DHCP_Options row and skipped creation for every subsequent subnet with the same CIDR. Those subnets' ports then attached nothing (FindDHCPOptionsByExternalID misses on subnet_id), VMs got no DHCPv4 lease, and SSH/ICMP into them silently failed even though SG ACLs and NAT were correct. Switch the gate to FindDHCPOptionsByExternalID keyed on spinifex:subnet_id so each subnet owns its own row, matching the attach lookup at line 203. Verified: account 000000000001 default VPC RunInstances now produces an LSP with dhcpv4_options populated, VM cloud-init reports a 172.31.0.5/20 lease + default route via 172.31.0.1, and ping + ssh TCP handshake to the EIP succeed end-to-end. Stale DHCP_Options row for the prior dead subnet was wiped manually; reconciler recreated the correct row for the live subnet on its next pass. * fix(ipsec): enable openvswitch-ipsec at provision time, drop daemon write Provisioning now runs `systemctl enable --now openvswitch-ipsec.service` alongside openvswitch-switch and ovn-controller. The spinifex-daemon process drops its `enable --now` shell-out and only polls is-active. Sudoers gains one read-only line: spinifex-daemon ALL=(root) NOPASSWD: /usr/bin/systemctl is-active openvswitch-ipsec.service This mirrors the existing vpcd ovn-controller is-active precedent and removes the daemon's need for systemd-write capability. Aligns with least-privilege service account pattern: provision enables services, daemon only reads status. If the unit is inactive at runtime, the daemon refuses to flip ipsec_encapsulation=true (silent-drop guard from mulga-siv-136) and surfaces a clear error pointing at scripts/setup-ovn.sh. * chore: unblock preflight on feat/network-layer-redesign Two pre-existing failures blocked preflight on the branch: - govulncheck flagged golang.org/x/net@v0.54.0 (GO-2026-5026, ASCII Punycode rejection bug in idna.ToASCII); reached via utils.DownloadFileWithProgress. Bump to v0.55.0; go mod tidy carries x/sys to v0.45.0. - TestTopologyHandler_SubnetCreate_RouterPortFails asserted subnet create fails when the parent VPC router is missing. That contract changed in aed7b2e (mulga-siv-133), which made handleSubnetCreate call EnsureVPC idempotently before EnsureSubnet to avoid a NATS publish-order race. Delete the now-impossible failure case. * refactor(network/ovn): relocate nbdb model package from services/vpcd Phase 4 / mulga-siv-125.6 Slice A. Pure import rename, no logic change. The OVN Northbound DB model (LogicalSwitch, LogicalRouter, DHCPOptions, etc.) lived under services/vpcd/nbdb/ because that was the original home of every OVN caller. After Phase 1+2 the L1 client (network/ovn) and all L3/L5 managers (network/policy, network/external, network/topology, network/reconcile) became the primary callers — 8 importers in network/* vs 2 in services/vpcd/. The directory move puts the schema model where its consumers are, and unblocks deleting services/vpcd/ entirely once the subscriber/launcher relocations land (Slices B–D). * refactor(vpcd): delete dead retrofit/reconcileIGW helpers + bridge-mode bleed Phase 4 / mulga-siv-125.6 Slice B. Production path no longer touches any of: - RetrofitAllGatewayPortNetworks / RetrofitAllExternalLocalnetOptions — the 5-pass startup leftovers that network/reconcile (Phase 2.2) made redundant. Nothing called them outside tests. - reconcileIGW + its allocateGatewayLRPIP / gwLrpRange helpers + the ipv4ToUint32 / uint32ToIPv4 utilities — superseded by external.IGWManager (Phase 2.4) which carries its own pool logic. - ensureLocalnetOptions / ensureGatewayPortNetworks / expectedGatewayPortNetwork — read-before-write retrofit shims for the same code paths. - generateMAC — only callers were reconcileIGW + its tests. useCentralizedNAT (the bridge-mode bleed flagged in plan §2.2) is replaced by a policy.NATMode value injected via WithNATMode(...). The mode is resolved once in vpcd.go from host.UplinkMode and threaded into both the topology-handler lazy managers and the reconciler stack, so "is this veth?" is no longer a question topology.go can ask. The TopologyHandler defaults to NATModeDistributed when WithNATMode is omitted, matching the previous default. Drops services/vpcd/topology.go from 1220 LOC to 693, deletes ~700 LOC of obsolete tests (TestGenerateMAC, TestEnsureLocalnetOptions_*, TestRetrofitAllExternalLocalnetOptions_FixesClearedOptions, TestReconcileIGW_*, TestGwLrpRange), and frees the nbdb + utils + net imports in topology.go. Stale "phase 4 migrates those onto the new managers" comment in vpcd.go fixed in passing — the subscribers were migrated in Phase 2.6 (mulga-siv-129); only the file-relocation parts of Phase 4 remain. Stale ensureLocalnetOptions / ensureGatewayPortNetworks references in network/ovn/{live.go,mock/mock.go} doc comments also cleaned up. * refactor(network): move NATS VPC subscribers to network/subscribers Phase 4 Slice C (mulga-siv-125.6). Promotes the NATS event handlers out of services/vpcd into a new spinifex/network/subscribers package. Subscriber holds the topology/policy/external managers directly — the lazy *Once adapter shim (topology_manager.go) is deleted along with vpcd's local event-struct copies. launchService now constructs topology.LiveManager eagerly with a DNS callback derived from external pools (preserving the unscoped-pool lookup old TopologyHandler.lmDNSServer did), builds every manager up front, then hands them to subscribers.New + Subscribe. reconcile.New receives topology.Manager directly instead of through the adapter. Bridge-mode constants and waitForFlowsHV move into vpcd.go alongside the launcher; they were left behind as topology.go's last responsibilities. Handler-layer tests (topology_test.go, security_test.go, integration_test.go) are dropped: substance is covered by network/topology, network/policy, network/external manager tests. subscribers/subscribers_test.go adds focused coverage: missing-dep rejection, topic registration smoke, and bad-JSON envelope per topic. services/vpcd is now down to the launcher + the ovn/mock compat aliases (removed in Slice D when the launcher promotes to cmd/vpcd/). * refactor(vpcd): relocate services/vpcd → spinifex/vpcd; drop dead aliases Phase 4 Slice D (mulga-siv-125.6). Final cleanup of the network-redesign package layout. services/vpcd/ disappears entirely; the launcher remains an in-process Service impl, just rehomed under spinifex/vpcd/ (no separate binary — spx vpcd start stays the cobra entrypoint). - Delete services/vpcd/mock_ovn.go + ovn.go (compat aliases now dead post Slice C; only callsite was launchService's NewLiveOVNClient, inlined to ovn.NewLiveClient). - git-rename services/vpcd/vpcd.go + vpcd_test.go to spinifex/vpcd/. - Retarget three importers (spinifex/service/service.go + service_test.go, cmd/spinifex/cmd/service.go) to the new path. - Fix stale services/vpcd / TopologyHandler comments in network/reconcile/ + network/external/types.go + spinifex/vpcd/vpcd.go. - service-interfaces.yaml vpcd path updated. - Plan doc §13/§14/§16 + §9.2/§19 updated: drop cmd/vpcd/main.go (was speculative; never built) in favour of in-process Service impl at spinifex/vpcd/vpcd.go. Plan §16 Created list now lists subscribers/. Phase 4 plan items 1+2 (remove services/vpcd, entrypoint location) satisfied. Items 3+4 remain blocked on mulga-siv-125.4 + 125.5 (bundled join token, plan archive). * fix(ovn): install openvswitch-ipsec + strongswan explicitly ovs-monitor-ipsec.service fails to start without an IKE daemon. The openvswitch-ipsec package only Recommends strongswan, so any apt config that drops Recommends (or a base image with the recommends already skipped) leaves the service in a Restart loop with 'IKE daemon is not installed'. Hoist both packages into setup-ovn.sh so provisioning is the single source of truth — the bootstrap shim in mulga is no longer needed. * test(network): pin ADR-0006 invariants to executable suite Add static-analysis + behavioural tests that bind every safety and liveness clause in ADR-0006 (S1-S9, L1-L3) to a named Go test. Failure output quotes the ADR clause verbatim and points at the offending file:line so future debugging sessions can't quietly bypass a layer contract to make a test green. Static-analysis suite (spinifex/network/invariants/): - S1 layers_test.go: parses `go list -json`; rejects upward layer imports and layers pulling from cross-cutter packages - S2 host_containment_test.go: AST-detects ovs-vsctl / ovn-nbctl / ovs-ofctl shell-out outside network/host/ - S3 nat_mode_test.go: rejects .UplinkMode() / NATModeFromUplinkMode() calls outside the init-time resolver locations - S4 names_test.go: AST-detects OVN object name construction (concat, fmt.Sprintf, strings.Join) outside topology/names.go - S5 policy_no_create_test.go: rejects CreateLogical{Switch,Router,...} and DeleteLogical{...} calls inside network/policy/ - S6 federation_isolation_test.go: rejects TransitSwitch / TransitRouterPort references and ovn-ic literals outside network/federation/ (pre-protects the layer before it lands) - S8 ipsec_test.go: rejects strongswan / xfrm / IKEv2 / libreswan / openswan imports anywhere in network/ - S9 reconciler_singleton_test.go: rejects Reconcile…Loop and Reconcile…Pass<n> function names anywhere in network/ - S7 nats_subjects_test.go: scans for `vpc.<verb>` subjects missing the {azID} segment; calls t.Skip with the gap report so every test run prints the work-to-do without breaking CI. Flip to t.Fatalf when the cluster-wide subject rename ships. Behavioural suite (spinifex/network/reconcile/liveness_test.go): - L1 reconciler converges: scaled-intent (N=1,3,8) double-Reconcile property test; asserts second pass is a no-op - L2 drift bound: structural check that DriftInterval is positive and ≤30 min support window; full fake-clock behavioural test deferred - L3 federation re-enables: t.Skip stub pointing at the missing network/federation/ package Fixes the one production S4 violation the new walker found: network/external/igw.go was constructing "gw-port-"+vpcID inline. Adds topology.GatewaySwitchPort(vpcID) helper and retargets both AttachIGW and DetachIGW callsites through L2. Plan + per-clause mechanic + phasing in docs/development/improvements/network-invariants.md. Refs: mulga-siv-140 * fix(ovn): install strongswan-charon (not strongswan metapackage) The strongswan metapackage's '\''charon-systemd | strongswan-charon'\'' alternative resolves to charon-systemd by default, which ships /usr/sbin/charon-systemd but not the legacy /usr/sbin/ipsec helper or /usr/lib/ipsec/charon binary that ovs-monitor-ipsec --ike-daemon=strongswan shells out to. Install strongswan-charon explicitly to pull strongswan-starter (legacy ipsec controller) + the matching charon binary, matching what openvswitch-ipsec.service expects. Also reset-failed + restart openvswitch-ipsec.service after enable to recover from the deb post-install starting the service before its dependencies finished unpacking. * chore: add CODEOWNERS — joshsiv-mulga gates network/ + vpcd/ Fall-through reviewer for any change is the maintainer. network/ and vpcd/ are explicitly listed so the matching branch protection rule ("Require review from Code Owners") routes those PRs to joshsiv-mulga. Those trees are governed by ADR-0006 invariant tests (spinifex/network/invariants/); CODEOWNERS adds a human review gate on top of the mechanical fence. Refs: mulga-siv-141 * chore: CODEOWNERS fallthrough → @mulgadc/mulgadc-engineers team Fallthrough reviewer should be the team, not a single maintainer, so any engineer can approve PRs outside the explicitly-gated paths. network/ and vpcd/ keep joshsiv-mulga as the required reviewer per ADR-0006 invariant ownership. Refs: mulga-siv-141 * feat(ci): add test_run workflow input for -test.run filter Pass an optional regex through to each suite's '\''go test -test.run'\'' so dispatches can target a single test (e.g. '\''^TestMultinodeIPSec$'\'') without running the full multinode suite. Threaded through single, multinode, and reboot suite invocations; empty input preserves current behaviour. * fix(daemon): flip NB_Global.ipsec=true on management node Setting Open_vSwitch.other_config:ipsec_encapsulation=true on each chassis is necessary but not sufficient for OVN native IPsec — without NB_Global.ipsec=true, ovn-controller never adds options:remote_name to the Geneve tunnel ports, so ovs-monitor-ipsec sees no IPsec-enabled tunnels and the kernel xfrm tables stay empty. After enabling ipsec_encapsulation locally, also write NB_Global.ipsec=true when the local ovnnb_db socket is present (management node only); workers skip silently — one writer is enough for a cluster-wide flag. Adds a scoped sudoers entry pinning the daemon to that exact ovn-nbctl invocation so the grant can't be reused for other NB mutations. * fix(ovn): grant charon AppArmor read on /etc/spinifex/ipsec The strongswan-charon profile only allows reads from /etc/ipsec.*, /etc/strongswan.*, and a small fixed allowlist. ovs-monitor-ipsec writes the per-tunnel strongSwan config with absolute paths to our peer cert + key under /etc/spinifex/ipsec/, so charon hits 'Permission denied' loading them (it runs as uid 0 but AppArmor still blocks) and no xfrm SAs come up. Drop a read grant for /etc/spinifex/ipsec/** into the profile's local override hook + reload. * fix(ovn): pin OVS system-id to hostname for IPsec cert identity match ovs-monitor-ipsec uses chassis-id as the IKEv2 `@<name>` peer identity in the strongSwan config it generates. Our per-node IPsec peer cert (admin.GenerateIPSecPeerCert) carries the node name as CN + dnsName SAN. With chassis-id left at the package-generated UUID, charon validates `@<UUID>` against a cert with dnsName=<hostname> — mismatch returns AUTHENTICATION_FAILED and no xfrm SAs are negotiated. Pin system-id to hostname (which equals the node name on Mulga clusters) by writing both the on-disk source of truth (/etc/openvswitch/system-id.conf, read by ovs-ctl on boot) and the running OVS DB value. Writing only one would drift on the next reboot — the original "do not override" comment was right about that hazard, just wrong about the resolution. Cross-link the invariant in admin/ipsec.go: changing the cert identity without updating setup-ovn.sh (or vice versa) breaks IKE silently. Refs: mulga-siv-136 * fix(ovn): take --node-name flag so chassis-id matches cluster identity CI hostnames are bead-themed (env20-casuarina) and don't match the cluster node name (node1/node2) baked into the IPsec peer cert dnsName SAN at `spx admin init/join --node NAME`. Pinning system-id to `hostname -s` produced env20-casuarina; charon then validated `@node1` against dnsName=env20-casuarina and IKE failed with AUTHENTICATION_FAILED. Add a --node-name=NAME flag to setup-ovn.sh, default to hostname for manual/dev invocations, and update the ansible install role to pass {{ spinifex_node_name }}. The matching bootstrap-install.sh change lives in mulga. Refs: mulga-siv-136 * refactor(daemon): relocate L0 helpers to network/host + widen ADR-0006 S2 Phase 4 Slice E of mulga-siv-125.6. Cleans up spinifex/daemon/network.go per the audit and widens the S2 invariant to forbid OVS/OVN shell-outs in spinifex/daemon/ (daemon is an L0 consumer; S2 applies). Moved to network/host/: - OVSNetworkPlumber + SetupTap/CleanupTap → host.OVSPlumber (host.NewOVSPlumber) - GetBridgeIPv4 → host.GetBridgeIPv4 - OVNHealthStatus + CheckOVNHealth → host.OVNHealth + host.HealthStatus - daemon/ipsec.go OVS/OVN ops → host.SetIPSecCertPaths, host.EnableIPSecEncapsulation, host.SetNBGlobalIPSec Moved to utils: - sudoCommand → utils.SudoCommand + utils.SetSudoCommandForTest test seam (cross-package reassign would trip the reassign linter; helper keeps the indirection inside utils). Moved to vm: - generateMgmtMAC → vm.GenerateMgmtMAC (pairs with vm.GenerateDevMAC) Deleted (dead): - SetupComputeNode (replaced by network/host/bridges.go:ensureBridges + scripts/setup-ovn.sh:572-579) - EnsureDataRoute - findInterfaceByIP - daemon/network.go + dead tests in daemon/network_test.go (duplicates of vm package tests retained; mgmt MAC test moved to vm/lifecycle_test.go) S2 invariant (network/invariants/host_containment_test.go) now walks both spinifex/network/ and spinifex/daemon/, skipping network/host/. * fix(ovn): broaden AppArmor read grant to /etc/spinifex (covers CA cert) The earlier override granted reads on /etc/spinifex/ipsec/** only, which covers the peer cert + key but NOT the cluster CA at /etc/spinifex/ca.pem. charon then can't load the CA, can't validate the peer cert chain, and surfaces the failure as `no trusted RSA public key found for '<peer>'` even though identity matching now succeeds — IKE_AUTH fails with AUTHENTICATION_FAILED and no xfrm SAs negotiate. Grant /etc/spinifex/** instead (covers ca.pem + ipsec/peer.{pem,key}) and always rewrite the override so re-runs replace the older narrower rule. Refs: mulga-siv-136 * fix(ipsec): symlink cluster CA into charon's default cacerts dir ovs-monitor-ipsec writes per-conn `cacert=/etc/spinifex/ca.pem` references in the strongSwan config, but strongSwan 6.0 only loads CAs declared in standalone `ca` sections or files placed in /etc/ipsec.d/cacerts at startup. Without this, charon authenticates the local side fine but rejects every inbound peer cert as `no trusted RSA public key found for '<peer>'`, AUTH_FAILED, and no xfrm SAs negotiate. Do the symlink in admin.GenerateIPSecPeerCert (right after writing the peer cert) — setup-ovn.sh runs before `spx admin init/join` creates /etc/spinifex/ca.pem, so the wiring can't live there. Use a stage-and- rename pattern so concurrent test invocations don't race on the symlink. /etc/ipsec.d/cacerts is on charon's default AppArmor allowlist so no profile change needed. Refs: mulga-siv-136 * chore(setup-ovn): drop apt-get install_packages block OVS/OVN/openvswitch-ipsec/strongswan-charon are baked into the CI gold image via mulga's image-builder/scripts/provision.sh (commits 116bb05, a9b3a9c). Keeping a runtime apt-get install path here pulled CI through apt-cacher-ng on every bootstrap and tripped on intermittent "Message has been manipulated" InRelease cache corruption (env21/env20 runs). Drop the install block. Gold-image rebuild is the canonical place to add new system packages. If a package is missing, downstream ovs/ovn commands fail loudly during setup-ovn.sh's later steps — clearer signal than silently re-installing through a flaky proxy. setup.sh already supports INSTALL_SPINIFEX_SKIP_APT=1 (passed by mulga's bootstrap-install.sh:237) so this removes the last runtime apt-get call on the CI provisioning path. * fix(ovn): gate system-id pin on explicit --node-name setup-ovn.sh used to fall back to `hostname -s` when --node-name wasn't passed, and then unconditionally wrote that value to /etc/openvswitch/system-id.conf + ovs-vsctl external_ids:system-id. On CI single-node, the gold image already registered a chassis with name=<UUID>; setup-ovn.sh then rewrote system-id to the bead-themed hostname (env10-banksia). vpcd's discoverChassis treats any chassis row whose name != local system-id but hostname == local hostname as a stale local chassis and skips it, so vpcd reported "no OVN chassis registered in SBDB" and refused to start, cascading into the e2e + e2e-go failures on run 26430839508. Drop the hostname fallback. If --node-name is given, pin both the on-disk file and the running OVS DB to it (callers that care about IPsec cert identity matching still get it). If no --node-name is given, leave the existing system-id alone — that's always safe whether the value came from a gold-image UUID, bootstrap-install.sh writing nodeN before this script runs, or a manual ansible value. * fix(network/reconcile): skip orphan prune on startup; drift owns GC The vpcd startup reconcile races against the daemon's EnsureDefaultVPC: the daemon writes SG KV rows + publishes vpc.create-sg on each node in parallel with vpcd starting up. The leader-gated reconcile can load intent before any SG row is visible on its local JetStream replica while subscribers on peer nodes are concurrently creating port groups from the same vpc.create-sg events. A startup orphan-port-group sweep then deletes managed sg_* port groups that intent hasn't caught up to. CI run 26434124790 trace: node2 reconcile/lock: elected holder=env20-bottlebrush node2 reconcile: starting intent_sgs=0 node2 reconcile/apply: removed orphan port group pg=sg_161c70ce23e704a2a node2 reconcile/apply: removed orphan port group pg=sg_a0d078650a3150391 node2 reconcile/apply: removed orphan port group pg=sg_54b564b374e9162bb Every subsequent RunInstances → vpc.create-port failed with "port group not found" for ~3 minutes until the 5-minute drift tick recreated the port groups. Tests give up after 60s; six multinode suites cascade-fail on InsufficientInstanceCapacity. Split the reconciler into Reconcile (full prune, drift loop) and ReconcileApplyOnly (no prune, startup). applySGs takes a pruneOrphans bool. vpcd startup calls ReconcileApplyOnly; drift loop unchanged. Legitimate orphans get garbage-collected on the next drift tick — at worst 5 minutes of lingering rows, no production impact. Add TestReconcile_ApplyOnlyKeepsOrphanPortGroup to lock the contract. * fix(ipsec): trigger charon rereadcacerts after CA symlink install strongSwan scans /etc/ipsec.d/cacerts only at charon startup. The unit openvswitch-ipsec.service is enabled at provision time and starts before admin init writes the cluster CA symlink, so charon ends up running with zero CAs loaded. Every IKE_AUTH then fails with `no trusted RSA public key found for '<peer>'` and no SAs negotiate — the failure mode exposed by TestMultinodeIPSec. Append a stroke rereadcacerts call at the end of installCAIntoCharonTrustStore so an already-running charon picks up the freshly-placed CA without a service restart. Failure is swallowed at debug — a missing/stopped charon at admin-init time is expected and a later startup will scan cacerts naturally. * refactor(network/host): move IPSec orchestrator from daemon (ADR-0006 S8) ADR-0006 S8: "IPSec is OVN-native only. No layer implements custom IKEv2, XFRM rules, or direct strongSwan management. IPSec SA lifecycle is delegated entirely to OVN native IPSec and is invisible above L0." The enableOVNIPSec orchestrator and its ensureOVSMonitorIPSecActive helper lived in daemon/ipsec.go but composed L0 primitives (SetIPSecCertPaths, EnableIPSecEncapsulation, SetNBGlobalIPSec) that already lived in network/host. The orchestrator now lives alongside its callees as host.EnableOVNIPSec, taking configPath and clusterConfig from the daemon at the call site. Helper and test-override package vars (systemctlActiveTimeout, ovnNBSocketPath) move with it; tests are translated to call EnableOVNIPSec directly rather than via the Daemon struct. Closes mulga-siv-142 * refactor(daemon): use topology.Port for OVN port names (ADR-0006 S4) ADR-0006 S4 forbids any layer above L2 from constructing OVN object names. Three daemon sites formatted "port-" + ENIId inline; route them through topology.Port instead. Closes mulga-siv-143 * refactor: use topology.Port for OVN port names in handlers + vm (ADR-0006 S4) Replace inline "port-"+eniID formatting across five sites (eip, instance x2, vpc/eni, vm/network_helpers) with topology.Port. OVSIfaceID stays as a thin wrapper to preserve the OVN binding contract comment and existing call sites. Closes mulga-siv-144 * test(network/invariants): widen scan roots to daemon/handlers/vm/testutil/utils (ADR-0006) Widen ADR-0006 invariant scan roots so drift in upstack consumers surfaces in CI. Backlog of violations in widened roots was drained by mulga-siv-142/-143/-144; this commit makes the wider scan permanent. - S2 (host_containment): add vm/ alongside network/ + daemon/ - S3 (nat_mode): add daemon/ alongside network/ + vpcd/ - S4 (names): add daemon/, handlers/, vm/ alongside network/ + vpcd/; add narrow AWS-ID exemption (vpc-cidr-assoc-) so AWS resource IDs are not misclassified as OVN object names - S6 (federation): add daemon/, handlers/ alongside network/; convert single-root walk to slice - S7 (nats_subjects): add daemon/, handlers/, testutil/, utils/ — stays t.Skip pending cluster-wide subject rename; gap report now lists 58 subjects CODEOWNERS: add /spinifex/network/invariants/ rule so widening or narrowing the scan needs explicit owner review. Closes mulga-siv-145 Refs: audit-adr-0006.md * chore(e2e): disable IPsec in pseudo multi-node test Pseudo multi-node runs three simulated nodes as unprivileged tf-user. The charon trust-store install in 904b45e writes /etc/ipsec.d/cacerts, which fails with EACCES under a non-root caller and aborts admin init. Disable --ipsec for this suite — loopback-only nodes don't exercise real Geneve encryption anyway. Real-multi-node + single-node e2e (both run admin init via sudo) continue to cover the IPsec path. * fix(ovn): atomic Ensure* primitives prevent duplicate logical entities OVN NB has no unique constraint on Name. The previous GetLogicalRouter→CreateLogicalRouter sequence in EnsureVPC was a TOCTOU race: when vpc.create on one node and the defensive vpc.create-subnet EnsureVPC call on another both observed absence, both inserted, yielding two routers per VPC. The orphan held DNAT references blocking EIP detach, and DNAT/flow installation landing on the orphan caused TestMultinodeGuestSSH SSH timeouts. Add EnsureLogicalRouter / EnsureLogicalSwitch / EnsurePortGroup to the libovsdb client. Each builds a single OVSDB transaction combining a wait-op (Until=equal, Rows=[]) gated on absence of a Name match with the insert. The server serialises the wait+insert atomically; concurrent callers see exactly one survivor and the rest fall through to a re-fetch that returns the canonical row. Switch all production callers (topology.EnsureVPC / EnsureSubnet / EnsureSGPortGroup, external.AttachIGW, reconcile.applyVPCs / applySubnets) to the Ensure* variants. Bare Create* remains available to the L1 wrapper. Add invariant test TestS4_OVNNamedCreatesUseEnsure (AST walk) to block future reintroduction of the bare Create* call sites outside network/ovn. Mock implements Ensure* under its existing mutex so the concurrency tests (50/30/30 goroutines for Router/Switch/PortGroup) prove a single UUID survives. Refs: mulga-siv-146 * fix(ovn): wait-op rows must be non-empty; flip to NotEqual predicate The Ensure* primitives sent a wait-op with `Rows: []ovsdb.Row{}` plus `Until: ==` (RFC 7047 §5.2.6 absence test). The libovsdb Operation struct tags Rows as `json:"rows,omitempty"`, so the empty slice was dropped on the wire and ovsdb-server rejected every transaction with "Required 'rows' member is missing", breaking reconciler bootstrap and every downstream create-port lookup ("logical switch X not found"). Flip the predicate to the equivalent `Until: !=` formulation with a single-row projection containing the Name we want absent: no row → projection [] != [{name}] → predicate holds row exists → projection [{name}] == [{name}] → predicate fails Same atomic guarantee, JSON now well-formed. Mock tests can't catch this — only a real ovsdb-server validates the envelope. * chore: remove generateMAC wrapper * chore: remove all bead / plan references * fix(network/policy): wait for chassis flows after AddNATGateway AddEIP fires m.barrier() to block until ovn-northd compiles NB→SB and every chassis installs the new dnat_and_snat flows; AddNATGateway returned right after the NB commit. So `aws ec2 create-route … --nat-gateway-id` would publish vpc.add-nat-gateway, the SNAT row would land in NB, and the API call would succeed before the gateway chassis had programmed the SNAT flow for the private subnet. Phase 8d's 10×5s=50s ping budget races that flow-install latency on slow CI hosts (last seen run 26455602615 single-node, identical flake on unmodified dev run 26434711010 — pre-existing, not introduced by Ensure*). Mirror AddEIP: call m.barrier() after AddNAT returns. Add unit test TestNATManager_AddNATGateway_FlowsBarrier_Fires covering the new path. * chore: remove more plan refs * chore: remove GenerateIPSecPeerCertIfEnabled * chore: remove dupliate subnetGatewayCIDR function * chore(network): trim superfluous comments across network/ tree Collapse multi-paragraph godocs to 1-2 sentences, drop WHAT-restatement comments, remove section dividers grouping <10 items, and cut prose narration in tests. ADR clause references, one-line WHY comments for non-obvious OVN/OVSDB/AWS/NATS semantics, and exported-symbol godoc are preserved. No behavioural changes. Packages: invariants, topology, host, external, ovn, reconcile, policy, subscribers. * fix(igw): restore attachment State="available" for AWS parity AWS InternetGatewayAttachment.State is "available" when an IGW is attached to a VPC, per the AWS SDK doc ("the state is available when attached to a VPC; otherwise, this value is not returned"). Commit b7505f8 wrote "attached"/"detached" to align with the reconciler filter, which inverted AWS parity. Fix the producer to write "available" again and update the consumer: - handlers/ec2/igw: AttachInternetGateway and DetachInternetGateway both persist State="available"; IGWRecord comment narrowed. - network/reconcile/intent.loadIGWs: filter accepts "available" (was "attached"); attachment is identified by VpcId != "". - tests updated: igw service_impl_test, routetable service_impl_test fixture, reconcile intent_test fixture. * Update spinifex.toml * refactor(subscribers): route EIP/NATGW through L5 wrappers (ADR-0006 §S1) Subscriber previously called policy.NATManager directly for EIP and NAT Gateway operations, bypassing the L5 wrappers (EIPManager, NATGWManager) that existed on paper. ADR-0006 §S1 requires every cross-layer call to pass through the immediate lower neighbor's typed interface; IGW already followed this pattern. Wire EIPManager and NATGWManager into the Subscriber struct, construct them in vpcd alongside IGWManager, and swap the four direct-L3 call sites (AddEIP, DeleteEIP, AddNATGateway, DeleteNATGateway) onto the L5 surface (AttachEIP, DetachEIP, AttachNATGateway, DetachNATGateway). AttachEIP now fires the flows-barrier, giving callers the same "reachable on return" guarantee IGW already had. The reconciler still calls policy.NATManager directly; that's a cross-cutter and a separate concern. * test(reboot): port 8MB WAL pad workaround from bash run-reboot-e2e.sh App VMs' user-data writes (unit file + index.html) sit in the on-host WAL until WriteWALToChunk crosses the 4MB threshold. Host reboot before that loses them — systemd then refuses to load the zero-byte unit and the responder never starts. Mirror the bash driver's dd+sync pad so the WAL flushes pre-reboot. Drop when the upstream WAL fix lands. * test(e2e): dump OVN/OVS/SG state on Phase 8d NATGW failure Phase 8d's existing failure path only prints lr-nat-list and lr-route-list. CI artifacts are not uploaded when run-e2e.sh exits nonzero, so debugging the NATGW regression requires inline diagnostics in the journal. Expand the dump to include nb/sb show, chassis bindings, port_group ACLs, br-int flows filtered to the NATGW IP/private subnet, host routes, and the private VM's own ip/route/neigh state. * fix(ovn,reconcile): scope NAT deletes to router; NATGW snat per associated subnet Two compounding bugs caused Phase 8d NATGW E2E flake (~60% CI failure): 1. ovn.DeleteNAT/DeleteNATByExternalIP searched all NAT rows globally and only mutated the named router afterwards. With AWS subnet CIDRs repeating across VPCs, (type, logicalIP/externalIP) is not globally unique — the delete picked a NAT row owned by a different router, the mutate was a no-op, and the row delete failed with "remaining reference(s)", leaving the original router's NAT orphaned. Both calls now resolve the LR first and filter NAT lookups to lr.NAT UUIDs. Mock client mirrors the scoping. 2. reconcile.loadNATGWs used the NATGW's home (public) subnet CIDR as SubnetCIDR, producing an snat for the wrong CIDR that collided with the correct event-published snat. loadNATGWs now walks route tables and emits one NATGWSpec per (NATGW, associated private subnet) pair, mirroring publishNatGatewayEventsForAssociation. Intent map keyed by natgwID|subnetCIDR so multi-subnet NATGWs produce distinct specs. Tests: - nat_test.go: cross-router same-logicalIP isolation for DeleteEIP and DeleteNATGateway - intent_test.go: NATGW snat uses associated private subnet CIDR, not the NATGW home subnet; NATGW with no associations produces no spec --------- Co-authored-by: Julian Sommer <115516918+juliansommer@users.noreply.github.com>	2026-05-27 21:05:01 +10:00
Josh Siviour	92fa467c90	refactor(e2e/multinode): flatten spread + parallel bucket + drop phase numbers (#245) ... * refactor(e2e/multinode): flatten phase 11 + parallelize bucket #1 + drop phase numbers Split monolithic phase11_PlacementAndNATGateway (~430 lines, 8 inline sub-steps) into 6 top-level Tests, each owning its own VPC graph via t.Cleanup and layered setup helpers (spreadSetupVPC, spreadSetupBastion, spreadSetupPrivateTrio, spreadSetupNATGW). The cleanup-ordering test flips *bool latches as each explicit teardown step succeeds so the LIFO unwind is a no-op on the success path. Add t.Parallel() to read-only / independent Tests: ClusterHealth, InstanceDistribution, GuestSSH, VPCNetworking. Deviates from the mulga-siv-127 bead's bucket-#1 listing by keeping CrossNodeGateway sequential (asserts a snapshot equality across gateways that the parallel VPC test would break by churning instance count mid-assert) and CrossNodeOps sequential (stops/starts trio[0], would race GuestSSH). Both deviations documented in tests_test.go + plan doc. Drop the bash phase numbering (phaseN_X funcs + "Phase N" log strings) from the rest of the suite — 10 funcs renamed to runX. Phase numbers were a port-time crutch; the Test names now describe each scenario and phase numbers ageing into noise as bash is retired in Stage H. * refactor(e2e/multinode): collapse spread Tests + control execution order Two changes wedged together since they touch the same files: 1. Collapse the 6 top-level TestMultinodeSpread* Tests into a single TestMultinodeSpread with 6 t.Run sub-tests sharing one VPC + bastion + private trio + NAT GW setup chain. The 6-test split paid setup cost 6× — bastion launch + private trio launch + NAT GW provision per Test pushed phase-11 wall-clock near 60min vs ~15min monolithic. Shared- setup layout keeps JUnit granularity (one row per sub-test) while restoring the monolithic budget. Setup helpers register t.Cleanup against the outer test so teardown runs once at the end (LIFO: NAT GW → trio → bastion → VPC). 2. Move the TestMultinodeSpread wrapper into tests_test.go so file-source order controls execution position. Previously the func lived in placement_nat_test.go which sorts before preflight_test.go and tests_test.go alphabetically — Spread ran FIRST in the sequential bucket, defeating Preflight's fail-fast on a sick cluster. Wrapper now runs after NodeRecovery so the cluster has been fully degrade-tested before Spread's 4-VM + NAT GW + custom-VPC graph spins up.	2026-05-21 20:53:59 +10:00
Josh Siviour	bedc35344f	feat(e2e/multinode) (#242) ... * feat(e2e/harness): multinode cluster wait helpers + AWSClientForGateway (Stage A) Stage A of mulga-siv-90. Compile-only; no tests wired yet. multinode_cluster.go: - Cluster.WaitNATSPeers polls /routez per node, dedupes by remote_id. want=2 on healthy 3-node, =1 after a node-down event. - Cluster.WaitPredastoreHealthy / WaitGatewayHealthy probe HTTPS per node via insecureHTTPClient (multinode certs are self-signed across peers). - Cluster.WaitDaemonReady runs DescribeInstanceTypes through every node's gateway, not just env.ServiceIPs[0] — catches a daemon that's only registered on one node. multinode_aws_via.go: - AWSClientForGateway clones *Env, swaps ServiceIPs to a single node's Addr, returns a fresh AWSClient. Lets phases 6/7 issue describe/ stop/start across every node in turn (bash aws_via equivalent). Init/Join helpers deferred to the pseudo-multinode bead (mulga-siv-91) where they belong — real-multinode bootstrap is owned by tofu + bootstrap-install.sh before the Go suite starts. * feat(e2e/multinode): TestMultinodePreflight + TestMultinodeClusterHealth (Stage B) Stage B of mulga-siv-90. First runnable Test* against a 3-node cluster. multinode/main_test.go: - Package-scoped singleton fixture, mirrors single/main_test.go. - requireMultiNodeFixture skips when SPINIFEX_E2E unset or mode != multinode. - Fixture = {Env, AWS, Harness, Cluster, Artifacts, TmpDir} — adds Cluster to the single-node shape via harness.ClusterFromEnv (reads SPINIFEX_NODES + SPINIFEX_SSH_USER/KEY). - TestMain drains harness.Fixture.Close + removes TmpDir at process exit. multinode/helpers_test.go: - needAZ / needInstanceTypeArch / needAMI shims (memoized via harness). - readyNodeCount substring counter — matches bash `grep -c "Ready"`. multinode/tests_test.go: - Top-level Test* wrappers (TestMultinodePreflight, TestMultinodeClusterHealth). multinode/preflight_test.go (phase 1): - /dev/kvm writable check (no /dev/kvm = test environment is wrong, fail fast). - peer_ssh hostname on every remote node. Skips node[0] to match bash — the runner's loopback SSHd is not part of the contract. multinode/cluster_health_test.go (phase 2): - Cluster.WaitNATSPeers >= 2 per node. - Cluster.WaitPredastoreHealthy / WaitGatewayHealthy across nodes. - Cluster.WaitDaemonReady (DescribeInstanceTypes per gateway). - spx get nodes shows N Ready, where N = cluster size. - spx get vms returns (length only — content may be empty). * feat(e2e/multinode): TestMultinodeInstanceDistribution + GuestSSH (Stage C) Stage C of mulga-siv-90. Phase 3 (distribution) + phase 4 (guest SSH). harness/multinode_distribution.go: - InstanceHostingNode: SSH each peer, ps grep qemu-system | <id>. Reuses NewPeerSSH; returns nil on no match (caller decides fatality). - CountInstancesPerNode + AssertSpreadAcrossNodes: fan-out + minNodes gate. Bash phase 3 warns-on-stacking; Go matches by accepting minNodes=1 + caller-side log. harness/multinode_guest_ssh.go: - GuestSSHPort polls hosting node's qemu cmdline for hostfwd=tcp:... reusing the existing hostfwdRE from ec2helpers.go (single regex, two capture groups: host + port). - GuestSSHEndpoint prefers PublicIpAddress, falls back to hosting-node + hostfwd. Mirrors the phase 4 conditional. - GuestSSHReady polls `echo ready` via SSH with bash-equivalent -o flags. multinode/helpers_test.go: - needKeyPair (EnsureKeyPair shim). - needInstanceTrio: pkg-level sync.Once launches 3 nano instances on default VPC, registers terminate-on-process-exit. Phase 4+ reuse the same trio without re-launch. multinode/distribution_test.go (phase 3): - launch trio + assert >= 1 node hosting (log warn on stacking). - spx get vms contains every trio ID. multinode/guest_ssh_test.go (phase 4): - For each trio instance: resolve endpoint, poll SSH ready, assert id reports ec2-user, log lsblk line count. multinode/tests_test.go: TestMultinodeInstanceDistribution + TestMultinodeGuestSSH. * feat(e2e/multinode): TestMultinode{Volume,CrossNodeGateway,CrossNodeOps} (Stage D) Stage D of mulga-siv-90. Phases 5/6/7 — pure AWS API exercise (no node-state churn). volume_test.go (phase 5): - Create 10GiB volume in discovered AZ, attach to trio[0] at /dev/sdf, detach, delete. Inline lifecycle — DeleteVolume in-test would race a registered Ensure* cleanup, surface as InvalidVolume.NotFound. gateway_test.go (phase 6): - Baseline DescribeInstances via the local gateway, then fan out AWSClientForGateway per non-local node. Require.Equal across all — cross-node fan-out regression surfaces immediately. crossnode_ops_test.go (phase 7): - Find trio[0]'s hosting node, stop via a different gateway, start via a third (or same other on 2-node fallback). Returns to running so later Test*s (phase 4 SSH, etc.) don't cascade-fail. otherTwoGateways mirrors the bash fallback chain. * feat(e2e/multinode): TestMultinodeNode{Failure,Recovery} (Stage E) Stage E of mulga-siv-90. Phase 8 (failure) + phase 9 (recovery) + diagnostic primitives. harness/multinode_node_lifecycle.go: - StopNode SSHes `sudo systemctl stop spinifex.target`. Non-fatal on exit error (bash matches — cluster shutdown races the SSH conn). - StartNode mirrors with sudo systemctl start. - WaitNodeServiceReady polls a single node's HTTPS gateway. Distinct from cluster-wide WaitGatewayHealthy so a recovering node can be tracked without re-checking peers. harness/multinode_diag.go: - DumpAllNodeLogs fans out journalctl -u 'spinifex-*' per node into artifactsDir/journal-<node>.log. Tolerates per-node SSH failure. - SpxGetNodesAcrossCluster strips trailing whitespace off SpxGetNodes. multinode/node_failure_test.go (phase 8): - Stop node2, register t.Cleanup restart safety net. - WaitNATSPeers want=1 (degraded), DescribeInstanceTypes via node1+node3, DescribeInstances still returns the trio. multinode/node_recovery_test.go (phase 9): - StartNode node2 (idempotent — phase 9 in isolation still works). - WaitNATSPeers want=2, WaitNodeServiceReady, spx get nodes 3 Ready, DescribeInstanceTypes via the recovered gateway. * feat(e2e/multinode): TestMultinodeVPCNetworking (Stage F) Ports phase 10 from run-multinode-e2e.sh. Stands up a fresh non-default VPC + subnet (10.200.0.0/16 + 10.200.1.0/24), launches 3 instances into the subnet, then asserts: - PrivateIpAddress is non-empty and matches 10.200.1.0/24 - All 3 IPs are unique - SubnetId / VpcId on the instance match the freshly-created ids - IPs persist across stop -> start cycles Independent of the package-level trio (which uses the default VPC) — this phase needs full control over subnet allocation so resources are created and cleaned up inline via t.Cleanup (LIFO: instances -> subnet -> VPC). * feat(e2e/multinode): TestMultinodePlacementAndNAT (Stage G) Ports phase 11 from run-multinode-e2e.sh — the largest single phase (~340 bash lines covering spread placement + NAT Gateway egress). Eight sub-steps preserved with matching harness.Step markers so the Go run cross-references cleanly with the bash log: Step 1: VPC + pub/priv subnets + IGW + main RT route + 2 SGs (bastion: tcp/22 anywhere; private: tcp/22 from bastion-sg, icmp from VPC) Step 2: bastion launch, wait SSH, scp key Step 3: spread placement group + 3 private instances via the group Step 4: validate spread (>=2 unique hosting nodes; >=3 = full PASS) Step 5: pre-NAT baseline — confirm NO internet from private VMs Step 6: allocate EIP + create NAT GW + private RT + 0.0.0.0/0 -> NGW Step 7: post-NAT — every private VM reaches 8.8.8.8 via NAT GW Step 8: inline teardown of NAT GW + private RT + EIP Each resource is owned by the test (no Ensure* memoization) so a failure mid-flight tears the full VPC graph down via t.Cleanup in LIFO order, matching the bash teardown sequence. NAT GW state polling is duplicated locally (waitForNATGatewayStateFatal/Best); the equivalents in single/natgw_test.go are unexported. Promote to harness/poll.go once a third consumer lands. * ci(e2e-go): add multi-node Go E2E job + cert/lb on 3-node cluster (Stage H) Adds e2e_go_multinode as a parallel job alongside e2e_go_single on the e2e-go workflow. The single-node job is untouched apart from extending its SUITES filter to drop the multinode suite (which only runs on the ci-multi cluster). What the new job does: - Provisions a 3-node tofu cluster via the same build-install-artifacts + bootstrap-install path as e2e_real_multi (bash). - Builds multinode.test / cert.test / lb.test on the runner, ships them to node1. - Runs each suite with SPINIFEX_MODE=multinode and the cluster csv in SPINIFEX_NODES / SPINIFEX_NODE_IPS. cert / lb branch internally on env.Mode so the same binaries cover both single + multi. - Pulls per-node journals (spinifex-journal-node1..N.log) for the e2e-analyze action so a node2/3 failure isn't blind. - Same JUnit + analyze + artifact-upload + status pipeline as the single-node job. Other moves: - Default SUITES now includes e2e-multinode so push triggers fan out across single + multi automatically. e2e_go_reboot stays explicit-opt-in (unchanged). - tests/e2e/Makefile: e2e-multinode now sets MODE=multinode so `make e2e-multinode` doesn't fall back to single (which would skip every TestMultinode* on the SPINIFEX_MODE check). - tests/e2e/TEST_COVERAGE.md: cross-reference the Go port at the head of the Real Multi-Node section. The bash driver stays in place as executable spec until the Go run is soak-tested. - e2e_go_status now needs e2e_go_multinode; multi=skipped counts as success so a single-only dispatch still posts green. * fix(e2e/multinode): NATS routez via peer_ssh + StartNode timeout + spx best-effort CI run 26161792146 surfaced three regressions in the freshly-landed multinode suite: 1. WaitNATSPeers dialled node.Addr:8222 directly. NATS monitor binds 127.0.0.1:8222 only (cmd/spinifex/cmd/templates/nats.conf:65), so every routez poll returned "connection refused". Switched to peer_ssh + curl http://127.0.0.1:8222/routez per node — matches the bash run-multinode-e2e.sh phase 2 pattern. 2. StartNode / StopNode capped ssh at a 30s context. spinifex.target has 6+ dependent units (predastore, NATS, awsgw, vpcd, daemon, ui) and a clean systemctl start runs 60-90s; the 30s budget was killing ssh mid-command ("signal: killed"). Bump StartNode to 5min and StopNode to 3min. 3. SpxGetVMs is best-effort in bash (`spx get vms --timeout 5s 2>/dev/null` with no exit-code check) because the CLI's NATS dial can race the cluster join shortly after bootstrap. The Go port fatal'd on non-zero exit and broke TestMultinodeClusterHealth + TestMultinodeInstanceDistribution downstream. Added harness.SpxRunBestEffort and rewired both call sites. * fix(e2e/multinode): spx CLI lenience + trio capacity retry + diag Run 26163994815 cascaded from two issues that bash silently tolerated. The Go port hard-failed where bash counts the failure and continues, so 3+ tests cascaded off transient first-failures. - spx get nodes: cluster_health + node_recovery now downgrade <Ready count to WARN with SpxRunBestEffort, matching bash phase 2 fail_test counter and phase 9 `|| echo ""`. Gateway+daemon health checks above already prove end-to-end NATS routing. - needInstanceTrio: retry RunInstances up to 6× on InsufficientInstanceCapacity (10s backoff). Daemon resourceMgr can briefly report 0 capacity right after node bootstrap before its first inventory scan completes — AWS itself surfaces this transiently. Stagger 2s between launches to match bash phase 3. - CI workflow: new "Pre-flight diagnostics on node1" step dumps groups/perms/spx output/systemctl status/journal tail to preflight-diag.log so next run's artifacts surface what tf-user sees before the test binary starts. * fix(e2e): strip SPINIFEX_* from spx child env Root cause of `spx get nodes` returning "nats: no servers available for connection" in the multinode Go suite (mulga-siv-90 runs 26163994815, 26195522455): spx CLI calls viper.AutomaticEnv() with prefix "SPINIFEX" inside config.LoadConfig. The CI workflow exports SPINIFEX_NODES (csv of cluster WAN IPs) so harness.ClusterFromEnv can build the *Cluster. viper picks SPINIFEX_NODES up as the `nodes` config key and silently clobbers the loaded `[nodes.*]` map → nodeConfig.NATS.Host = "" → nats.Connect("") → ErrNoServers. Bash never set SPINIFEX_NODES so the bash port was unaffected. Direct ssh `spx get nodes` from the runner also worked because the env was clean. Only the test-binary-spawned spx hit the collision. Fix: SpxRun / SpxRunBestEffort build a sanitized child env that preserves the small SPINIFEX_* set spx's root command explicitly BindEnvs (config path, AWS keys, host, base-dir, NATS dial overrides) and drops everything else. * fix(vpcd): make handleAddNAT idempotent (mulga-siv-124) PR #232 (commit 5887927) added a second vpc.add-nat publish in daemon/vm_adapters.go:onInstanceUpHook to cover instance recovery. The publish fires on every instance up event including fresh launches, racing the original publish from handlers/ec2/instance/service_impl.go. Both publishes are queue-grouped to "vpcd-workers" so two cluster vpcd instances handle them ~13s apart. handleAddNAT's existing DeleteAllNATsByExternalIP -> AddNAT sequence is not idempotent under concurrent writers: the second invocation deletes the rule the first installed (after ovn-northd compiled SB flows and chassis installed them), creating a flow-install gap during which the EIP is unreachable. Symptom: TestMultinodePlacementAndNAT fails with ssh: Connection timed out to the bastion EIP for 150s (mulga-siv-90 CI runs 26195522455, 26197248656). Bash port last green on e2e-baremetal run 26008057481 before #232 merged. Fix: before delete+add, look up the existing rule via FindNATByExternalIP. If it exists and its LogicalIP + spinifex:vpc_id ExternalID (plus ExternalMAC + LogicalPort under distributed-NAT mode) already match the target, no-op. Falls through to the existing delete-then-add for the IP-reuse-across-VPCs case the original code already handled. Adds TestTopologyHandler_AddNAT_Idempotent covering the UUID-witness + flow-barrier counter (two adds with identical state must not recreate the row or re-arm the barrier). * test(e2e/multinode): bump bastion SSH wait + ICMP probe Run 26198221557 confirmed the vpcd dnat_and_snat race (mulga-siv-124) was fixed — no "cleaned up stale NAT" lines for the bastion EIP, only one writer + an idempotent skip. But TestMultinodePlacementAndNAT still timed out at exactly 150s on SSH to the bastion's public IP, suggesting a separate OVN flow-install lag on the freshly-created VPC's external switch. - Bump waitForBastionSSH budget 150s -> 5min. Default-VPC GuestSSH reaches sshd in ~70s; a brand-new VPC needs longer for ovn-northd to compile SB + every chassis to install the external ARP responder + dnat_and_snat flow. 5min covers cloud-init + OVN warm. - Add an ICMP probe per iteration. Pure diagnostic — ping failure does not fail the step, but the next iteration's SSH error carries ping_output so the log splits "datapath down" (ping fail) from "sshd not up" (ping ok, ssh refused/timeout). Matches the RCA agent's fix candidate 3. * test(e2e/multinode): dump cluster datapath state on phase 11 failure waitForBastionSSH currently fails opaquely when the EIP datapath is broken — the log shows "ping=fail" but doesn't pin which node owns the gw chassis or whether ARP / OVN NAT rules / chassis encap IPs are sane. Register a t.Cleanup hook (after bastionPubIP is known) that fans out arp / ip route / ovs-vsctl / ovn-sbctl / ovn-nbctl probes to every cluster node and writes results to <Artifacts>/diag-<node>-<probe>.log on failure only. Plus a local-side ping / traceroute / ip route block so a future post-mortem can distinguish runner-network breakage from cluster datapath breakage. Best-effort: per-node SSH errors log but never re-fail the test. * test(e2e/multinode): use IpPermissions form for SG authorize calls Two AuthorizeSecurityGroupIngress calls in phase 11 used the legacy flat form (top-level IpProtocol/FromPort/ToPort/CidrIp) instead of the IpPermissions array. The Spinifex VPC handler at spinifex/handlers/ec2/vpc/security_group.go:667 reads only input.IpPermissions and silently drops the flat fields — newRules=0, no error returned. Run 26200626265 journal: groupId=sg-922de57bc74b85c9c (natgw-bastion) newRules=0 groupId=sg-758b05b4a5e7161fd (natgw-private) newRules=1 Bastion SG had zero ingress rules; TCP SYN to bastion :22 was dropped by ACL, causing waitForBastionSSH to time out after 5min. Bash version passes because AWS CLI v2 normalizes --protocol/--port/--cidr shorthand into IpPermissions.N.* on the wire. Fix: re-shape both calls (bastion tcp/22, private icmp) to use IpPermissions, matching the third authorize call in the same function (line 163-175) that already works. Server-side fix to honor flat form follows in a separate bead.	2026-05-21 15:40:00 +10:00