diff --git a/qcow/api_external_test.go b/qcow/api_external_test.go new file mode 100644 index 0000000..0c19b2f --- /dev/null +++ b/qcow/api_external_test.go @@ -0,0 +1,301 @@ +// This file is deliberately in package qcow_test and not in package qcow. +// +// What it asserts is not a behaviour of Open, it is that Open can be *reached* from +// another module: the Recovery it insists on is implemented here out of nothing but the +// package's exported surface — its interfaces, its types and its sentinels — the way +// spinbox has to implement it (ADR-0021 §4). An in-package test proves nothing about +// that, because it can reach internal/commit and every other thing a consumer cannot. +// +// It went red for the reason it exists: the only spelling of "this volume has never +// published" was internal/commit.ErrNoHead, so nothing outside this module could say the +// one condition Recovery is built around, and every volume handed to Open was refused. +// Delete qcow.ErrNoHistory and this file stops compiling. +package qcow_test + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + "path" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/spin-stack/storage/qcow" +) + +// memPaths is a Paths a consumer could write: a map, and not one syscall. It is what the +// interface being implementable from outside means in practice, so it uses no helper of +// this repository's either. +type memPaths struct { + files map[string][]byte + dirs map[string]bool +} + +func newMemPaths() *memPaths { + return &memPaths{files: map[string][]byte{}, dirs: map[string]bool{}} +} + +func (m *memPaths) MkdirAll(dir string) error { + for d := filepath.Clean(dir); d != "/" && d != "."; d = filepath.Dir(d) { + m.dirs[d] = true + } + return nil +} + +func (m *memPaths) Exists(p string) (bool, error) { + _, ok := m.files[p] + return ok, nil +} + +func (m *memPaths) Size(p string) (int64, error) { + body, ok := m.files[p] + if !ok { + return 0, fmt.Errorf("size %s: %w", p, fs.ErrNotExist) + } + return int64(len(body)), nil +} + +func (m *memPaths) ReadFile(p string) ([]byte, error) { + body, ok := m.files[p] + if !ok { + return nil, fmt.Errorf("read %s: %w", p, fs.ErrNotExist) + } + return body, nil +} + +func (m *memPaths) WriteAtomic(p string, data []byte) error { + if err := m.MkdirAll(filepath.Dir(p)); err != nil { + return err + } + m.files[p] = data + return nil +} + +// List answers fs.ErrNotExist for a directory that is not there, which is the contract +// the interface states and the one a sweep depends on. +func (m *memPaths) List(dir string) ([]string, error) { + dir = filepath.Clean(dir) + if !m.dirs[dir] { + return nil, fmt.Errorf("list %s: %w", dir, fs.ErrNotExist) + } + seen := map[string]bool{} + for p := range m.files { + if filepath.Dir(p) == dir { + seen[path.Base(p)] = true + } + } + for d := range m.dirs { + if filepath.Dir(d) == dir { + seen[path.Base(d)] = true + } + } + names := make([]string, 0, len(seen)) + for n := range seen { + names = append(names, n) + } + sort.Strings(names) + return names, nil +} + +func (m *memPaths) Rename(oldPath, newPath string) error { + body, ok := m.files[oldPath] + if !ok { + return fmt.Errorf("rename %s: %w", oldPath, fs.ErrNotExist) + } + delete(m.files, oldPath) + m.files[newPath] = body + return nil +} + +func (m *memPaths) Remove(p string) error { + delete(m.files, p) + return nil +} + +// memRunner stands in for qemu-img: `create` puts a file where the package says the layer +// goes, `info` answers about the files that are there. A consumer outside this module +// gets exactly this interface — a process to run — and nothing about how it is run. +type memRunner struct { + paths *memPaths + size int64 + created []string +} + +func (r *memRunner) Run(_ context.Context, _ string, args ...string) ([]byte, error) { + switch args[0] { + case "create": + image := args[len(args)-2] + r.created = append(r.created, image) + return nil, r.paths.WriteAtomic(image, []byte("qcow2")) + case "info": + image := args[len(args)-1] + if ok, _ := r.paths.Exists(image); !ok { + return nil, fmt.Errorf("qemu-img: %s: %w", image, fs.ErrNotExist) + } + info := []map[string]any{{ + "format": "qcow2", + "virtual-size": r.size, + "filename": image, + }} + if !strings.Contains(strings.Join(args, " "), "--backing-chain") { + return json.Marshal(info[0]) + } + return json.Marshal(info) + } + return nil, fmt.Errorf("qemu-img: unexpected %v", args) +} + +// extRecovery is the whole point: a Recovery written with nothing but the exported +// surface, saying "this volume has never published" the only way a consumer can. +type extRecovery struct { + // head is the commit the object store holds, empty when it holds none — which is + // what ErrNoHistory says, and what an outside implementation could not say at all. + head string + restored qcow.Restored + // storeDown is an error that is not about history: a store that would not answer, + // which must never be read as "born empty". + storeDown error +} + +func (e extRecovery) RestoreFrom(context.Context, qcow.Lineage, int64) (qcow.Restored, error) { + switch { + case e.storeDown != nil: + return qcow.Restored{}, fmt.Errorf("rebuilding this volume: %w", e.storeDown) + case e.head == "": + return qcow.Restored{}, fmt.Errorf("volume has no HEAD object: %w", qcow.ErrNoHistory) + } + return e.restored, nil +} + +func (e extRecovery) Current(context.Context, string) (string, error) { + switch { + case e.storeDown != nil: + return "", fmt.Errorf("reading HEAD: %w", e.storeDown) + case e.head == "": + return "", fmt.Errorf("volume has no HEAD object: %w", qcow.ErrNoHistory) + } + return e.head, nil +} + +const ( + extRoot = "/data" + extVolume = "0199bd2f-0000-7000-8000-00000000c0de" + extLayer = "0199bd2f-0001-7000-8000-00000000face" + extSize = int64(64 << 20) +) + +func extOpen(t *testing.T, p *memPaths, r *memRunner, req qcow.OpenRequest) (*qcow.Chain, error) { + t.Helper() + req.Root, req.SizeBytes, req.NewLayerID = extRoot, extSize, extLayer + req.VolumeID = extVolume + return qcow.Open(t.Context(), r, p, "qemu-img", req) +} + +// TestExternalRecoveryBornEmpty is the path a consumer's very first volume takes: nothing +// has ever published it, its Recovery says so with qcow.ErrNoHistory, and Open creates +// the first layer instead of refusing. +func TestExternalRecoveryBornEmpty(t *testing.T) { + p := newMemPaths() + r := &memRunner{paths: p, size: extSize} + + chain, err := extOpen(t, p, r, qcow.OpenRequest{Recovery: extRecovery{}}) + if err != nil { + t.Fatalf("a volume nothing has published must be born empty, and this consumer's Recovery said so: %v", err) + } + image := qcow.LayerImage(extRoot, extLayer) + if chain.Active != image { + t.Fatalf("active layer = %q, want %q", chain.Active, image) + } + if len(r.created) != 1 || r.created[0] != image { + t.Fatalf("qemu-img create calls = %v, want exactly [%s]", r.created, image) + } + // What the launcher reads, and the only half of the contract that leaves this + // process: the pointer must name the layer QEMU is to be started against. + pointer, err := p.ReadFile(qcow.ActivePointer(extRoot, extVolume)) + if err != nil { + t.Fatalf("reading the active pointer: %v", err) + } + if string(pointer) != image { + t.Fatalf("active pointer = %q, want %q", pointer, image) + } +} + +// TestExternalRecoveryKeepsLocalChain is Current's half of the same sentence: a chain +// already on this disk is served when the store holds no history, so a consumer that +// cannot say ErrNoHistory loses its volume on the second open as well as the first. +func TestExternalRecoveryKeepsLocalChain(t *testing.T) { + p := newMemPaths() + r := &memRunner{paths: p, size: extSize} + if _, err := extOpen(t, p, r, qcow.OpenRequest{Recovery: extRecovery{}}); err != nil { + t.Fatalf("first open: %v", err) + } + + chain, err := extOpen(t, p, r, qcow.OpenRequest{Recovery: extRecovery{}}) + if err != nil { + t.Fatalf("reopening a chain the store has not moved past: %v", err) + } + if want := qcow.LayerImage(extRoot, extLayer); chain.Active != want { + t.Fatalf("active layer = %q, want the layer already on disk %q", chain.Active, want) + } + if len(r.created) != 1 { + t.Fatalf("qemu-img create calls = %v, want the first open's one and no more", r.created) + } +} + +// TestExternalRecoveryRefusals is the other side, and the reason ErrNoHistory has to be a +// sentinel and not "any error": every one of these is a volume Open must refuse rather +// than create empty, and each is expressible from outside this module too. +func TestExternalRecoveryRefusals(t *testing.T) { + storeDown := errors.New("the object store did not answer") + + tests := []struct { + name string + req qcow.OpenRequest + // local seeds a chain on this host before the open under test. + local bool + want error + }{{ + // §14's recovery path: a host that has never seen the volume, a bucket whose + // HEAD is gone. "No history" is true of the bucket and false of the volume, and + // creating it empty here is the blank-disk defect. + name: "the catalog says this volume published and the store has no HEAD", + req: qcow.OpenRequest{Recovery: extRecovery{}, HeadCommitID: "0199bd2f-0002-7000-8000-0000000000c1"}, + want: qcow.ErrChainMissing, + }, { + name: "the store could not be asked at all", + req: qcow.OpenRequest{Recovery: extRecovery{storeDown: storeDown}}, + want: qcow.ErrChainMissing, + }, { + // Two hosts have held this volume. The local chain is a fork of a history that + // moved on, and which of the two survives is not this process's to decide. + name: "the published history moved past the chain on this disk", + local: true, + req: qcow.OpenRequest{Recovery: extRecovery{head: "0199bd2f-0003-7000-8000-0000000000c2"}}, + want: qcow.ErrStaleChain, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p := newMemPaths() + r := &memRunner{paths: p, size: extSize} + if tc.local { + if _, err := extOpen(t, p, r, qcow.OpenRequest{Recovery: extRecovery{}}); err != nil { + t.Fatalf("seeding a local chain: %v", err) + } + } + before := len(r.created) + + chain, err := extOpen(t, p, r, tc.req) + if !errors.Is(err, tc.want) { + t.Fatalf("Open = (%v, %v), want an error wrapping %v", chain, err, tc.want) + } + if len(r.created) != before { + t.Fatalf("a refused volume had layers created for it: %v", r.created[before:]) + } + }) + } +} diff --git a/qcow/chain.go b/qcow/chain.go index b8b8f77..85f5c3c 100644 --- a/qcow/chain.go +++ b/qcow/chain.go @@ -67,6 +67,21 @@ var ErrChainMissing = errors.New("qcow: this volume has published commits and th // volume and somebody has to decide which of the two histories is the one to keep. var ErrStaleChain = errors.New("qcow: this host's chain is behind the published history") +// ErrNoHistory means the object store holds no HEAD for this volume: nothing has ever +// published it, and a chain born empty is the correct answer. It is the one condition a +// Recovery must be able to say, so it has to be sayable with nothing but this package — +// which is what it was not while the only spelling lived in internal/commit, where a +// consumer in another module cannot reach it. An implementation there could return every +// error meaning "refuse" and none meaning "permit", so Open refused every volume. +// +// It *is* internal/commit's ErrNoHead and not a second sentinel that means the same +// thing: two values would have to be kept mutually wrapping by hand, and the day one of +// them was returned bare, errors.Is would answer differently on the two spellings of one +// condition. One value under two names cannot drift. The name is this package's because +// this is where the surface is, and the sentence — "this volume has no history" — is the +// one a caller of Open is asking. +var ErrNoHistory = commit.ErrNoHead + // Recovery rebuilds a volume's published chain on this host. // // An interface and not a bool the caller computes, because the question — may this volume @@ -78,13 +93,12 @@ var ErrStaleChain = errors.New("qcow: this host's chain is behind the published // "born empty". type Recovery interface { // RestoreFrom rebuilds this volume's published chain locally, including its parent's - // up to the named commit when it is a clone. A wrapped commit.ErrNoHead means the + // up to the named commit when it is a clone. A wrapped ErrNoHistory means the // volume has never published and an empty chain is correct; every other error means - // refuse. commit.ErrNoHead is reused rather than a sentinel of our own because the - // condition *is* "this volume has no HEAD". + // refuse. RestoreFrom(ctx context.Context, l Lineage, sizeBytes int64) (Restored, error) // Current is the commit the object store says is this volume's newest, wrapping - // commit.ErrNoHead when it has never published. It is Restore's question without + // ErrNoHistory when it has never published. It is Restore's question without // Restore's work, and it is asked on every open of a chain that is already here: // a local chain is only current if the published history has not moved past it. Current(ctx context.Context, volumeID string) (string, error) @@ -253,6 +267,10 @@ type Paths interface { // List names the entries of a directory, without their paths. What a sweep is looking // for is precisely the files no record names, so it cannot be found by asking about // paths this host already knows. + // + // A directory that is not there is fs.ErrNotExist and never an empty list: a host on + // its first cycle and a listing that failed are the same answer otherwise, and + // sweeping against the second deletes every chain on the machine. List(dir string) ([]string, error) // Rename moves a file within the layers directory. It is how a file that is built in // several steps — a compaction's flattened root — only ever appears under its real @@ -448,7 +466,7 @@ func Open(ctx context.Context, r Runner, p Paths, qemuImg string, req OpenReques func checkNotStale(ctx context.Context, p Paths, req OpenRequest, image string) error { head, err := req.Recovery.Current(ctx, req.VolumeID) switch { - case errors.Is(err, commit.ErrNoHead): + case errors.Is(err, ErrNoHistory): return nil case err != nil: return fmt.Errorf("%w: volume %s has a local chain and the object store could not say whether it is current: %w", @@ -486,7 +504,7 @@ func checkNotStale(ctx context.Context, p Paths, req OpenRequest, image string) func born(ctx context.Context, r Runner, p Paths, qemuImg string, req OpenRequest, pointer string, local State) (*Chain, error) { restored, err := req.Recovery.RestoreFrom(ctx, req.Lineage, req.SizeBytes) switch { - case errors.Is(err, commit.ErrNoHead): + case errors.Is(err, ErrNoHistory): // This host's own record outranks the bucket's answer, in exactly one direction and // only here. An Agent that published commits for this volume and is pointed at no // object store — or at the wrong one — is told "never published", and would create a @@ -543,7 +561,7 @@ func born(ctx context.Context, r Runner, p Paths, qemuImg string, req OpenReques // // Restore and not Current, and the difference is the whole decision: Current asks whether // the local chain is behind and can only refuse, while a host with a guest waiting needs -// the store to put the history on this disk. Its ErrNoHead is the case that keeps the +// the store to put the history on this disk. Its ErrNoHistory is the case that keeps the // local chain — nothing was ever published by anyone, so replacing the local layers with // an empty image is the blank-disk defect with a fence in front of it. // @@ -551,7 +569,7 @@ func born(ctx context.Context, r Runner, p Paths, qemuImg string, req OpenReques func regrant(ctx context.Context, r Runner, p Paths, qemuImg string, req OpenRequest, pointer, image string, local State) (chain *Chain, keepLocal bool, err error) { restored, err := req.Recovery.RestoreFrom(ctx, req.Lineage, req.SizeBytes) switch { - case errors.Is(err, commit.ErrNoHead): + case errors.Is(err, ErrNoHistory): slog.Info("this volume was granted back to this host and the object store holds no history for it, so the local chain is the only one there is", "volume_id", req.VolumeID, "tip", image, "fenced_at_epoch", local.Fenced.Epoch) if err := clearFence(p, req.Root, req.VolumeID); err != nil { diff --git a/qcow/compact.go b/qcow/compact.go index a9af5f3..43b0f10 100644 --- a/qcow/compact.go +++ b/qcow/compact.go @@ -9,7 +9,6 @@ import ( "slices" storagev1 "github.com/spin-stack/storage/api/gen/spin/storage/v1" - "github.com/spin-stack/storage/internal/commit" "github.com/spin-stack/storage/internal/ids" ) @@ -389,12 +388,12 @@ func (m *Manager) collapse(ctx context.Context, v *volume, st *State) error { } if err := m.pub.Publish(ctx, layer); err != nil { switch { - case errors.Is(err, commit.ErrRootSuperseded): + case errors.Is(err, ErrRootSuperseded): // The object store's own statement of the check above, for the window this // host cannot see from its record: HEAD is not the commit these bytes // reconstruct. return m.abandonCompaction(v, st, err.Error()) - case errors.Is(err, commit.ErrHeadMoved): + case errors.Is(err, ErrNotWriter): // Another host is this volume's writer. A collapse is a publish, so it meets // the same fence as any other one, and for the same reason: the alternative is // this host serving a guest whose writes can never land. diff --git a/qcow/manager.go b/qcow/manager.go index 07555ac..2197c2d 100644 --- a/qcow/manager.go +++ b/qcow/manager.go @@ -172,11 +172,52 @@ type SealedLayer struct { // // A nil Publisher is a host that keeps its layers locally and publishes nothing, which // is every lane before v6 §23.3 and is not an error. +// +// Two of its failures are not failures of the transfer and the Manager branches on them, +// so an implementation must be able to say them: ErrNotWriter, which stops the volume, +// and ErrRootSuperseded, which abandons a collapse. Any other error is a publish that did +// not land and is retried. They are exported for the same reason ErrNoHistory is — a +// Publisher living in another module has no other way to say either sentence, and an +// implementation that cannot say ErrNotWriter leaves a fenced host serving a guest whose +// writes can never be published. type Publisher interface { Publish(ctx context.Context, layer SealedLayer) error } +// ErrNotWriter means the compare-and-set on HEAD lost: something else published for this +// volume, so this host is not its writer any more. It is the one publishing failure that +// stops the volume rather than being retried — a host that goes on taking writes it can +// never publish is the fencing defect §15 is about. +// +// It *is* internal/commit's ErrHeadMoved, under this package's name; ErrNoHistory carries +// why one value under two names is the shape and two values is not. +var ErrNotWriter = commit.ErrHeadMoved + +// ErrRootSuperseded means a compacted root was published for a HEAD that has moved on: +// the flattened bytes reconstruct a commit that is no longer the newest, so taking them +// would move HEAD backwards over a commit that returned SUCCESS. Nobody else took the +// volume — that is ErrNotWriter — so this host goes on serving it and plans the next +// collapse over the history as it is now. +// +// It *is* internal/commit's ErrRootSuperseded, under the same name. +var ErrRootSuperseded = commit.ErrRootSuperseded + // Deps are the Manager's injected collaborators (INV-01). +// +// **Three of them cannot be supplied from outside this module today, so Manager is not +// yet the facade ADR-0021 §4 promises spin's runner — Open and the chain functions are.** +// Clock and Disk are internal/simio interfaces whose own methods return internal types +// (clock.Instant, clock.Timer, disk.File, disk.Usage), so no implementation of them can +// be written elsewhere, and internal/simio/real — the implementations a real host wants +// — is unreachable for the same reason. Dialer is nameable in shape but lives internal +// too. Closing this means deciding where the simio boundary sits relative to the public +// surface, which is a decision of its own and not a doc comment's to make; it is not +// closed by re-exporting, because what a consumer needs is the *real* implementations and +// those are the ones INV-01 confines. +// +// Publisher, Witness, Recovery, Runner and Paths are the ones that were designed to be +// implemented by a caller, and they are: every type in their signatures is public, and +// every sentinel their errors must carry is exported by this package. type Deps struct { Clock clock.Clock Disk disk.Disk @@ -824,7 +865,7 @@ func (m *Manager) publish(ctx context.Context, v *volume) error { return m.refuse(v, storagev1.VolumeRefusal_VOLUME_REFUSAL_DURABILITY_LOST, err) } if err := m.pub.Publish(ctx, layer); err != nil { - if errors.Is(err, commit.ErrHeadMoved) { + if errors.Is(err, ErrNotWriter) { // Another host published for this volume, so this one is not its writer: it // would keep accepting writes that can never be published. That is the one // publishing failure that stops the volume. @@ -1517,6 +1558,9 @@ type volumeGap struct { } // Volumes reports what this host is holding, ordered by volume id (deterministic). +// +// It returns internal/agent's type, which a caller outside this module can read but +// cannot name — part of the same gap Deps carries, and it moves when that one does. func (m *Manager) Volumes(context.Context) ([]agent.VolumeStatus, error) { m.mu.Lock() defer m.mu.Unlock()