The sentinels a Recovery and a Publisher must say are sayable from outside

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:])
+			}
+		})
+	}
+}

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,15 +561,15 @@ 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.
 //
 // keepLocal true means the caller carries on with the chain that is already here.
 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 {