mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-20 11:21:49 +00:00
8ab96b29a1
* fix: replicate every JetStream KV bucket across the cluster A KV bucket created without a replica count gets one replica, and one replica lives on one JetStream-chosen server recorded in no config. Losing, restarting or partitioning that server makes the bucket return "no responders available" from every node at once, so a single-node event becomes a cluster-wide outage. The reconciler leader-lease buckets were the worst case: acquiring a lease is a write, so a lease bucket without a quorum cannot be acquired by anyone, and every reconciler sharing it stops cluster-wide at exactly the moment a node has failed. The cluster's node count is now declared once, in config.LoadConfig, and read by the one helper every bucket creation goes through. Every process that creates a bucket loads the cluster config first, so no service can forget to pass it and no signature has to carry it. An undeclared size is an error at creation rather than a silent fall back to one replica. Existing buckets are raised rather than left behind: a service opening one raises it, the daemon sweeps them all after formation, and "spx admin kv replicas --repair" does it on demand. Raising reads the live stream config and changes only the replica count, and never lowers one. Removes the ~30 replicas/clusterSize parameters that only forwarded a number, and JetStreamManager.UpdateReplicas, which set len(nodes) with no clamp and would have failed outright on a cluster of six or more. Proved by a repo-wide invariant test pinning bucket creation to the one helper, a whole-cluster sweep in every multi-node nightly cell, and a new kvquorum suite (cell 33) that takes away the node leading the most buckets and requires the survivors to keep answering for all of them. * chore: name cell 33 in the nightly dispatch input * fix: a KV bucket that cannot be raised must still open The first version of the replica fix treated a raise it could not place as a hard error, and on a real three-node cluster that turned the fix into the outage it prevents. A node whose config named a fourth server wanted four replicas, NATS answered "no suitable peers for placement" for three, and the daemon retried for its full five-minute budget without finishing bootstrap. The bucket was present, quorate and serving on three nodes throughout. A config naming a node before that node serves is the ordinary middle of growing a cluster, so that is a state to tolerate rather than refuse. The line is now drawn between the two cases instead of around both. Creating a bucket below the cluster's replica count still fails, because that is the defect and a fallback there reintroduces it where it is hardest to notice. Failing to raise an existing bucket warns and hands the bucket back, and the raise is retried on every open besides the daemon sweep and --repair. This matters most in kvlease, which runs on every reconcile tick: refusing there would stop every reconciler sharing a lease bucket that was working. getOrCreateBucket now attaches before it creates. A create asking for more replicas than the cluster can place is refused before the server looks at the name, so with create-first "this bucket already exists" was an inference from which error came back, and the wrong one comes back on a single-node server. RaiseAllBucketReplicas attempts every bucket and joins the failures, and --repair keeps going too. Stopping at the first would leave the rest of a cluster short because one bucket could not be placed. Also adds nats.ServerName/NodeFromServerName beside the template that renders server_name, with a test tying them together. The kvquorum suite had mapped stream peers to nodes by comparing them to node names, which they are not: they are the node name behind a "spinifex-nats-" prefix, so nothing matched and the node-loss test could not choose a victim. * test: wait for the right NATS peer count after a node rejoins /routez reports a node's routes to its peers, not its own membership, so a fully formed three-node cluster is two peers per node. The kvquorum suite waited for three and spent its whole five-minute rejoin budget on a cluster that had already reformed. Every other caller passes len(Nodes)-1; the helper's doc comment now says so. The assertion the test exists for had already passed: node1 was leading 18 of 47 buckets when it was taken away, and node2 and node3 each still answered for all 47. * fix: replicate KV buckets across the largest odd count a cluster can place A bucket was replicated across min(nodes, 5) servers, which stores a fourth full copy on a four-node cluster and still survives exactly one loss — the same tolerance R3 gives for three quarters of the write amplification. A Raft majority needs an odd count, so the rule becomes the largest odd count the cluster can place, capped at five: 1, 3, 3, 5, 5. It is one expression with two entry points, so no caller can hold a stale copy of the policy and keep passing after the policy changes. Three retry loops could not tell a configuration fault from a cluster that had yet to form. An undeclared cluster size never becomes declared by waiting, so each of them spent its whole budget on one and then reported quorum as the cause. They now stop on a permanent error and say what is actually wrong. This is what the integration suite was hitting: it builds its gateway by hand and never loads a cluster config, so the size was never declared and the IAM retry sat in its sleep until the test timed out. Both ends are fixed. The audit reported configuration alone. A bucket at three replicas with one peer offline and one lagging is one failure from unavailable and read as ok. BucketReport now carries the leader and a count of the peers JetStream itself calls current and reachable, quorum is measured against that count, and spx admin kv replicas exits 0, 1 or 2 so a deployment gate can tell a problem --repair fixes from one needing a human. Declaration is write-once: daemon.Start refuses a conflicting redeclaration because it is about to create buckets, and it declares before JetStream init rather than after it. config.LoadConfig warns instead, because it also reads config files that are not this process's own cluster. spx admin init --nodes 2 is refused — no replica count on two nodes both survives a loss and accepts writes — and single-node replication is asserted in the single e2e suite, so it runs in every nightly host permutation. * fix: withhold the vLLM serving image until its artifact is published The catalog offers ubuntu-26.04-vllm-serving-x86_64, and the qcow2 it names 404s on iso.mulgadc.com, so spx admin images import would fail for every operator who tried it. Its GPU siblings are published; this one never was. The entry moves to WithheldImages rather than being deleted, because the entry is the definition — the URL, the checksum and the two tags resolveServingAMI filters on. Publishing the artifact is then a move back, not a retype. Bedrock already handles the image not being registered: resolveServingAMI returns ErrServingAMINotFound, which is the honest answer while there is nothing to launch. The bedrock tag contract is still pinned, now against the map the definition lives in, and it additionally asserts the entry is not offered for import. The e2e reachability walk covers AvailableImages alone, so a withheld entry is out of it by construction. * fix: drop the cluster size from the call sites dev added Two constructors on dev still pass a replica count that no longer exists as a parameter: the cluster size is a declared process fact, so EKSServiceDeps has no ClusterSize field and NewIAMServiceImpl takes three arguments. The merge is textually clean and breaks the build, which is the shape of collision a signature removal always has. * docs: explain what an operator sees when a cluster server fails The replication section said how many copies a bucket has and stopped there, which leaves the question an operator actually has during an incident: one server is gone, what still works. It now covers the majority side carrying on, an isolated server refusing changes on purpose and why that is the behaviour you want, guests continuing to run while anything that has to be recorded does not, the risk of reading state from a server that is cut off, and a returning server catching up by itself. The surrounding prose reads from the operator's side rather than the implementation's: what you see and what to do, not which consensus protocol is underneath.
204 lines
7.0 KiB
Go
204 lines
7.0 KiB
Go
//go:build integration
|
|
|
|
package integration
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"strings"
|
|
"sync"
|
|
"sync/atomic"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/mulgadc/spinifex/spinifex/clustersize"
|
|
"github.com/mulgadc/spinifex/tests/fixtures/scratch"
|
|
"github.com/nats-io/nats-server/v2/server"
|
|
"github.com/nats-io/nats.go"
|
|
"github.com/nats-io/nats.go/jetstream"
|
|
)
|
|
|
|
// sharedNATSHarness is the package-wide handle TestMain installs before
|
|
// m.Run() and every StartGateway call reads from.
|
|
var sharedNATSHarness *sharedNATS
|
|
|
|
// sharedNATS owns the one embedded NATS+JetStream server shared by every
|
|
// test in the package, plus the dynamic per-account authenticator that
|
|
// gives each test its own isolated JetStream namespace on that server. See
|
|
// the TestMain doc comment (main_test.go) for why the server — and every
|
|
// connection into it — is shared across the whole package rather than
|
|
// booted fresh per test.
|
|
type sharedNATS struct {
|
|
srv *server.Server
|
|
auth *accountAuthenticator
|
|
storeDir string
|
|
|
|
connsMu sync.Mutex
|
|
conns []*nats.Conn
|
|
}
|
|
|
|
// natsStorePrefix names each run's JetStream store directory. It is a constant
|
|
// because the sweep below matches on it.
|
|
const natsStorePrefix = "spinifex-integration-nats-"
|
|
|
|
// startSharedNATS boots the one embedded, JetStream-enabled NATS server the
|
|
// whole package's tests connect into.
|
|
func startSharedNATS() (*sharedNATS, error) {
|
|
// One embedded, non-clustered server, so every bucket this binary creates is
|
|
// on one replica. Bucket creation refuses until it is told, and this package
|
|
// builds its gateway by hand rather than through config.LoadConfig, which is
|
|
// where a real process declares it.
|
|
if err := clustersize.Declare(1); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
// The store cannot be a t.TempDir(): the server outlives every individual
|
|
// test, so the first test to finish would delete it out from under it.
|
|
scratch.SweepAbandoned(os.TempDir(), natsStorePrefix, scratch.DefaultMaxAge)
|
|
|
|
storeDir, err := os.MkdirTemp("", natsStorePrefix)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("nats store tempdir: %w", err)
|
|
}
|
|
|
|
auth := newAccountAuthenticator()
|
|
opts := &server.Options{
|
|
Host: "127.0.0.1",
|
|
Port: -1,
|
|
JetStream: true,
|
|
StoreDir: storeDir,
|
|
NoLog: true,
|
|
NoSigs: true,
|
|
CustomClientAuthentication: auth,
|
|
}
|
|
ns, err := server.NewServer(opts)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("new nats server: %w", err)
|
|
}
|
|
auth.srv = ns
|
|
|
|
go ns.Start()
|
|
if !ns.ReadyForConnections(10 * time.Second) {
|
|
return nil, fmt.Errorf("nats server not ready for connections")
|
|
}
|
|
|
|
return &sharedNATS{srv: ns, auth: auth, storeDir: storeDir}, nil
|
|
}
|
|
|
|
// removeStore reclaims the JetStream store once the server is fully down.
|
|
// Called after WaitForShutdown so nothing is still writing into it.
|
|
func (h *sharedNATS) removeStore() {
|
|
if h.storeDir == "" {
|
|
return
|
|
}
|
|
if err := os.RemoveAll(h.storeDir); err != nil {
|
|
fmt.Fprintf(os.Stderr, "tests/integration: remove nats store %s: %v\n", h.storeDir, err)
|
|
}
|
|
}
|
|
|
|
// connectIsolated opens a fresh connection scoped to its own dynamically
|
|
// provisioned NATS account keyed by name, giving the caller a JetStream
|
|
// namespace no other test's account can see or collide with. The returned
|
|
// connection is tracked and closed by TestMain once every test has
|
|
// finished, not by the caller — see the TestMain doc comment for why.
|
|
func (h *sharedNATS) connectIsolated(name string) (*nats.Conn, jetstream.JetStream, error) {
|
|
nc, err := nats.Connect(h.srv.ClientURL(), nats.UserInfo(name, "x"))
|
|
if err != nil {
|
|
return nil, nil, fmt.Errorf("connect: %w", err)
|
|
}
|
|
js, err := jetstream.New(nc)
|
|
if err != nil {
|
|
nc.Close()
|
|
return nil, nil, fmt.Errorf("jetstream: %w", err)
|
|
}
|
|
|
|
h.connsMu.Lock()
|
|
h.conns = append(h.conns, nc)
|
|
h.connsMu.Unlock()
|
|
return nc, js, nil
|
|
}
|
|
|
|
// closeAll closes every connection opened via connectIsolated. Only called
|
|
// from TestMain after every test has finished.
|
|
func (h *sharedNATS) closeAll() {
|
|
h.connsMu.Lock()
|
|
defer h.connsMu.Unlock()
|
|
for _, nc := range h.conns {
|
|
nc.Close()
|
|
}
|
|
}
|
|
|
|
// accountAuthenticator implements nats-server's Authentication interface
|
|
// (wired in via Options.CustomClientAuthentication). Every distinct
|
|
// username that connects gets its own dedicated, JetStream-enabled NATS
|
|
// account, created on first sight and reused after. This is what gives each
|
|
// test a private JetStream namespace on the one shared server: fixed KV
|
|
// bucket names production services hard-code (e.g. IAM's "iam-users")
|
|
// resolve to a distinct JetStream store per account instead of one global
|
|
// bucket every test would otherwise collide on. NATS subjects are equally
|
|
// account-scoped by default, so per-test daemon-side stubs (StubSubject)
|
|
// stay isolated the same way they were when each test had its own server.
|
|
type accountAuthenticator struct {
|
|
srv *server.Server
|
|
|
|
mu sync.Mutex
|
|
accounts map[string]*server.Account
|
|
}
|
|
|
|
func newAccountAuthenticator() *accountAuthenticator {
|
|
return &accountAuthenticator{accounts: make(map[string]*server.Account)}
|
|
}
|
|
|
|
// Check implements server.Authentication. The account name is the username
|
|
// the client connected with (see accountNameFor), so it is only ever
|
|
// created once even if a test somehow reconnected.
|
|
func (a *accountAuthenticator) Check(c server.ClientAuthentication) bool {
|
|
name := c.GetOpts().Username
|
|
if name == "" {
|
|
return false
|
|
}
|
|
acc, err := a.accountFor(name)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
c.RegisterUser(&server.User{Username: name, Account: acc})
|
|
return true
|
|
}
|
|
|
|
func (a *accountAuthenticator) accountFor(name string) (*server.Account, error) {
|
|
a.mu.Lock()
|
|
defer a.mu.Unlock()
|
|
if acc, ok := a.accounts[name]; ok {
|
|
return acc, nil
|
|
}
|
|
acc, err := a.srv.RegisterAccount(name)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("register account %s: %w", name, err)
|
|
}
|
|
if err := acc.EnableJetStream(nil, nil); err != nil {
|
|
return nil, fmt.Errorf("enable jetstream for account %s: %w", name, err)
|
|
}
|
|
a.accounts[name] = acc
|
|
return acc, nil
|
|
}
|
|
|
|
// accountNameReplacer strips characters NATS account names / on-disk
|
|
// JetStream store directories don't tolerate well (subtests separate their
|
|
// path with "/", and some test names contain spaces).
|
|
var accountNameReplacer = strings.NewReplacer("/", "_", " ", "_")
|
|
|
|
// accountSeq disambiguates repeat runs of one test. t.Name() is unique among
|
|
// tests in a binary but NOT across passes: -count=N replays identical names
|
|
// inside the same TestMain, so a name-only account key hands the second pass
|
|
// the first pass's JetStream namespace. That namespace still holds an ECR
|
|
// signing key sealed with the first pass's master key, which StartGateway
|
|
// regenerates per call — so the reused key failed to decrypt and took down
|
|
// every gateway-starting test on the second pass.
|
|
var accountSeq atomic.Uint64
|
|
|
|
// accountNameFor derives a fresh NATS account name for t, so each gateway gets
|
|
// an empty JetStream namespace no matter how often the test is replayed.
|
|
func accountNameFor(t *testing.T) string {
|
|
return fmt.Sprintf("%s_%d", accountNameReplacer.Replace(t.Name()), accountSeq.Add(1))
|
|
}
|