mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
828b5df4aa00f3ec12cb97244bc26df1b8ae73ae
2185 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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
|
e2c5397b5d |
feat: gateway-side instance record cache (Phase 2) (#992)
* feat: add read-only instance record cache (Phase 2) Adds spinifex/instancecache: a revision-fenced snapshot+watch informer over the instance record KV space, with a per-account index and OTel metrics. Wired up and running in awsgw at startup, but not yet consulted by any read path. * fix: cover the fence with real syncs, bound decode-failure cardinality, expose degraded Adds end-to-end fencing tests that drive freshSync/periodicResync against the embedded NATS server via a test-only postSnapshotHook seam, since drainBuffered unit tests alone don't exercise the buffer-before-snapshot ordering. Drops the per-key attribute from the decode_failures counter to keep it O(1) time series; the key is still logged where the decode happens. Adds a Degraded accessor set on sync failure and cleared on the next success, per D7, without changing what List or Get answer. * refactor: replace instancecache's buffering fence with ResumeFromRevision The watch/snapshot fence no longer needs a side buffer, watcher modes, or an overflow failure mode. Every sync now snapshots first, then opens a watcher with kvstore.Bucket.WatchFrom (ResumeFromRevision), and replays onto a private candidate map until the end-of-replay marker arrives. Only a fully caught-up candidate is swapped in as the live map, together with its watcher becoming active; the previous watcher is stopped and joined only after, so two watchers never apply to the same map. kvstore.Bucket gains WatchFrom as a sibling to Watch, opened through the same withKV wrapper, for a caller that needs to replay from a snapshot's high-water mark rather than an UpdatesOnly live feed. * fix: cover the fence with production entry points, not a reimplementation The four fence tests recomputed hw+1 and called kvstore.WatchFrom/replay themselves instead of going through syncOnce, so they verified nats.go's resume behavior (already covered by TestResumeFromRevision_...) rather than the cache's own revision handling. A mutation inside syncOnce (hw+1 -> hw+1_000_000) passed all four. Reinstate a postSnapshotHook seam (nil in production) so the initial-sync pair can call syncOnce/installSync directly and the periodic-resync pair can call periodicResync directly, landing the write inside syncOnce's internal snapshot-to-watch window without duplicating it. The same mutation now fails all four; reverting passes all four. * feat: count the records a resumed watch replays, so the fence is observable live |
||
|
Ben Duncan
|
b6d01370dd |
fix: render northstar's recursion allowlist so a node is not an open resolver (#993)
* fix: render northstar's recursion allowlist so a node is not an open resolver northstar.toml is written with allow_recursion = false and an allow_recursion_from list built from cluster membership, so a freshly formed node serves recursion to the cluster and refuses it to the internet. The list is the node's own bind and advertise addresses plus every peer's host, with any port stripped. Both local addresses matter: a node's resolv.conf points at its own advertised address, so the query arrives from that public IP over lo rather than from 127.0.0.1, and an allowlist of loopback alone would stop every node resolving internal names. Guests are covered without appearing in the list. vpcd's per-tap DNS shim relays guest queries to a node's :5300 from a node address, so trusting the nodes trusts every guest behind them and no guest CIDR is needed. The template comment carries the caveats for allow_recursion = true, since the cost of setting it is remote from the setting: an open resolver is found by scanners within hours and gets the node's whole address range blocklisted, which is an outage for every tenant on it. The list is rendered at formation, so a node added later is not in its peers' files. Regenerating it live from membership, as the daemon already does for peers.nft, is follow-on work tracked on mulga-t3mub. * chore: pin northstar to the recursion ACL and atomic zone reload The recursion allowlist this branch renders is enforced by northstar's RecursionPolicy, which lands in dev at a0e741e. CI resolves northstar from go.mod rather than the workspace, so the build failed on NewRecursionPolicy being undefined until this pin moved. The AWS SDK bumps come with it. |
||
|
Josh Siviour
|
06df1b799b |
fix: tunnel apiserver controlplane egress through konnectivity (#991)
The EgressSelectorConfiguration written to every EKS control-plane VM defined only the `cluster` egress selection, which carries kubelet exec/logs/portforward. Admission webhooks and aggregated apiservers dial via the `controlplane` selection instead, and an undefined selection falls back to a direct dial. The control-plane VM sits alone in its own VPC with no route to the service CIDR, so every webhook call timed out after 10s and konnectivity never saw a single dial request for one. Define both selections on the konnectivity-server UDS so webhook traffic rides the agent-initiated reverse tunnel alongside kubelet traffic. |
||
|
Julian Sommer
|
4d3ba022ce |
fix: poll for the outcome point instead of racing the reply
natsMetricsHandler records after the handler has already replied, so a test that collects the moment nc.Request returns can beat the point to the reader. Matches the poll the ec2.cmd outcome tests already use. |
||
|
Josh Siviour
|
d9593c0a48 |
feat: repair instance records on a ticker, not only on state change (#988)
* feat: repair instance records on a ticker, not only on state change A record write is best-effort, and a failed one forgets its digest so the next reconcile retries it. But the only caller is persistState, reached from WriteState, which fires on state change. An instance that launches, has its record write fail, and then simply runs has no next state change, so its record is absent for as long as it lives. A 60s sweep on the node now reconciles the live instance set against the digest of what it has published. The digest guard means a node with nothing outstanding writes nothing, so the cost tracks failures rather than instance count. The interval is the point of the change — it is what turns an unbounded window into a bounded one — so it is asserted directly rather than inferred from a sweep. The sweep does not go through persistState: the local state file is already correct and rewriting it would advance the state revision for no reason. Only the record half needs repairing. WriteRunningSet now reports what it did, so a sweep that found work can say so and a healthy node stays silent. * fix: keep the QMP liveness stamp out of record change detection Found on env19: the node owning a running instance rewrote its record on every sweep, forever. The digest guard was working exactly as coded — it correctly found a real difference each tick, because Health.LastQMPSuccess advances on every successful QMP poll and the collector runs on its own cadence. Left alone this is a KV write per running instance per node per minute, indefinitely. The stamp is now held out of the digest. Nothing reads it back: impairment is judged from QMPConsecutiveFailures and ImpairedSince, and node liveness comes from the heartbeat store, so holding it out costs no reader anything and restores the publication cadence the record had before the sweep existed. QMPConsecutiveFailures and ImpairedSince stay in, because an impairment is a real transition that has to reach the record space. Both halves are covered: a stamp moving on its own must not write, and an impairment must still write. The first fails without this change. |
||
|
Julian Sommer
|
7b4a83ccfe |
fix: expect InvalidSnapshot.NotFound for cross-account DeleteSnapshot
Snapshot metadata is now keyed by owning account, so a snapshot outside the caller's prefix is absent rather than forbidden. The e2e assertion still encoded the pre-partitioning UnauthorizedOperation contract. |
||
|
Julian Sommer
|
d8d3e2268d | feat: partition EBS metadata keys by owning account (#989) | ||
|
Josh Siviour
|
de44fa3264 |
fix: cover the full ELBv2 attribute key set (#990)
ModifyLoadBalancerAttributes and ModifyTargetGroupAttributes reject any
key absent from the per-type default maps with ValidationError, and the
rejection fails the whole call rather than the single key. Terraform and
the AWS Load Balancer Controller send the full attribute set on every
apply, so one missing key breaks the resource outright.
Adds the 17 keys the ELBv2 API reference documents but the default maps
lacked:
ALB: health_check_logs.s3.{enabled,bucket,prefix}, ipv6.deny_all_igw_traffic
NLB: secondary_ips.auto_assigned.per_subnet
Target group: load_balancing.algorithm.anomaly_mitigation,
stickiness.app_cookie.{cookie_name,duration_seconds},
lambda.multi_value_headers.enabled, target_group_health.* (4),
deregistration_delay.connection_termination.enabled,
preserve_client_ip.enabled, target_health_state.unhealthy.* (2),
target_failover.{on_deregistration,on_unhealthy}
health_check_logs.s3.* is what surfaced this: it is ALB-only, and an
aws_lb apply against the sandbox failed on it.
The target group map stays a single flat union rather than splitting by
protocol, so it remains a permissive superset. Values are store-and-echo
only; nothing in the data plane reads them.
|
||
|
Bryn Mailer
|
eccc61a69e |
fix: hold back JetStream recovery headroom on the store filesystem (#967)
Filling the filesystem behind the JetStream store to zero free bytes leaves nats-server with nowhere to write while it recovers a stream. Every error return inside the locked region of newFileStoreWithCreated assigns nil to the named return value fs, and the deferred unlock then dereferences it, so a recovery write failure surfaces as a nil-pointer panic instead of an error. That structure is unchanged in v2.14.3, in the pinned v2.14.5 and in the current v2.14.6, so no available version bump addresses it. The nats service now keeps a preallocated reserve file beside the jetstream directory and hands it back at startup when the filesystem can no longer supply that headroom on its own, re-arming once space returns. Recovery therefore always has somewhere to write, whichever process filled the disk. |
||
|
Bryn Mailer
|
18aa67b0e9 |
feat: add the e2e guest disk-performance gate (#964)
* feat: add the e2e guest disk-performance gate Nothing measured guest disk performance in CI, which is why a quadratic index scan shipped in viperblock 35dd9f2 and survived three subsequent fixes to adjacent code. It is also why the two availability clauses at the end of the ebs-performance plan's Stage 0 have never been evaluated: a sustained random-write workload took the guest, and then the host, down, and nothing since has demonstrated that it no longer does. tests/e2e/diskperf/ launches an instance, attaches a dedicated blank volume per repetition, and drives a pinned fio profile over guest SSH. Two profiles: randwrite-4k at 4 jobs x 4 GiB, the shape the exit criterion names, and randrw-70-30-4k, the mixed workload the headline IOPS came from. The target is a raw device with direct=1 rather than a file on the boot disk, so guest page cache, filesystem allocation and journal traffic are out of the path. Two classes of assertion, kept apart because folding them together lets a run that left the host unreachable pass on a respectable IOPS figure. Availability is hard pass/fail and absolute, so it gates from the first run: the job completes inside its budget, a guest responsiveness probe answers throughout on a second SSH channel, and the host's own sshd answers throughout. The guest probe reads the root volume while the workload runs on a different one, so a pass is evidence of isolation between volumes rather than of the job merely being slow. The host probe is a TCP connect plus an SSH banner read, which needs no credential and still proves the kernel scheduled sshd. Throughput and latency are banded against a committed baseline: fail above 25% regression, warn above 10%, per-metric median across repetitions. baseline.json is committed and embedded, with every figure zeroed, and a zero reference records and warns rather than failing. Only the availability class gates until numbers are captured on the bare-metal cell; a throughput band nobody measured would be a fabricated gate. A fresh volume per repetition is the reset. Extent count grows monotonically, so reusing a volume makes the gate order-dependent and drifts its own numbers upward over time, slowly normalising the regression it exists to catch. Each job is its own guest invocation and fio's JSON is fetched by a separate command, so a wedged job loses only itself and reports as a budget breach rather than a parse failure. e2e-baremetal.yml gains suites, suite-env and cell inputs so one reusable workflow serves both the correctness cell and nightly cell 28, which queues behind cell 20 on the shared concurrency group. Cells 25-27 are the dedicated eks, ecs and rds cells. The per-suite Go test timeout is resolved from e2e_suite_timeout in suite-sets.sh, where diskperf declares its 180-minute budget beside rds's rather than in a second place. The gate's own arithmetic runs in preflight via test-harness: band edges, the unmeasured cases, per-stream metric selection, fio JSON parsing, and the probe accounting including a slow-but-successful answer counting as a breach. A gate whose comparison logic is untested is the failure mode the plan is otherwise full of. The extent-count ceiling named as a fourth availability assertion is not implemented: it needs viperblock.index.entries from S1.5, which does not exist. * feat: let a manual bare-metal run choose its e2e suites A dispatch run only sees inputs declared under workflow_dispatch, which had none, so suites and suite-env resolved empty and fell back to the correctness set. Capturing a diskperf baseline by hand was therefore unreachable without a workflow_call caller. * fix: build the bare-metal suites the run step will execute The build step packs pre-built binaries into the tarball and did not see the suite selection, so a selected suite was absent from the node unless it happened to be one of the three the build script hardcoded. * feat: commit the measured disk-performance baseline Medians of three repetitions on the bare-metal cell. The throughput and latency bands now gate instead of warning, so a 2x regression in either job fails the suite. randwrite-4k's write tail is two orders above the mixed job's. It is recorded as measured rather than zeroed: an ungated metric would miss the regression class the gate exists to catch. |
||
|
Josh Siviour
|
c767f2b6ee |
feat: judge DescribeInstances completeness by responder identity, and resolve IMDS locally (#985)
* feat: stamp the owning node's AZ on the instance record RunInstance never persists a node's AZ onto the stored instance, so today only the launching node's own ProjectInstance call can produce a correct AZ for a describe. Add VM.AZ, carry it on InstanceStatus since it is placement observed by the running node, and stamp it in WriteRunningSet alongside Status.LastNode from the node's own config.AZ. * docs: trim the AZ field comment to the project's three-line limit * fix: identify Gather responders by node, not by frame count utils.Gather previously counted reply frames toward completeness, so two subscriptions replying from one host (a daemon mid-restart) could satisfy a fan-out while a genuinely configured node stayed silent. Every daemon reply now carries an X-Node-ID header set by the shared response helpers via nats.Msg.RespondMsg, so Gather can attribute each frame to the node that sent it. Gather gains a CollectionMode (CollectServeData preserves today's early-exit behavior; CollectUntilDeadline collects to the full deadline for absence-proof callers) and returns node-identified Frames plus Responders/ SuccessResponders/ErrorResponders sets. Identity mode retains only the first payload seen per node; a disagreeing later frame from the same node flags a conflict but never overwrites the retained data. Node identity uses one namespace across the reply header, config.ClusterConfig.Nodes keys, daemon.Heartbeat.Node, and vm.VM.LastNode, all traced back to Daemon.node - confirmed to already agree, with a new test asserting it at runtime. This is a mechanical caller-side follow-on: every existing utils.Gather caller now reads frame.Data instead of frame directly. * feat: thread configured node set to DescribeInstances GatewayConfig gains NodeIDs, the keys of config.ClusterConfig.Nodes, distinct from the existing ExpectedNodes count. awsgw builds it from cluster config and wires it into GatewayConfig. The DescribeInstances action passes gw.NodeIDs through instead of relying on gw.DiscoverActiveNodes(ctx) at that call site, so completeness can be judged against the nodes that are actually configured rather than the nodes that happened to answer a discovery probe. * fix: judge DescribeInstances completeness by node identity DescribeInstancesChecked now judges completeness on decoded frames against the configured node set (ValidResponders) rather than on Gather's frame/byte counts, which cannot know the describe schema. A node that answers with a nil Reservations list is a valid responder, not an invalid one - the existing merge guard on Reservations != nil governs how frames combine, not whether the node counted as having answered. The explicit-instance-ID 404 path now collects with CollectUntilDeadline so a node that hasn't replied yet cannot be mistaken for a node that has nothing to report; general listing stays on CollectServeData. Any ambiguity - an unidentified frame, a node appearing in both the success and error sets, a node replying twice with disagreeing frames, a Gather cap hit, or an empty/unset node set - suppresses the NotFound rather than risking a false one. * fix: let a describe by instance id stop once every id is found CollectUntilDeadline made every DescribeInstances with explicit ids wait the full 3s fan-out deadline, measured at 4650ms against 1655ms before, on a healthy cluster where all three nodes answer in about a millisecond. Gather gains a Settled predicate, consulted only under CollectUntilDeadline, that ends collection once the replies already answer the question. Absence still pays the whole deadline, and an instance the caller cannot see is filtered out of every frame so it takes the absence path too, which keeps the two NotFound answers indistinguishable in time. * refactor: resolve IMDS instance metadata local-first, drop the fan-out IMDS only ever serves a guest on its own node, so a DescribeInstances fan-out to the whole cluster is unnecessary latency and load. Resolve from the node's own on-disk VM state first, falling back to a direct record-space read for an instance that has moved off this node. Neither path issues a NATS request. * perf: cache the local instance state parse for IMDS lookups IMDS request rate is guest-driven and unbounded, so re-reading and re-parsing the whole on-disk VM state on every request does not scale with VM count on a node. Cache the parsed state and only re-read when the file's mtime or size changes; a stat or parse failure clears the cache and falls back to the record space rather than serving a stale hit. * fix: share ManagedBy visibility rule between DescribeInstances and IMDS Extracts IsInstanceVisibleToCaller so account ownership and the platform-managed hide rule live in one place instead of two independently maintained copies. * refactor: decouple IMDS instance lookup from the daemon package handlers/imds no longer imports daemon: NewIMDSServiceImpl now takes a localStateReader and a recordLoader interface instead of a dataDir string, and vpcd (the composition root for both planes) constructs both adapters, including the stat-guarded local-state cache that used to live inline in handlers/imds. Either adapter may be nil, degrading that source to a miss rather than failing IMDS to start. * test: move the local state reader test to an external package * chore: exempt export_test.go from the external test package check The check's own remedy text tells you to move unexported hooks into an export_test.go, then flags that file for being in-package — which it is by definition, since exporting internals is its whole purpose. * fix: bound how many describes may hold a full fan-out deadline A describe naming an instance the caller cannot see is filtered out of every frame, so it never settles early and holds a gateway goroutine and inbox subscription for the whole 3s deadline. Issuing those is free for the caller, so nothing stopped a burst of them pinning unbounded gateway state. Absence proofs now draw on a fixed budget. Past it, callers get a retryable RequestLimitExceeded rather than being queued, since queueing would let the backlog grow. The decision is taken before any reply is read, so a nonexistent id and another account's id throttle identically and the throttle cannot become the timing oracle the deadline exists to close. * perf: settle describe-by-id on responder coverage, not the full deadline Every describe-by-id was paying the whole 3s fan-out deadline whenever the requested instance was not in a running-instance frame — which includes a stopped instance, a terminated one, an id belonging to another account, and an id that does not exist. Gather gains ResponderGrace: once every ExpectedResponders node has answered, collection closes after a short grace window instead of running to the deadline. Core NATS gives a request only to the subscribers present when it was published, so an overlapping subscription replies alongside the first and one that arrives later never receives the request at all. Waiting past full coverage cannot find a reply that full coverage missed. The window is armed once and never extended, so a node replying repeatedly cannot hold the fan-out open. The full deadline is now paid only when a node stays silent, which is the case it was always meant to cover. The stopped and terminated bucket queries now run alongside the fan-out rather than after it, so their latency overlaps it instead of adding to it. They feed a shared instanceSettler that ends collection as soon as every requested id has been seen, skipping even the grace window. The timing oracle is unchanged. An instance the caller cannot see is filtered out of every source before the reply is sent, so it takes the same path as an id that does not exist: both answer NotFound after full responder coverage, and remain indistinguishable in time. * fix: stop parallel tests racing the terminate backoff seam terminateRetrySleep is a package global, and every test that needed it neutered swapped it and restored the old value from t.Cleanup. Those tests run in parallel, so two cleanups could write the seam at once — a real data race the detector caught once enough parallel tests shared the binary. Installed once in TestMain instead. Nothing in this package wants the real backoff, so there is no value in restoring it per test. |
||
|
Julian Sommer
|
a5fc2b517c | fix: supply aws:userid on the gateway authorization path (#987) | ||
|
Julian Sommer
|
ab167a192c | fix: reject an unresolvable policy variable on the IAM write path (#986) | ||
|
Julian Sommer
|
4522494c17 |
fix: define the per-suite e2e test timeout once, in suite-sets.sh
The timeout was written out separately in each workflow and drifted: rds ran 50m on the PR gate and 30m nightly, where it panicked and took six in-flight subtests with it. e2e-suites.yml had the same hardcoded 30m on its single path, which also runs rds. A new e2e_suite_timeout helper sits beside the eligibility lists it already guards, and all four call sites resolve through it. e2e.yml loops over suites on the remote VM, which has no checkout, so it resolves the pairs on the runner and looks them up in the loop. Also lift the nightly cell ceiling to 50 minutes so the rds cell is not cut short by the job before its own timeout applies. |
||
|
Julian Sommer
|
0b518d2f79 |
fix: name the blocking resource in DeleteSecurityGroup DependencyViolation
checkSGDependencies knew which ENI or referencing security group blocked the delete and discarded it, returning a bare DependencyViolation with no log line, so the blocking id had to be recovered by cross-referencing the ENI table. Return it via awserrors.Errorf and log it at the point of refusal. |
||
|
Bryn Mailer
|
e07f25ccb2 |
fix: fail DRAIN when a volume seal failed (#966)
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. |
||
|
Julian Sommer
|
be94beb52a | ci: run eks, ecs and rds nightly in dedicated cells (#984) | ||
|
Julian Sommer
|
0247f57d6c | fix: derive S3 Service Metrics cluster health from raft quorum (#983) | ||
|
Julian Sommer
|
677055a478 | test: cover the launch-template boot path end to end (#982) | ||
|
Julian Sommer
|
e8f1718ebd | fix: seed the gateway nexthop MAC binding for a host-local nexthop (#981) | ||
|
Julian Sommer
|
dc9b8429ae |
fix: derive the EKS nodegroup ARN discriminator so an exact-ARN Deny fires
The policy gate had the cluster and nodegroup name but not the trailing UUID, so it evaluated nodegroup/<cluster>/<name>/*. That value matches the AWS-documented wildcard policy spelling but not one naming the exact stored ARN, which is what a tenant gets by pasting DescribeNodegroup output, so that Deny stayed inert. The discriminator is now derived from (account, cluster, name) as an RFC 9562 version-8 UUID, so the gateway and the handler compute the same value independently and the gate names the stored ARN exactly with no lookup. Nodegroups created before this keep their random discriminator and their existing wildcard-only behaviour. |
||
|
Julian Sommer
|
f7bcccb1fe | test: cover spread placement distribution and the default VPC handlers | ||
|
Julian Sommer
|
e53e7a5884 |
test: cover subscriber topology handler happy paths
Drives every vpc.* topology topic end to end against the mock OVN client and asserts the resulting rows: router, subnet switch and router port, logical switch port addresses and port-group membership, gateway router port, NAT rules and egress policies. Invalid CIDR, IP and MAC events are pinned to a failed reply envelope with no partial write. Moves the elbv2 dispatch table to an external test package. subscribers 53.5% -> 73.4%. |
||
|
Julian Sommer
|
13c5b00066 |
test: pin elbv2 gateway subject wiring and validation guards
Adds a 35-row dispatch table covering every NATS-backed elbv2 gateway wrapper. Each row subscribes a stub only to the expected subject, so a mis-wired subject fails as no-responders, and asserts the returned value, the forwarded request body and the account header. A second table covers the 28 validation guards that had no test, each pinned to its AWS error code. Package coverage 49.5% -> 93.8%. |
||
|
Julian Sommer
|
80f4db05bc |
test: cover the ebs provider NATS server, memory provider and null provider
Tables the twelve subjects Serve subscribes through the malformed-body, version-skew and provider-failure arms, and round-trips every verb through NATSProvider against a served MemoryProvider so client encoding and server decoding are asserted against each other. Covers MemoryProvider.ListVolumes, ListSnapshots and CopySnapshot, all of which were at zero, plus NATSProvider's nil-payload and subject-token guards. natsserve 28.8% -> 91.3%, ebsprovider 67.5% -> 86.3%, nullprovider 0% -> 100%. |
||
|
Julian Sommer
|
bd8ac8efa1 |
test: cover the installer wizard key handlers and screen rendering
Drives the model through Update with real key messages: a full walkthrough asserting the config it produces, every validation gate keeping the operator on the screen with the cursor on the field at fault, and the rendered output for facts that appear nowhere else (role layout, pool geometry, folded planes). |
||
|
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. |
||
|
Julian Sommer
|
d535718bcb |
test: add an ovn.Client conformance suite across live and mock
One behavioural suite runs against LiveClient over the in-memory NB server and against mock.Client, so the semantics reconcile, policy, external, topology, subscribers and vpcd rely on are pinned in both. Covers NAT router scoping and duplicate deletion, the not-found sentinels, gateway chassis idempotence, ReplaceACLs no-churn, port group membership and DHCP lookups. |
||
|
Julian Sommer
|
babde88de0 |
test: cover waitForClusterReady via a sleep and deadline seam
The readiness loop terminates on wall clock, so a stubbed sleep alone still costs two minutes; clusterReadyMaxWait becomes a var alongside clusterReadySleep so the timeout branch is reachable. Adds the in-package marker to the EIP association tests, which drive setupTestEIP and read svc.natsConn. |
||
|
Julian Sommer
|
ce9a03bc72 |
test: cover the EIP association path and the SG rule adapter
setupTestEIP built the EIP service with a nil VPCService, so every ENI lookup AssociateAddress performs was structurally unreachable: lookupENI, lookupENIByInstance and AssociatedPublicIPForInstance had no test at all and any test reaching them would have panicked. Wire a real VPC service over the same embedded NATS and cover association by ENI id and by instance id, the private-IP override, both not-found codes, and the add-nat/delete-nat events the NAT rule is built from. sgRulesToPolicyRules was never carried a tenant security-group rule: the golden scenarios build IntentState by hand and bypass the adapter, so a transposed FromPort/ToPort or a dropped SourceSG would have shipped silently. Load real SG records through KV and pin the field mapping. |
||
|
Julian Sommer
|
5b09cc9172 | test: add a live E2E identity-policy authorization suite (#980) | ||
|
Julian Sommer
|
675243753e |
test: add completeness gates for the ECS, ECR and RDS scope tables and the condition keys
ECS and ECR take the existing both-directions shape against their Actions tables. RDS carries its scopes on the dispatch table itself, so the gate is the account-level half instead: every served action is either scoped or carries a written reason, and one in both sets or neither fails. HasScope and ScopedActions are added to drive it. The condition-key gate pins the door's key set and asserts the IAM write path accepts exactly the operator/key pairs the evaluator enforces. |
||
|
Julian Sommer
|
a9a457489d |
fix: re-anchor a caller-supplied policy ARN onto the caller's account
The seven sourcePolicyARN actions resolved to the PolicyArn the caller sent, so the gate decided against an ARN whose account the caller chose. The scope now takes the ARN's path and name and rebuilds it on the caller's account, as sourceOIDCARN beside it already does, and widens to account-wide when the ARN carries no policy component. Lands with the cross-tenant integration suite that found it. |
||
|
Julian Sommer
|
b033a10200 | fix: scope tagging policy checks and pin the table exhaustive (#979) | ||
|
Josh Siviour
|
28324fd6c6 |
fix: take the volume lease when snapshotting an unmounted volume (#978)
snapshotVolumeEngine's unmounted branch built its engine by hand instead of going through openVolumeVB, so it was the last control-plane path that opened an engine over a volume without holding the cluster-wide lease. A snapshot request landing on a node that does not have the volume mounted would open a second engine over storage another node was writing. Two engines on one encrypted volume read the same SeqNumHighWater and issue the same AES-GCM nonces, which discloses the XOR of the plaintexts and permits recovery of the authentication subkey, so this is not only a consistency question. Route the branch through openVolumeVB and give the lease the engine's lifetime. The tail both branches shared moves into createSnapshotWithVB, mirroring copySnapshotOnVolume, which was converted earlier and is what this one was measured against. A refusal is reported as unavailable rather than internal: the holder gives the volume up on detach, so the caller should retry instead of treating a live mount elsewhere as permanent. |
||
|
Josh Siviour
|
0ff37c313a |
fix: render S3-scoped gateway errors in the flat S3 error envelope (#976)
The gateway does not serve S3, so an S3-scoped credential is rejected on scope by the SigV4 middleware. That rejection was written through GenerateEC2ErrorResponse, giving the client the EC2 query envelope <Response><Errors><Error>. SDKs look for a flat top-level <Error><Code> on an S3 response, find nothing, and report an empty code and an empty message, which reads as a credential fault rather than a wrong endpoint. Add the flat S3 envelope and resolve the envelope from the service the client actually signed with: GetService reports an unserved scope as empty, which is exactly the case that has to render as S3. The service-to-envelope decision was copied into four emitters and the copies had already drifted - writeClusterUnavailable put ELBv2 on the EC2 envelope while ErrorHandler and writeThrottleError put it on the IAM one. Collapse them onto a single xmlErrorBody so a new service cannot be wired into some emitters and missed by others. s3 is deliberately not added to supportedServices: only the shape of the rejection changes, not the rejection. |
||
|
Josh Siviour
|
687974995f |
fix: ECS RunTask and CreateLaunchTemplateVersion honour ClientToken (#974)
* fix: ECS RunTask and CreateLaunchTemplateVersion honour ClientToken A retried RunTask placed the tasks again: it mints a fresh task ID per task, reserves CPU/memory/GPU, hot-plugs an ENI in awsvpc mode and starts the containers, so a resend with Count 5 left ten running tasks, double the reserved capacity and five leaked ENIs. CreateLaunchTemplateVersion added a second identical version and moved $Latest. RunTask gets a per-action wrapper over idempotency.Do rather than a dispatch decorator: it is the only ECS action that needs one. A partial placement finalizes rather than aborts, since re-running would add tasks on top of the ones already started. CreateLaunchTemplateVersion reuses the EC2 dispatch decorator, so it is an opt-in rather than new machinery. CreateLaunchTemplate, ModifyLaunchTemplate and ECS CreateService accept a token they do not store; each is name-keyed or a CAS and cannot duplicate, which is now recorded at the handler. * test: cover the RunTask client-token binding layer Split the token layer out of RunTask's NATS binding as runTaskIdempotent, the same seam runTask already has, so the no-token, store-failure and replay paths are reachable without a live connection. |
||
|
Josh Siviour
|
44abc812b6 |
fix: seven EC2 creates honour ClientToken instead of duplicating on a retry (#973)
CreateVolume, CreateNetworkInterface, CreateNatGateway, CreateRouteTable, CreateEgressOnlyInternetGateway, CopyImage and CreateCapacityReservation parsed a ClientToken and never read it, so a retried request created a second resource. Adds ec2IdempotentHandler at the EC2 dispatch table rather than seven hand-written wrappers, keyed <action>.<account>.<token> in the bucket RunInstances already uses. RunInstances keeps its unprefixed keys: re-keying live records would make a retry re-launch instead of replay. Promotes the shared orchestration to idempotency.Do, now that eight callers share it, and wraps store failures in ErrUnavailable so callers map every non-mismatch token failure onto one API error. |
||
|
Julian Sommer
|
1fd4373b35 | fix: verify caller-supplied role ARNs against the stored canonical ARN (#972) | ||
|
Josh Siviour
|
e2c38ca7ef | refactor: one paging walk and one client-token store, not six and two (#971) | ||
|
Julian Sommer
|
4d4f7f84f1 |
fix: wait for the NATS subscriber callback before asserting on recorded spans
ServeNATSRequestCtx ends its consumer span in a defer that runs after the reply is published, so a test returning from NATSRequest could read the span recorder before the consumer span had ended and see no account attribute. The subscriber callbacks now signal completion and both tests wait on that signal instead of on the reply. |
||
|
Julian Sommer
|
db97c90a9a | fix: bind the EKS internal control-plane routes to the calling CP VM (#970) | ||
|
Julian Sommer
|
12408268be |
chore: trim unused entries from the go build cache before saving
The cache was a union that only grew: each run restored everything, added what it compiled and saved the result, so objects superseded by later commits stayed for Go's full five-day window. Deleting every entry and letting it rebuild took Build and Test from 3434 MB to 478 MB, restore from 75s to 13s and save from 64s to 7s. Dropping entries nothing has touched in four hours holds it near that. Go bumps an entry's mtime when a run uses it, so the working set survives; the step runs after the tests, so a pruned entry costs a later run a partial recompile at worst and never the current one. |
||
|
Julian Sommer
|
da92c7a083 |
chore: only save the go build cache from dev and main
Restore is what makes CI fast; save is what got expensive. A same-era A/B across the switch to this cache shows what restore buys: wall time went 6:50 to 3:52, Build Spinifex 63s to 10s, test-race 385s to 188s. Since then the cache has grown to its steady state and the work has not moved, so Build and Test now spends 75s restoring and 64s saving where it once spent 13s and 8s. A PR's save is read by nothing else. Splitting restore from save and gating the save on dev and main keeps every branch reading a cache a dev merge refreshes minutes later, and takes the upload off the 56% of runs that are pull requests. |
||
|
Josh Siviour
|
bbd0ebebed |
feat: retry a transient reconcile failure before the resync, not after it (#969)
pass and keyPass logged every failure at WARN as "retrying next cycle" and returned the callee's deadline, which for a failed callee is zero. Zero means no deadline, so the next attempt was the resync — five minutes by default. Two things were wrong with that. A fault that clears in seconds left the world unconverged for minutes, when the loop already had the mechanism to ask for sooner. And a terminal failure was announced as recoverable: "retrying next cycle" is false for a 400, which will be a 400 every cycle, forever, at WARN. Both now go through failed(), which classifies with awserrors.IsTerminal — the predicate added in #923 for exactly this decision and until now called by nothing. A terminal error is logged at ERROR and asks for no early retry; any other error asks to be revisited after DefaultErrorRetry. A deadline the callee named survives when it is sooner, and clampRevisit still caps every request at the resync, so a loop can only become more eager than it is today. |
||
|
Julian Sommer
|
91bab3b910 | chore: merge integration job into lint | ||
|
Julian Sommer
|
3cfa94d49a |
chore: drop the go build cache age census
It answered its question: of 18 GB on disk, 1.45 GB is untouched for a day and nothing at all survives three. Go's own trim and the rate CI runs already keep the cache live, so pruning before the save would shed under 4% of the bytes at a two-day cut and risk recompiles for it. |
||
|
Julian Sommer
|
eccf068a25 | fix: run the instance handler subtests in parallel too | ||
|
Josh Siviour
|
a0dfd311b2 |
refactor: make the elected reconcile node the only DNS zone writer (#968)
The lifecycle handlers published record changes onto the dns.recordset.change queue group, which load-balances rather than serialises, so any daemon could be read-modify-writing a zone object at any moment. zonelock.go existed to make that safe. With the reconcile pass now applying its own batch on the node that won the election, the desired set is the only path into a zone and the elected node is its only writer, so both the publish path and the lock go. Deleted: the four lifecycle publish call sites and their helpers, publish.go, zonelock.go, SubjectRecordsetChange and QueueGroup, and the lock from applyZone. Every record they published is already in the desired set the reconcile rebuilds each pass, and every record they withdrew is already pruned by it, so the classes DNS carries are unchanged. |