mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
main
9 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Julian Sommer
|
36d5788923 | fix: require iam:PassRole for EKS cluster and node roles (#1139) | ||
|
Julian Sommer
|
2b4ddad78a |
chore: drop dead and redundant nolint directives
Removes directives for linters that are not enabled (noctx, exhaustive, deadcode) and gosec suppressions already covered by config exclusions. Tags ochrevector QueryResult and Filter with their existing field names so musttag passes without changing the NATS wire format, and bounds the RDS window slot conversion instead of suppressing G115. |
||
|
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>
|
||
|
Josh Siviour
|
9f5f877e50 |
fix: batched hygiene — slog key, ui pid file, race-scaled timeouts, GPU console capture (#637)
* fix(ec2): rename ambiguous 'code' slog key to firmwarePath The EFI variable store prep log used 'code' for the OVMF firmware path, which reads as an error/status code in log search. Rename to firmwarePath to avoid confusion. * fix(spinifexui): write pid file to a service-owned BaseDir spinifexui.Start() called the legacy utils.WritePidFile, which has no MkdirAll and falls back to $HOME/spinifex. Under the spinifex-ui unit's ProtectHome=yes this directory is neither guaranteed to exist nor writable. Add Config.BaseDir, switch to utils.WritePidFileTo (which does MkdirAll) for Start/Stop/Status, wire it through the CLI as a spinifex-ui-base-dir flag/SPINIFEX_UI_BASE_DIR env var, provision /var/lib/spinifex/spinifex-ui in setup.sh, and add it to the unit's ReadWritePaths. * test(viperblockd): replace fixed sleep with a readiness poll TestEBSConfigQueueGroup_DetachedOpenFails slept 500ms before assuming launchService's ebs.config subscriber was bound, then raced a hardcoded 5s request deadline. Poll with a cheap malformed request (rejected at json.Unmarshal, before the slow S3 path) until the subscriber answers, then issue the real request with room for openVolumeVB's S3 backoff. * fix(e2e): capture GPU worker console log on test failure Wire harness.OnFailure into TestEKSGPUPodExposure to dump the GPU worker's serial console via the established EC2 GetConsoleOutput path, matching lb_test.go's captureLBConsoleOnFailure. A driver or device-plugin hang after boot only ever surfaces on the guest console. |
||
|
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. |
||
|
Josh Siviour
|
1e1d2f3c6d |
ci: GPU e2e workflow on ci-gpu self-hosted runner (#518)
* ci: GPU e2e workflow on ci-gpu self-hosted runner Runs tests/e2e/gpu (EC2/ECS/EKS GPU passthrough) against the persistent, hand-bootstrapped wattle shared host, driven over SSH by a banksia runner labelled ci-gpu (matches the ci-single/ci-multi convention) — no runner installed on the shared box. - runs-on [self-hosted, ci-gpu]; concurrency ci-gpu, cancel-in-progress false (single A1000 = single-flight, queue not stomp) - workflow_dispatch + GPU-path filter (not every push) - manual-work mutex gate (~/.ci-gpu-paused / CI_GPU_PAUSED) so hands-on GPU testing blocks CI - if: always() cleanup terminates launched GPU instances; never resets the box (no setup-ovn.sh/dev-reset/cluster-reset) - drives the control plane with spx + the already-configured aws spinifex profile directly (no wrapper scripts, nothing to install on wattle) * ci(gpu-e2e): advisory AMI preflight, fail only if zero GPU AMIs The suite is self-guarding — each test skips when its AMI is absent. The hard-fail on the base ubuntu GPU AMI blocked the whole run even when the ECS GPU AMI (the only one imported) could execute. Inventory the AMIs and fail only when none are present. * ci(gpu-e2e): self-provision base GPU AMI, hard-require it in preflight The GPU E2E workflow was green-because-skipped: with no GPU AMIs imported the self-guarding suite skipped every test, passing as a false green. Make the raw-EC2 GPU passthrough test the always-on baseline. - Add an import-if-absent setup step that pulls the prebuilt base GPU image from the catalog by name (ubuntu-26.04-nvidia-gpu-x86_64), naming the AMI exactly what tests/e2e/gpu filters for. Idempotent; caches on wattle's persistent loop-fs. Runs spx under sg spinifex (local storage write). - Preflight now hard-requires the base AMI (fail, not skip) so a silent import failure can't pass as green; ECS/EKS node AMIs stay opt-in warn. - Reconcile the local build path: ubuntu-gpu-nvidia.conf AMI_NAME and launch-gpu.sh's nvidia default now use the same canonical name, so a locally built+imported image also satisfies the suite. * ci(gpu-e2e): pass --ami-name on base GPU import so the AMI name matches spx admin images import --name selects the catalog entry but names the resulting AMI generically (ami-<distro>-<version>-<arch>), so the import landed as ami-ubuntu-26.04-x86_64 and the tests/e2e/gpu name filter never matched. Pass --ami-name ubuntu-26.04-nvidia-gpu-x86_64 explicitly (same flag build-system-image.sh uses) and poll briefly for availability. * fix(network,vm): reconcile auto-assigned public-IP NAT + scale VFIO QMP timeout Auto-assigned public IPs were never carried through the network reconciler. applyEIPs ran AddEIP + ensureGuestPortDatapath for user EIPs only; auto-assigned IPs (intent.Ports[].PublicIP) got egress exemption but no NAT re-assert and no gatewayLRP->guest convergence. Their dnat_and_snat was created once at RunInstances and never touched, so a vpcd/OVN rebuild dropped it and the guest-port hop never converged: inbound DNATs at the gateway but never reaches the guest (TCP/SSH dark) while ICMP is answered by the gateway LR (ping OK). floatingIPSpecs now folds every auto-assigned/ELB public IP into the same AddEIP + guest-port convergence path as user EIPs (deduped by private IP; a user EIP wins), making the datapath self-healing across reconcile ticks and deploys. qmpGreetingTimeout for VFIO guests now scales with guest RAM (base 30s + 8s/GiB, floored at 180s, capped at 600s) instead of a flat 180s, so a large guest is not SIGKILLed mid-pin while the whole guest RAM is IOMMU-locked before QMP answers. * fix(vm): scale QMP handshake command deadline for VFIO launches The greeting timeout scaling alone did not fix the VFIO launch SIGKILL: a GPU guest sends its QMP greeting before the RAM pin completes, then cannot service qmp_capabilities until the pin finishes. sendQMPCommand hardcoded a 30s response deadline, so the handshake decode timed out (~43s for a 16 GiB g5.xlarge) and the launch cleanup SIGKILLed a QEMU that would have come up. Parameterize the command deadline (sendQMPCommandWithTimeout); the handshake now uses the RAM-scaled qmpGreetingTimeout (base 30s + 8s/GiB, floor 180s, cap 600s) for qmp_capabilities. Other callers keep the 30s default. * test(gpu-e2e): share one GPU instance across passthrough subtests to avoid mid-suite IP recycle |
||
|
Josh Siviour
|
98fc01764b |
feat(ecs): GPU passthrough to task containers (Epic C) (#512)
* feat(ecs): thread resourceRequirements GPU count through conv, ContainerDef, AssignContainer and RunSpec * feat(ecs): add GPU placement dimension — InstanceRecord capacity, fits/reserve/release, STRINGSET resource reporting * feat(ecs): agent GPU discovery via nvidia-smi, pinning ledger, device-UUID report-back and DescribeTasks gpuIds * feat(ecs): inject assigned GPU devices into containerd OCI spec via CDI (oci.WithCDIDevices) * test(ecs): add GPU task e2e (nvidia-smi container, gpuIds assertion) to gpu e2e suite * test(gpu): decouple base-AMI requirement from GPU fixture so ECS e2e runs without the ubuntu EC2 image * test(gpu): pick smallest advertised GPU instance type to fit a single host * fix(ecs): preserve pinned GPU UUIDs across STOPPED state change so DescribeTasks reports gpuIds for one-shot tasks |
||
|
Josh Siviour
|
9f349e86ca |
feat(eks): expose NVIDIA GPUs to pods (Epic B) — VFIO QMP, worker registration, device-plugin GPU access (#507)
* feat(eks): expose NVIDIA GPUs to pods via CDI device plugin GPU nodegroups now label workers nvidia.com/gpu.present=true and taint them nvidia.com/gpu=present:NoSchedule (via a k3s config.yaml.d drop-in, since k3s has no K3S_NODE_TAINT env), and stage a hidden nvidia-device-plugin addon that addon-sync renders into the K3s auto-deploy dir. The device plugin runs with --device-list-strategy=cdi-cri and is gated to GPU nodes by nodeSelector + toleration, advertising nvidia.com/gpu allocatable so pods land on GPU nodes and get the device injected via the CDI spec baked by mulga-cdi-generate. Relies on containerd 2.0.5 default enable_cdi=true (no config fork). * fix(vm): scale QMP greeting deadline for VFIO passthrough guests A GPU passthrough guest (-device vfio-pci + large -m) must lock and DMA-map its entire guest RAM through the IOMMU before QEMU's QMP monitor answers, which exceeds the fixed 30s greeting deadline and gets the VM SIGKILLed at launch. Parameterize the greeting timeout (NewQMPClientWithGreetingTimeout) and scale it to 180s for VMs with GPU attachments; plain VMs keep the 30s default and reconnects against an already-running QEMU keep it too. * fix(eks): register GPU workers reliably and expose GPUs to pods Three fixes let a GPU nodegroup worker boot, register, and advertise its GPU: - k3s-agent.service gains Delegate=yes (plus KillMode/TasksMax/limits and overlay/br_netfilter modprobe): k3s embeds containerd+kubelet in-process, so on cgroup-v2 the kubelet cannot create /kubepods without a delegated subtree and ContainerManager.Start fails before Node registration. - The GPU node label rides a k3s config drop-in (20-gpu.yaml node-label + node-taint) instead of K3S_NODE_LABEL, which k3s ignores; without the label the nvidia-device-plugin nodeSelector never matches. - Raise the nodegroup ready timeout to 20m for the ~7min GPU first boot. Adds the GPU e2e (VPC + NAT-GW egress harness) and a slog-restore unit test. * fix(eks): install ecr-credential-provider on the GPU node AMI The GPU worker's kubelet is started (from nodegroup userdata) with --image-credential-provider-config + --image-credential-provider-bin-dir= /usr/local/bin, but the eks-node-gpu AMI never built or installed the ecr-credential-provider binary. kubelet treats a referenced-but-missing provider binary as a fatal RegisterCredentialProviderPlugins error, so the node crash-loops and never registers (while the Alpine eks-node worker, which ships the binary, joins fine). Build + install it in the GPU manifest; the server-only binaries stay omitted since GPU nodes are agent-only. Also tee k3s-agent stderr (and its embedded kubelet klog) to the serial console: Ubuntu logs only to journald, which is off-box-invisible on a worker in an isolated customer VPC, so a kubelet that fails to register left no trace. * fix(eks): give the nvidia-device-plugin its own GPU access The CDI-mode device plugin (--device-list-strategy=cdi-cri) must run nvml.Init and generate the CDI spec in its own container, but the DS had no GPU wiring, so nvml.Init crash-looped and nvidia.com/gpu was never advertised. Run the DS under the k3s-auto-configured nvidia runtimeClass with NVIDIA_VISIBLE_DEVICES/CAPABILITIES=all, the host root mounted read-only at /driver-root so it can locate the driver libs + ld.so.cache, and /var/run/cdi mounted writable so the generated spec is visible to containerd. No enable_cdi drop-in is needed on k3s v1.32.5 / containerd 2.0.5. GPU workload pods still need only resources.limits nvidia.com/gpu. Live-validated end-to-end on the single-GPU host (RTX A1000): baked control-plane AMI, GPU nodegroup, nvidia.com/gpu allocatable, nvidia-smi pod, zero manual intervention. * test(ec2): drop obsolete restoreSlogDefault test Rebased onto dev, which dropped the restoreSlogDefault workaround in #506 (viperblock no longer mutates the global slog). The test referenced the now-deleted helper and no longer compiles. Real GPU/registration diff is independently covered (diff coverage 95.3%). |
||
|
tomnewton-mulga
|
412eb618b0 | Adding GPU passthrough e2e tests - intended to be run on Wattle |