mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
1e00425bb302e56e97a4eaeb389dc0cdc59641b8
342 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
d52245001e |
feat: Ochre vector store (tenant pgvector, embeddings, ingestion, operator teardown) (#788)
* feat: add /v1/embeddings gateway adapter * feat: Ochre vector store pgvector backend, KV index registry, orchestration Adds handlers_ochrevector: the VectorBackend interface + pgx/v5 impl with per-account schema/role isolation enforced via SET LOCAL ROLE + search_path inside every transaction, the KV index registry (CREATING/READY/DELETING/STALE), and the create/delete/list/reconcile orchestration tying them together with crash-safe rollback and re-entrant recovery of stuck records. * feat: singleton appliance-ensure state machine for ochre vector store Adds the encrypted control-plane credential store and re-entrant Ensure state machine for the platform pgvector appliance (D2): a fixed-key KV claim mirroring Registry.Reserve elects exactly one winner to launch the appliance via an injected ApplianceLauncher, losers wait or resume a stale crashed provision, and the master password is generated once, stored only AES-256-GCM encrypted, and decrypted solely inside Connect to build a pgxBackend. * feat: Ochre vector store async ingestion (chunker, job store, worker) Adds Stage 3: a deterministic recursive chunk splitter, a per-account ingestion job-record store mirroring the index registry, the transactional per-document replace on the backend, and the IngestService worker that lists a predastore source, chunks and embeds each object, and replaces its rows idempotently. * feat: Ochre vector store similarity query and metadata filter grammar Adds VectorBackend.Query (cosine kNN, k default/cap, iterative HNSW scan under a filter) and a Bedrock-KB-shaped metadata Filter AST that compiles to parameterized SQL over jsonb, closing off arbitrary SQL by allowlisting jsonb keys and always binding values as params. Also fixes RunJob reporting READY on a job that ingested nothing regardless of failure cause, and stamps SourceSpec.Metadata onto ingested rows so the new filters have data to match against. * feat: Ochre vector store request/response API, VectorService, and CLI Adds the daemon-facing VectorService interface (index CRUD, ingest, describe job, query), its server implementation over the existing registry/backend/embedder, a NATS-forwarding client, and the `spx admin ochre vector` operator CLI. Query resolves and embeds against each index's own pinned model; ingest stamps the same model/dimension onto every source spec server-side. accountID is always taken from the caller context, never the request payload. Daemon subscription wiring and manifest registration are a later change; this is pure request/response plumbing with no construction into daemon.go or awsgw.go. * feat: wire Ochre vector store VectorService into the daemon Constructs the platform Postgres appliance and VectorService behind a new OchreVector config section that defaults off, so existing daemons are unaffected. Adds a real RDS-backed ApplianceLauncher that provisions the appliance over rds.CreateDBInstance/DescribeDBInstances, registers the six ochre.vector.* NATS subjects mirroring the bedrock endpoint service pattern, and degrades to a disabled (nil) service on any boot-time appliance failure rather than blocking daemon startup. * fix: fix Ochre vector store boot ordering and self-register subjects startOchreVector ran before subscribeAll, so its appliance launch fired rds.CreateDBInstance/DescribeDBInstances requests with no responder yet registered, timing out the launch on a first-enable boot. It also ran synchronously, blocking daemon startup up to the appliance launch timeout. Move it to a background goroutine started after subscribeAll returns, and have it register its own ochre.vector.* subjects once the VectorService is actually constructed, via a new registerNatsSubs helper shared with subscribeAll's boot-time table. Guard d.natsSubscriptions with d.mu, since this background registration can now race the shutdown path's unsubscribe-all sweep. * feat: bake pgvector into the shared rds-postgres image * fix: reach the ochre appliance over a routed daemon port on its ENI IP The daemon dialled the RDS appliance over rec.Endpoint, a vanity spx3.net hostname the control-plane host can neither resolve nor route to, so a healthy appliance was never reachable and the vector store disabled itself. Resolve the appliance's customer-ENI private IP read-only via ec2.DescribeNetworkInterfaces filtered on the spinifex:rds-db-instance tag, dial that IP for the DSN instead of the hostname, and give the daemon its own routed ENI in the appliance's subnet bound to an OVS host port so the IP is reachable. Retry appliance startup with bounded backoff instead of hard-disabling on the first transient. * fix: install pgvector at bootstrap for the Ochre vector appliance The daemon connects to the appliance as its NOSUPERUSER master, and the image's pgvector is not a trusted extension, so CREATE EXTENSION vector failed with permission denied. rds-init now installs it via the bootstrap superuser, gated on the appliance's fixed master name so tenant instances are unaffected. * fix: grant the Ochre appliance master CREATE on its database The daemon's EnsureAccount runs CREATE SCHEMA per account as the NOSUPERUSER master, which does not own the postgres database, so bootstrap now grants it CREATE on the database alongside the pgvector install. Both are gated to the appliance master, so tenant instances are unaffected. * fix: make the Ochre appliance's pgvector reachable under per-account isolation The daemon connects as a NOSUPERUSER master and runs every account operation under SET ROLE with a search_path pinned to the account schema. Two gaps blocked that against a real non-superuser Postgres: the master could not SET ROLE into the roles it creates, and the account role could not see the vector type. Bootstrap now grants the appliance master createrole_self_grant and installs pgvector into a read-only extensions schema granted to PUBLIC; the backend carries that schema on each account search_path so the vector type resolves without weakening per-account write isolation. All appliance-scoped. * fix: prune stale VPC host port before installing the daemon appliance port A replaced Ochre appliance is handed the same subnet IP, so a new daemon ENI mints a fresh vhp- port at an address a superseded port still holds. The old port survives at the OS/OVS layer even after its ENI record is wiped, so two interfaces claim one address and the kernel can route to the dead one (EHOSTUNREACH). installVPCHostPort now prunes any other vhp- port holding the target address before installing. * fix: gate VM restore's stale-socket kill on backing-store readiness On daemon start, a live VM whose NBD sockets fail to dial was force-killed as an orphan. While predastore/viperblock is still coming up after a restart those sockets are transiently unreachable, so a healthy VM (the Ochre appliance was the observed victim) was destroyed and its RDS record flipped to failed. The kill now only fires when the backing store is confirmed up; otherwise the live QEMU is kept and QMP reconnected. Genuine orphan reaping under a healthy store is unchanged. * fix: drive ochre ingest jobs from a daemon-side sweep scheduler * feat: add operator teardown verb for the ochre platform appliance Adds Appliance.Teardown to remove the singleton platform Postgres appliance (RDS instance, host port, KV record), an admin-only NATS subject and daemon handler to drive it, and `spx admin ochre appliance teardown --confirm` to invoke it without exposing the action through the tenant awsgw surface. * fix: register ochre appliance teardown before Connect so it survives a broken appliance * fix: purge ochre index and job metadata on appliance teardown * test: mark ochre in-package test files for the external-test gate |
||
|
|
ee1ee4f033 |
fix(preflight): report services still executing a replaced binary (#781)
Overwriting /usr/local/bin/spx while spinifex.target is running does not move the services onto it. Each keeps executing the replaced inode until it restarts, so a node serves a mixture of two builds, and because the handlers are NATS queue-group workers the skew shows up as roughly one request in three behaving like the old build — an intermittent fault rather than a broken node. The ansible deploy path already detects and remediates this, but only there: an out-of-band binary copy leaves nothing to report it, and on env19 that state went unnoticed for a full day. spx admin preflight now reports it, so an operator has a command to run and a script has a non-zero exit to gate on. Detection follows the deploy path's: a (deleted) marker on /proc/<pid>/exe, a target path that no longer resolves, or a path that resolves to a different inode than the one being executed. |
||
|
|
2fe885441b |
fix(shutdown): scope a local drain to its own node and stop counting failed phases as done (#780)
* fix(shutdown): scope a local drain to its own node and stop counting failed phases as done A node's spinifex-shutdown.service publishes its GATE and DRAIN requests to the shared spinifex.cluster.shutdown.* subjects, which every daemon subscribes to as a fan-out. The node filter only decided which ACK the stopping node waited for, so by the time it applied, every other node had already stopped its gateway, UI and VPC daemon and powered down its guests. Stopping one node took down every guest in the cluster. Requests now carry the node they apply to, and a daemon drops one addressed elsewhere without ACKing. A cluster shutdown leaves the field empty, so it still reaches every node. The STORAGE phase also ran 'pkill -f nbdkit', which matches every nbdkit on the host: another node's backends on a shared host, and a live guest's backend that survived a daemon restart under KillMode=process. Cleanup now sweeps this node's own nbdkit-vol-*.pid files and signals a PID only while /proc still agrees it is nbdkit, so a recycled PID is never hit. Finally, the coordinator counted errored ACKs toward its per-phase quorum, so a node that failed to drain reported N/N and the run advanced into STORAGE and PERSIST with guests still running. Only clean ACKs count now, and any errored one aborts the phase unless --force. * test(shutdown): cover the phase gate, every handler's target check and the real proc paths Diff coverage came in at 52.9% against a 60% threshold. The gaps were the per-phase target checks beyond GATE, the real procComm and signalProcess bodies that every cleanup test stubs out, and the coordinator's abort branch, which sat inside runClusterShutdown where no unit test could reach it. The abort decision moves into shutdownPhaseOutcome, a pure function returning the completed count and the message to print, so the rule that an errored node does not count toward the quorum is testable on its own rather than only reachable through a full cluster shutdown. |
||
|
|
660dfe3925 | fix: os.exit on viperblock failure so it restarts properly | ||
|
|
58e0f2879f |
feat: serve the deployment CA from IMDS instead of baking it into images (#784)
Image import shelled out to virt-customize to install the deployment CA into an imported image's trust store. The step was best-effort and skipped whenever virt-customize was absent, which is the default: nothing in provisioning installs guestfs-tools on a node. When it did run it mutated the image after its checksum had been verified, it could not handle Windows guests, and it tied CA rotation to re-importing every affected AMI. Nothing the stack launches depended on it. ECS container instances, EKS k3s servers and BootDirect microVMs all receive the gateway CA at launch via user-data or fw_cfg, and boot itself needs no CA because IMDS is plain HTTP. The only uncovered case is a customer instance whose own workload calls a Spinifex HTTPS endpoint, which needs a pull mechanism rather than an image mutation. The IMDS responder now serves the CA at /spinifex/ca.pem, outside the /latest tree so the AWS-compatible surface stays byte-identical to EC2's. The route is token-free because a CA certificate is public material and the console already serves it unauthenticated; the X-Forwarded-For SSRF guard still applies. The PEM is cached and re-read on modtime change, so rotating ca.pem on a node reaches guests without restarting vpcd. |
||
|
|
65d2501e6b | feat: add MariaDB as the second RDS engine (#762) | ||
|
|
9c995ad114 |
fix(test): restore in-package tests that reach unexported symbols (#779)
* fix(test): restore in-package tests that reach unexported symbols admin_upgrade_test.go and units_invariants_test.go were moved to external test packages while, in parallel, validateUpgradeFlags and unitVersionRe grew references in those same files. Both sides merged cleanly and the result did not compile, which silenced the systemd unit guards entirely. Also regenerates units_gen.go, which had drifted from build/systemd: it was missing spinifex-firewall.service and carried a stale spinifex-daemon.service. The drift landed unnoticed because those guards were the tests that could not build. Note that `go generate ./...` does not regenerate this file. The directive lives in generate.go, which is //go:build ignore, so go generate never scans it; the generator has to be run directly from spinifex/systemd. * fix(systemd): bump the daemon unit version so the firewall path reaches hosts The nftables firewall work added `-/etc/spinifex/firewall` to the daemon unit's ReadWritePaths but left the version marker at 1. Reconcile replaces a unit only when the installed version is lower than the embedded one; same version with different content is treated as operator-modified and left alone. An upgraded host would therefore keep its old unit and the daemon would never be able to write peers.nft. Bump the daemon unit to version 2 and give the firewall unit a marker of its own — it had none, so it read as version 0. Pin both body hashes. * fix(systemd): make go generate actually regenerate units_gen.go The directive sat in generate.go, which is //go:build ignore, so go generate never scanned the file and never ran it. Every drift failure told the reader to run `go generate ./...`, which silently did nothing for this file, and the units_gen.go drift that hid the daemon unit version bug landed that way. Move the directive to reconcile.go, which is part of the package and is scanned, and drop the dead one. |
||
|
|
fd8be8a814 |
refactor(test): route duplicated NATS bootstraps through testutil (#776)
* refactor(test): route duplicated NATS bootstraps through testutil Thirteen test sites hand-rolled a NATS or JetStream server bootstrap that spinifex/testutil already provides, and one seedKV clone duplicated testutil.SeedKV. Route them through the shared helpers. Sites whose options genuinely diverge are left alone: fixed ports needed for reconnect and restart scenarios, custom TLS and token auth, a custom MaxPayload, and TestMain entry points that have no testing.TB for t.Cleanup. The natgw and routetable seedKV clones use CreateOrUpdateKeyValue rather than CreateKeyValue, so their failure behaviour differs and they stay as they are. Also moves eight test files to external test packages where they declare no helper used by a sibling in the same package. Coverage is unaffected: Go compiles internal and external test packages into one binary per package, so attribution is identical either way. * chore(test): require new test files to use an external test package Adds a preflight check that fails when a newly added test file declares an in-package test. External test packages bind tests to the exported API and let packages share test doubles, which an in-package double cannot be. Only added files are checked, so the existing in-package tests are untouched. e2e and integration suites are exempt: both share a package-level fixture built in main_test.go, so their files cannot move individually. A file that needs unexported access can opt out with a //test:in-package comment. |
||
|
|
6f4aa495e6 |
feat: reconcile systemd unit files on upgrade (#771)
* feat: reconcile systemd unit files on upgrade Unit files were written once at install time by install_systemd() in scripts/setup.sh and never re-asserted. A node upgraded by swapping the spx binary kept whatever units it was installed with, so any fix that lives in a unit rather than in code never reached it. This is not theoretical. A node whose spinifex-viperblock.service predates KillMode=process defaults to KillMode=control-group, so on a target stop TimeoutStopSec expires mid-flush and systemd cgroup-SIGKILLs every nbdkit while guest QEMU survives in the daemon's own cgroup: the guest keeps running with no storage path. Units are now stamped with a spinifex-unit-version marker, embedded in the binary via a generated file, and reconciled by spx admin upgrade. Missing or version-stale units are replaced after a timestamped backup; a unit at the current version whose content differs is reported as operator-modified and never overwritten. Drop-ins under <unit>.d are untouched, so systemctl edit overrides survive. Nothing is restarted. A daemon-reload is enough for KillMode and TimeoutStopSec to apply to the next stop, so drift is corrected without disturbing a running guest. Adds --dry-run, --units-only and --skip-units to spx admin upgrade. * test: cover systemd reconcile error paths and the admin upgrade CLI flow Diff coverage on the unit-migration PR sat at 45.8%, below the 60% gate. Adds tests for reconcile.go's error/edge paths (unreadable unit, backup and write failures, daemon-reload failure, malformed version markers, downgrade-is-conflict) and drives admin_upgrade.go's CLI flow directly via an injectable exit and a decision matrix extracted into validateUpgradeFlags/upgradeSummary for testability. |
||
|
|
f14dd2ae1a |
fix: register EC2 snapshot metadata for provider-backed AMI imports (#774)
The catalog image-import command wrote a volume's blocks through the
EBS provider but never wrote the EC2 control plane's
{snapshotID}/metadata.json, so the imported AMI's snapshot was
invisible to DescribeSnapshots and any resolution path without the
system-AMI fallback. Register it the same way the ochre weights
import already does, and log when the system-AMI fallback masks a
missing document instead of silently succeeding.
|
||
|
|
98ec781629 |
feat: decouple the EC2 control plane from the viperblock engine behind an EBSProvider contract (#770)
* feat: introduce the EBSProvider interface and control-plane metadata store
Phase 1 of decoupling the control plane from viperblock. Adds a
provider-neutral block-storage contract that spinifex owns end to end.
ebsprovider defines EBSProvider, whose operation set and idempotency rules
follow the CSI controller service, with NATS request/reply as the transport
rather than CSI's gRPC. Every request and response carries an explicit schema
version so wire skew fails loudly at the boundary instead of silently decoding
a changed payload. The package deliberately exposes no viperblock types:
provider-specific state is an opaque handle. Ships with a NATS client against
the ebs.provider.v1.* subjects and an in-memory fake for tests.
ebsmetadata gives spinifex its own volume and AMI documents under a spinifex
key prefix, so the control plane can answer DescribeVolumes and DescribeImages
without reading the storage engine's persisted state.
The EC2 volume and snapshot handlers gain provider-backed paths for create,
describe, modify, attach-state and delete. Provider injection is optional for
now, so the existing engine path stays the default until the viperblockd side
serves the new subjects and the metadata migration lands.
vm/lifecycle.go no longer imports viperblock; FindFreePort was the only symbol
it used and is now a local helper.
* feat: serve the EBS provider contract from viperblockd
Gives the ebsprovider NATS client a server. Volume create, describe, expand
and delete, plus snapshot create and delete, are now handled by the daemon
that owns BaseDir and the mounted-volume registry, which is where viperblock
engine construction belongs.
Every handler prefers an already-mounted engine over opening a second one on
the same volume, and expand refuses outright while the volume is mounted.
Opening a second engine on a live volume is the double-writer hazard this
decoupling exists to remove, so the boundary enforces it rather than trusting
callers. Requests carrying an unrecognised schema version are rejected at the
boundary instead of being decoded on a best-effort basis.
Snapshot creation replies immediately with an operation ID and completes
asynchronously on a per-snapshot subject, since a large volume will outrun the
request timeout. Draining the volume stays with the caller: it coordinates
with the guest and is not something the storage boundary can reach.
The snapshot create subject gains an explicit create verb. Addressing it as
ebs.provider.v1.snapshot.<volumeID> collided with the snapshot delete subject
and left the wildcard unsubscribable.
Publish and unpublish are not served yet; the mount and unmount logic they
would front is still inline in launchService and needs extracting first.
Also adds a conformance suite that runs one set of contract assertions against
both the in-memory fake and the NATS client, so each side is checked against
the contract rather than against the other's structs.
* feat: let an operator select the EBS provider
Adds an [ebs] provider key and wires the NATS provider into the EC2 volume,
snapshot, image and instance services at the daemon's composition root. Until
now nothing injected a provider, so the provider-backed code paths in those
services were unreachable.
The key is provider-neutral rather than nested under [viperblock], because it
names the boundary rather than one provider's settings; a second provider will
not need it renamed. An empty value resolves to "embedded", so deployed config
files that predate the key keep today's behaviour, and an unrecognised value
is a startup error rather than a silent fallback.
Startup probes the provider's capability verb and warns when nothing answers.
The daemon and viperblockd start independently, so an unanswered probe is not
fatal, but without it a misconfigured node fails much later as an unexplained
request timeout on the first EBS call.
"viperblockd" stays experimental: under it volume metadata lives in the
control plane's own document, and the leak reaper, GC and admin paths still
read viperblock's config.json, so they cannot see tenant or tag data for
volumes created that way.
* fix: make every volume-metadata consumer provider-aware
Several readers and writers of volume metadata were never given a provider
branch, so under the viperblockd provider they silently used the wrong store.
Each of these is a correctness bug rather than a missing feature, because the
control plane had already stopped writing the document they read.
Modifications had nowhere to live: ModifyVolume built its result in memory and
dropped it, and DescribeVolumesModifications read it back out of viperblock's
config.json, so a describe straight after a successful modify returned nothing.
Volume metadata now carries its own modification record.
The leak reaper read attachment and tags from config.json and wrote orphan
markers to tags.json, neither of which exists for a provider-managed volume, so
a leaked volume was invisible to it and any mark it did write could not be read
back. Tag mutations had the same split: a tag applied after creation never
reached the document DescribeVolumes reads.
Snapshot deletion could not see a provider-managed clone, because a clone
records its source only in the control-plane document. That let a delete strip
chunks a live clone still reads, so it is fixed here rather than left for the
migration.
Volume enumeration and DescribeVolumeStatus were likewise scanning object
prefixes that a provider-managed volume does not create.
Also adds an encryption flag to the volume document, which DescribeVolumes had
been reporting as false unconditionally and snapshots could not carry at all,
and a list operation for AMIs so the image path can follow later.
The legacy path is unchanged; its invariant tests pass unmodified.
* feat: backfill ebsmetadata documents from legacy volume and AMI state
DescribeVolumes under the viperblockd provider enumerates only ebsmetadata
documents, so a volume created before the switch had no document and was
invisible rather than merely slow to read. A per-object read fallback cannot
fix enumeration, so both a backfill and a fallback are needed.
Add an object-store migration kind to the migrate framework. Predastore has no
conditional write, so an object-based version stamp would be last-writer-wins
across nodes; RunObject stamps progress in a caller-supplied JetStream KV
bucket instead, exactly as RunKV stamps a bucket's own version. The stamp is a
read-then-write rather than a compare-and-swap, so correctness rests on every
ObjectMigration.Run being safe to re-execute.
The backfill itself lives in migrate/ebsmetadatabackfill rather than beside the
numbered config migrations: it needs viperblock's VBState schema to decode the
legacy config.json, and migrate is imported by every handler that stamps a KV
bucket version. It converts what it can reach and skips unreadable objects with
a log, so one corrupt volume cannot hide the rest of the fleet.
ebsmetadata gains the legacy read fallback for the window before the migration
has run everywhere. It stays free of viperblock by taking the decode as an
injected function value, wired at the composition root, and the fallback is
retired by leaving it unset. A fallback read never writes the converted value
back, which would race the migration.
Prefix enumeration pages to exhaustion. Stopping at the first page would let
the backfill stamp itself complete having converted only the volumes it
happened to see, reintroducing the invisible-volume failure it exists to
prevent.
The migration runs from configureEBSProvider, so only when the provider is
selected; backfilling while the embedded engine is still authoritative would
produce documents that immediately go stale. Failure is fatal rather than
advisory, unlike the capability probe beside it. This makes the switch to
viperblockd one-way, which is now recorded on the config field.
* feat: serve volume publish and unpublish from viperblockd
PublishVolume and UnpublishVolume were the last unserved verbs on the provider
contract, and they are the ones that attach a volume to a VM. Until they are
served the provider path cannot boot an instance, so provider = "viperblockd"
cannot be verified end to end.
The mount and unmount logic was two large anonymous handler bodies inline in
launchService. Rather than write a second mount path, extract each into a named
function and have both the legacy ebs.mount/ebs.unmount handlers and the new
provider handlers call it. Two divergent implementations of nbdkit spawning and
the seal decision would be far worse than the inline bodies. The extraction
relocates code without rewriting it; response publishing stays in each handler
so the provider never publishes to ebs.mount.response.
Publish is idempotent: a volume this node already has mounted returns its
existing attachment rather than starting a second nbdkit against it, which is
the double-writer hazard the boundary exists to prevent. Unpublish preserves
the invariant that a failed seal leaves the volume mounted so a retry can
re-attempt it, and reports an error rather than success.
Both verbs are node-addressed, so they subscribe directly rather than through
the spinifex-workers queue group, and are registered only when NodeName is set.
There is no queue-group fallback: a subject that already names one node has
nothing sensible to fall back to.
Read-only attachment is refused rather than silently granted read-write, since
nbdkit is started read-write or not at all.
Also fixes a latent panic carried by the extracted mount path: viperblock.New
returns a nil VB alongside its error, and SetCacheSize was called on it before
the error was checked, so a failing New would have taken down the handler.
* feat: read and write AMI metadata through ebsmetadata
The backfill migration wrote ebsmetadata AMI documents that nothing read: every
AMI path still went through viperblock.AMIMetadata inside a VBState wrapper at
{imageID}/config.json. Give the image service provider branches for the AMI
read, write and enumerate paths, with enumeration going through ListAMIs rather
than an object-prefix scan, as DescribeVolumes already does.
AMIMetaLoader declared GetAMIConfig returning viperblock.AMIMetadata, putting a
viperblock type on an interface boundary in the instance package. It now returns
ebsmetadata.AMI on both paths, the embedded one converting before it returns, so
the boundary names no viperblock type whichever provider is selected.
The legacy-to-document mapping had accumulated three copies. Consolidate them in
ebsmetadata/vblegacy, a sibling package rather than a member so ebsmetadata
itself stays free of viperblock, and cover it with a reflective test that fails
when a field is added to either schema without being wired through. Sixteen
fields copied by hand in three places would have dropped one silently.
The admin image paths read whichever representation exists and write back to
that same one, so a provider-managed AMI never gains a spurious config.json.
FindAMIDependents now unions ebsmetadata volumes with the legacy prefix scan:
missing a dependent there deletes an AMI a live volume still needs, the same
class of bug as the snapshot clone path fixed earlier on this branch.
That union is built by hand rather than by wiring Store.ListVolumes' fallback,
which swallows per-ID read errors. This path deliberately fails closed on a
transport error so a delete cannot proceed on a partial picture, and the
fallback would have quietly turned that into failing open.
The CLI image import now also writes the document, best-effort, so an AMI
imported after the migration is not left reachable only through the fallback.
* fix(ec2): resolve volume config through ebsmetadata when a provider is configured
A provider-created volume has no vol-*/config.json — its metadata lives in
the ebsmetadata document. getBaseVolumeConfigAndEncryption read the legacy
key unconditionally, so AttachVolume reported a volume that DescribeVolumes
had just listed as available as InvalidVolume.NotFound.
Routing through ebsmetadata.Store also puts this path behind the legacy
fallback, so a volume created before the provider switch stays attachable.
The NoSuchKey remap now names the key it missed; its silence made the
original failure invisible in the daemon logs on every node.
* test(bench): add an EC2 control-plane latency benchmark
Measures the EBS control-plane operations either side of the provider
boundary, so the cost of the NATS hop can be stated rather than guessed.
Reports median and p95 with a bootstrap CI on the median difference, and
labels a delta indicative unless that CI excludes zero.
* fix(ec2): drain the volume before a provider snapshot
The legacy CreateSnapshot branch drains first so the snapshot reads a
current checkpoint; the provider branch did not, and viperblockd documents
the drain as the caller's responsibility. Both sides deferred to the other,
so an attached volume snapshotted stale data with no error raised.
DrainVolume and volumeAttachment now take the fallback state and instance
as strings rather than a viperblock.VolumeMetadata, removing a viperblock
type from an exported signature.
* feat(ec2): route the instance root volume through the EBS provider
RunInstances built its own viperblock engine to clone the AMI snapshot into
the root volume, which made every launch a second writer against a volume
viperblockd already serves. The launch now calls the provider's CreateVolume
with the AMI's snapshot and source volume, and records the result in
ebsmetadata so DescribeVolumes and attach can see it.
A snapshot clone needs both the snapshot ID and its source volume ID to
resolve base blocks, so CreateVolume in the volume handler now sends
SourceVolumeID alongside SourceSnapshotID; without it the daemon rejected
every create-volume --snapshot-id outright.
The gp3 IOPS and throughput constants move to spinifex/types so the instance
and volume handlers share one definition rather than duplicating literals.
* feat(ebsprovider): add a bounded volume seed to CreateVolume
The EFI variable store must be initialised from the launching node's firmware
VARS template, byte for byte. Passing a host path would not work: CreateVolume
is queue-subscribed, so any node can service it, and a foreign node's OVMF
package would bind the guest's pflash region to the wrong firmware build.
CreateVolumeRequest carries the bytes instead, as SeedData, and viperblockd
writes them at offset 0 in the engine it already builds. The seed applies only
when the create actually allocates the volume, so a relaunch never overwrites
a BootOrder the guest has since written.
JSON base64-encodes a []byte, inflating the payload by 4/3, so MaxSeedBytes
bounds the seed well inside the NATS max_payload the cluster runs with and
both ends reject an oversized seed with a clear error rather than letting the
publish fail. VolumeSeeding advertises the behaviour so a provider that cannot
seed says so instead of silently dropping the bytes.
* feat(ec2): route the EFI variable store through the EBS provider
prepareEFIVolume built a control-plane viperblock engine to seed the variable
store from the firmware VARS template, making every UEFI launch a second
writer against a volume viperblockd already serves. It now calls the
provider's CreateVolume with the template as seed bytes, so the store is
allocated and written by the daemon that owns the volume.
The template is still read on the launching node and shipped with the request.
CreateVolume is queue-subscribed, so any node can service it, and a VARS
region seeded from a foreign node's OVMF build would bind the guest's pflash
to the wrong firmware.
pflash refuses a VARS region that is not byte-exact, so the returned capacity
is checked against the template length; a provider that rounded up to a GiB
boundary would otherwise yield a guest that cannot boot.
The engine path stays behind a nil-provider guard until the legacy branch goes.
* feat(ec2): route image snapshots through the EBS provider
snapshotRunningVolume and snapshotStoppedVolume each built a control-plane
viperblock engine to take the snapshot backing a new AMI, making CreateImage
a second writer against a volume viperblockd already serves. Both now call
the provider's CreateSnapshot with the volume's provider handle.
The running path keeps its drain, which is what makes the snapshot current
rather than one predating a guest-triggered write. The stopped path needs no
drain, and the daemon already prefers a live mounted engine over opening a
second one, so it handles both cases itself.
getVolumeConfig read the legacy config.json, which a provider-created volume
does not have, so every CreateImage against one failed. It now reads the
ebsmetadata document under a provider, the same way attach does.
* fix(daemon): only the owning node creates an image for a stopped instance
handleEC2CreateImage is a fan-out. Ownership is established by vmMgr, which
only the node holding the VM passes, so a running instance is imaged once.
A stopped instance lives in the cluster-replicated KV instead, which every
node reads successfully, so all of them ran the full snapshot and AMI
creation. The caller saw the first reply and the rest leaked: on a three-node
cluster one create-image produced three AMIs and three snapshots.
A stopped instance records its owner in LastNode, so a node that is not the
owner now declines. Gather tallies error frames and keeps waiting, returning
only on a success, so declining cannot preempt the owner's reply.
An instance stopped by a build that never set LastNode elects an owner
through the KV's compare-and-swap, since having every node decline would make
create-image fail permanently for those records.
* perf(ec2): serve DescribeVolumes by ID from the volume documents
The provider branch called ListVolumes unconditionally and filtered in
memory, so asking for one volume read and decoded every volume document in
the bucket. The non-provider branch already had the equivalent fast path.
Explicit IDs now fetch their documents directly. The tenant check moves with
them: a document belonging to another account reports not-found rather than
being returned, matching what the enumeration filter did.
Measured against a memory object store, the fast path stays flat near 20us
regardless of bucket size while the enumeration grows linearly, reaching
4.6ms at 500 volumes.
* chore(lint): ban the viperblock import from the EC2 control plane
The control plane reaches block storage through ebsprovider now, but nothing
stopped a handler from importing the engine again. That is not hypothetical:
the instance handler first imported viperblock for a config struct alone, and
the engine construction followed thirteen days later.
depguard denies the import across handlers/, which matters because
viperblock.New starts a chunk uploader — every construction is a writer
against a volume viperblockd already serves. A types-only import is denied
too, since that is exactly how the last one started.
The four files still carrying the pre-provider engine path behind a
nil-provider guard are exempted by name. The exemption list is the remaining
work: it shrinks to nothing when that path goes.
* fix(ec2): refuse CopySnapshot instead of creating an unusable snapshot
CopySnapshot wrote a control-plane snapshot document under the new ID and
nothing else. The copy described correctly and CreateVolume against it
succeeded, so it looked like it worked, but the new ID had no viperblock
snapshot metadata behind it and the volume failed when the guest attached it.
Producing the metadata means re-sealing it under the destination ID. On an
encrypted volume it is AEAD-sealed with the snapshot ID in the AAD and the
sealing helpers are unexported, so spinifex cannot write it. Copying only the
unencrypted form would be dead code on any cluster with encryption enabled.
Refusing with UnsupportedOperation fails at the call the caller can act on
rather than at attach time. Source lookup and the ownership check still run
first, so a bad ID or another tenant's snapshot reports that instead.
* test(ec2): give handler test fixtures their own legacy config.json shape
Handler tests built the legacy config.json fixture by importing
viperblock's VBState/VolumeConfig/VolumeMetadata/AMIMetadata directly.
Mirror that shape in ebsmetadata/vblegacy, next to the production code
that already reads it (ebsmetadatabackfill), and switch the
fixture-only usages across the volume/snapshot/image handler tests to
it. A byte-for-byte equivalence test against the real viperblock types
guards the mirror from drifting out of sync.
Call sites that hand a viperblock type into putVolumeConfig,
mergeVolumeConfig, or buildVBConfig are untouched, since those
functions still name the real type in their signature.
* feat(ebsprovider): add CopySnapshot to the EBS provider contract
Adds a CopySnapshot operation to EBSProvider (types, ebs.provider.v1.snapshot.copy
NATS subject, MemoryProvider and NATSProvider implementations, conformance
coverage) and a viperblockd handler that duplicates a snapshot's metadata via
viperblock.CopySnapshotMeta on the source volume's VB.
* feat(ec2): serve CopySnapshot through the EBS provider
CopySnapshot has been refusing with UnsupportedOperation because a copy needs
its own snapshot metadata on the backend under the new ID, and on an encrypted
volume that metadata is AEAD-sealed with the snapshot ID in the AAD, so the
control plane cannot re-seal it. The provider contract now carries a copy
operation, so the storage engine does it.
The provider path generates a new snapshot ID, has the provider duplicate the
backend metadata under it, then writes the control-plane document. Rollback
mirrors CreateSnapshot: a failure after the copy deletes the provider snapshot
rather than leaving one nothing references. Ownership is still checked against
the source before anything is created.
The no-provider path keeps refusing. It has no way to produce the backend
metadata, and the copy it used to write described fine and then failed at
attach — a resource that fails as far as possible from the call that made it.
Adds MemoryProvider.Snapshot so a control-plane test can assert a snapshot
reached the provider rather than existing only as a control-plane document.
That assertion is what catches the old behaviour.
* refactor: remove the embedded EBS engine path from the EC2 control plane
viperblockd is now the only EBS provider. The pre-provider path that
constructed a viperblock engine in-process is deleted, and with it every
github.com/mulgadc/viperblock import under spinifex/handlers/ (tests
included). The depguard ec2-control-plane rule drops all four file
exemptions and the !$test carve-out, so the rule now reads simply as
"nothing under spinifex/handlers/** may import viperblock".
viperblock.VolumeConfig was also a data carrier on the provider path. It
is replaced by the spinifex-native ebsmetadata.Volume rather than the
vblegacy mirror: vblegacy mirrors a legacy on-disk shape whose only job is
decoding old objects, while ebsmetadata.Volume already carries every field
the call sites read, so the conversion step disappears instead of being
reimplemented against a legacy shape.
The embedded-only helpers (volumeConfigWrapper, putVolumeConfig,
mergeVolumeConfig, configIsEncrypted, putVolumeConfigViaDetached,
deleteS3Prefix, buildVBConfig, snapshotVolume, the ami-*/config.json
prefix scan) had exactly one production caller chain each, all through a
nil-provider branch, and are removed. The snapshot-dependency guard on
DeleteVolume is provider-agnostic and is kept, now running on the provider
path where it previously did not.
config: viperblockd is the default and an empty Provider resolves to it.
An explicit provider = "embedded" is rejected at config load rather than
silently upgraded, because starting anyway would migrate a node's volumes
to ebsmetadata one-way without the operator asking. The generated
spinifex.toml template emits viperblockd.
* fix(images): write the imported AMI document from the config the import wrote
ImportDiskImage creates the AMI's snapshot and records it on its own
*viperblock.VB, which viperblock.New allocates fresh, so the value lands only
in {imageID}/config.json. The import command was persisting the ebsmetadata
document from its pre-import manifest copy, whose SnapshotID is still empty.
Store.GetAMI only consults the legacy config.json fallback when the document
is missing, so that empty-SnapshotID document permanently shadowed the real
value and every launch from an imported AMI failed the zero-copy clone check
with ServerInternal.
Rebuild the document by reading back the config.json the import just wrote. A
missing or unreadable config is skipped rather than written from the stale
manifest: no document still resolves correctly through the legacy fallback.
* test(e2e): read NODE from spx get vms instead of VCPU in the spread check
spx get vms gained HEALTH and CRASHES columns, shifting NODE from field 6 to
field 8. The spread placement check still read field 6, so every instance
reported its vCPU count instead of its node, the unique-node count always
collapsed to 1, and the check failed on clusters that had spread correctly.
Read field 8 and strip the ANSI colour codes wrapping each cell.
* feat(viperblockd): add a --debug flag so viperblock debug logging is reachable
viperblockd hardcoded Info, and viperblock's VB.logger() falls back to
slog.Default() because vb.log is only ever set by SetDebug. That left the
engine's debug records, including the chunk uploader's start and disabled
lines emitted during New(), unreachable in a deployed environment.
Binds the namespaced viper key viperblock-debug and SPINIFEX_VIPERBLOCK_DEBUG
rather than the bare debug key, which several commands share. Also sets the
Config.Debug field, which existed but was set only by tests.
* fix(viperblockd): release the chunk uploader when openVolumeVB fails
viperblock.New starts a chunk uploader and a WAL syncer before the backend is
touched, so a failure in Backend.Init or loadStateWithRetry returned nil to the
caller while leaving both running with no handle to stop them. Repeated backend
failures on CopySnapshot, ExpandVolume or DescribeVolume accumulated uploaders
on a 30s ticker for the life of the process.
Adds invariant tests for both halves of the ownership rule: a mounted volume's
snapshot create and copy reuse the live engine and log zero volume opens, an
unmounted volume opens exactly one, and a failed open leaves no uploader behind.
The open counts are read from the same log record a production alarm watches.
* fix(ebsprovider): route requests to the node that owns the mount
Every viperblockd queue-subscribed the ebs.provider.v1.* verbs in one
spinifex-workers group, so NATS delivered each request to a random node. Only
the node holding the mount can reuse the live engine through findMountedVolume;
any other node constructed a second viperblock engine, with a live 30s chunk
uploader, against a volume nbdkit was serving on a different host. Measured on
a three-node cluster, this was roughly two snapshots in three.
Rather than resolve the owner, let the owner answer. A node subscribes
ebs.provider.v1.owner.<volumeID>.<verb> as a plain subscription when it mounts
a volume and drops it on unmount, mirroring the per-volume ebs.config
subscription. Clients try that subject first and fall back to the queue group
only on ErrNoResponders, which means nothing is mounted and opening an engine
is correct. A timeout deliberately does not fall back, since the owner may
still be running the operation.
Applies to snapshot create and copy, expand and describe, the four verbs that
reuse a mounted engine. Delete keeps queue dispatch.
* fix: rebuild viperblockd mounted-volume registry after a restart
Config.MountedVolumes is in-memory and rebuilt from nothing on start, so a
viperblockd restart while a VM keeps running leaves nbdkit serving a volume
the daemon has forgotten. Owner routing then infers from ErrNoResponders that
nothing is mounted and falls back to the queue group, which opens a second
viperblock engine, with a live chunk uploader, on another node. The same empty
registry also defeats handlePublishVolume's double-mount guard, so a republish
could start a second nbdkit.
Discover surviving nbdkit processes from /proc at startup and rebuild the
registry from them, following orphan_scan.go's precedent: match comm exactly,
parse the argv for volume=<id> and --unix/-p, and adopt a PID only when
nbdkit-vol-<volume>.pid names that same PID. Nothing is started, signalled or
killed; anything that fails corroboration is logged and skipped. Recovery runs
before the first subscription so no handler can race it.
Extract mountVolume's engine construction into constructMountedVB so mount and
recovery cannot drift, and release the WAL syncer viperblock.New starts when
construction fails after it.
* fix: corroborate recovered nbdkit processes from argv, not a pidfile
nbdkit is launched with -f (required for the Go plugin) and never writes the
--pidfile it is given, so no such file exists on any node; the daemon's own
/tmp is a PrivateTmp namespace besides. Corroborating against it rejected
every genuine mount, making recovery a no-op in production.
Corroborate against what the kernel actually has: the process must serve our
plugin, name this daemon's base dir, and still hold its socket. Adopt a volume
once, so two live nbdkits for it leave one registry entry and a warning rather
than two conflicting ones.
* feat: add a second EBS provider backed by qcow2 and qemu-nbd
The decoupling was justified by swappability that nothing demonstrated: the
conformance suite ran against MemoryProvider and against a NATSProvider wired
to a MemoryProvider, so both arms were the same implementation behind
different transports. A fake is not a second implementation.
Add three pieces. natsserve serves the ebs.provider.v1.* contract from any
EBSProvider, depending on nothing but the interface, and replaces the
test-only server twin so conformance now exercises production code. The
conformance suite moves to its own package and is exported so implementations
outside ebsprovider can run it, with no assertion changed. qemunbdd is the
second implementation: one qcow2 file per volume, qemu-nbd for publish, real
snapshot copies instead of chunk references, and every exec behind a runner
seam so its tests need no qemu.
Owner-subject routing is deliberately not served neutrally: it needs a
per-node mounted-volume registry the interface does not model, so it stays in
viperblockd and a neutral server's clients correctly fall back to the queue
group.
* test: run the conformance suite against the qcow2 provider
The suite passes unmodified against an implementation sharing no code with
viperblock, both in-process and through natsserve over the real wire
contract, which is the swappability claim the provider boundary was built to
support.
Two real defects it caught. ExpandVolume and GetVolume inspected the image
before consulting the publication map, so qemu-img could not open a volume
qemu-nbd held the write lock on and the refusal surfaced as an internal
error instead of ErrVolumeInUse; qemu-img info now runs with --force-share,
so describing a volume no longer depends on whether it is attached. Publish
also failed with qemu-nbd's raw exit status when the socket path exceeded the
108-byte AF_UNIX limit, and now reports that reason itself.
* feat: make the EBS provider selectable by config
Add the qemunbd selector alongside viperblockd, a spx service command that
serves the qcow2 provider over natsserve, and service registration. Config
validation accepts the new value everywhere viperblockd is accepted, while
embedded stays rejected by name.
The daemon logged a hard-coded viperblockd constant as the active provider
rather than the resolved one, in configureEBSProvider and in both of
probeEBSProvider's lines; all three now report what is actually configured.
Both daemons answer the same subjects, so exactly one may run on a cluster.
* test: add a live lifecycle check for the qcow2 provider
Drives create, publish, write, snapshot, restore and read-back entirely
through ebsprovider.NewNATSProvider against a real qemunbdd daemon on a
cluster node, so the client under test is the same shape the control plane
uses and never touches the provider type. Skipped unless SPINIFEX_QEMUNBD_LIVE
is set, so it stays out of preflight.
* fix: publish NBD URIs in the form standard clients accept
PublishedVolume.NBDURI carried nbd:unix:/path, which is QEMU's legacy
pre-URI filename syntax rather than an NBD URI. libnbd rejects it outright
with "URI must begin with 'nbd://'", so no NBD client but QEMU could ever
consume what the EBS provider boundary hands back.
Emit nbd+unix:///?socket=/path instead, the form libnbd, qemu and nbdkit
all accept. ParseNBDURI now takes both that and the TCP form, and still
accepts the legacy nbd:unix: syntax for state written by older daemons, so
a conformant third-party provider's URI is no longer rejected on the way
in either.
Verified against qemu 10.0.11: qemu-img and a real -drive option string
both open the URI form, and qemu reports both forms as nbd+unix://?socket=,
so it already treats the URI as canonical.
* test: validate published exports with libnbd's client tools
Every other assertion about the EBS provider goes through our own Go
client, so nothing has ever checked whether the export we publish is usable
by an NBD client that knows nothing about this codebase. That is the layer
neither the conformance suite nor a CSI-shaped contract can reach.
RunNBDClientSuite drives nbdinfo and nbdcopy against a published volume:
the URI connects, the export advertises its size and block-size
constraints, data written externally reads back byte for byte, a ReadOnly
publish sets the read-only transmission flag, and UnpublishVolume tears the
export down. It skips when libnbd-bin is absent rather than passing
vacuously.
Wired to qemunbdd, where all five failed before the URI fix. The publish
assertion in the main suite now requires a dialable URI instead of merely
a non-empty string, which is what let the wrong form through.
* test: run the external NBD client suite against a live viperblockd
The suite so far only ever saw qemu-nbd. Pointing it at viperblockd's nbdkit
export is the comparison worth having: one external client, two providers it
cannot tell apart, and every difference is a candidate leak in the boundary.
RunNBDClientSuiteWithConfig takes the node name and volume prefix a live
cluster needs, and publishForNBD now unpublishes and deletes what it created
so a run against a real cluster leaves nothing behind. Subtests dial a
normalised URI so a provider emitting the legacy syntax still gets its export
tested rather than every subtest failing on the same string; whether the URI
itself is standard is now its own subtest.
Adds a reconnect check, since publication holds the volume rather than any one
connection. It retries the real operation because a cheaper liveness probe is
itself a connection and re-triggers whatever refused it.
* feat: make the conformance suite capability-driven
The suite asserted a fixed Capabilities literal, so it could only ever pass
against a provider advertising exactly that set. Every implementation had to
match the suite rather than the suite testing the contract. RunSuite now reads
GetCapabilities from the provider under test and gates optional behaviour on
what it advertises, and the fixed literal is demoted to ReferenceCapabilities,
a convenience for callers constructing a MemoryProvider.
Add ReadOnlyPublish to Capabilities. A provider that leaves it false must
refuse a ReadOnly publish rather than hand back a writable export, which is
what viperblockd already does; it now has a way to say so up front.
Add ErrUnsupportedCapability and the unsupported_capability error code. A
request for optional behaviour a provider does not advertise is well formed,
so classifying it as invalid_argument told the caller to fix a request that
was never wrong. viperblockd's ReadOnly refusal is reclassified accordingly.
Declared capabilities now match observed behaviour: qemunbdd advertises
SparseExtentReporting (qemu-nbd exports base:allocation, confirmed with
nbdinfo) and ReadOnlyPublish (PublishVolume passes -r). viperblockd advertises
neither: the nbdkit Go binding exposes no extents callback, and the mount path
never passes -r.
The external NBD client suite gains a sparse-extent subtest that checks the
export's advertised contexts against the capability, so SparseExtentReporting
is falsifiable from outside rather than taken on trust, and its read-only
subtest now branches on the capability instead of assuming refusal.
Cover the new branches with a MemoryProvider advertising no optional
capabilities and one advertising OnlineExpansion, the branch no provider we
ship exercises.
* refactor: rename SourceVolumeID to SourceSnapshotVolumeID
CreateVolumeRequest.SourceVolumeID meant three different things. viperblockd
required it whenever SourceSnapshotID was set and used it to resolve the
snapshot's chunk references, so there it named the snapshot's origin volume.
MemoryProvider ignored it entirely. qemunbdd implemented it as an independent
volume-to-volume clone, which is what the name and doc comment implied. The
conformance suite could not catch the divergence because the reference
provider had no branch for the field at all.
The contract already has a field meaning "the volume this snapshot came from":
Snapshot.SourceVolumeID, which every provider populates. So the request field
is the same fact travelling back the other way, and its name was the problem.
Rename it to SourceSnapshotVolumeID, document it as the source snapshot's
origin volume, and state that it is required with SourceSnapshotID and
meaningless without it.
All three providers now enforce one rule: a source snapshot must be
accompanied by its origin volume, or the request is invalid_argument.
MemoryProvider additionally rejects an origin that does not match the
snapshot's own SourceVolumeID, so a wrong name fails instead of resolving
blocks against a volume the snapshot never came from. qemunbdd loses its
volume-to-volume clone branch, which implemented behaviour the contract no
longer offers and no caller ever requested.
The suite pins both the requirement and a full create-snapshot-restore round
trip, so the field is exercised end to end rather than only on the error path.
* feat: make the conformance suite runnable against a live provider
The suite had only ever run against MemoryProvider and qemunbdd, both of which
are discarded with the test binary. viperblockd, the provider production
actually uses, was the one implementation the suite never judged, because the
suite assumed each subtest got fresh state: hard-coded volume IDs collide with
an earlier run's leftovers on a store that keeps them, and a hard-coded node ID
addresses a node that does not exist.
SuiteConfig adds NamePrefix, NodeID and OtherNodeID. A tracking wrapper records
every volume, snapshot and publication the suite creates and tears them down in
dependency order, logging rather than failing so a teardown problem cannot mask
the assertion a subtest was making. Two new tests cover this by running the
whole suite twice against one shared provider instance, which passes only
because cleanup works, and once per prefix to cover concurrent runs sharing a
daemon.
TestLive_ViperblockdConformance wires this to a real cluster behind
SPINIFEX_VIPERBLOCK_LIVE, alongside the existing external NBD client suite.
* fix(viperblockd): close the four contract gaps the live conformance run found
Unpublishing a volume this node has no mount for now succeeds instead of
returning not_found, matching the legacy unmount path: nothing mounted here is
already the state the request asks for.
CreateSnapshot refuses a snapshot ID that already exists against a different
source volume with already_exists, rather than overwriting it.
DeleteVolume refuses a published volume with volume_in_use instead of tearing
the mount down, on this node or on whichever node answers the volume's owner
subject. The export and the guest writing through it survive the metadata
deletion, so the old behaviour surfaced later as corruption.
CreateVolume reads the source snapshot before writing the volume, returning
not_found when it is absent and invalid_argument when it was taken from a
different volume than the request names. Previously both produced a blank
volume and a successful response.
CopySnapshot classifies viperblock's foreign-volume refusal as invalid_argument
now that it carries a sentinel.
* feat(ebsprovider): pin the source snapshot's origin volume in the contract
The suite only required source_snapshot_volume_id to be present, so a caller
naming a volume the snapshot never came from was accepted everywhere. The
snapshot records its own origin, so this is checkable: creating from a snapshot
with a mismatched source volume is now invalid_argument for every provider,
enforced by qemunbdd against its snapshot metadata and by viperblockd against
the snapshot's persisted SourceVolumeName.
* test(ebsprovider): run the contract suite against viperblockd in CI
CI installed neither qemu nor nbdkit, so every conformance test that needs a
real implementation skipped silently and MemoryProvider was the only provider
the suite judged. CI now installs both, builds viperblock's nbdkit plugin, and
sets SPINIFEX_REQUIRE_CONFORMANCE_TOOLS so a runner image that loses them fails
instead of skipping.
TestViperblockdConformance stands the provider handlers up over an embedded
NATS and a real predastore and runs the same suite against them, which is what
caught CopySnapshot accepting a foreign volume's snapshot on unencrypted
volumes.
* feat(viperblockd): honour read-only publish and advertise block size constraints
ReadOnlyPublish was advertised false and read_only=true was rejected,
so the capability was true only by omission. nbdkit -r now starts the
export when the request asks for it, which sets the NBD read-only
transmission flag and makes the plugin refuse writes. Access mode is
fixed when nbdkit starts, so a republish asking for the other mode is
refused with VolumeInUse rather than handed the running export.
Block size constraints were absent entirely: the nbdkit Go binding has
no block-size callback, so the plugin cannot advertise them. The
blocksize-policy filter can, without a binding callback. Minimum 1
because viperblock's WriteAt handles unaligned requests with a
read-modify-write, preferred 4096 to match its block size, maximum 32M
to match what qemu-nbd advertises.
* test(conformance): judge sparse reporting by the allocation map, not the context
The subtest asserted that base:allocation was offered. nbdkit offers
that context whether or not the plugin can compute extents, answering
'all allocated' when it cannot, so the assertion passed for a server
with no extent support at all — it could not fail. What separates the
two is the answer: a volume with nothing written to it is entirely
holes to a server that really tracks allocation.
Read the map with nbdinfo --map --json and compare holes against the
advertised capability. qemunbdd reports holes and advertises true;
viperblockd reports none and advertises false.
* test(fixtures): keep SSL_CERT_FILE set for the fixture's whole lifetime
The predastore fixture is one daemon per test binary, but its cert was
exported with t.Setenv, which reverts when the first test that called
Start finishes. In-process clients did not notice because crypto/x509
caches the pool behind a sync.Once, loaded while the variable was still
set. Child processes did: nbdkit spawned by a later test inherited an
environment with no SSL_CERT_FILE and could not verify the daemon, so
the whole NBD client suite failed when the package ran as a whole while
passing when run alone.
Also install libnbd-bin in CI, so the NBD client suite runs there
instead of failing on missing tools.
* ci: install nbdkit-plugin-dev so the viperblock plugin can build
* test(volume): give the snapshot fixture an owner
CreateVolume now fails closed on a snapshot whose owner it cannot match,
including an unset owner_id. The fixture predates that and wrote none,
so the provider test failed with InvalidSnapshot.NotFound before ever
reaching the provider.
* test(integration): give the volume harness a real provider
The control plane no longer builds volumes itself, so CreateVolume in
the integration harness failed with "no EBS provider configured": the
harness wired a VolumeServiceImpl with nothing behind it. It now serves
viperblockd's provider subjects against the same predastore fixture, so
the blocks are still written by real viperblock to a real daemon.
Three persisted-state assertions moved with it. TenantID, VolumeType
and AvailabilityZone are control-plane facts, held in the ebsmetadata
document rather than viperblock's own state, so the storage oracle now
asserts their absence and DescribeVolumes -- whose answer is
account-scoped -- is what checks them.
* refactor: route image import through the EBS provider and drop the legacy metadata path
The control plane could still only run against viperblock: the AMI import and
the ochre weights import both linked the storage library directly, and the
ebsmetadata store kept a read fallback onto viperblock's config.json.
Import now goes through the provider contract with no new verb: CreateVolume,
PublishVolume, a bulk copy over the published NBD export with qemu-img, then
UnpublishVolume and CreateSnapshot. Bulk data rides NBD rather than NATS,
whose request size the contract caps far below a disk image.
The migration scaffolding goes with it. ebsmetadata documents are now the only
thing the control plane reads: no legacy prefix scan, no config.json fallback,
no backfill on daemon start. config.json belongs to the provider, which is the
single writer of it.
ListAMIs keeps its tolerant/strict split. Skipping an undecodable document is
right for DescribeImages, where one corrupt document must not hide the fleet,
and wrong for a name-uniqueness check, which cannot rule out a name it failed
to read.
spinifex and cmd no longer import viperblock outside services/viperblockd and
testutil/vbscan.
* feat: add a volume enumeration verb so a lost document cannot strand blocks
Making ebsmetadata documents the sole index of volumes left one failure with
no recovery: a volume whose document is gone disappears from DescribeVolumes,
from the leak reaper (which enumerates documents), and from the provider,
whose contract had no verb that asks what it holds.
ListVolumes closes that. It is capability-gated on VolumeEnumeration and
paginated, following CSI's optional LIST_VOLUMES rather than inventing a
shape. Refs are deliberately not Volumes: enumeration answers what exists,
and a provider must not open every volume's engine to say so.
Each implementation reads what actually exists rather than what it remembers.
qemunbdd walks its volumes directory, not its in-memory map, which does not
survive a restart; viperblockd walks the bucket's top-level prefixes. A
volume the provider has forgotten is precisely what a caller is looking for.
The consumer is a read-only orphan report: provider volumes minus documented
ones, exposed as spx admin volumes orphans. It never deletes. An EFI store is
excused only when its base volume is documented, so a stranded pair is still
reported. Listing documents stays strict, because reporting a volume as
orphaned on the strength of a document that failed to decode would point an
operator at live data.
* fix: stop the orphan scan accusing storage of holding things it does not
A live env19 run reported three orphans against a healthy cluster: an AMI's
backing volume, a snapshot, and the cluster key store. All three were wrong,
and a report that cries wolf on a clean cluster is worse than no report.
Enumeration treated every top-level bucket prefix as a volume. Snapshots sit
beside volumes at that level and have a config of their own, so presence of a
config cannot separate them; the snap- name is what does, which is already how
the AMI removal path reads them back. keys/ and spinifex/ are not volumes at
all.
An AMI's blocks are a genuine provider volume, so enumeration keeps reporting
them. What was missing is on the reading side: an AMI is recorded as an image
document, not a volume one, so the scan now treats a documented image as
known. Listing images is strict for the same reason listing volumes is —
unreadable and absent are different answers, and only one means orphaned.
* fix: let an orphaned image's blocks be reclaimed
The orphan report tells an operator that reclaiming a stranded volume is their
decision, and images remove --force is the tool it points them at. On env19
that tool failed on exactly those volumes: removal drops the ebsmetadata
document first, and predastore answers NoSuchKey for a document that is
already gone, which is the definition of an orphan.
Deleting the document stays first and stays fatal. That ordering is the
barrier that stops a launch landing on blocks about to be deleted, and an AMI
still advertised after its blocks are gone is the worse failure. Only an
already-absent document is now tolerated: the barrier is satisfied, and the
blocks are the sole thing left to reclaim.
S3ObjectStore.DeleteObject normalises a missing key the way GetObject already
does, so this is recognised rather than matched on error text.
The obvious unit test passed without the fix, because the in-memory store is
silently tolerant on delete and real S3 answers 204 — neither behaves like the
backend this runs against. The test now drives a store that reports the miss,
and fails without the fix.
* feat: add per-verb OTel spans to the EBS provider contract
The ten provider verbs cross a single NATS seam and nothing measured it.
The only spans on the storage path were the legacy ebs.mount/ebs.unmount
pair, so control-plane storage latency could not be attributed to a verb
or compared across provider implementations.
NATSProvider.request is the one client-side funnel every verb passes
through, including both legs of an owner-subject fallback, so a single
span there covers the whole contract and measures what the control plane
experiences regardless of which provider answers.
Span names come from a closed verb set rather than the subject. Owner
and snapshot-create subjects embed a volume ID and the mount/unmount
subjects embed a node ID; naming spans after them would give every
volume a span name of its own. The IDs move to attributes and an
unrecognised subject collapses to a single unknown verb.
Trace context rides the message headers, so the serving provider joins
the caller's trace instead of rooting one of its own. The carrier is a
copy of the one in spinifex/utils rather than an import: that package
pulls in the aws-sdk and the cluster key store, which the wire contract
must not depend on.
Responses carry no shared accessor for their error field, so both sides
re-read just that field from the encoded reply, and only when the span
is recording. Without it a failed verb looks like a successful round
trip.
viperblockd serves the contract with its own handlers rather than
natsserve, so its handlers now take the per-message context and its
publish/unpublish pair drops the subject-named consumer span it opened
for itself.
* feat: add null and hostile providers, and a per-verb bench suite
The provider boundary made it possible to put something other than
viperblock behind the control plane, and nothing yet did.
nullprovider is the floor. It is not a new type: MemoryProvider already
answers every verb from a map and natsserve already serves any
EBSProvider, so the package names that composition and states what it
measures. On loopback with no storage behind it a round trip costs
around 195us against 74ns in process, which is the seam's own price and
is not attributable to any storage engine.
conformance.RunBenchSuite prices one round trip per verb and drives
every implementation through the same calls in the same order, so two
providers' numbers are comparable. It measures control operations only:
guest I/O never crosses these verbs.
hostile wraps any provider in one that misbehaves. Faults derive from
the seed and a per-target sequence rather than a global counter, so
concurrent callers on different volumes cannot reorder each other's
faults and a failure found deep into a soak run replays exactly. Error
and error-after-work are separate faults on purpose: the second reports
failure after the work has already happened, which is how an orphan is
made, and is what makes an enumeration scan the right oracle for a soak.
Enumeration itself is never faulted, since an oracle that can be told
what to say cannot falsify anything.
The soak asserts that every volume the provider still holds is
attributable to an injected fault. A leftover with nothing behind it is
the control plane losing track of storage on its own.
Two bugs were found by falsifying the new tests rather than by running
them. The FNV-derived draw clustered badly enough that two fault bands
were never reachable in 1200 draws, so the draw now goes through a
SplitMix64 finalizer. And the first version of the lie test was
worthless: MemoryProvider returns copies, so a lie could not corrupt it
whether or not the injector copied, and the test now drives a provider
that hands out pointers into its own state.
* test: bench the provider contract against qemunbdd and a live viperblockd
RunBenchSuite was only ever pointed at the null provider, so the numbers
priced the seam and nothing else. Point it at the two real implementations.
BenchConfig gains a VolumeBytes knob: 1 GiB per iteration is fine against a
provider that stores nothing and wrong against one that does, so qemunbdd and
viperblockd are both measured at 64 MiB.
liveProvider splits into a testing.TB flavour. Benchmarks need the same
connection and the same node and cannot be handed a *testing.T.
* fix(viperblockd): carry the trace context into every viperblock engine call
Both S3 clients only emit a span when the request already carries one, so
calling the no-context Init, LoadState, SaveState, LoadBlockState and Close
left the whole engine open untraceable. Every provider verb traced as one flat
server span, and the 342ms describe measured on env19 could not be attributed
to anything.
viperblock already exposes the Ctx variants and Backend already declares
InitCtx, so this is threading, not new instrumentation. openVolumeVB,
openLoadedVolumeVB, sealVolumeVB, describeVolumeEngine, snapshotVolumeEngine,
copySnapshotOnVolume, seedVolume and applyConfigUpdate now take a context, and
mount/unmount keep the context their own span derives instead of dropping it.
completeCreateSnapshot gets WithoutCancel: the background half of
accept-then-publish has to outlive the request it was accepted by and still be
attributable to it.
The per-volume ebs.config handler opens a consumer span like the other legacy
subjects, so a config update is no longer the one path with no trace at all.
* perf: describe a volume without opening an engine on it
describeVolumeEngine fell through to openVolumeVB for any volume not mounted
on the node serving the request, which constructed a full viperblock engine
to read one integer. On env19 that decomposed as a 22 ms reachability list, a
3 ms state read and a 16 ms PutObject of the volume's own config.json — the
write coming from the SeqNum bootstrap a read-write open performs.
Use viperblock's read-only state load instead. A describe is now the 3 ms
read: no background goroutines, no reachability probe, no SeqNum window
claimed, and no write issued by a worker that does not own the volume.
Falsified: restoring the openVolumeVB call fails
TestDescribeVolume_ReadsWithoutOpeningTheVolume on both the write and the
list, with the fake backend answering each so neither assertion hides behind
the other.
* perf: share one HTTP client across every viperblock backend
Four sites built the same S3Config by hand, each leaving HTTPClient unset, so
every engine open and state read got its own connection pool and paid its own
TLS handshake. That was invisible while Backend.Init issued a whole-bucket
list — the list paid for the connection and the state read rode it — and it
became the dominant cost once describe was reduced to a single request.
Route all four through cfg.volumeS3Config, which builds the client once.
Falsified: dropping the HTTPClient field takes
TestDescribeVolume_ReusesOneConnection from one connection to three.
* perf: wait for nbdkit to be ready instead of waiting a fixed second
mountVolume spawned nbdkit and then ran a confirm loop until a one-second
deadline passed. The loop only broke early when the process exited, so the
fast-failure path worked and the success path always cost the full second.
That second was ~1015 ms of a ~1040 ms publish, and it emitted no spans, so
the timings never contradicted the assumption that it was process spawn.
Wait for the endpoint to accept a connection instead. That is also the
question the caller needs answered, since it is about to hand the URI to
something that will connect to it — where 'still alive after a second' only
correlates with it. A nbdkit that exits during startup still fails the mount
immediately, and one that never comes up now fails on a deadline rather than
being assumed healthy.
Falsified: reintroducing a fixed sleep in pollUntil fails both the readiness
timing assertion and the fast-failure one.
* test: measure the data path, not just the control verbs
The conformance benchmarks price create, publish and describe. Guest
I/O never crosses those verbs, so none of those numbers say anything
about what a guest experiences. This drives the published export with
qemu-img bench, which sits on the same block layer virtio-blk does.
It asserts only that the I/O completes and logs a table: what a given
store is capable of is not a contract, and pinning a number here would
only produce a flaky test.
Read profiles stay inside the region the write profiles covered, or
they would be timing how fast the export answers with zeroes, and one
write profile flushes, because an unflushed write measures how fast the
WAL accepts data rather than how fast the store takes it.
Profiles are spaced apart because the plugin tears an engine down per
disconnect and removes the volume's local directory as it goes, so a
connection opened while the previous one is closing fails its
handshake.
* test: run data-path profiles back to back
The spacing was a workaround for a volume lock that did not hold across
close. With that fixed, consecutive profiles are worth more than the
workaround was: each is its own NBD connection, so a run that completes
has also shown that an arriving connection waits out the previous one's
close rather than racing it.
* fix: refuse a delete that cannot establish whether a volume is published
volumeMountedElsewhere returned false on every failure path, including a
request timeout, so a busy or partitioned owner was indistinguishable
from no owner at all and the delete proceeded. Deleting a published
volume leaves the nbdkit export and the guest writing through it alive
over storage that is going away.
Report the three outcomes separately. NATS distinguishes no-responders
from no-answer, so only the former means unmounted; an unanswered probe
is refused and the caller may retry once the owner is reachable.
* feat: exclude a second viperblock engine with a JetStream volume lease
Nothing in the stack stopped two viperblock engines opening one volume from
different nodes. For an encrypted volume that is not just a consistency risk:
the AES-GCM nonce is fully consumed by SeqNum, VolumeUUID and a domain byte,
and the SeqNum window claim is a read-then-write with no CAS, so two engines
opening concurrently issue overlapping nonces under the same key.
Every engine open now takes a lease first, keyed by volume in a JetStream KV
bucket. A lease is a KV create, which JetStream serialises, so a second
claimant loses instead of both winning. The revision that won is the
generation: monotonic across the bucket, so a later opener always carries a
higher one than the writer it displaced. Entries carry a TTL and a renewal
loop, so a node that dies mid-mount does not wedge its volumes forever.
Losing a lease is logged, not enforced. Until the object store can refuse a
stale writer's PUT, failing live guest I/O on lease loss would add a failure
mode without stopping the zombie it is aimed at.
A daemon with no lease store refuses to open rather than opening blind, so
nbdkit recovery binds the store before it adopts surviving exports. Repeat
acquisitions on one node share a lease: the unmount seal opens a detached
engine while the mount being torn down still holds it, and cross-node
exclusion is what the lease is for. A restart reclaims this node's own stale
entries, since the exports that outlived the daemon are still running.
* refactor: consume masterkey from bluebottle in the test this branch added
* feat(ebsprovider): classify refusals a caller may retry
Every ErrorCode said what went wrong and none said whether the same
request could succeed later, so a caller could not tell a permanent
refusal from a transient one. Transient conditions shipped as internal,
which tells a caller to give up when it should retry.
Adds ErrUnavailable / ErrorCodeUnavailable and ErrorCode.Retryable(),
which callers branch on instead of on the code itself. DeleteVolume's
"could not establish whether the volume is published" refusal moves off
internal onto it: the owner that did not answer may answer later.
TestErrorCodeRetryable names every code, so adding one without deciding
its retry meaning fails rather than silently defaulting to permanent.
TestClassifyError pins the sentinel-to-wire-code mapping, which had no
coverage at all; a sentinel missing from classifyError falls through to
internal and loses both the reason and the retry signal.
* fix(ebsprovider): drop CopyOnWriteClone, which no request can ask for
Four providers advertised the capability and nothing consumed it: there
is no clone verb on the interface. CreateVolume's SourceSnapshotID
restores from a snapshot and CopySnapshot duplicates snapshot metadata;
neither is a volume clone, and the field documenting SourceSnapshotID
says so explicitly. CSI expresses clone as a volume_content_source with
a volume source, which this contract does not implement.
Advertising it told a caller a feature was available and then gave them
no way to use it. Removed rather than backed by a method: adding a clone
verb is a feature, not a contract fix, and the capability can come back
alongside it. The Capabilities doc comment now states the rule, and
qemunbdd's stale comment about sparse extents is corrected while there.
* feat(ebsprovider): add ListSnapshots so snapshots are reconcilable
The contract could create, copy and delete snapshots but never say which
ones it held. That left the control-plane metadata document as the only
index of them, so losing one stranded the data it pointed at with no way
to find it again. This is the same exposure ListVolumes closed for
volumes, and it stayed invisible because ListSnapshots is optional in
CSI too.
ListSnapshots pages exactly as ListVolumes does, gated on a new
SnapshotEnumeration capability. SnapshotRef is deliberately not a
Snapshot: enumeration answers what exists, and reading a snapshot's size
or state can cost far more than listing it.
viperblockd walks the bucket's snap- prefixes, sharing the top-level
walk with ListVolumes. It leaves SourceVolumeID empty because the link
lives inside the snapshot's own encrypted state and reading it would
mean a fetch per snapshot listed. qemunbdd walks its snapshots
directory rather than its in-memory map, so a snapshot that outlived the
process still appears; its SourceVolumeID is empty in that case for the
same reason CreateSnapshot refuses to reuse the ID.
Five conformance subtests cover it, and they run green against
MemoryProvider, qemunbdd and viperblockd over a real predastore.
* feat(ebsprovider): state the exclusion guarantee in the contract
A caller could not tell which providers prevent two writers from holding
one volume, at what scope, or for how long. The guarantee existed in
viperblockd and was invisible from the outside, which is why the
conformance suite had no concurrency dimension at all.
Capabilities.Exclusion states it, and deliberately separates three
questions the contract used to run together: how far exclusion reaches
(none, node, cluster), whether a dead owner's claim ever expires, and
whether a writer that lost its claim is actually stopped. That last one
matters most and is the least flattering: exclusion refuses the second
opener, fencing stops the first. viperblockd advertises cluster scope
with a 45s TTL and FencesLostClaim false, because a partitioned node
whose lease lapsed keeps writing until the object store can refuse it.
Four new conformance subtests. Two required a second viperblockd node in
the harness: cluster exclusion cannot be exercised by one node, because
the second publish goes to a subject nobody serves and times out rather
than being refused. Both nodes share the NATS the leases live in and the
bucket the volumes live in, so a refusal comes from the lease.
Standing that second node up immediately found a real gap, in a test
that had been silently skipping for want of one. A publish refused by
another node's lease came back as internal rather than volume_in_use, so
a caller was told something had broken when the answer was that someone
else held the volume. Mapped in handlePublishVolume.
* build: pin viperblock to the release carrying the provider's dependencies
The branch has been building against a local replace pointing at a
viperblock worktree, so its committed pin never actually built: it named
a version without CopySnapshotMeta, ErrSnapshotVolumeMismatch,
NewStateReader or s3.NewHTTPClient, all of which this branch calls.
Points at the merged viperblock dev instead and drops the replace.
GOWORK=off now builds clean, which is the deploy path ansible uses.
* fix: let viperblockd's delete sweep tolerate an object another sweep removed
deleteObjectPrefix claimed to mirror handlers/ec2/volume's deleteS3Prefix and
give DeleteVolume and DeleteSnapshot idempotent-when-absent behaviour, but it
returned on any DeleteObject error. Two overlapping deletes for one volume left
the loser reporting an internal failure once the winner had removed a chunk
between the list and the delete.
The control-plane copy that carried this fix is gone with the embedded engine
path, so the provider path reintroduced the bug it had just fixed.
* fix: stop CreateVolume leaving local state on whichever node served it
A state read prefers this node's local copy over the backend, and provider RPCs
are not owner-routed, so the node that happened to serve a create kept a copy
that answered describe as if the volume still existed after another node
deleted it. The two-node conformance harness caught it as DeleteVolume/success
failing about one run in three.
The seeded path already removed its local files after closing. The unseeded
path saved state and left them, which was the only difference between the two.
* fix: guard the mount path itself against a second export of one volume
handlePublishVolume refused a volume this node already exported, but
mountVolume did not, and the legacy ebs.<node>.mount subject is the route
production takes for instance launch and hot-attach. A retried attach racing a
fresh one started two real nbdkit against one volume, with no refusal and no
warning: the volume lease only refuses a different node, and a same-node
re-acquire finds the volume already held and increments its refcount.
That also made the advertised exclusion inaccurate. viperblockd claims
cluster-scoped exclusion on the grounds that a second opener is refused, while
the conformance subtests reach it only through PublishVolume, so they passed
with the legacy route unguarded. Moving the check into mountVolume gives both
entry points the same behaviour and makes the claim true for the path
production uses.
* fix: tolerate a missing legacy AMI config on deregister
DeregisterImage deletes the AMI document, then the pre-provider
{imageID}/config.json. Predastore answers a delete of an absent key with
NoSuchKey, and that was treated as fatal, so every AMI created since the
metadata store landed failed to deregister after its document was already
gone. A retrying caller then saw InvalidAMIID.NotFound for an image it had
just been told it could not delete.
The regression test uses a store that answers a missing-key delete the way
Predastore does; MemoryObjectStore silently succeeds, which is why the unit
suite stayed green.
* fix: reap ENIs left behind by an abandoned instance launch
A launch creates the instance's ENI and then attaches it. When the daemon
dies in between, the record survives with no instance and nothing ever looks
at it again. The security group dependency check counts an ENI that lists the
group, so the group cannot be deleted for as long as the record exists.
The in-process launch path already rolls back on error; a process that dies
cannot. This adds a cluster-wide sweep over records that are launch-created,
never attached, and older than the launch RPC timeout by a wide margin. The
description prefix is now a shared constant because it is the only thing
separating an abandoned launch from a standalone ENI a caller asked for.
|
||
|
|
4dca2c7b59 |
fix: drain guests on a plain systemctl stop spinifex.target (#766)
* fix: drain guests on a plain target stop, not just restart
`systemctl is-system-running` cannot tell a plain `systemctl stop
spinifex.target` apart from the stop half of a restart -- both report
`running`, and the old `--only-if-host-stopping` gate skipped the drain
for both. That left guests running with no QMP shutdown and no volume
unmount while predastore/viperblock stopped underneath them, so a real
stop discarded any writes still in flight.
Add a second signal: `systemctl list-jobs` shows a pending START job for
spinifex.target during a restart (the target's stop leg resolves
instantly, since it owns no processes) or a pending RESTART job for
spinifex-shutdown.service itself when that unit is restarted directly
(PartOf propagates target-to-member only). Renamed the flag to
`--unless-restarting` to match: drain on a real shutdown or a plain
stop, skip only when one of those jobs shows a restart is already
queued. Any step systemctl can't answer fails toward draining, and a
skipped drain now warns loudly (naming each instance still running) so
an operator sees the risk in the journal instead of discovering it via
`ps`. `--only-if-host-stopping` stays as a deprecated, still-functional
alias so a partial upgrade between unit file and binary doesn't break.
* fix: retry the local drain GATE/DRAIN ACK instead of failing on first miss
A drain that fails at GATE is indistinguishable from a skipped drain from
the guest's perspective: it keeps running while storage goes away
underneath it. Stopping spinifex.target shortly after a restart hit exactly
this: the daemon had not finished re-subscribing to NATS, so the ACK
request came back with zero subscribers and NATS bounced it back near-
instantly ("no responders"), which is why the failure logged in about a
second despite --timeout=120s. collectLocalShutdownACK now retries with
backoff bounded by the phase timeout instead of failing on the first bounce,
and an exhausted retry budget logs at ERROR and names the guests still
running via a severity-aware refactor of warnIfGuestsLeftRunning, so a
failed drain is unmistakable in the journal rather than silent.
* fix: reap idle nbdkit concurrently and re-derive viperblock's stop budget
KillMode=process made viperblockd the sole reaper of nbdkit on SIGTERM, but
shutdownVolumes reaped them one at a time and TimeoutStopSec was never
re-derived for that change. With several mounted volumes the serial loop
blew past the 30s budget, systemd SIGKILLed viperblockd mid-loop, and
whichever nbdkit hadn't been reaped yet was orphaned.
Fan the per-volume utils.KillProcess calls out concurrently so the reap
costs one worst-case nbdkit, not N of them summed, and raise
TimeoutStopSec to 150s to strictly exceed that single-volume worst case
(120s KillProcess grace + nbdkit's own 20s drain + headroom). Add an
invariant asserting the two stay in that relationship, and a test proving
the reap is actually concurrent (fails against the old serial loop).
* docs: document the host and guest lifecycle contract
States what happens to running guests for every host lifecycle event: reboot, target restart, target stop, daemon restart, daemon crash, power loss and a coordinated cluster shutdown. Records the older-release behaviour where a plain target stop left guests with no data path, and how to tell which behaviour a node has.
|
||
|
|
c9e330d8ae |
refactor: consume bluebottle otelsetup, drop the local bootstrap
otelsetup/otelsetup.go and otelsetup/slog.go were a near-copy of the shared bootstrap. Entrypoints now call bluebottle directly, passing WithEnvironmentFallback for SPINIFEX_CI_ENV; the package keeps only the HTTP middleware and request metrics. Init reattaches the OTLP bridge to a default logger installed before it ran, so the local addFanoutHandler is no longer needed. The bridge is now level-gated, so services no longer ship Debug records to the sink while running at Info. |
||
|
|
8f930e6275 |
refactor: consume the shared packages from bluebottle
Takes iampolicy, masterkey and sigv4 from github.com/mulgadc/bluebottle rather than predastore, so the control plane no longer depends on the S3 server for shared auth and policy code. |
||
|
|
c92294e7d5 |
feat: render predastore templates with the cluster and S3 planes split (#763)
* feat: render predastore templates with the cluster and S3 planes split The templates set one bind_addr per host, which put raft and blob wherever S3 was published. A host now binds the cluster plane on the address peers dial and the gate carries its own bind_addr for the S3 API, so the installer can hand S3 the WAN without exposing replication with it. The single-node template also drops to one node of each role. Extra blob nodes only hash-partition keys across more badger indexes on the same drive, and extra meta replicas add fsyncs for a quorum that dies with the machine. * docs: regenerate the AWS model operation coverage table * fix: repin predastore to the merged dev commit |
||
|
|
1394fbf488 |
docs: describe the predastore host and node model
The port tables were wrong in both directions: inter-node traffic is QUIC over UDP rather than TCP, and the shard range 9991-9993 no longer exists. Ports are unique within a host rather than across the cluster, so every machine uses the same three, and a single-node install opens only the S3 port because colocated nodes talk over an in-process pipe. The reference architectures scripted a move of node data out of a directory layout that no longer exists, which would have broken any install following them. The update guide now says object storage has no migration path. |
||
|
|
a7a62addeb |
test: rebuild the predastore fixture and service tests on the facade
The fixture wrote a config the strict decoder rejects and built its cluster through clusterrun, so it now renders a v1 TOML, loads it through LoadConfig and serves under Run. Loading the real file means the fixture trips over the same validation an install does. Its data directory holds node-<id> directly, and it drains on cancellation rather than being abandoned at exit. The runtime tests drove buildRuntime, serve and the old on-disk layout, all of which predastore owns now, so they go. What replaces them covers what spinifex still decides: the flag merge and its precedence, a host id or gate node the config does not have, and the pid file behind Stop and Status. Two fixes to earlier commits in this series: the host id conversion moves behind its own guard, and the node status probes use WaitGroup.Go. |
||
|
|
89df817601 |
refactor: run the predastore service through predastore.Run
The facade owns every node of a host, the S3 gate among them, so the service no longer builds a cluster runtime or a gateway of its own. Waiting for a leader, draining the gateway and the shutdown grace all moved behind Run; pprof and the base path have no equivalent and are gone. spinifex.toml stays authoritative for the S3 endpoint: the host entry is settled from the service flags before Run, and the gate is found by role rather than by position. A host id is now required, so init writes one for single-node installs too, where the topology was previously left implicit. BasePath survives as the pid directory alone. Stop and Status now read the same value start writes, which they did not before. |
||
|
|
588162c136 |
feat: rewrite the predastore config templates onto the v1 schema
The decoder is strict now, so every key the reorganised predastore no longer reads is a startup failure rather than an ignored line. Both templates move to nested [[host.node]] tables with the gate, blob and meta roles, portless bind_addr and addr, singular [[bucket]], and the host TLS and encryption-key paths the schema requires. version resets to 1 with the format. A gate node is what serves the S3 API, so a host without one answers nothing. Ports are unique within a host rather than across the cluster: gate 8443, blob from 6660, meta from 7660. Colocated nodes bind no socket and still need one. |
||
|
|
96dc1af568 |
refactor: delete the predastore cluster topology migration
The migration rewrote a v4 topology into v5 and recovered raft state through predastore's clusterrun package, which no longer exists. The on-disk layout it relocated data into has moved too: node directories are now node-<id> under the host root, with no host-N level. Nothing migrates a pre-cutover install; it is re-initialised from scratch. node-reset.sh keeps directories by design, so it now removes the predastore cluster tree outright rather than leaving the old skeleton standing. |
||
|
|
57d3db1683 | feat: installer multidisk roles (#760) | ||
|
|
7440446bbb | feat: install ext4 with role-assigned drives for spinifex and predastore (#759) | ||
|
|
835dbbaf1c |
feat: parameterise bedrock endpoint create/delete by account and pinned (#757)
* feat: parameterise bedrock endpoint create/delete by account and pinned Widens EnsureEndpointInput/DescribeEndpointInput/DeleteEndpointInput with AccountID and (Ensure-only) Pinned, and keys the daemon's Ensure/Describe/Delete on the resolved account instead of the hardcoded GlobalAccountID. An empty AccountID still resolves to GlobalAccountID and Pinned defaults to false, so every existing caller (resolver, reaper, evictForCapacity, the NATS gateway client) is unchanged. This is stage 1 of provisioned throughput: it reuses the existing launch/capacity/readiness machinery for a dedicated, account-scoped, pinned endpoint without adding a parallel surface. * feat(bedrock): add provisioned-throughput CRUD and PT ARN Implements CreateProvisionedModelThroughput, Get, List, Update, and Delete over a new bedrock-provisioned KV store, plus the provisioned-model ARN format/parse. Create rejects provider-hosted models and requests a pinned, account-scoped endpoint from the daemon; Get derives status live from the pinned endpoint's state; Update honours name changes only and rejects a served-model swap; Delete tears down the pinned endpoint before removing the record. EndpointProvisioner is declared in gateway_bedrock with primitive-typed methods (handlers_bedrock already imports gateway_bedrock, so it can't be imported back for its EndpointService interface directly); a small adapter in handlers_bedrock bridges the two, always setting Pinned:true and the caller's real account on every Ensure. Does not yet wire a PT ARN into the Converse/InvokeModel inference path (next stage). * feat(bedrock): accept a provisioned-throughput ARN on Converse/InvokeModel Translates a PT ARN before resolve: the gateway inference entry parses it, loads the bedrock-provisioned record, and resolves against the pinned endpoint for the commitment's own account rather than the GlobalAccountID shorthand every bare modelId uses. Adds EndpointResolver.EndpointForAccount (gateway-side interface, implemented by DynamicEndpointResolver) so the seam stays import-cycle-safe, and threads it through vllm.go/invoke_llama.go's resolve call sites. A foreign-account or unknown PT ARN reads as ResourceNotFoundException, same as any other unresolvable model identifier. * fix(bedrock): accept a provisioned-throughput ARN on streaming inference ConverseStream and InvokeModelWithResponseStream built their router/adapter with a nil ProvisionedStore and the GlobalAccountID-shorthand provider, so a PT ARN was silently never translated on the streaming path even though the non-stream Converse/InvokeModel siblings already handled it. Thread the gateway's provisioned store through both entry points and give Router.ConverseStream / InvokeStreamRouter.InvokeModelWithResponseStream the same translate-before-resolve treatment Converse/InvokeModel already have. * fix(bedrock): surface pinned endpoints in the operator CLI Service.List only ever enumerated the GlobalAccountID key prefix, and 'endpoint describe' always looked up GlobalAccountID too, so a pinned, account-scoped endpoint held a GPU while reading ABSENT and missing from list. List now scans every account's records, and describe gains an optional --account flag (default empty, preserving GlobalAccountID resolution) to look up a specific account's endpoint. list/describe both render the endpoint's account and pinned status. |
||
|
|
9b2334b964 |
fix(ochre): self-heal weights stage when the recorded snapshot is gone (#753)
* fix(ochre): self-heal weights stage when the recorded snapshot is gone runStageWeights's idempotency check trusted a matching source URI without verifying the snapshot it names still exists, so a host rebuild or volume GC that lost the snapshot left stage permanently short-circuited on a useless KV record until an operator ran 'weights remove' first. Inject a weightsSnapshotChecker, verify liveness before the no-op return, and fall through to re-materialise when the snapshot is gone. 'weights list' uses the same checker to mark dangling entries so an operator can see which staged models can actually serve without waiting for endpoint launch to fail. * docs: note weights stage self-heal and list STATUS column |
||
|
|
3601bb4d36 | fix: don't output secret keys on installs | ||
|
|
53b31cb76c | fix: keep the RDS master password out of the server and console logs (#754) | ||
|
|
d178ef5992 |
feat: version-aware host-asset preflight for binary-swap deploys (#750)
A binary-swap deploy (make deploy or a manual spx copy) ships a new binary whose host-config prerequisites — the privileged helper scripts and their sudoers grant, placed only by setup.sh / setup-ovn.sh — it does not ship. The node looks healthy and the gap surfaces later as a cryptic launch failure or a silent security regression. Add a preflight package that checks the host against what this build expects, comparing asset content, not existence, so a stale-but-present asset is caught too. The managed helpers install from quoted heredocs, so their on-disk bytes equal the heredoc body verbatim; the binary carries the canonical sha256s and compares. - spinifex/preflight: CheckHost / CheckHostAt, content hash of the two managed helpers and a NOPASSWD grant-line check for the sysctl helper. - Generated canonicalHashes plus a drift guard that re-extracts the heredocs and fails if the committed manifest no longer matches. - Three surfaces: spx admin preflight, a make deploy gate (SKIP_ASSET_PREFLIGHT=1 to bypass), and a non-fatal daemon boot warning. |
||
|
|
bc1dd3413e |
fix(bedrock): make the reaper's decision visible to an operator (#746)
Three gaps found while verifying idle reclaim on live hardware. The reclaim log line passed a time.Duration, which slog renders as an int64 nanosecond count in JSON: the line read idleFor:135000000000 rather than 2m15s, in journalctl and in the metrics sink alike. It now logs an integer second count under a unit-suffixed field, which is both readable and still aggregatable — a string would have cost the latter. The same shape appears at around ten other sites across the fleet and is tracked separately. The endpoint command's help still told operators the gateway could not request endpoints itself, which stopped being true when dynamic resolution landed. It now describes the lifecycle that actually runs, and says what the manual commands are still for. describe showed nothing the idle sweep observed, so an operator asking why an endpoint was or was not reclaimed had to read the KV. It now reports in-flight count, last activity, idle duration, pinned status and consecutive scrape failures for a READY endpoint, and nothing for any other state, since no other state is swept. LastActive moves onto EndpointRecord so the idle clock, the LRU eviction order and this output all read one definition rather than three. |
||
|
|
0a1d837f40 |
feat(bedrock): gate Ochre models behind per-account access grants (#741)
* feat(bedrock): gate Ochre models behind per-account access grants Self-host models were invokable by any authenticated account: tieredCatalog returned them unconditionally and no runtime router checked the caller. GetFoundationModel also described models the caller could not use. Add a bedrock-model-access JetStream KV bucket holding presence-only grants, resolved through a new AccessResolver. Access is deny-by-default: a gateway with no store configured falls back to DenyAllAccessResolver, so an unconfigured deployment serves nothing rather than everything. The system account bypasses grants, matching how quotas exempt it. All four runtime routers share one grantedCatalogEntry gate so they cannot drift on who may invoke what. List and describe agree: an ungranted model is absent from ListFoundationModels and reported as ResourceNotFoundException by GetFoundationModel, so the error cannot confirm a model exists. Administration lands on two surfaces: Grant/Revoke/ListModelAccess actions on the spinifex gateway service for the UI, and `spx admin ochre access` for operators and provisioning, which opens the bucket over NATS directly rather than requiring a SigV4 client the CLI does not have. * feat(bedrock): seed the admin account's model grants at gateway start Deny-by-default left a fresh install with a catalog nobody could see, and the first cut filled that gap from ansible — which is a dev tool, so any install that never ran it came up unusable. Seed from the product instead: on start awsgw grants the platform admin account the full catalog, once, behind a marker key in the same bucket. The marker makes it a bootstrap step rather than a policy: tenant accounts still begin with no access, and an operator who revokes a grant does not have it restored by the next restart. Grants are written before the marker so a crash midway repeats the idempotent grants rather than leaving a half-seeded account, and the marker is a conditional create so only the first node to reach it need win. Seeding is best-effort: a KV failure logs and leaves the gateway serving, and the next start retries because the marker is written only on success. * test(bedrock): cover the model-access CLI and gateway admin actions Route the access CLI runners through the same loadConfigAndConnectFn / ochreExit seam the weights runners use, so grant, revoke and list can be driven against an embedded JetStream server, and add gateway tests for the grant admin actions including the non-admin denial. |
||
|
|
3379b16586 | fix: encrypt and acknowledge the RDS bootstrap password (#739) | ||
|
|
272b4b22e8 |
feat(ochre): add the vLLM serving AMI and give the daemon a route to it (#740)
* feat(images): add a vllm-serving AMI as a stacked mkosi profile
LaunchServingVM resolves its AMI by the tags spinifex:managed-by=bedrock and
spinifex:bedrock-role=vllm-serving. No image carried either, so every Ensure
failed at resolveServingAMI. The only GPU image ships the NVIDIA driver and
Docker but no Python, no vLLM and no serving unit, so tagging it would have
moved the failure from admission to first boot.
Add a vllm profile that stacks onto gpu-nvidia and owns the whole guest side
of the launcher contract: the runtime, vllm-serve.service, the weights mount
and the tags. vLLM is baked into a uv-managed venv rather than pulled as a
container at boot — a runtime docker pull of vllm/vllm-openai is known to
stall this platform's write plane, and mkosi's unprivileged build has no
daemon to bake an image from anyway.
Also bring images/mkosi.profiles/ under make test-images. Its scripts have no
.sh suffix, so the existing name glob skipped exactly the files worth linting;
sources are found by shebang instead.
* fix(images): make the vllm postinst survive the build sandbox
Two failures found by actually running the build.
uv installs to /usr/local/bin, which mkosi's postinst PATH need not carry, so
`command -v uv` reported a good install as missing. Call it by absolute path,
and check it the same way.
`vllm --version` initialises device config and dies with "Failed to infer
device type" on a builder with no GPU, so it could never have verified
anything. Assert the pinned version through package metadata instead, which
also catches a version mismatch rather than merely a broken entrypoint.
* fix(images): pin the serving AMI datasource and mirror cloud-init to the console
A serving VM boots through LaunchSystemInstance, so it comes up dual-NIC with a
fw_cfg netcfg blob and takes its user-data solely from IMDS — the same shape as
the EKS control plane. That path carries a cloud-init datasource pin; the vllm
profile never got one, and the base config deliberately leaves datasource_list
unpinned, so the serving image was the only system VM booting without it.
Reuse the EKS pin rather than copying it. The 99- prefix is load-bearing:
read_conf_d() applies cloud.cfg.d in REVERSE and mergemanydict() is
first-source-wins, so a 90- name loses to cloud-init's own 90_dpkg.cfg ('_'
sorts above '-') and the pin is discarded in silence. The postinst asserts the
MERGED datasource_list via cloud-init's own read_conf_with_confd rather than
checking the file exists, because it existed at 90- too and did nothing there.
Also mirror every cloud-init stage to the serial console. A serving VM is
headless, and a guest that wedges before handover never surrenders
/var/log/cloud-init-output.log, so the console get-console-output reads is the
only channel an operator has.
The images_test.go case covers the two tags resolveServingAMI filters on; it
matches on those alone and never on name, so a typo hides the image entirely.
* fix(images): make the dhcpcd data-NIC lease genuinely one-shot
dhcp_oneshot() promised to configure the link and exit "leaving no daemon
to fight cloud-init", but the dhcpcd branch passed only -q, which is quiet
rather than one-shot. Every Ubuntu image takes that branch, so a dhcpcd
daemon survived on the data NIC holding the control socket.
cloud-init's Ec2 datasource then runs its own ephemeral dhcpcd on the same
NIC. Instead of spawning, dhcpcd forwards the invocation to the resident
daemon as a control command and exits 0, and --dumplease returns the
resident daemon's lease, so discovery looks successful. Cleanup then waits
for a pid file at /run/dhcpcd/<iface>-4.pid that no process ever wrote,
spinning its full 30000 x 10ms budget: init-local stalls for exactly 300s
on every system-instance boot.
Adding -1 exits once the interface is configured and -p keeps the address
and routes across that exit. Both are needed; -1 alone tears the link back
down. The test now models the real clients — the stub stays resident unless
given the flag that makes its client exit, and the dhcpcd branch is forced
by dropping udhcpc from PATH rather than left to whether the host happens
to have /usr/share/udhcpc/default.script.
* fix(images): serve without a CUDA toolkit and stop vllm-serve crash-looping
vLLM picks FlashInfer for top-k/top-p sampling, and FlashInfer JIT-compiles
that kernel on first use. The serving image ships the driver and the
container toolkit but no nvcc, so the sampler warmup died with "Could not
find nvcc and default cuda_home='/usr/local/cuda' doesn't exist" minutes
into a GPU cold start, after the model had already loaded.
VLLM_USE_FLASHINFER_SAMPLER=0 selects vLLM's native top-k/top-p sampler,
which is the same operation with no CUDA toolchain at runtime. Everything
else on the serving path is already precompiled in the wheels: the attention
backend resolves to FLASH_ATTN and CUDA graph capture completes, both
without nvcc. Baking FlashInfer's AOT modules is the alternative if the
fused kernel is ever wanted, and it keeps the toolchain in the builder
rather than the appliance.
The unit had Restart=on-failure with no bound, so an unfixable start failure
restarted forever and buried its own first traceback under identical copies
on the serial console — the only view an operator has of a headless guest.
StartLimitIntervalSec/StartLimitBurst make it report instead.
The postinst asserts the installed unit carries the sampler setting, for the
same reason the datasource pin is asserted there: the alternative is finding
out at runtime, on a GPU, five minutes in.
Also corrects the shared datasource pin's header, which still claimed it
installs as 90-spinifex-ec2-imds.cfg. It is installed at 99- by the vllm
profile precisely because a 90- name loses the reverse-sorted, first-source-
wins cloud.cfg.d merge to cloud-init's own 90_dpkg.cfg.
* fix(bedrock): raise the serving-endpoint readiness bound to 15 minutes
The 5 minute bound was chosen before any cold start had been measured, and
its own comment asked to be corrected once a model actually ran on wattle.
It now has: Llama 3.2 1B on an RTX A1000 reached "Application startup
complete" 257.6s after launch, leaving ~42s of margin on the smallest model
this platform can serve, with a cold torch.compile accounting for 26.3s of
it. A bigger card and a bigger model are unmeasured.
The two failure modes are not symmetric. Too low tears down an endpoint that
was about to come up and reports it as a failed launch; too high only makes
a genuinely stuck launch take longer to give up. So the bound is set well
clear of the single data point rather than close to it.
The CLI's own wait moves to 20 minutes to stay above it. The daemon starts
its clock when the VM is launched, up to a minute after the CLI starts its
own, so leaving both at 15 would expire the client first and report a bare
"timed out" in place of the daemon's abort, which carries the reason.
* fix: give the Ochre daemon a routed port in the Bedrock system VPC
The readiness probe and every InvokeModel call dial a serving VM's
system-VPC address from inside the daemon, but that subnet lives in OVN
on br-int and the host has no route to it, so a healthy VM never reached
READY.
Each daemon now owns an ENI in the system VPC's private subnet, bound to
an OVS internal port on br-int by iface-id, exactly as ovn-controller
binds a guest tap. The ENI address is installed at the subnet's prefix
length, which is what supplies the connected route. BaseURL is unchanged:
the address it already carried is now reachable.
Describe-or-create keyed on a per-node description, since ENIs carry no
tag filter, so a restart or a second endpoint reuses one address instead
of leaking one per launch. The port is ensured before anything else the
launch creates: a VM the daemon cannot dial holds a GPU for the whole
readiness window before unwinding.
|
||
|
|
e3b5701f15 | fix: authorize RDS data-volume formatting by exact volume identity (#737) | ||
|
|
688ce5ccfd | feat: add AWS model conformance coverage (#736) | ||
|
|
506a8126b7 | fix: propogate rds heartbeat failures | ||
|
|
68671cdf19 | fix: prevent parameter groups bricking RDS instances (#731) | ||
|
|
871513dc6c |
Reapply "feat: match new predastore config surface area (#591)"
This reverts commit
|
||
|
|
b4701c78de |
Revert "feat: match new predastore config surface area (#591)"
This reverts commit
|
||
|
|
615578a2e7 | refactor: simplify RDS internals (#692) | ||
|
|
2e09461080 | chore: cleanup code comments | ||
|
|
43975b01e6 |
feat: add spx admin ochre endpoint ensure/describe/list/delete (#691)
* feat: add spx admin ochre endpoint ensure/describe/list/delete The daemon subscribes all four bedrock.endpoint.* subjects, but nothing in production calls their NATS client, so the serving-endpoint lifecycle could not be exercised at all in a running system. This gives an operator a direct way to drive it. ensure --wait polls describe until READY and reports the elapsed time, since the daemon replies STARTING as soon as it claims the model and the launch outlives the request. That is the measured cold start. Also corrects the weights stage tokenizer rule in COMMANDS.md, which still described both tokenizer files as required rather than either. * test: cover the ochre endpoint Run wrappers' connect and exit paths * fix: treat an empty endpoint bucket as no endpoints, not an error ListEndpoints was the only kvutil.Keys caller not handling ErrNoKeysFound, so 'ochre endpoint list' failed on a cluster that had never launched one. |
||
|
|
49c5a04bed |
fix: register ochre weights snapshot with the EC2 control plane (#690)
weights stage created the snapshot at the viperblock layer only, writing the block checkpoint and config.json but not the metadata.json that CreateVolume resolves a SnapshotId through. The endpoint launcher clones the weights volume via ec2.CreateVolume, so every launch failed with InvalidSnapshot.NotFound against a snapshot that was present on disk. |
||
|
|
050dbdb060 | fix: disallow reserved master names during rds password apply | ||
|
|
8cf44f3310 | fix: harden RDS beta provisioning and replies (#689) | ||
|
|
b76aab8b50 | feat(installer): three-plane networking, ZFS root, distributed NAT and multi-node install (#663) | ||
|
|
f3dd3c251e |
feat: add spx admin ochre weights stage/list/remove (#680)
* feat: add spx admin ochre weights stage/list/remove Give operators a supported path to stage a self-hosted model's weights: point at a predastore S3 prefix holding ordinary Hugging Face files, and stage materialises them into a viperblock volume, snapshots it, and records the source URI and snapshot ID in the bedrock-weights KV bucket. stage refuses before downloading anything if the model is not a self-host catalog entry, or if the prefix is missing any required file. Re-staging the same source is a no-op; a different source replaces the entry and reports the previous snapshot for separate reclamation. WeightsStore gains GetWeights and a source-URI field on its record to support this. LookupServingSpec (catalog.go) is reused unchanged to validate --model-id. * fix: unify ServingSpec after rebase onto dev's daemon-launcher spec Rebasing onto dev pulled in mulga-vmfng.7.5's daemon-side ServingSpec and LookupServingSpec, textually duplicating the ochre weights stage CLI's own versions in catalog.go. Collapse both into one type carrying ModelID and VLLMArgs, and one three-value LookupServingSpec so callers distinguish an unknown model (found=false) from a provider-tier one (selfHost=false). Update the daemon launcher's call site accordingly. * test: cover ochre weights stage/list/remove command behaviour Splits runOchreWeightsStage/List/Remove into testable decision functions (runStageWeights, listWeightsOutput, removeWeights) separate from cobra and NATS wiring, and makes mkfs.ext4 execution and the connect/exit steps swappable so the full command flow can be driven against an embedded JetStream server instead of live infrastructure. Also relaxes the tokenizer file requirement to accept either tokenizer.json or tokenizer.model, since canonical Llama 3.x repos ship only the former at the prefix root. * fix: resolve mkfs.ext4 from sbin when it is absent from PATH e2fsprogs installs mkfs.ext4 into sbin, which a non-login shell omits, so weights staging failed on an unprivileged operator's otherwise fine host. * fix: size the weights volume to a whole GiB so the tail block fits viperblock rejects a write whose block overruns VolumeSize, so importing an image of unaligned length failed on its last block. Rounding up also makes the volume's byte size and its manifest SizeGiB describe the same volume. Name the volume in the import error so a part-written import is traceable. |
||
|
|
8be25c93f9 |
fix: surface faults on the AMI-read and service-ownership paths (#679)
DescribeImages skipped any ami-* prefix whose config.json could not be fetched, read, or parsed, logging each at DEBUG. At the daemon's default INFO level the result was an empty Images list and a completion line reading count=0, indistinguishable from a healthy empty registry. Promote the three fault branches plus the body-close failure to WARN, leave NoSuchKey at DEBUG since a prefix with no config is a legitimate non-AMI directory, and report prefixes examined alongside images returned so a mismatch is visible without raising the log level. SetServiceOwnership chowned per-service directories to service users without propagating a missing user, so spx admin init reported success while leaving /etc/spinifex/northstar root-owned 0600. ChownRecursive and SetServiceOwnership now return errors, failures are aggregated across every service rather than stopping at the first, and finalizeNodeSetup prints the unapplied paths instead of a success mark. |
||
|
|
b5fa0ffb7a |
fix: stop viperblockd discarding the configured S3 region (#677)
The viperblock start command applies --s3-host, --s3-bucket and --s3-region over spinifex.toml whenever viper reports a non-empty value. s3-bucket and s3-region carried non-empty defaults, so the override fired on every start even though the service unit passes neither flag nor environment variable, and the region read from the config was replaced with ap-southeast-2. Any cluster deployed to another region could not launch an instance: viperblockd signed its S3 requests with ap-southeast-2, predastore verified them against its own region and rejected them as AuthorizationHeaderMalformed, the volume mount failed and RunInstances ended in launch_failed. The bare-metal e2e cell, which installs with --region bare-metal, failed every instance-backed test this way. Default all three to empty, matching s3-host, so an unset flag leaves the configured value alone while an explicit flag or SPINIFEX_VIPERBLOCK_* still wins. The predastore.toml template also pinned the main bucket to a literal ap-southeast-2 while the top-level key and the northstar bucket both rendered the configured region; it now renders it too. |
||
|
|
5e17169661 | feat(e2e): complete the RDS end-to-end suite (#608) | ||
|
|
c2b80a9b93 |
feat: match new predastore config surface area (#591)
* match new predastore config surface area
* fix(migrate): carry existing installs onto the predastore v5 topology
The cluster topology refactor breaks an install three ways at once. The old
config carries a top-level host key, which the new schema binds to the [[host]]
table array, so TOML rejects the document and the daemon dies before it reads
any topology. Node state moved from distributed/{db,nodes}/node-N to a per-host
data directory keyed by a node id that is now unique across roles. And because
object metadata records which node holds each shard, the shard nodes had to
keep their ids, so the state replicas took new ones and every replica's
persisted raft configuration stopped listing it.
predastore.toml gains a 4 to 5 migration covering all three: it rewrites the
tables, relocates whichever node directories exist on this machine, and
recovers each relocated state replica onto the new member set. The rewrite is
line-oriented rather than a re-render from the template, so buckets, auth,
rate limits and iam settings an operator edited survive.
spinifex.toml gains a 3 to 4 migration renaming the predastore node_id to
host_id, which predastore-start.sh greps to select the host this machine runs.
The predastore migration lives in its own package rather than in migrate,
which every handler that stamps a KV bucket version imports; reaching the
cluster runtime from there would pull raft, badger and the QUIC client into all
of them. admin_upgrade.go blank-imports it, so only the binary that runs the
upgrade links it, and a test pins both registrations.
* test(predastore): cover the service runtime, gateway and shutdown paths
The service wrapper's new seams had no unit tests: buildRuntime, serve and
the errgroup Start runs the cluster and the S3 gateway under. CI's diff
coverage gate measured the PR at 34.6% against a 60% threshold as a result.
Cover every buildRuntime error wrap and its host selection, both ways the
gateway can fail serve, and the full lifecycle: Start brings the nodes and
the gateway up together, Shutdown drains both by cancelling the context
they share. Topologies are written per test as real config, with node ids
from a process-wide counter — the pipe transport's listener registry is
global, and this package's integration tests hold a fixture daemon on the
low ids for the life of the test binary.
|
||
|
|
bda08e1afd |
fix: namespace colliding viper keys and bind viperblock flags to their own command (#661)
* fix: namespace remaining viper keys shared across service subcommands
predastore, nats and awsgw registered "port", "host", "debug", "tls-cert",
"tls-key", "encryption-key-file" and "node-id" as bare, unqualified viper
keys. viper.BindPFlag overwrites the same key's registered pflag object on
each call, so only the last subcommand registered in init() kept its CLI
flag live; the others read a foreign flag's (usually unset) value.
Bare keys are also vulnerable to AutomaticEnv: viper checks the
SPINIFEX_<KEY> derived name before an explicit BindEnv target, so a
same-named env var set for one subcommand also feeds viper.Get calls in
every other subcommand sharing that key, or a generic SPINIFEX_DEBUG-style
var could shadow all of them at once.
Namespace each affected key per subcommand ("predastore-port",
"nats-debug", "awsgw-host", etc.) following the pattern already used for
predastore-host/base-path/config-path and spinifex-ui-*, so each key's
AutomaticEnv-derived name matches its own BindEnv target. CLI flag names
and environment variable names are unchanged. nats's own bind-host flag
uses "nats-service-host" rather than "nats-host", since that key is already
bound by rootCmd's own cluster-wide --nats-host override.
* fix: bind viperblock flags through the command that declares them
s3-host, s3-bucket, s3-region and plugin-path are declared on viperblockCmd
but were looked up on predastoreCmd, which declares none of them. The lookup
returned nil, BindPFlag rejected it silently, and the discarded error hid it.
Both the flags and their declared defaults were invisible to viper, so
s3-bucket resolved "" rather than "predastore".
|