mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
Josh Siviour
45555dde49
* test(ddil): flip Scenarios A/B/C to real assertions Replaces the t.Skip stubs landed in #204 with the assertions described in docs/development/improvements/ddil-e2e-test-harness.md §3: - A NATSKill — KillNATS on node3, assert ModeStandalone within 30s, /local/instances still serves on the isolated node, majority /health on node1+node2 stays up, witness counters on all three nodes keep advancing, snapshot survives reconnect. - B DaemonRestartWithoutNATS — KillNATS + RestartDaemonOnly, assert ModeStandalone within 30s of restart, recovered instances present with live QEMU PIDs, cluster rejoin after StartNATS. Strict pre/post PID equality dropped — see file comment; recovery + a non-zero PID give the same regression coverage without coupling to the daemon unit's KillMode. - C CleanPartition — PartitionNode peer-IP DROP, majority daemons keep serving /health and /local/instances, isolated node3 reports ModeStandalone with /local/* responsive, witness counter on the partitioned side keeps advancing, heal converges and snapshot is preserved. Supporting changes: - harness/daemon_client.go — daemonPort 8443→4432; 8443 is predastore, the deployed daemon ClusterManager binds [nodes.<id>.daemon].host. The Phase-1 stub used a placeholder that would have blocked any /local/* assertion against the real cluster. - harness/witness.go — export WitnessVM.Terminate so scenarios can clean up their counter VMs from t.Cleanup. - scenarios/main_test.go — requireLiveCluster + launchWitnesses helpers to keep the three flipped scenarios tight. - TEST_COVERAGE.md — A/B/C status flipped SKIPPED → ENABLED; TestCoverageDrift continues to enforce alignment with the package. Validation: built with -tags=e2e, dry-run scenarios pass, coverage drift passes, preflight clean. workflow_dispatch e2e-ddil.yml against tofu cluster is the next step. * fix(ddil): countDropRules — single iptables -S call + filter Phase 1's countDropRules ran `iptables -S INPUT -S OUTPUT` to grep DROPs across both chains, but iptables -S takes a single optional chain — the second -S errors to stderr, the combined SSH buffer poisons the grep input, and Atoi fails ("ip" is what we saw on tofu-cluster). Hit by TestHarnessSmoke and every flipped scenario (A/B/C) via AssertCleanState. Switch to one `iptables -S` (lists every chain in iptables-save format), filter for INPUT/OUTPUT lines, count `-j DROP`. `2>/dev/null` silences sudo/iptables stderr so a noisy MOTD or warning cannot leak into stdin. Error path now also surfaces the raw SSH output for the next time this breaks somewhere unexpected. * ci(e2e-ddil): export spinifex AWS creds for witness SDK The witness EC2 client (harness/witness.go NewWitness) calls session.NewSession with no static credentials, so the AWS SDK falls through its default chain — env vars → shared profile → IAM role — and on the CI runner none of those exist. Result on the live cluster: "NoCredentialProviders: no valid providers in chain. Deprecated." launches every Scenario A/B/C attempt straight to t.Fatal at launchWitnesses. spx admin init writes [spinifex] aws_access_key_id / aws_secret_access_key to the operator's ~/.aws/credentials on node1 during bootstrap. Pull them over SSH and export as AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY so the SDK's env-var step resolves before it gets to the empty profile/IAM checks. Hard-fail the step if either field is missing — better than silently watching every scenario t.Fatal on the same line. * fix(ddil): CI green-up — instance-type discovery, gotestsum, EC2 envelope Three fixes to take the DDIL E2E suite from "scenarios flipped" to actually green on the tofu cluster. mulga-siv-56. 1. Witness auto-discovers the smallest registered instance type (DescribeInstanceTypes, sort by memory then vCPUs). The hard-coded t2.micro default returned InvalidInstanceType on every Scenario A/B/C launch because the cluster doesn't register that type. DDIL_WITNESS_- INSTANCE_TYPE stays as an operator override. 2. e2e-ddil.yml uses gotestsum so the Actions UI shows per-test PASS/FAIL/ SKIP lines instead of a JSON wall. results.json artifact preserved for future flake-rate dashboarding. 3. GenerateEC2ErrorResponse emits the real EC2 query-API envelope <Response><Errors><Error>... instead of the IAM-shaped <ErrorResponse>. aws-sdk-go v1's ec2query handler rejects the latter with SerializationError and drops the code, leaving Go callers without an awserr.Code() to inspect. New SDK round-trip test in gateway_test.go guards the regression. * ci(e2e-ddil): install gotestsum from prebuilt tarball `go install gotest.tools/gotestsum@v1.12.0` failed on the self-hosted runner because v1.12.0 pulls golang.org/x/tools v0.19.0, whose tokeninternal static-assert pegs a narrow Go window (~1.22). Both older and newer runner Go versions trip `invalid array length -delta * delta`. A prebuilt linux_amd64 binary sidesteps toolchain coupling entirely without changing the gotestsum version pin. * ci(e2e-ddil): collect cluster logs on failure Witness `findHost` is currently failing with "could not locate QEMU host for i-XXX across 3 nodes" — daemon reports running but no QEMU fork is visible on any node. Cluster gets destroyed during teardown, so logs have to be pulled before that runs. Adds a failure-only step that ssh's each node and bundles journalctl spinifex-*, process listings, and state dirs into the ddil-results artifact for offline triage. * fix(ddil): support tap-mode QEMU networking in witness harness findHost no longer extracts hostfwd port — production launches go through auto-allocated ENI on the OVS bridge and emit `-netdev tap,...`, never hostfwd. Witness was timing out across all 3 nodes. Discover guest IP via DescribeInstances.PrivateIpAddress (populated by PrepareRunInstances on ENI auto-create) and tunnel SSH through the host node to guestIP:22. Host routes to the tap bridge subnet, so the relay works without runner-side bridge routing. WitnessVM.SSHPort → WitnessVM.GuestIP. hostfwdPortRE removed. Closes mulga-siv-57. * fix(ddil): SSH witness via public IP, drop unreachable tunnel path tofu-cluster bootstraps with --external-mode=pool which forces dev_networking=false → no QEMU hostfwd dev NIC and no host route to the ENI's PrivateIpAddress (tap on OVS br-int, 172.31.0.0/16 invisible to host kernel). Previous tunnel-via-host fix could find the host but couldn't dial the guest. Switch to the path shell e2e already uses: default subnet has MapPublicIpOnLaunch=true, daemon auto-allocates an EIP from the external pool, and the runner reaches that IP via the same WAN subnet as the cluster nodes. No host relay required, survives Scenario C (partition only DROPs peer cluster IPs, not the runner's source IP). Workflow now scp's ~/.ssh/spinifex-key from the primary node (the counterpart to the public material bootstrap-install.sh imports) and exports DDIL_GUEST_SSH_KEY so the harness can auth as ubuntu against cloud-init's authorized_keys. WitnessVM.GuestIP → WitnessVM.PublicIP. hostSigner / dialHost removed. Continues mulga-siv-57. * fix(ddil): tunnel guest SSH through host node EIP from MapPublicIpOnLaunch sits on the cluster's external pool subnet, which is OVN-routed — reachable from any cluster node, not from the off-cluster runner sitting on the DDIL_NODES WAN subnet. Restore the host-jump pattern: SSH the hosting node (already known from findHost), dial publicIP:22 over that connection, run a guest SSH handshake over the tunnel. Symmetric with how shell e2e reaches guests from inside a node. Continues mulga-siv-57. * fix(ddil): wait for witness sshd before baseline read Previous run reached the guest EIP via the host tunnel but hit 'Connection refused' — sshd hadn't bound yet. waitForRunning only checks EC2 state, which flips on QEMU start, not cloud-init completion. The OVN dnat_and_snat rule for the auto-allocated EIP is also written asynchronously, so the tunnel can land before either layer is ready. awaitGuestSSH polls the host-tunnelled SSH handshake to publicIP:22 for up to 90s, retrying on transient transport errors (refused/reset/EOF/ timeout/no-route) and failing fast on auth errors so a bad key surfaces immediately. Called once after resolvePublicIP, before the baseline counter read. Continues mulga-siv-57. * fix(ddil): case-insensitive transient SSH error match awaitGuestSSH's retry matcher used lowercase substrings, but x/crypto/ssh emits "ssh: rejected: connect failed (Connection refused)" with capital C — strings.Contains returned false and the helper bailed on first attempt instead of polling. Last run failed in 49s, not the 90s budget. Lowercase the error before matching, and add "connect failed" as the canonical signal for "jump host dial through tunnel failed". Continues mulga-siv-57. * fix(ddil): default guest SSH user to ec2-user Shell E2E SSHes the same ami-ubuntu-* images as ec2-user (tests/e2e/run-multinode-e2e.sh:650). DDIL harness defaulted to ubuntu and hit 'unable to authenticate, attempted methods [none publickey]' — authorized_keys is provisioned under the AMI default account, which is ec2-user, not ubuntu. Continues mulga-siv-57. * fix(ddil): wait for witness-counter.service to seed /var/lib/counter awaitGuestSSH passed at sshd-up, but cloud-init's runcmd that enables witness-counter.service runs in the final cloud-init stage — after sshd binds. Baseline read landed in the window where sshd accepts but ExecStartPre hasn't created /var/lib/counter: cat: /var/lib/counter: No such file or directory Collapse awaitGuestSSH + readCounter into awaitBaseline that polls the full path (tunnel + SSH handshake + cat) and retries on the transport errors plus "no such file or directory". Single 2-minute budget covers cloud-init final stage on tofu-cluster hardware. Continues mulga-siv-57. * fix(systemd): drop Wants=spinifex-nats from daemon unit Wants= reactivates a stopped unit on `systemctl restart`, so a deliberately killed spinifex-nats was getting pulled back up the moment the daemon restarted — masking the DDIL Tier 1 standalone-startup path and breaking the Scenario B assertion (mulga-siv-58). Cold-boot coverage stays intact: spinifex.target's Wants= activates every spinifex unit on boot. After= remains for ordering and is non-blocking when NATS is absent. scripts/test-ddil-phase1.sh hit the same trap and worked around it with a runtime drop-in plus sentinel file; the fix here makes that workaround unnecessary for any caller that restarts the daemon while NATS is stopped. * feat(daemon): peer-health probe drives standalone mode on partition Mode() is now derived from two atomic signals — natsConnected (NATS client TCP link, flipped by the existing disconnect/reconnect callbacks) and peersReachable (any peer daemon's /health responded in the most recent probe cycle). Reports cluster iff both are up; standalone otherwise. The peer-health probe is the missing signal for DDIL Scenario C: an iptables DROP between peer IPs blocks NATS cluster-routing traffic but leaves each node's local NATS client TCP connection healthy, so nats.Conn.IsConnected() keeps returning true and onNATSDisconnect never fires. The probe dials each peer's /health over the cluster network with a 2s timeout on a 2s ticker; worst-case detection of total partition is ~6s on a 3-node cluster, well inside the 30s Scenario C budget. Single-node clusters keep peersReachable permanently true (nothing to probe) so the derived mode flips to cluster as soon as the local NATS connect succeeds. The probe runs from startLocal so the partition signal exists even before (or in the absence of) a successful NATS connect. It does not touch NATS or JetStream, so the §1e Tier 1 invariant asserted by assertNoClusterServicesInitialised is unaffected. Tests: - TestMode_RequiresBothSignals — every nats×peers combination. - TestProbePeersOnce_FlipsOnHealthChange — deterministic probe-only test. - TestProbePeerHealth_RejectsBadAddr — fails closed on missing addresses. - TestMonitorPeerReachability_EndToEnd — ticker-driven false→true. Closes mulga-siv-58. * fix(daemon): peer probe falls back to local daemon port `spx admin init` only renders `[nodes.<self>.daemon]` in spinifex.toml; remote-node blocks carry just `host = "<ip>"` with no daemon sub-table. So `peer.Daemon.Host` was always empty in a deployed cluster and the probe failed closed, leaving peersReachable permanently false. Tofu-cluster run 25665158964 surfaced this as a Scenario A regression: after StartNATS, natsConnected flipped true on reconnect but Mode() stayed standalone because the derived signal required peersReachable. `a_nats_kill_test.go:75: node3 did not return to cluster mode … (still reporting another mode)`. Add peerDaemonPort(p) that prefers an explicit peer Daemon.Host port when present and otherwise reuses the local daemon's port (clusters are symmetric — every node binds the same port the template renders). New TestPeerDaemonPort_FallbackToSelf covers both branches plus the no-port- available case; TestProbePeerHealth_RejectsBadAddr is updated to clear self config so it still asserts the fail-closed path. * fix(daemon): SIGTERM preserves VMs + KillMode=process on the unit DDIL Scenario B asserts that `systemctl restart spinifex-daemon` preserves every guest VM so the new daemon can reattach via the persisted local state file. The previous setupShutdown SIGTERM handler called vmMgr.StopAll(), gracefully QMP-powering-down every guest before the daemon exited — ~90s wall-clock and zero surviving QEMU on the other side. CI run 25666369604 surfaced this as `recovered instance i-... has no live QEMU PID`. Two changes restore the Tier 1 contract: - setupShutdown no longer calls StopAll. It still flips d.shuttingDown so crash/restart handlers bail out, persists state, unsubscribes from NATS, and exits. VMs are only stopped when coordinated cluster shutdown drains them — handleShutdownDrain still calls StopAll and flips the flag before the SIGTERM path runs. Operator-visible behaviour change: `systemctl stop spinifex-daemon` leaves guest VMs running. To stop VMs, use the cluster-shutdown topics or terminate via the AWS API. - The systemd unit gains `KillMode=process` so systemd only signals the daemon's main PID. With the default control-group mode, QEMU children were killed when systemd tore down the daemon's cgroup — defeating the SIGTERM change above. `TimeoutStopSec=15` bounds the new fast-exit grace window. Also nil-guards natsConn.Close in the shutdown path: when a daemon restarts while NATS is unreachable (Scenario B), natsConn is nil and the close would panic. Mulga-siv-58 (reopened). * fix(vm): QEMU stdout/stderr to shared log file (SIGPIPE survives daemon restart) QEMU stdout/stderr were piped to daemon-side scanner goroutines. When the daemon exits (e.g. `systemctl restart spinifex-daemon`), the pipe read ends close; QEMU's next write triggers SIGPIPE and the guest dies — defeating DDIL Scenario B's "daemon restart preserves running instances" guarantee. Open `<runtimeDir>/qemu.log` and assign it to cmd.Stdout/cmd.Stderr instead. The FD is dup'd into QEMU at fork, so closing it on the daemon side after Start() leaves QEMU writing into the file indefinitely. POSIX O_APPEND makes concurrent QEMU writes interleave atomically per line, so a single shared log is safe and avoids per-instance clutter in RuntimeDir. Trade-off: QEMU lines no longer land in journalctl -u spinifex-daemon. Operators inspect <runtimeDir>/qemu.log directly (path logged at instance start under the qemu_log field). mulga-siv-58 (DDIL Scenario B iteration 4) * fix(daemon): pin XDG_RUNTIME_DIR=/run/spinifex; revert SIGPIPE workaround Root cause of Scenario B's "no live QEMU PID" was not SIGPIPE — QEMU ignores SIGPIPE (SIG_IGN set early in QEMU init). It was systemd's `PrivateTmp=yes` on the daemon unit combined with `utils.pidPath()` falling through to `os.TempDir()` (=/tmp): - spinifex-daemon HOME=/var/lib/spinifex/spinifex, so `$HOME/spinifex` (=/var/lib/spinifex/spinifex/spinifex) does not exist and pidPath() returns /tmp. - Every daemon start gets a fresh private /tmp namespace. QEMU stays in the OLD namespace after the daemon dies (mount namespaces aren't changed by reparenting). The new daemon's /tmp is empty. - `/local/instances` reads `<pidPath>/<id>.pid`; in the new namespace the file doesn't exist, so PID=0. Fix: set `Environment=XDG_RUNTIME_DIR=/run/spinifex` on the daemon unit. /run/spinifex is host-wide tmpfs (declared via tmpfiles.d, already in ReadWritePaths) and survives daemon restart. pidPath() short-circuits on XDG_RUNTIME_DIR, so QEMU pidfiles, QMP sockets, and RuntimeDir() all resolve to the stable path. Both daemon incarnations and QEMU children see the same files. Revert iteration 4's pipe→file QEMU log change (commit8a9a6f9f). SIGPIPE was never killing QEMU, and the file-redirect dropped QEMU lines from `journalctl -u spinifex-daemon` for no gain. mulga-siv-58 (DDIL Scenario B iteration 5) * feat(daemon): siv-60 heal reconcile — phase 1 (probe-heal trigger) (#211) * feat(daemon): wire reconcileOnHeal on peer-probe heal edge (siv-60 phase 1) Scenario C heal triggered zero resync: the local NATS client stayed connected to its loopback server throughout the partition, so onNATSReconnect never fired. The peer-probe false→true transition was the only heal signal but had no resync action wired to it. Adds reconcileOnHeal, called from both edges and coalesced via a reconciling atomic.Bool so the two callbacks landing on the same heal don't double-push. For phase 1 the body is the existing WriteState (re-asserts our per-node row in the instance-state KV). Phase 2 will extend it to pull KV → merge → push for the stopped/terminated buckets and re-fire per-node bootstrap. Plan: docs/development/feature/ddil-heal-reconcile.md * feat(daemon): siv-60 phase 2 — idempotent default-VPC bootstrap re-fire on heal Calls ensureDefaultVPCInfrastructure() from reconcileOnHeal after the WriteState push. Idempotent: each branch (default VPC lookup, IGW attach, default-SG create, default route insert) checks for existing infrastructure before creating. Covers the failure mode where startCluster ran during partition and the initial creates returned errors against an unreachable NATS — without a heal-time retry the cluster would be stuck without a default VPC until the next daemon restart. Scope note: the bead's "pull KV → merge → push" assumed the daemon cached peer-node running state, stopped instances, terminated instances, and VPC/IGW/SG state locally. The codebase audit (see plan doc) confirms it doesn't — every consumer reads its KV bucket fresh per request, and vmMgr only tracks VMs running on this node. The bootstrap re-fire is the one heal-time action that has a real in-memory target. Watchdog sweep is unnecessary because vm.Manager.CrashHandler is event-driven from cmd.Wait(). * fix(daemon): drop ensureDefaultVPCInfrastructure from reconcileOnHeal (siv-60 follow-up) (#212) * fix(daemon): drop ensureDefaultVPCInfrastructure from reconcileOnHeal DDIL Scenario C regressed at the witness-SSH step after siv-60 landed in #211. Root cause: the first peer-probe tick on multi-node startup flips peersReachable false→true (its zero value is false), so reconcileOnHeal fires concurrently with startCluster's own ensureDefaultVPCInfrastructure call. The Describe→Create→Attach sequence is not cluster-wide singleton, so multiple daemons + double calls per daemon raced and produced duplicate IGW attachments, corrupting default-VPC routing — which broke the witness VM's tunnel SSH path once Scenario C's partition was installed. Phase 1 (the heal trigger + coalescing) is the meaningful fix and stays. Phase 2's bootstrap re-fire needs leader election or a debounce against the startup-time path before being re-introduced; leaving an inline NOTE so the next attempt has the context. Failed run: actions/runs/25706585637 (Scenario C, attempts 1+2). Last green DDIL run: 25671044714 (pre-merge, SHA40e3d847). * fix(daemon): init peersReachable=true to suppress startup heal edge DDIL Scenario C kept flaking at attempt-1 even after dropping the bootstrap re-fire. Remaining trigger: peersReachable starts as the zero value (false) on multi-node clusters, and the first probePeersOnce tick flips it false→true. That counts as a "heal" edge under siv-60 phase 1, so reconcileOnHeal fires at boot — concurrent with startCluster's bootstrap — perturbing timing in a way that the harness's Scenario C exposes (witness sshd handshake EOF mid-test). Initialise peersReachable optimistically. Mode() also requires natsConnected (starts false), so the optimistic peer init can never falsely report cluster mode — but it does eliminate the spurious startup edge. Real partition→heal transitions still fire the trigger exactly as designed: true→false on partition, false→true on heal. --------- Co-authored-by: Julian Sommer <115516918+juliansommer@users.noreply.github.com>