test(uefi): separate the embedded store from the injected error in the failing-get fake

A launch whose record points at a variable store keeps the live copy at
<DataDir>/uefi/<instance-id>/vars.fd and opens it as pflash unit 1 with
format=raw. It reuses the copy when seeded.json beside it names the
pointer, and otherwise gets the object, decompresses it, checks it is
exactly the node's VARS size and writes vars.fd then seeded.json by
temporary file and rename. A failed get is retryable like a mount; a
missing or malformed object is not.

A store whose firmware arch or VARS size differs from the node's fails
the launch before any volume is mounted, with a Server.InternalError
state reason and a log naming both firmwares. A different CODE hash
boots, and is logged and counted.

A UEFI instance with no pointer and no EFI volume, or with both, is
refused with InternalError before its record is changed, in launch,
Start and the claimed-instance start. An instance whose variables are on
an EFI volume still launches through it.

docs/COMMANDS.md

@@ -135,7 +135,7 @@ Operational commands for inspecting cluster state. These fan out NATS requests t
 
 | Command | Flags | Description |
 |---------|-------|-------------|
-| `spx admin volumes orphans` | — | Lists volumes the storage provider holds that the control plane has no record of. Their blocks consume space but they have no API handle, so they can be neither described nor deleted through the EC2 API. **Reports only, never deletes**: the evidence that an orphan is still wanted is the record that went missing, so removal is an operator decision taken per volume. It also lists UEFI variable stores nothing owns: a `uefi/i-*/` prefix with no instance record and no store record, a `uefi/ami-*/` prefix with no AMI document, and a store record that is not `deleting` and has no instance record. These are reported and never deleted, because an absent instance record is not proof of a terminate. |
+| `spx admin volumes orphans` | — | Lists volumes the storage provider holds that the control plane has no record of. Their blocks consume space but they have no API handle, so they can be neither described nor deleted through the EC2 API. **Reports only, never deletes**: the evidence that an orphan is still wanted is the record that went missing, so removal is an operator decision taken per volume. |
 | `spx admin volumes unsealed` | — | Lists volumes a node has open, or had open, without a confirmed seal to the backend — the node named holds writes predastore may not have. Starting such a volume elsewhere is refused while that node is alive (its daemon heartbeat is under 60 seconds old, or cannot be read) and is otherwise allowed, in which case it opens from the last checkpoint that did reach the backend. This is the list of volumes where that would cost something. |
 | `spx admin storage status` | `--endpoint` (default: this node's AWS gateway), `--region` (default: this node's region), `--ca-bundle` (default: this node's CA) | Sends the signed `GetStorageStatus` action to the AWS gateway with credentials from the standard AWS chain (e.g. `AWS_PROFILE=spinifex`) and prints what the console's Service Metrics page shows: predastore encoding, meta and blob node counts, cluster health, a meta node table (health, raft state, leader, term, commit and applied index) and the blob nodes. Cluster health is `Degraded` when any meta node is unreachable or the nodes reporting raft state do not agree on one leader. A meta node read only through its host's `/readyz` shows healthy with blank raft fields. |
 

spinifex/domains/ec2/instance/service.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/gpu"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
 )
@@ -122,15 +121,6 @@ type StoppedInstanceStore interface {
 	ClaimStoppedInstance(instanceID string) (*vm.VM, error)
 }
 
-// UEFIStoreMarker records a terminated instance's UEFI variable store for
-// deletion. Implemented by uefistore.Store.
-type UEFIStoreMarker interface {
-	Exists(ctx context.Context, instanceID string) (bool, error)
-	MarkDeleting(ctx context.Context, instanceID string) (bool, error)
-}
-
-var _ UEFIStoreMarker = (*uefistore.Store)(nil)
-
 // InstanceTagWriter projects an instance record's full tag set into the
 // central tag store, and removes it on terminate. Implemented by
 // domains/ec2/tags.TagsServiceImpl.

spinifex/domains/ec2/instance/service_impl.go

@@ -234,7 +234,6 @@ type InstanceServiceImpl struct {
 	eniDeleter        ENIDeleter
 	ipReleaser        PublicIPReleaser
 	tagWriter         InstanceTagWriter
-	uefiStores        UEFIStoreMarker
 	gpuClaimer        GPUClaimer
 	amiLoader         AMIMetaLoader
 	ebsProvider       ebsprovider.EBSProvider
@@ -315,12 +314,6 @@ func (s *InstanceServiceImpl) SetTerminationDeps(vd VolumeDeleter, ed ENIDeleter
 	s.tagWriter = tw
 }
 
-// SetUEFIStores wires the store records a stopped instance's terminate marks
-// for deletion. nil leaves terminate recording no UEFI store obligation.
-func (s *InstanceServiceImpl) SetUEFIStores(m UEFIStoreMarker) {
-	s.uefiStores = m
-}
-
 // SetGPUClaimer wires the GPU passthrough claim/release dependency used by
 // StartStoppedInstance. nil disables GPU passthrough for the service.
 func (s *InstanceServiceImpl) SetGPUClaimer(g GPUClaimer) {
@@ -2696,6 +2689,15 @@ func (s *InstanceServiceImpl) capacityRefusal(ctx context.Context, instance *vm.
 // start returns it to the stopped store, a recovery returns it to the node it
 // was taken from.
 func (s *InstanceServiceImpl) launchClaimedInstance(ctx context.Context, instance *vm.VM, undo func()) error {
+	// Before anything is stamped or claimed, so the record handed back is the
+	// one that was claimed.
+	if err := vm.CheckUEFIVarStore(instance); err != nil {
+		slog.ErrorContext(ctx, "launchClaimedInstance: refusing to launch a UEFI instance without exactly one variable store",
+			"instanceId", instance.ID, "err", err)
+		undo()
+		return errors.New(awserrors.ErrorServerInternal)
+	}
+
 	instance.ResetNodeLocalState()
 
 	// AWS restamps LaunchTime on every start. The pending watchdog times a
@@ -2934,7 +2936,6 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 	s.deleteInstanceVolumes(ctx, instance, input.InstanceID)
 	s.releaseInstancePublicIP(ctx, instance, input.InstanceID)
 	s.deleteInstanceENI(ctx, instance, input.InstanceID)
-	hasUEFIStore := s.stampUEFIStoreTeardown(ctx, instance)
 
 	// Write to terminated KV FIRST so the instance is visible in DescribeInstances.
 	// If this fails the instance stays in the stopped bucket — safe to retry.
@@ -2944,16 +2945,6 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 		return nil, errors.New(awserrors.ErrorServerInternal)
 	}
 
-	// Marked only once the terminate is durable: a stopped instance whose
-	// terminate failed keeps its store. A failed mark leaves the teardown entry
-	// pending, from which the store reaper completes it.
-	if hasUEFIStore {
-		if _, err := s.uefiStores.MarkDeleting(ctx, input.InstanceID); err != nil {
-			slog.WarnContext(ctx, "TerminateStoppedInstance: failed to record UEFI variable store for deletion",
-				"instanceId", input.InstanceID, "err", err)
-		}
-	}
-
 	// Now safe to remove from stopped KV. Retry once on failure so the instance
 	// doesn't appear in both buckets.
 	if err := s.stoppedStore.DeleteStoppedInstance(input.InstanceID); err != nil {
@@ -2971,29 +2962,6 @@ func (s *InstanceServiceImpl) TerminateStoppedInstance(ctx context.Context, inpu
 	return &TerminateStoppedInstanceOutput{Status: "terminated", InstanceID: input.InstanceID}, nil
 }
 
-// stampUEFIStoreTeardown records the UEFI store teardown entry the terminated
-// record carries, and reports whether the instance has a store to mark. An
-// unreadable bucket is stamped failed, which the store reaper resolves.
-func (s *InstanceServiceImpl) stampUEFIStoreTeardown(ctx context.Context, instance *vm.VM) bool {
-	if s.uefiStores == nil {
-		return false
-	}
-	has, err := s.uefiStores.Exists(ctx, instance.ID)
-	state := vm.TeardownPending
-	switch {
-	case err != nil:
-		slog.WarnContext(ctx, "TerminateStoppedInstance: failed to read UEFI store record", "instanceId", instance.ID, "err", err)
-		state = vm.TeardownFailed
-	case !has:
-		return false
-	}
-	if instance.Teardown == nil {
-		instance.Teardown = make(map[string]string)
-	}
-	instance.Teardown[vm.TeardownUEFI] = string(state)
-	return has
-}
-
 func (s *InstanceServiceImpl) deleteInstanceVolumes(ctx context.Context, instance *vm.VM, instanceID string) {
 	instance.EBSRequests.Mu.Lock()
 	defer instance.EBSRequests.Mu.Unlock()

spinifex/domains/ec2/instance/service_impl_test.go

@@ -2605,6 +2605,34 @@ func TestStartStoppedInstance_InstanceTypeUnknown(t *testing.T) {
 	assert.Equal(t, vm.StateStopped, store.WroteStopped[id].Status)
 }
 
+// A UEFI instance with no variable store pointer is refused before anything is
+// stamped or claimed, so the record handed back is the one that was claimed.
+func TestStartStoppedInstance_UEFIInstanceWithoutAPointerIsRefused(t *testing.T) {
+	id := "i-uefi-nopointer"
+	itype := "t3.micro"
+	launched := time.Unix(1700000000, 0).UTC()
+	stopped := &vm.VM{
+		ID: id, Status: vm.StateStopped, AccountID: "acc", InstanceType: itype, BootMode: "uefi",
+		Instance: &ec2.Instance{LaunchTime: aws.Time(launched)},
+	}
+	before := stopped.Record()
+	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{id: stopped}}
+	prov := &fakeResourceCapacityProvider{instanceTypes: map[string]*ec2.InstanceTypeInfo{
+		itype: {InstanceType: aws.String(itype)},
+	}}
+	svc := &InstanceServiceImpl{stoppedStore: store, resourceMgr: prov, vmMgr: vm.NewManager()}
+
+	_, err := svc.StartStoppedInstance(context.Background(), &StartStoppedInstanceInput{InstanceID: id}, "acc")
+
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorServerInternal, err.Error())
+	assert.Empty(t, prov.allocated, "nothing is allocated for a start that cannot boot")
+	require.NotNil(t, store.WroteStopped[id], "the claimed instance is handed back")
+	assert.Equal(t, before, store.WroteStopped[id].Record(), "the record is unchanged")
+	_, inManager := svc.vmMgr.Get(id)
+	assert.False(t, inManager)
+}
+
 func TestStartStoppedInstance_AllocateFails(t *testing.T) {
 	id := "i-alloc-fail"
 	itype := "t3.micro"

spinifex/domains/ec2/instance/uefi_terminate_test.go

@@ -1,129 +0,0 @@
-package instance
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"testing"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
-	testutil "github.com/mulgadc/spinifex/internal/testkit"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
-	vmmock "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm/mock"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-const uefiTerminateID = "i-0123456789abcdef0"
-
-// stoppedUEFIFixture is a stopped instance with a live store record and three
-// retained objects, the state a launch and a few stops leave.
-func stoppedUEFIFixture(t *testing.T) (*InstanceServiceImpl, *vmmock.StateStore, *uefistore.Store, *objectstore.MemoryObjectStore) {
-	t.Helper()
-	_, _, js := testutil.StartTestJetStream(t)
-	stores := uefistore.NewStore(js)
-	require.NoError(t, stores.Open(t.Context()))
-	_, err := stores.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: uefiTerminateID, AccountID: "acc", State: uefistore.StateLive})
-	require.NoError(t, err)
-
-	objects := objectstore.NewMemoryObjectStore()
-	for _, k := range []string{"a", "b", "c"} {
-		_, err := objects.PutObject(t.Context(), &s3.PutObjectInput{
-			Bucket: aws.String("spinifex"), Key: aws.String(uefistore.KeyPrefix(uefiTerminateID) + k), Body: bytes.NewReader([]byte("vars")),
-		})
-		require.NoError(t, err)
-	}
-
-	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
-		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
-	}}
-	svc := &InstanceServiceImpl{stoppedStore: store, objectStore: objects}
-	svc.SetUEFIStores(stores)
-	return svc, store, stores, objects
-}
-
-func storeState(t *testing.T, stores *uefistore.Store) string {
-	t.Helper()
-	rec, _, err := stores.Get(t.Context(), uefiTerminateID)
-	require.NoError(t, err)
-	return rec.State
-}
-
-// TestS8_TerminateStoppedInstanceRecordsTheUEFIStoreObligationAndReturns
-// enforces ADR-0003:S8: the request deletes no store object, it records the
-// obligation on the store record and the terminated record, and returns.
-func TestS8_TerminateStoppedInstanceRecordsTheUEFIStoreObligationAndReturns(t *testing.T) {
-	svc, store, stores, objects := stoppedUEFIFixture(t)
-
-	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
-	require.NoError(t, err)
-
-	keys, err := uefistore.ListObjects(t.Context(), objects, "spinifex", uefiTerminateID)
-	require.NoError(t, err)
-	assert.Len(t, keys, 3, "ADR-0003:S8: the request must leave the object deletion to the reaper")
-	assert.Equal(t, uefistore.StateDeleting, storeState(t, stores), "the store record carries the obligation")
-	require.NotNil(t, store.WroteTerminated[uefiTerminateID])
-	assert.Equal(t, string(vm.TeardownPending), store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI])
-}
-
-// TestS7_FailedStoppedTerminateKeepsTheUEFIStoreLive enforces ADR-0003:S7: a
-// terminate that fails before it is durable leaves the instance stopped, so
-// its store must stay live rather than be handed to the reaper.
-func TestS7_FailedStoppedTerminateKeepsTheUEFIStoreLive(t *testing.T) {
-	svc, store, stores, _ := stoppedUEFIFixture(t)
-	store.WriteTerminatedErr = errors.New("kv write boom")
-
-	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
-	require.Error(t, err)
-
-	assert.Equal(t, uefistore.StateLive, storeState(t, stores),
-		"ADR-0003:S7: a stopped instance whose terminate failed must keep its store")
-}
-
-func TestTerminateStoppedInstance_NoUEFIStoreStampsNoEntry(t *testing.T) {
-	_, _, js := testutil.StartTestJetStream(t)
-	stores := uefistore.NewStore(js)
-	require.NoError(t, stores.Open(t.Context()))
-	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
-		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
-	}}
-	svc := &InstanceServiceImpl{stoppedStore: store}
-	svc.SetUEFIStores(stores)
-
-	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
-	require.NoError(t, err)
-
-	_, stamped := store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI]
-	assert.False(t, stamped)
-}
-
-type unreadableUEFIStores struct{ marked bool }
-
-func (u *unreadableUEFIStores) Exists(context.Context, string) (bool, error) {
-	return false, errors.New("kv unavailable")
-}
-
-func (u *unreadableUEFIStores) MarkDeleting(context.Context, string) (bool, error) {
-	u.marked = true
-	return false, nil
-}
-
-// An unreadable bucket does not fail the terminate; the entry is stamped
-// failed so the store reaper settles it from the terminated record.
-func TestTerminateStoppedInstance_UnreadableUEFIStoreIsStampedFailed(t *testing.T) {
-	store := &vmmock.StateStore{Stopped: map[string]*vm.VM{
-		uefiTerminateID: {ID: uefiTerminateID, Status: vm.StateStopped, AccountID: "acc"},
-	}}
-	stores := &unreadableUEFIStores{}
-	svc := &InstanceServiceImpl{stoppedStore: store}
-	svc.SetUEFIStores(stores)
-
-	_, err := svc.TerminateStoppedInstance(t.Context(), &TerminateStoppedInstanceInput{InstanceID: uefiTerminateID}, "acc")
-	require.NoError(t, err)
-
-	assert.Equal(t, string(vm.TeardownFailed), store.WroteTerminated[uefiTerminateID].Teardown[vm.TeardownUEFI])
-	assert.False(t, stores.marked)
-}

spinifex/domains/ec2/instance/uefistore/cleanup.go

@@ -1,141 +0,0 @@
-package uefistore
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"slices"
-	"strings"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/nats-io/nats.go/jetstream"
-)
-
-// markAttempts bounds MarkDeleting's retries against concurrent writers. Each
-// lost race means another writer changed the record, and none of them can
-// return it to live, so a few rounds always settle.
-const markAttempts = 5
-
-// Exists reports whether instanceID has a store record, without decoding it,
-// so a record this node cannot read still counts as one.
-func (s *Store) Exists(ctx context.Context, instanceID string) (bool, error) {
-	return s.kv.Exists(ctx, instanceID)
-}
-
-// MarkDeleting records that instanceID's store is to be deleted, and drops any
-// write-back still owed. It reports false with no error when the instance has
-// no store record. Only a terminate or an operator's forget may call it.
-func (s *Store) MarkDeleting(ctx context.Context, instanceID string) (bool, error) {
-	for range markAttempts {
-		rec, rev, err := s.Get(ctx, instanceID)
-		if errors.Is(err, kvstore.ErrNotFound) {
-			return false, nil
-		}
-		if err != nil {
-			return false, err
-		}
-		if rec.State == StateDeleting {
-			return true, nil
-		}
-		rec.State = StateDeleting
-		rec.WriteBack = nil
-		_, err = s.CompareAndSet(ctx, rec, rev)
-		if err == nil {
-			return true, nil
-		}
-		if !errors.Is(err, kvstore.ErrConflict) {
-			return false, err
-		}
-	}
-	return false, fmt.Errorf("uefistore: mark %s deleting: %w after %d attempts", instanceID, kvstore.ErrConflict, markAttempts)
-}
-
-// InstanceIDs lists the key of every store record, sorted. It decodes
-// nothing, so a caller reads each record through Get and its validation.
-func (s *Store) InstanceIDs(ctx context.Context) ([]string, error) {
-	kv, err := s.kv.KV(ctx)
-	if err != nil {
-		return nil, err
-	}
-	lister, err := kv.ListKeys(ctx)
-	if err != nil {
-		if errors.Is(err, jetstream.ErrNoKeysFound) {
-			return nil, nil
-		}
-		return nil, fmt.Errorf("uefistore: list %s: %w", BucketName, err)
-	}
-	defer func() { _ = lister.Stop() }()
-
-	var ids []string
-	for key := range lister.Keys() {
-		if key != kvutil.VersionKey {
-			ids = append(ids, key)
-		}
-	}
-	slices.Sort(ids)
-	return ids, nil
-}
-
-// validOwner reports whether ownerID is an instance or image ID that names
-// exactly one prefix, so a malformed ID can never widen a delete.
-func validOwner(ownerID string) bool {
-	if strings.ContainsAny(ownerID, "/*") {
-		return false
-	}
-	for _, p := range []string{"i-", "ami-"} {
-		if rest, ok := strings.CutPrefix(ownerID, p); ok && rest != "" {
-			return true
-		}
-	}
-	return false
-}
-
-// ListObjects returns the key of every object under ownerID's prefix, across
-// every page of the listing.
-func ListObjects(ctx context.Context, objects objectstore.ObjectStore, bucket, ownerID string) ([]string, error) {
-	if !validOwner(ownerID) {
-		return nil, fmt.Errorf("uefistore: %q is not an instance or image ID", ownerID)
-	}
-	contents, _, err := objectstore.ListAll(ctx, objects, &s3.ListObjectsV2Input{
-		Bucket: aws.String(bucket),
-		Prefix: aws.String(KeyPrefix(ownerID)),
-	})
-	if err != nil {
-		return nil, fmt.Errorf("uefistore: list %s: %w", KeyPrefix(ownerID), err)
-	}
-	keys := make([]string, 0, len(contents))
-	for _, obj := range contents {
-		keys = append(keys, aws.StringValue(obj.Key))
-	}
-	return keys, nil
-}
-
-// DeleteObjects deletes every object under ownerID's prefix and reports how
-// many it removed. It succeeds only once a fresh listing finds the prefix
-// empty, so an object written during the delete is not left unaccounted for.
-func DeleteObjects(ctx context.Context, objects objectstore.ObjectStore, bucket, ownerID string) (int, error) {
-	keys, err := ListObjects(ctx, objects, bucket, ownerID)
-	if err != nil {
-		return 0, err
-	}
-	deleted := 0
-	for _, key := range keys {
-		_, err := objects.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: aws.String(bucket), Key: aws.String(key)})
-		if err != nil && !objectstore.IsNoSuchKeyError(err) {
-			return deleted, fmt.Errorf("uefistore: delete %s: %w", key, err)
-		}
-		deleted++
-	}
-	left, err := ListObjects(ctx, objects, bucket, ownerID)
-	if err != nil {
-		return deleted, err
-	}
-	if len(left) > 0 {
-		return deleted, fmt.Errorf("uefistore: %d object(s) remain under %s", len(left), KeyPrefix(ownerID))
-	}
-	return deleted, nil
-}

spinifex/domains/ec2/instance/uefistore/cleanup_test.go

@@ -1,213 +0,0 @@
-package uefistore_test
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"testing"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-const bucket = "spinifex"
-
-func putObject(t *testing.T, objects objectstore.ObjectStore, key string) {
-	t.Helper()
-	_, err := objects.PutObject(t.Context(), &s3.PutObjectInput{
-		Bucket: aws.String(bucket), Key: aws.String(key), Body: bytes.NewReader([]byte("vars")),
-	})
-	require.NoError(t, err)
-}
-
-func keysUnder(t *testing.T, objects objectstore.ObjectStore, prefix string) []string {
-	t.Helper()
-	contents, _, err := objectstore.ListAll(t.Context(), objects, &s3.ListObjectsV2Input{Bucket: aws.String(bucket), Prefix: aws.String(prefix)})
-	require.NoError(t, err)
-	var keys []string
-	for _, o := range contents {
-		keys = append(keys, aws.StringValue(o.Key))
-	}
-	return keys
-}
-
-// Deletion is final: a writer holding the revision of a deleting record, such
-// as a write-back retry, must not bring the store back to live.
-func TestCompareAndSet_RefusesDeletingToLive(t *testing.T) {
-	s, _ := openStore(t)
-	rev, err := s.Create(t.Context(), liveRecord())
-	require.NoError(t, err)
-	deleting := liveRecord()
-	deleting.State = uefistore.StateDeleting
-	rev, err = s.CompareAndSet(t.Context(), deleting, rev)
-	require.NoError(t, err)
-
-	revived := liveRecord()
-	revived.WriteBack = writeBack()
-	_, err = s.CompareAndSet(t.Context(), revived, rev)
-	require.ErrorIs(t, err, uefistore.ErrDeleting)
-
-	got, gotRev, err := s.Get(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.Equal(t, rev, gotRev)
-	assert.Equal(t, uefistore.StateDeleting, got.State)
-}
-
-// A write-back owed on a deleting store would put an object behind the delete.
-func TestCompareAndSet_RefusesDeletingWithAWriteBack(t *testing.T) {
-	s, _ := openStore(t)
-	rev, err := s.Create(t.Context(), liveRecord())
-	require.NoError(t, err)
-	rec := liveRecord()
-	rec.State = uefistore.StateDeleting
-	rec.WriteBack = writeBack()
-
-	_, err = s.CompareAndSet(t.Context(), rec, rev)
-	require.ErrorIs(t, err, uefistore.ErrInvalidRecord)
-}
-
-func TestMarkDeleting_MovesLiveToDeletingAndDropsTheWriteBack(t *testing.T) {
-	s, _ := openStore(t)
-	rec := liveRecord()
-	rec.WriteBack = writeBack()
-	_, err := s.Create(t.Context(), rec)
-	require.NoError(t, err)
-
-	has, err := s.MarkDeleting(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.True(t, has)
-
-	got, rev, err := s.Get(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.Equal(t, uefistore.StateDeleting, got.State)
-	assert.Nil(t, got.WriteBack)
-	assert.Equal(t, "111122223333", got.AccountID)
-
-	has, err = s.MarkDeleting(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.True(t, has, "a repeated terminate finds the obligation already recorded")
-	_, again, err := s.Get(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.Equal(t, rev, again, "a repeat writes nothing")
-}
-
-func TestMarkDeleting_NoRecordIsNoStore(t *testing.T) {
-	s, _ := openStore(t)
-	has, err := s.MarkDeleting(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.False(t, has)
-	_, _, err = s.Get(t.Context(), instanceID)
-	require.ErrorIs(t, err, kvstore.ErrNotFound, "marking must not create a record")
-}
-
-// A record this node cannot trust is reported, never rewritten as deleting.
-func TestMarkDeleting_RefusesAnUntrustedRecord(t *testing.T) {
-	s, js := openStore(t)
-	rawPut(t, js, instanceID, `{"schema_version":2,"instance_id":"i-0123456789abcdef0","state":"live"}`)
-
-	_, err := s.MarkDeleting(t.Context(), instanceID)
-	require.ErrorIs(t, err, uefistore.ErrInvalidRecord)
-}
-
-func TestExists_CountsAnUndecodableRecord(t *testing.T) {
-	s, js := openStore(t)
-	has, err := s.Exists(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.False(t, has)
-
-	rawPut(t, js, instanceID, `not json`)
-	has, err = s.Exists(t.Context(), instanceID)
-	require.NoError(t, err)
-	assert.True(t, has)
-}
-
-func TestInstanceIDs_ListsRecordsButNotTheVersionKey(t *testing.T) {
-	s, _ := openStore(t)
-	ids, err := s.InstanceIDs(t.Context())
-	require.NoError(t, err)
-	assert.Empty(t, ids)
-
-	for _, id := range []string{"i-0000000000000000b", "i-0000000000000000a"} {
-		rec := liveRecord()
-		rec.InstanceID = id
-		_, err := s.Create(t.Context(), rec)
-		require.NoError(t, err)
-	}
-	ids, err = s.InstanceIDs(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, []string{"i-0000000000000000a", "i-0000000000000000b"}, ids)
-}
-
-// The prefix carries its trailing slash, so an instance whose ID extends
-// another's keeps its objects.
-func TestDeleteObjects_RemovesOnlyTheOwnersPrefix(t *testing.T) {
-	objects := objectstore.NewMemoryObjectStore()
-	const other = instanceID + "1"
-	for _, key := range []string{"a", "b", "c"} {
-		putObject(t, objects, uefistore.KeyPrefix(instanceID)+key)
-	}
-	putObject(t, objects, uefistore.KeyPrefix(other)+"a")
-	putObject(t, objects, uefistore.KeyPrefix("ami-0123456789abcdef0")+"a")
-
-	deleted, err := uefistore.DeleteObjects(t.Context(), objects, bucket, instanceID)
-	require.NoError(t, err)
-	assert.Equal(t, 3, deleted)
-	assert.Empty(t, keysUnder(t, objects, uefistore.KeyPrefix(instanceID)))
-	assert.Len(t, keysUnder(t, objects, uefistore.KeyPrefix(other)), 1)
-	assert.Len(t, keysUnder(t, objects, uefistore.KeyPrefix("ami-0123456789abcdef0")), 1)
-
-	deleted, err = uefistore.DeleteObjects(t.Context(), objects, bucket, instanceID)
-	require.NoError(t, err, "an empty prefix is already deleted")
-	assert.Zero(t, deleted)
-}
-
-func TestDeleteObjects_RefusesAnIDThatWidensTheDelete(t *testing.T) {
-	objects := objectstore.NewMemoryObjectStore()
-	putObject(t, objects, uefistore.KeyPrefix(instanceID)+"a")
-	for _, id := range []string{"", "i-", "ami-", "vol-0123", "i-0/../", "i-*", "x"} {
-		_, err := uefistore.DeleteObjects(t.Context(), objects, bucket, id)
-		require.Errorf(t, err, "owner %q", id)
-	}
-	assert.Len(t, keysUnder(t, objects, uefistore.ObjectRoot), 1)
-}
-
-// failingDeletes refuses every delete, as predastore does while it is down.
-type failingDeletes struct {
-	objectstore.ObjectStore
-}
-
-func (failingDeletes) DeleteObject(context.Context, *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
-	return nil, errors.New("connection refused")
-}
-
-func TestDeleteObjects_ReportsAFailedDelete(t *testing.T) {
-	mem := objectstore.NewMemoryObjectStore()
-	putObject(t, mem, uefistore.KeyPrefix(instanceID)+"a")
-
-	_, err := uefistore.DeleteObjects(t.Context(), failingDeletes{mem}, bucket, instanceID)
-	require.Error(t, err)
-	assert.Len(t, keysUnder(t, mem, uefistore.KeyPrefix(instanceID)), 1)
-}
-
-// noopDeletes reports success without deleting, so the closing listing is
-// what notices the prefix is not empty.
-type noopDeletes struct {
-	objectstore.ObjectStore
-}
-
-func (noopDeletes) DeleteObject(context.Context, *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
-	return &s3.DeleteObjectOutput{}, nil
-}
-
-func TestDeleteObjects_FailsWhileAnObjectRemains(t *testing.T) {
-	mem := objectstore.NewMemoryObjectStore()
-	putObject(t, mem, uefistore.KeyPrefix(instanceID)+"a")
-
-	_, err := uefistore.DeleteObjects(t.Context(), noopDeletes{mem}, bucket, instanceID)
-	require.Error(t, err)
-}

spinifex/domains/ec2/instance/uefistore/uefistore.go

@@ -28,19 +28,13 @@ const (
 	// StateDeleting is a store whose objects are to be deleted, and then the record.
 	StateDeleting = "deleting"
 
-	// ObjectRoot is the object-store prefix every store object is under.
-	ObjectRoot = "uefi/"
+	objectRoot = "uefi/"
 )
 
 // ErrInvalidRecord is returned for a store record that breaks its invariants,
 // whether it is about to be written or was just read.
 var ErrInvalidRecord = errors.New("uefistore: invalid store record")
 
-// ErrDeleting is returned for a change that would return a deleting store to
-// live. Deletion follows only a terminate or an operator's forget, and both are
-// final, so a writer that meets it drops whatever it was about to record.
-var ErrDeleting = errors.New("uefistore: store is being deleted")
-
 // FirmwareIdentity names the firmware a variable store was written under. A
 // store only boots where Arch and VarsSize match the node's firmware, because
 // pflash needs a byte-exact region; CodeSHA256 is recorded, not enforced.
@@ -74,7 +68,7 @@ type UEFIWriteBack struct {
 // KeyPrefix is the object-store prefix every store object of ownerID (an
 // instance or image ID) is written under.
 func KeyPrefix(ownerID string) string {
-	return ObjectRoot + ownerID + "/"
+	return objectRoot + ownerID + "/"
 }
 
 // ownsKey reports whether key names one object directly under ownerID's prefix.
@@ -98,9 +92,6 @@ func (rec *UEFIStoreRecord) validate(key string) error {
 		return fmt.Errorf("%w: instance_id %s stored at key %s", ErrInvalidRecord, rec.InstanceID, key)
 	case rec.State != StateLive && rec.State != StateDeleting:
 		return fmt.Errorf("%w: %s has state %q", ErrInvalidRecord, key, rec.State)
-	case rec.State == StateDeleting && rec.WriteBack != nil:
-		// A write-back retry puts an object, which would land behind the delete.
-		return fmt.Errorf("%w: %s is deleting with a write-back still owed", ErrInvalidRecord, key)
 	}
 	if wb := rec.WriteBack; wb != nil {
 		switch {
@@ -143,15 +134,6 @@ func NewStore(js jetstream.JetStream) *Store {
 	return &Store{kv: kvstore.New[UEFIStoreRecord](js, bucketConfig())}
 }
 
-// NewReader returns a Store over the existing bucket that neither creates it
-// nor migrates it, for an operator report that must not change the cluster.
-func NewReader(js jetstream.JetStream) *Store {
-	cfg := bucketConfig()
-	cfg.AttachOnly = true
-	cfg.OnOpen = nil
-	return &Store{kv: kvstore.New[UEFIStoreRecord](js, cfg)}
-}
-
 // Open creates the bucket if it is missing and stamps its schema version.
 func (s *Store) Open(ctx context.Context) error {
 	_, err := s.kv.KV(ctx)
@@ -187,24 +169,12 @@ func (s *Store) Create(ctx context.Context, rec *UEFIStoreRecord) (uint64, error
 }
 
 // CompareAndSet replaces the record only if it is still at rev, returning
-// kvstore.ErrConflict when it is not, and the revision written otherwise. A
-// record at rev that is deleting cannot be set live again: that is ErrDeleting.
+// kvstore.ErrConflict when it is not, and the revision written otherwise.
 func (s *Store) CompareAndSet(ctx context.Context, rec *UEFIStoreRecord, rev uint64) (uint64, error) {
 	rec.SchemaVersion = RecordSchemaVersion
 	if err := rec.validate(rec.InstanceID); err != nil {
 		return 0, err
 	}
-	if rec.State == StateLive {
-		// Read unvalidated: only the state at rev matters, and any other
-		// revision fails the guarded write below anyway.
-		cur, curRev, err := s.kv.Get(ctx, rec.InstanceID)
-		if err != nil && !errors.Is(err, kvstore.ErrNotFound) {
-			return 0, err
-		}
-		if err == nil && curRev == rev && cur.State == StateDeleting {
-			return 0, fmt.Errorf("%w: %s", ErrDeleting, rec.InstanceID)
-		}
-	}
 	return s.kv.CompareAndSet(ctx, rec.InstanceID, rec, rev)
 }
 

spinifex/operator/admin/orphan_uefi_stores_test.go

@@ -1,173 +0,0 @@
-package admin
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"testing"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
-	testutil "github.com/mulgadc/spinifex/internal/testkit"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-type staticInstanceIndex struct {
-	ids map[string]bool
-	err error
-}
-
-func (s staticInstanceIndex) InstanceIDs(context.Context) (map[string]bool, error) {
-	return s.ids, s.err
-}
-
-type uefiOrphanFixture struct {
-	objects  *objectstore.MemoryObjectStore
-	metadata *ebsmetadata.Store
-	stores   *uefistore.Store
-	index    staticInstanceIndex
-}
-
-func newUEFIOrphanFixture(t *testing.T) *uefiOrphanFixture {
-	t.Helper()
-	_, _, js := testutil.StartTestJetStream(t)
-	stores := uefistore.NewStore(js)
-	require.NoError(t, stores.Open(t.Context()))
-	objects := objectstore.NewMemoryObjectStore()
-	return &uefiOrphanFixture{
-		objects:  objects,
-		metadata: ebsmetadata.NewStore(objects, orphanTestBucket),
-		stores:   stores,
-		index:    staticInstanceIndex{ids: map[string]bool{}},
-	}
-}
-
-func (f *uefiOrphanFixture) putStoreObject(t *testing.T, owner string) {
-	t.Helper()
-	_, err := f.objects.PutObject(t.Context(), &s3.PutObjectInput{
-		Bucket: aws.String(orphanTestBucket),
-		Key:    aws.String(uefistore.KeyPrefix(owner) + "0f1e2d3c"),
-		Body:   bytes.NewReader([]byte("vars")),
-	})
-	require.NoError(t, err)
-}
-
-func (f *uefiOrphanFixture) record(t *testing.T, id string, deleting bool) {
-	t.Helper()
-	_, err := f.stores.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: id, AccountID: "111122223333", State: uefistore.StateLive})
-	require.NoError(t, err)
-	if deleting {
-		_, err = f.stores.MarkDeleting(t.Context(), id)
-		require.NoError(t, err)
-	}
-}
-
-func (f *uefiOrphanFixture) scan(t *testing.T) (*OrphanUEFIStores, error) {
-	t.Helper()
-	return FindOrphanUEFIStores(t.Context(), f.objects, orphanTestBucket, f.metadata, f.stores, f.index)
-}
-
-// seedEveryCase plants one store of each shape the report must tell apart.
-func (f *uefiOrphanFixture) seedEveryCase(t *testing.T) {
-	t.Helper()
-	// Owned: an instance record, a store record, and a registered image.
-	f.index.ids["i-00000000000000live"] = true
-	f.record(t, "i-00000000000000live", false)
-	f.putStoreObject(t, "i-00000000000000live")
-	f.record(t, "i-000000000recordonly", false)
-	f.putStoreObject(t, "i-000000000recordonly")
-	f.record(t, "i-0000000000deleting", true)
-	f.putStoreObject(t, "i-0000000000deleting")
-	require.NoError(t, f.metadata.PutAMI(t.Context(), ebsmetadata.AMI{ImageID: "ami-0000000000000live", Name: "img", State: "available"}))
-	f.putStoreObject(t, "ami-0000000000000live")
-
-	// Unowned.
-	f.putStoreObject(t, "i-00000000000noowner")
-	f.putStoreObject(t, "ami-000000000noimage")
-	f.putStoreObject(t, "vol-00000000000other")
-}
-
-func TestFindOrphanUEFIStores_ListsEachUnownedShape(t *testing.T) {
-	f := newUEFIOrphanFixture(t)
-	f.seedEveryCase(t)
-
-	orphans, err := f.scan(t)
-	require.NoError(t, err)
-
-	assert.Equal(t, []string{"uefi/i-00000000000noowner/"}, orphans.InstancePrefixes,
-		"only a prefix with neither an instance record nor a store record is unowned")
-	assert.Equal(t, []string{"uefi/ami-000000000noimage/"}, orphans.ImagePrefixes)
-	assert.Equal(t, []string{"uefi/vol-00000000000other/"}, orphans.OtherPrefixes)
-	assert.Equal(t, []OrphanUEFIStoreRecord{{InstanceID: "i-000000000recordonly", AccountID: "111122223333", State: uefistore.StateLive}},
-		orphans.Records, "a deleting record is already on its way out, and the reaper owns it")
-	assert.False(t, orphans.Empty())
-}
-
-// TestS7_UEFIOrphanReportDeletesNothing enforces ADR-0003:S7 for the report:
-// an absent instance record is not a terminate, so a scan that lists every
-// unowned store leaves every object and every record as it found them.
-func TestS7_UEFIOrphanReportDeletesNothing(t *testing.T) {
-	f := newUEFIOrphanFixture(t)
-	f.seedEveryCase(t)
-	objectsBefore := f.objects.Count()
-	ids, err := f.stores.InstanceIDs(t.Context())
-	require.NoError(t, err)
-	revs := map[string]uint64{}
-	for _, id := range ids {
-		_, rev, err := f.stores.Get(t.Context(), id)
-		require.NoError(t, err)
-		revs[id] = rev
-	}
-
-	_, err = f.scan(t)
-	require.NoError(t, err)
-
-	assert.Equal(t, objectsBefore, f.objects.Count(), "ADR-0003:S7: the orphan report must delete no object")
-	for id, rev := range revs {
-		rec, got, err := f.stores.Get(t.Context(), id)
-		require.NoError(t, err, "ADR-0003:S7: the orphan report removed the store record %s", id)
-		assert.Equalf(t, rev, got, "ADR-0003:S7: the orphan report changed the store record %s", id)
-		assert.NotEmpty(t, rec.State)
-	}
-}
-
-func TestFindOrphanUEFIStores_EmptyClusterReportsNothing(t *testing.T) {
-	f := newUEFIOrphanFixture(t)
-	orphans, err := f.scan(t)
-	require.NoError(t, err)
-	assert.True(t, orphans.Empty())
-}
-
-// Reading an unreadable record or a failed index as absent would point an
-// operator at live data, so either aborts the scan.
-func TestFindOrphanUEFIStores_UnreadableInputsAbortTheScan(t *testing.T) {
-	t.Run("store record", func(t *testing.T) {
-		_, _, js := testutil.StartTestJetStream(t)
-		stores := uefistore.NewStore(js)
-		require.NoError(t, stores.Open(t.Context()))
-		kv, err := js.KeyValue(t.Context(), uefistore.BucketName)
-		require.NoError(t, err)
-		_, err = kv.Put(t.Context(), "i-0123456789abcdef0", []byte(`{"schema_version":9,"instance_id":"i-0123456789abcdef0","state":"live"}`))
-		require.NoError(t, err)
-		objects := objectstore.NewMemoryObjectStore()
-
-		_, err = FindOrphanUEFIStores(t.Context(), objects, orphanTestBucket, ebsmetadata.NewStore(objects, orphanTestBucket), stores, staticInstanceIndex{ids: map[string]bool{}})
-		require.ErrorIs(t, err, uefistore.ErrInvalidRecord)
-	})
-	t.Run("instance index", func(t *testing.T) {
-		f := newUEFIOrphanFixture(t)
-		f.index.err = errors.New("kv unavailable")
-		_, err := f.scan(t)
-		require.Error(t, err)
-	})
-	t.Run("AMI document", func(t *testing.T) {
-		f := newUEFIOrphanFixture(t)
-		putRawAMIDocument(t, f.objects, "ami-corrupted000", []byte("{not json"))
-		_, err := f.scan(t)
-		require.Error(t, err)
-	})
-}

spinifex/operator/admin/orphan_volumes.go

@@ -2,18 +2,12 @@ package admin
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"sort"
 	"strings"
 
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
 	"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
 	"github.com/mulgadc/spinifex/spinifex/providers/ebs"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
 )
 
 // efiVolumeSuffix marks a volume derived from another one. An EFI variable
@@ -121,119 +115,3 @@ func listProviderVolumes(ctx context.Context, provider ebsprovider.EBSProvider)
 		token = resp.NextToken
 	}
 }
-
-// UEFIStoreRecords is the read side of the UEFI store-record bucket.
-type UEFIStoreRecords interface {
-	InstanceIDs(ctx context.Context) ([]string, error)
-	Get(ctx context.Context, instanceID string) (*uefistore.UEFIStoreRecord, uint64, error)
-}
-
-var _ UEFIStoreRecords = (*uefistore.Store)(nil)
-
-// InstanceIndex reports every instance ID the control plane holds a record
-// for, live or terminated. A record it cannot decode still counts.
-type InstanceIndex interface {
-	InstanceIDs(ctx context.Context) (map[string]bool, error)
-}
-
-// OrphanUEFIStoreRecord is a store record that is not deleting and whose
-// instance has no record, so nothing will ever delete its objects.
-type OrphanUEFIStoreRecord struct {
-	InstanceID string
-	AccountID  string
-	State      string
-}
-
-// OrphanUEFIStores is what the object store and the store records hold for
-// UEFI variable stores that nothing owns.
-type OrphanUEFIStores struct {
-	// InstancePrefixes are uefi/i-*/ prefixes with no instance record and no
-	// store record.
-	InstancePrefixes []string
-	// ImagePrefixes are uefi/ami-*/ prefixes with no AMI document.
-	ImagePrefixes []string
-	// OtherPrefixes are prefixes named for neither an instance nor an image,
-	// which no owner can ever claim.
-	OtherPrefixes []string
-	Records       []OrphanUEFIStoreRecord
-}
-
-// Empty reports whether the scan found nothing.
-func (o *OrphanUEFIStores) Empty() bool {
-	return len(o.InstancePrefixes) == 0 && len(o.ImagePrefixes) == 0 && len(o.OtherPrefixes) == 0 && len(o.Records) == 0
-}
-
-// FindOrphanUEFIStores reports UEFI variable stores that nothing owns. Like
-// FindOrphanVolumes it never deletes: an absent instance record is not proof
-// of a terminate, so settling each one is an operator's decision.
-//
-// Every read is strict. A record or document this cannot read aborts the scan
-// rather than reading as absent, which would point an operator at live data.
-func FindOrphanUEFIStores(ctx context.Context, objects objectstore.ObjectStore, bucket string,
-	metadata *ebsmetadata.Store, stores UEFIStoreRecords, instances InstanceIndex) (*OrphanUEFIStores, error) {
-	if objects == nil || bucket == "" || metadata == nil || stores == nil || instances == nil {
-		return nil, errors.New("uefi orphan scan: object store, bucket, metadata, store records and instance index are all required")
-	}
-
-	known, err := instances.InstanceIDs(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("uefi orphan scan: list instance records: %w", err)
-	}
-	images, err := metadata.ListAMIsStrict(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("uefi orphan scan: list control-plane images: %w", err)
-	}
-	imageIDs := make(map[string]bool, len(images))
-	for _, image := range images {
-		imageIDs[image.ImageID] = true
-	}
-
-	out := &OrphanUEFIStores{}
-	ids, err := stores.InstanceIDs(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("uefi orphan scan: list store records: %w", err)
-	}
-	recorded := make(map[string]bool, len(ids))
-	for _, id := range ids {
-		recorded[id] = true
-		rec, _, err := stores.Get(ctx, id)
-		if errors.Is(err, kvstore.ErrNotFound) {
-			continue
-		}
-		if err != nil {
-			return nil, fmt.Errorf("uefi orphan scan: read store record %s: %w", id, err)
-		}
-		if rec.State != uefistore.StateDeleting && !known[id] {
-			out.Records = append(out.Records, OrphanUEFIStoreRecord{InstanceID: id, AccountID: rec.AccountID, State: rec.State})
-		}
-	}
-
-	_, prefixes, err := objectstore.ListAll(ctx, objects, &s3.ListObjectsV2Input{
-		Bucket:    aws.String(bucket),
-		Prefix:    aws.String(uefistore.ObjectRoot),
-		Delimiter: aws.String("/"),
-	})
-	if err != nil {
-		return nil, fmt.Errorf("uefi orphan scan: list store objects: %w", err)
-	}
-	for _, p := range prefixes {
-		prefix := aws.StringValue(p.Prefix)
-		owner := strings.TrimSuffix(strings.TrimPrefix(prefix, uefistore.ObjectRoot), "/")
-		switch {
-		case strings.HasPrefix(owner, "i-"):
-			if !known[owner] && !recorded[owner] {
-				out.InstancePrefixes = append(out.InstancePrefixes, prefix)
-			}
-		case strings.HasPrefix(owner, "ami-"):
-			if !imageIDs[owner] {
-				out.ImagePrefixes = append(out.ImagePrefixes, prefix)
-			}
-		default:
-			out.OtherPrefixes = append(out.OtherPrefixes, prefix)
-		}
-	}
-	sort.Strings(out.InstancePrefixes)
-	sort.Strings(out.ImagePrefixes)
-	sort.Strings(out.OtherPrefixes)
-	return out, nil
-}

spinifex/operator/cli/admin.go

@@ -32,7 +32,6 @@ import (
 	"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
 	"github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata"
 	ec2image "github.com/mulgadc/spinifex/spinifex/domains/ec2/image"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
 	ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
 	iamdomain "github.com/mulgadc/spinifex/spinifex/domains/iam"
 	"github.com/mulgadc/spinifex/spinifex/domains/network/host"
@@ -46,7 +45,6 @@ import (
 	"github.com/mulgadc/spinifex/spinifex/providers/ebs/viperblock/dirty"
 	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/gpu"
-	"github.com/mulgadc/spinifex/spinifex/runtime/daemon"
 	"github.com/mulgadc/spinifex/spinifex/runtime/formation"
 	hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command"
 	"github.com/mulgadc/spinifex/spinifex/runtime/host/dns"
@@ -954,13 +952,6 @@ func runVolumesOrphansCmd(_ *cobra.Command, _ []string) {
 		fmt.Fprintln(os.Stderr, "Orphan scan failed:", err)
 		os.Exit(1)
 	}
-	// The variable stores are reported first, so the volume report's early
-	// return on a clean scan cannot hide them.
-	if err := reportUEFIStoreOrphans(context.Background(), nc, store, node.Predastore.Bucket); err != nil {
-		fmt.Fprintln(os.Stderr, "UEFI store orphan scan failed:", err)
-		os.Exit(1)
-	}
-	fmt.Println()
 	if len(orphans) == 0 {
 		fmt.Println("No orphaned volumes: every volume the provider holds has a control-plane record.")
 		return
@@ -979,53 +970,6 @@ func runVolumesOrphansCmd(_ *cobra.Command, _ []string) {
 	fmt.Println("through the EC2 API; removing one is a per-volume operator decision.")
 }
 
-// reportUEFIStoreOrphans prints the UEFI variable stores that nothing owns.
-// It reads only, and attaches to the buckets rather than creating them.
-func reportUEFIStoreOrphans(ctx context.Context, nc *nats.Conn, store objectstore.ObjectStore, bucket string) error {
-	js, err := jetstream.New(nc)
-	if err != nil {
-		return fmt.Errorf("jetstream: %w", err)
-	}
-	orphans, err := admin.FindOrphanUEFIStores(ctx, store, bucket, ebsmetadata.NewStore(store, bucket),
-		uefistore.NewReader(js), kvInstanceIndex{js: js})
-	if err != nil {
-		return err
-	}
-	if orphans.Empty() {
-		fmt.Println("No orphaned UEFI variable stores.")
-		return nil
-	}
-	fmt.Println("Orphaned UEFI variable stores:")
-	for _, p := range orphans.InstancePrefixes {
-		fmt.Printf("  %s  (no instance record and no store record)\n", p)
-	}
-	for _, p := range orphans.ImagePrefixes {
-		fmt.Printf("  %s  (no AMI document)\n", p)
-	}
-	for _, p := range orphans.OtherPrefixes {
-		fmt.Printf("  %s  (names neither an instance nor an image)\n", p)
-	}
-	for _, r := range orphans.Records {
-		fmt.Printf("  store record %s  (account %s, %s, no instance record)\n", r.InstanceID, r.AccountID, r.State)
-	}
-	fmt.Println()
-	fmt.Println("Nothing was deleted. An absent instance record is not proof of a terminate,")
-	fmt.Println("so settling each of these is an operator decision.")
-	return nil
-}
-
-// kvInstanceIndex is the instance domain's index of every instance ID it holds
-// a record for.
-type kvInstanceIndex struct {
-	js jetstream.JetStream
-}
-
-var _ admin.InstanceIndex = kvInstanceIndex{}
-
-func (k kvInstanceIndex) InstanceIDs(ctx context.Context) (map[string]bool, error) {
-	return daemon.KnownInstanceIDs(ctx, k.js)
-}
-
 func runimagesRemoveCmd(cmd *cobra.Command, args []string) {
 	imageID, _ := cmd.Flags().GetString("image-id")
 	force, _ := cmd.Flags().GetBool("force")

spinifex/operator/cli/admin_uefi_orphans_test.go

@@ -1,90 +0,0 @@
-package cli
-
-import (
-	"bytes"
-	"testing"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/nats-io/nats.go/jetstream"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	testutil "github.com/mulgadc/spinifex/internal/testkit"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/mulgadc/spinifex/spinifex/runtime/daemon"
-)
-
-func putKeys(t *testing.T, js jetstream.JetStream, bucket string, keys ...string) {
-	t.Helper()
-	kv, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{Bucket: bucket})
-	require.NoError(t, err)
-	for _, key := range keys {
-		_, err := kv.Put(t.Context(), key, []byte("not decoded"))
-		require.NoError(t, err)
-	}
-}
-
-// Every key space an instance record has lived in counts, and the node
-// blobs and presence markers name nodes rather than instances.
-func TestKVInstanceIndex_CountsEveryInstanceKeySpace(t *testing.T) {
-	_, _, js := testutil.StartTestJetStream(t)
-	putKeys(t, js, daemon.InstanceStateBucket, "i.i-running", "instance.i-frozenstopped", "node.node-1", "nodepresence.node-1")
-	putKeys(t, js, daemon.TerminatedInstanceBucket, "i.i-terminated", "terminated.i-frozenterminated")
-
-	ids, err := kvInstanceIndex{js: js}.InstanceIDs(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, map[string]bool{
-		"i-running": true, "i-frozenstopped": true, "i-terminated": true, "i-frozenterminated": true,
-	}, ids)
-}
-
-// The report attaches to the buckets rather than creating them, so a cluster
-// missing one fails the scan instead of reporting every store as unowned.
-func TestReportUEFIStoreOrphans_FailsWhenABucketIsMissing(t *testing.T) {
-	_, nc, _ := testutil.StartTestJetStream(t)
-	objects := objectstore.NewMemoryObjectStore()
-
-	require.Error(t, reportUEFIStoreOrphans(t.Context(), nc, objects, "spinifex"))
-}
-
-func TestReportUEFIStoreOrphans_PrintsAndDeletesNothing(t *testing.T) {
-	_, nc, js := testutil.StartTestJetStream(t)
-	putKeys(t, js, daemon.InstanceStateBucket)
-	putKeys(t, js, daemon.TerminatedInstanceBucket)
-	stores := uefistore.NewStore(js)
-	require.NoError(t, stores.Open(t.Context()))
-	_, err := stores.Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: "i-0000000000recorded", AccountID: "111122223333", State: uefistore.StateLive})
-	require.NoError(t, err)
-	objects := objectstore.NewMemoryObjectStore()
-	for _, owner := range []string{"i-00000000000noowner", "ami-000000000noimage", "vol-00000000000other"} {
-		_, err := objects.PutObject(t.Context(), &s3.PutObjectInput{
-			Bucket: aws.String("spinifex"), Key: aws.String(uefistore.KeyPrefix(owner) + "a"), Body: bytes.NewReader([]byte("vars")),
-		})
-		require.NoError(t, err)
-	}
-
-	out := captureStdout(t, func() {
-		require.NoError(t, reportUEFIStoreOrphans(t.Context(), nc, objects, "spinifex"))
-	})
-
-	assert.Contains(t, out, "uefi/i-00000000000noowner/")
-	assert.Contains(t, out, "uefi/ami-000000000noimage/")
-	assert.Contains(t, out, "uefi/vol-00000000000other/")
-	assert.Contains(t, out, "store record i-0000000000recorded")
-	assert.Contains(t, out, "Nothing was deleted")
-	assert.Equal(t, 3, objects.Count())
-}
-
-func TestReportUEFIStoreOrphans_CleanCluster(t *testing.T) {
-	_, nc, js := testutil.StartTestJetStream(t)
-	putKeys(t, js, daemon.InstanceStateBucket)
-	putKeys(t, js, daemon.TerminatedInstanceBucket)
-	require.NoError(t, uefistore.NewStore(js).Open(t.Context()))
-
-	out := captureStdout(t, func() {
-		require.NoError(t, reportUEFIStoreOrphans(t.Context(), nc, objectstore.NewMemoryObjectStore(), "spinifex"))
-	})
-	assert.Contains(t, out, "No orphaned UEFI variable stores.")
-}

spinifex/runtime/compute/vm/firmware.go

@@ -1,14 +1,21 @@
 package vm
 
 import (
+	"crypto/sha256"
+	"encoding/hex"
 	"fmt"
+	"io"
 	"os"
+	"sync"
+	"time"
+
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
 )
 
 // FirmwareCandidate is a (CODE, VARS template) pair shipped together by a
-// firmware package. CODE is mounted readonly as pflash unit 0; VARS is copied
-// into a per-VM viperblock volume on first boot and mounted as pflash unit 1
-// so EFI variables (BootOrder, BootNext, secure-boot state) survive reboots.
+// firmware package. CODE is mounted readonly as pflash unit 0; VARS seeds the
+// instance's variable store, which is mounted as pflash unit 1 so EFI
+// variables (BootOrder, BootNext, secure-boot state) survive reboots.
 type FirmwareCandidate struct {
 	Code         string
 	VarsTemplate string
@@ -48,3 +55,67 @@ func FirmwarePaths(arch string) (code, varsTemplate string, varsSize int64, err
 	}
 	return "", "", 0, fmt.Errorf("no UEFI firmware found for architecture %q; install ovmf (x86_64) or qemu-efi-aarch64 (arm64)", arch)
 }
+
+// NodeFirmware is the UEFI firmware this node runs for one architecture: the
+// candidate FirmwarePaths matched and the identity a variable store is
+// checked against.
+type NodeFirmware struct {
+	Code         string
+	VarsTemplate string
+	Identity     uefistore.FirmwareIdentity
+}
+
+// codeHashKey identifies one version of a CODE blob on disk, so a package
+// update that replaces the file is hashed again.
+type codeHashKey struct {
+	path    string
+	size    int64
+	modTime time.Time
+}
+
+// codeHashes caches CODE blob hashes by codeHashKey. The arm64 blob is 64 MiB,
+// and every UEFI launch needs its hash.
+var codeHashes sync.Map
+
+// NodeFirmwareFor returns the firmware this node runs for arch, with the
+// SHA-256 of its CODE blob.
+func NodeFirmwareFor(arch string) (NodeFirmware, error) {
+	code, vars, varsSize, err := FirmwarePaths(arch)
+	if err != nil {
+		return NodeFirmware{}, err
+	}
+	sum, err := codeSHA256(code)
+	if err != nil {
+		return NodeFirmware{}, err
+	}
+	return NodeFirmware{
+		Code:         code,
+		VarsTemplate: vars,
+		Identity:     uefistore.FirmwareIdentity{Arch: arch, VarsSize: varsSize, CodeSHA256: sum},
+	}, nil
+}
+
+func codeSHA256(path string) (string, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return "", fmt.Errorf("open UEFI CODE %s: %w", path, err)
+	}
+	defer f.Close()
+	info, err := f.Stat()
+	if err != nil {
+		return "", fmt.Errorf("stat UEFI CODE %s: %w", path, err)
+	}
+	key := codeHashKey{path: path, size: info.Size(), modTime: info.ModTime()}
+	if cached, ok := codeHashes.Load(key); ok {
+		if sum, ok := cached.(string); ok {
+			return sum, nil
+		}
+	}
+	h := sha256.New()
+	if _, err := io.Copy(h, f); err != nil {
+		return "", fmt.Errorf("hash UEFI CODE %s: %w", path, err)
+	}
+	sum := hex.EncodeToString(h.Sum(nil))
+	codeHashes.Store(key, sum)
+	return sum, nil
+}

spinifex/runtime/compute/vm/lifecycle.go

@@ -92,6 +92,9 @@ func (m *Manager) Start(ctx context.Context, id string) error {
 	if !ok {
 		return fmt.Errorf("%w: %s", ErrInstanceNotFound, id)
 	}
+	if err := CheckUEFIVarStore(instance); err != nil {
+		return err
+	}
 	// StateError is rejected by launchStillValid; move to Pending first so
 	// launch proceeds (resource re-allocation is the caller's responsibility).
 	if m.Status(instance) == StateError && m.deps.TransitionState != nil {
@@ -127,6 +130,13 @@ func (m *Manager) launch(ctx context.Context, instance *VM) (err error) {
 		return nil
 	}
 
+	// Before the reason below is cleared, so a refused start changes nothing.
+	if err := CheckUEFIVarStore(instance); err != nil {
+		slog.ErrorContext(ctx, "Refusing to launch a UEFI instance without exactly one variable store",
+			"instanceId", instance.ID, "err", err)
+		return err
+	}
+
 	// A reason left by the stop, or by a restore that found this node could not
 	// schedule the instance, describes a state the instance is now leaving.
 	// Clearing it here keeps the node the only writer of the field.
@@ -148,6 +158,16 @@ func (m *Manager) launch(ctx context.Context, instance *VM) (err error) {
 		}
 	}
 
+	// Before the mount, so a store this node cannot boot fails the launch
+	// with nothing to roll back.
+	uefiCtx, uefiSpan := otel.Tracer(vmTracerName).Start(ctx, "vm.launch.uefi_var_store")
+	uefiErr := m.prepareUEFIVarStore(uefiCtx, instance)
+	endSpanWithError(uefiSpan, uefiErr)
+	if uefiErr != nil {
+		slog.ErrorContext(ctx, "Failed to prepare the UEFI variable store", "instanceId", instance.ID, "err", uefiErr)
+		return uefiErr
+	}
+
 	_, mountSpan := otel.Tracer(vmTracerName).Start(ctx, "vm.launch.mount_volumes")
 	mountErr := m.deps.VolumeMounter.Mount(ctx, instance)
 	endSpanWithError(mountSpan, mountErr)
@@ -324,6 +344,14 @@ func (m *Manager) startQEMU(instance *VM, appendConsole bool) error {
 		instance.Config.IOThreads = append(instance.Config.IOThreads, driveCfg.IOThreads...)
 		instance.Config.Devices = append(instance.Config.Devices, driveCfg.Devices...)
 		instance.Config.Blockdevs = append(instance.Config.Blockdevs, driveCfg.Blockdevs...)
+
+		varsDrive, err := m.uefiVarsDrive(instance)
+		if err != nil {
+			return err
+		}
+		if varsDrive != nil {
+			instance.Config.Drives = append(instance.Config.Drives, *varsDrive)
+		}
 	}
 
 	if instance.DirectBoot {
@@ -1322,7 +1350,7 @@ func buildBaseVMConfig(instanceID, instanceType, pidFile, consoleLogPath, serial
 		CPUCount:       vCPUs,
 		Architecture:   architecture,
 		InstanceType:   instanceType,
-		UseUEFI:        bootMode == "uefi" || bootMode == "uefi-preferred",
+		UseUEFI:        isUEFIBootMode(bootMode),
 
 		// Present EC2-shaped DMI so stock cloud-init activates the Ec2 datasource.
 		SMBIOSUUID:         ec2SMBIOSUUID(instanceID),
@@ -1415,7 +1443,8 @@ func buildDiskBootDrives(requests []EBSRequest, cpuCount int, machineType string
 //
 // The boot volume is a -blockdev node named "os" under a bootindex=1 device
 // that carries werror/rerror; it is never detached, so it keeps fixed names.
-// EFI volumes emit pflash unit=1; Config.Execute adds the CODE blob (unit=0).
+// EFI volumes emit pflash unit=1, as uefiVarsDrive does for a variable store
+// pointer; Config.Execute adds the CODE blob (unit=0).
 //
 // Every other (data) volume is given the same named node-name/device-id/
 // iothread-id shape AttachVolume's hot-plug path uses, via a -blockdev entry

spinifex/runtime/compute/vm/manager.go

@@ -36,7 +36,6 @@ type Deps struct {
 	Resources          ResourceController
 	VolumeStateUpdater VolumeStateUpdater
 	InstanceCleaner    InstanceCleaner
-	UEFIStores         UEFIStoreTerminator
 	Hooks              ManagerHooks
 
 	// ShutdownSignal returns true once the daemon has begun coordinated
@@ -88,6 +87,9 @@ type Deps struct {
 	// (predastore + viperblock) is confirmed up. Nil is treated as ready,
 	// preserving behaviour for tests and nodes with storage unwired.
 	BackingStoreReady func() bool
+
+	// UEFIVarStore is where UEFI variable stores are read from and kept.
+	UEFIVarStore UEFIVarStoreDeps
 }
 
 // Manager owns the in-memory map of running VMs on this node and every

spinifex/runtime/compute/vm/shutdown.go

@@ -433,7 +433,6 @@ func (m *Manager) finalizeTerminated(instance *VM) error {
 // already spent and terminated is terminal.
 func (m *Manager) reconcileVanishedQEMU(instance *VM) error {
 	m.markTeardown(instance, TeardownQEMU, TeardownDone)
-	m.markUEFIStoreForDeletion(instance)
 	m.stampOutstandingTeardownFailed(instance)
 	return m.finalizeTerminated(instance)
 }
@@ -461,7 +460,6 @@ func (m *Manager) forceFinalizeStuckTerminate(instance *VM) error {
 	if m.deps.InstanceCleaner != nil {
 		m.markTeardownResult(instance, TeardownVolumes, m.deps.InstanceCleaner.DeleteVolumes(instance))
 	}
-	m.markUEFIStoreForDeletion(instance)
 	m.stampOutstandingTeardownFailed(instance)
 	return m.finalizeTerminated(instance)
 }
@@ -550,8 +548,8 @@ func (m *Manager) releaseAutoAssignedPublicIP(instance *VM) {
 }
 
 // terminateCleanup is stopCleanup plus the AWS-resource cleanup that
-// only applies on terminate: volume deletion, the UEFI store's deletion
-// record, public IP release, ENI deletion, placement-group removal.
+// only applies on terminate: volume deletion, public IP release, ENI
+// deletion, placement-group removal.
 func (m *Manager) terminateCleanup(instance *VM) {
 	// Terminate deletes the volumes next, so an unsealed block map loses
 	// nothing a caller can still ask for. Tolerated to keep terminate
@@ -562,7 +560,6 @@ func (m *Manager) terminateCleanup(instance *VM) {
 	if m.deps.InstanceCleaner != nil {
 		m.markTeardownResult(instance, TeardownVolumes, m.deps.InstanceCleaner.DeleteVolumes(instance))
 	}
-	m.markUEFIStoreForDeletion(instance)
 
 	m.cleanupTapDevices(instance)
 	m.markTeardown(instance, TeardownTap, TeardownDone)

spinifex/runtime/compute/vm/teardown.go

@@ -23,9 +23,6 @@ const (
 	TeardownENI       = "eni"
 	TeardownOVN       = "ovn"
 	TeardownPlacement = "placement"
-	// TeardownUEFI is pending once the variable store is recorded for
-	// deletion; the cluster-wide store reaper marks it done.
-	TeardownUEFI = "uefi"
 )
 
 // markTeardown records dep → state on the VM under the manager lock. Inspect

spinifex/runtime/compute/vm/uefi_terminate.go

@@ -1,35 +0,0 @@
-package vm
-
-import "log/slog"
-
-// UEFIStoreTerminator settles a terminated instance's UEFI variable store. The
-// objects are deleted by a cluster-wide reaper, so terminate only records the
-// obligation and removes this node's local copy.
-type UEFIStoreTerminator interface {
-	// MarkUEFIStoreDeleting moves the instance's store record to deleting. It
-	// reports false with no error when the instance has no store.
-	MarkUEFIStoreDeleting(instanceID string) (bool, error)
-	// RemoveLocalUEFIStore removes this node's copy of the store. Idempotent.
-	RemoveLocalUEFIStore(instanceID string) error
-}
-
-// markUEFIStoreForDeletion records a terminating instance's store for the
-// reaper. A failed mark is stamped failed, which the reaper completes from the
-// terminated record; an instance with no store gets no teardown entry.
-func (m *Manager) markUEFIStoreForDeletion(instance *VM) {
-	if m.deps.UEFIStores == nil {
-		return
-	}
-	has, err := m.deps.UEFIStores.MarkUEFIStoreDeleting(instance.ID)
-	switch {
-	case err != nil:
-		slog.Warn("Failed to record UEFI variable store for deletion; the store reaper completes it",
-			"instanceId", instance.ID, "err", err)
-		m.markTeardown(instance, TeardownUEFI, TeardownFailed)
-	case has:
-		m.markTeardown(instance, TeardownUEFI, TeardownPending)
-	}
-	if err := m.deps.UEFIStores.RemoveLocalUEFIStore(instance.ID); err != nil {
-		slog.Warn("Failed to remove local UEFI variable store", "instanceId", instance.ID, "err", err)
-	}
-}

spinifex/runtime/compute/vm/uefi_terminate_test.go

@@ -1,153 +0,0 @@
-package vm
-
-import (
-	"errors"
-	"sync"
-	"testing"
-	"time"
-
-	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// fakeUEFIStores records each terminator call; has and markErr shape the mark.
-type fakeUEFIStores struct {
-	mu      sync.Mutex
-	has     bool
-	markErr error
-	marked  []string
-	removed []string
-}
-
-var _ UEFIStoreTerminator = (*fakeUEFIStores)(nil)
-
-func (f *fakeUEFIStores) MarkUEFIStoreDeleting(id string) (bool, error) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	f.marked = append(f.marked, id)
-	return f.has && f.markErr == nil, f.markErr
-}
-
-func (f *fakeUEFIStores) RemoveLocalUEFIStore(id string) error {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	f.removed = append(f.removed, id)
-	return nil
-}
-
-func (f *fakeUEFIStores) calls() (marked, removed []string) {
-	f.mu.Lock()
-	defer f.mu.Unlock()
-	return append([]string(nil), f.marked...), append([]string(nil), f.removed...)
-}
-
-func withUEFIStores(m *Manager, f *fakeUEFIStores) {
-	deps := m.deps
-	deps.UEFIStores = f
-	m.SetDeps(deps)
-}
-
-func runningUEFIInstance(id string) *VM {
-	return &VM{ID: id, Status: StateRunning, InstanceType: "t3.micro", Instance: &ec2.Instance{}}
-}
-
-// TestS8_TerminateRecordsTheUEFIStoreObligationAndReturns enforces ADR-0003:S8
-// for the store: terminate deletes no object, it records the obligation and
-// returns, and the terminated record carries it for the reaper to converge.
-func TestS8_TerminateRecordsTheUEFIStoreObligationAndReturns(t *testing.T) {
-	store := newFakeStateStore()
-	m, _, _, _, _ := terminateTestManager(t, store)
-	stores := &fakeUEFIStores{has: true}
-	withUEFIStores(m, stores)
-	v := runningUEFIInstance("i-uefi-terminate")
-	m.Insert(v)
-
-	require.NoError(t, m.Terminate(v.ID))
-
-	marked, removed := stores.calls()
-	assert.Equal(t, []string{v.ID}, marked, "terminate must record the store for deletion")
-	assert.Equal(t, []string{v.ID}, removed, "the node that ran the instance removes its local copy")
-	terminated := store.terminated[v.ID]
-	require.NotNil(t, terminated, "terminate must write the terminated record")
-	assert.Equal(t, string(TeardownPending), terminated.Teardown[TeardownUEFI],
-		"ADR-0003:S8: the terminated record must carry the store obligation while the reaper converges it")
-	assert.False(t, terminated.TeardownComplete(), "a pending store keeps the record from early purge")
-}
-
-func TestTerminate_InstanceWithNoUEFIStoreStampsNoEntry(t *testing.T) {
-	store := newFakeStateStore()
-	m, _, _, _, _ := terminateTestManager(t, store)
-	withUEFIStores(m, &fakeUEFIStores{has: false})
-	v := runningUEFIInstance("i-bios")
-	m.Insert(v)
-
-	require.NoError(t, m.Terminate(v.ID))
-
-	_, stamped := store.terminated[v.ID].Teardown[TeardownUEFI]
-	assert.False(t, stamped, "a BIOS instance has no store, so nothing is owed")
-}
-
-// A mark that fails is stamped failed rather than dropped, so the reaper can
-// complete it from the terminated record.
-func TestTerminate_FailedUEFIMarkIsStampedFailed(t *testing.T) {
-	store := newFakeStateStore()
-	m, _, _, _, _ := terminateTestManager(t, store)
-	withUEFIStores(m, &fakeUEFIStores{has: true, markErr: errors.New("kv unavailable")})
-	v := runningUEFIInstance("i-uefi-mark-fails")
-	m.Insert(v)
-
-	require.NoError(t, m.Terminate(v.ID), "a store mark never fails the terminate")
-
-	assert.Equal(t, string(TeardownFailed), store.terminated[v.ID].Teardown[TeardownUEFI])
-}
-
-// A terminate that wedged before its cleanup still records the store when the
-// backstop finalises it.
-func TestReconcileVanishedQEMU_RecordsTheUEFIStoreForDeletion(t *testing.T) {
-	store := newFakeStateStore()
-	m, _, _, _, _ := terminateTestManager(t, store)
-	stores := &fakeUEFIStores{has: true}
-	withUEFIStores(m, stores)
-	v := runningUEFIInstance("i-uefi-wedged")
-	v.Status = StateShuttingDown
-	m.Insert(v)
-
-	require.NoError(t, m.reconcileVanishedQEMU(v))
-
-	marked, _ := stores.calls()
-	assert.Equal(t, []string{v.ID}, marked)
-	assert.Equal(t, string(TeardownPending), store.terminated[v.ID].Teardown[TeardownUEFI])
-}
-
-// TestS7_OnlyTerminateRecordsTheUEFIStoreForDeletion enforces ADR-0003:S7 at
-// the node: a stop, a node-wide stop and a failed recovery of an instance
-// that has run never move its store toward deletion or remove its local copy.
-func TestS7_OnlyTerminateRecordsTheUEFIStoreForDeletion(t *testing.T) {
-	m, _, _, _, rt := shutdownTestManager(t)
-	stores := &fakeUEFIStores{has: true}
-	withUEFIStores(m, stores)
-
-	stopped := runningUEFIInstance("i-uefi-stop")
-	m.Insert(stopped)
-	require.NoError(t, m.Stop(stopped.ID))
-
-	drained := runningUEFIInstance("i-uefi-drain")
-	m.Insert(drained)
-	require.NoError(t, m.StopAll())
-
-	failed := &VM{ID: "i-uefi-has-run", Status: StatePending, InstanceType: "t3.micro", Instance: &ec2.Instance{}}
-	m.Insert(failed)
-	errored := rt.waitFor(failed.ID, StateError)
-	m.MarkFailed(t.Context(), failed, "relaunch_timeout")
-	select {
-	case <-errored:
-	case <-time.After(markFailedDeadline):
-		t.Fatalf("MarkFailed on an instance that has run did not reach error within %s", markFailedDeadline)
-	}
-	m.goroutineWg.Wait()
-
-	marked, removed := stores.calls()
-	assert.Empty(t, marked, "ADR-0003:S7: only a decided terminate may record a store for deletion")
-	assert.Empty(t, removed, "ADR-0003:S7: a stop or failure keeps the node's copy of the store")
-}

spinifex/runtime/compute/vm/uefi_varstore.go

@@ -0,0 +1,390 @@
+package vm
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	otelsetup "github.com/mulgadc/spinifex/spinifex/foundation/telemetry"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/metric"
+)
+
+const (
+	// uefiStoreDirName is the directory under DataDir that holds one
+	// subdirectory per instance with its live variable store.
+	uefiStoreDirName   = "uefi"
+	uefiVarsFileName   = "vars.fd"
+	uefiSeededFileName = "seeded.json"
+
+	// uefiStoreGetTimeout bounds the one get a launch makes for a fixed-size
+	// object, so a wedged object store fails the start rather than holding it.
+	uefiStoreGetTimeout = 10 * time.Second
+
+	// uefiStoreBodySlack is how far a compressed body may exceed its region
+	// before it is refused; gzip grows incompressible data by far less.
+	uefiStoreBodySlack = 1 << 20
+
+	// uefiStateReasonCode is the StateReason a launch refused over its
+	// variable store records, as EC2 reports an internal launch failure.
+	uefiStateReasonCode = "Server.InternalError"
+)
+
+// ErrUEFIVarStoreMissing is a UEFI instance whose record names no variable
+// store and carries no EFI volume, so it has nothing to boot its variables from.
+var ErrUEFIVarStoreMissing = errors.New("UEFI instance has no variable store")
+
+// ErrUEFIVarStoreConflict is a UEFI instance whose record names a variable
+// store and also carries an EFI volume, so either one would be ignored.
+var ErrUEFIVarStoreConflict = errors.New("UEFI instance has both a variable store pointer and an EFI volume")
+
+// ErrUEFIFirmwareMismatch is a variable store written under firmware whose
+// architecture or VARS size differs from this node's, which pflash cannot load.
+var ErrUEFIFirmwareMismatch = errors.New("UEFI variable store firmware does not match this node")
+
+// ErrUEFIVarStoreUnusable is a variable store object that is missing or does
+// not decode to a region of this node's VARS size. Retrying does not help.
+var ErrUEFIVarStoreUnusable = errors.New("UEFI variable store object is unusable")
+
+// UEFIVarStoreDeps is where a node reads instance variable stores from and
+// keeps the live copy QEMU opens. DataDir is the daemon's persistent data
+// directory, so the copy survives a daemon restart and a host reboot.
+type UEFIVarStoreDeps struct {
+	Objects objectstore.ObjectStore
+	Bucket  string
+	DataDir string
+}
+
+// uefiSeed is seeded.json: the pointer key vars.fd beside it was seeded from.
+type uefiSeed struct {
+	Key      string    `json:"key"`
+	SeededAt time.Time `json:"seeded_at"`
+}
+
+// isUEFIBootMode reports whether bootMode boots the guest through UEFI firmware.
+func isUEFIBootMode(bootMode string) bool {
+	return bootMode == "uefi" || bootMode == "uefi-preferred"
+}
+
+// usesUEFIVarStore reports whether v boots through UEFI and so needs a
+// variable store. A direct-boot microvm boots qboot and has none.
+func usesUEFIVarStore(v *VM) bool {
+	return !v.DirectBoot && isUEFIBootMode(v.BootMode)
+}
+
+// CheckUEFIVarStore refuses a UEFI instance that has no variable store, or
+// two, before anything about it is changed. An EFI volume still counts as a
+// store until RunInstances seeds pointers instead. The caller owns v.
+func CheckUEFIVarStore(v *VM) error {
+	if !usesUEFIVarStore(v) {
+		return nil
+	}
+	v.EBSRequests.Mu.Lock()
+	hasEFIVolume := slices.ContainsFunc(v.EBSRequests.Requests, func(r EBSRequest) bool { return r.EFI })
+	v.EBSRequests.Mu.Unlock()
+
+	switch {
+	case v.UEFIVarStore == nil && !hasEFIVolume:
+		return fmt.Errorf("%w: %s", ErrUEFIVarStoreMissing, v.ID)
+	case v.UEFIVarStore != nil && hasEFIVolume:
+		return fmt.Errorf("%w: %s points at %s", ErrUEFIVarStoreConflict, v.ID, v.UEFIVarStore.Key)
+	}
+	return nil
+}
+
+// uefiInstanceDir is the directory holding instanceID's vars.fd and
+// seeded.json. An ID that is not a single path segment is refused.
+func uefiInstanceDir(dataDir, instanceID string) (string, error) {
+	if !strings.HasPrefix(instanceID, "i-") || strings.ContainsAny(instanceID, `/\`) || instanceID != filepath.Clean(instanceID) {
+		return "", fmt.Errorf("instance ID %q cannot name a UEFI store directory", instanceID)
+	}
+	return filepath.Join(dataDir, uefiStoreDirName, instanceID), nil
+}
+
+// uefiVarsPath is the live variable store QEMU opens as pflash unit 1.
+func (m *Manager) uefiVarsPath(instanceID string) (string, error) {
+	dir, err := uefiInstanceDir(m.deps.UEFIVarStore.DataDir, instanceID)
+	if err != nil {
+		return "", err
+	}
+	return filepath.Join(dir, uefiVarsFileName), nil
+}
+
+// uefiVarsDrive is pflash unit 1 from this node's copy of the store the
+// record points at, or nil with no pointer. Launch seeds the copy and a
+// reboot reuses it, so a missing one fails rather than booting without it.
+func (m *Manager) uefiVarsDrive(instance *VM) (*Drive, error) {
+	hasPointer := false
+	m.Inspect(instance, func(v *VM) { hasPointer = v.UEFIVarStore != nil })
+	if !hasPointer || !usesUEFIVarStore(instance) {
+		return nil, nil
+	}
+	path, err := m.uefiVarsPath(instance.ID)
+	if err != nil {
+		return nil, err
+	}
+	if _, err := os.Stat(path); err != nil {
+		return nil, fmt.Errorf("UEFI variable store for %s: %w", instance.ID, err)
+	}
+	return &Drive{File: path, Format: "raw", If: "pflash", Unit: 1}, nil
+}
+
+// prepareUEFIVarStore leaves vars.fd holding the store the record points at,
+// reusing this node's copy when seeded.json names the pointer and fetching
+// the object otherwise. The object and the record's pointer are only read.
+func (m *Manager) prepareUEFIVarStore(ctx context.Context, instance *VM) error {
+	var ref *UEFIVarStoreRef
+	m.Inspect(instance, func(v *VM) {
+		if v.UEFIVarStore != nil {
+			cp := *v.UEFIVarStore
+			ref = &cp
+		}
+	})
+	if ref == nil || !usesUEFIVarStore(instance) {
+		return nil
+	}
+
+	deps := m.deps.UEFIVarStore
+	if deps.Objects == nil || deps.Bucket == "" || deps.DataDir == "" {
+		return errors.New("UEFI variable store access is not wired on this node")
+	}
+	if name, ok := strings.CutPrefix(ref.Key, uefistore.KeyPrefix(instance.ID)); !ok || name == "" || strings.Contains(name, "/") {
+		return fmt.Errorf("%w: pointer %q is not under %s", ErrUEFIVarStoreUnusable, ref.Key, uefistore.KeyPrefix(instance.ID))
+	}
+	if m.deps.InstanceTypes == nil {
+		return errors.New("InstanceTypeResolver not wired")
+	}
+	spec, ok := m.deps.InstanceTypes.Resolve(instance.InstanceType)
+	if !ok {
+		return fmt.Errorf("instance type %s not found", instance.InstanceType)
+	}
+	fw, err := NodeFirmwareFor(spec.Architecture)
+	if err != nil {
+		return err
+	}
+	if err := m.checkUEFIFirmware(instance, ref, fw); err != nil {
+		return err
+	}
+
+	dir, err := uefiInstanceDir(deps.DataDir, instance.ID)
+	if err != nil {
+		return err
+	}
+	if reuseUEFIVars(dir, ref.Key, fw.Identity.VarsSize) {
+		recordUEFIStoreSeed("local")
+		slog.InfoContext(ctx, "Reusing this node's UEFI variable store", "instanceId", instance.ID, "key", ref.Key)
+		return nil
+	}
+
+	start := time.Now()
+	region, err := fetchUEFIVars(ctx, deps, ref.Key, fw.Identity.VarsSize)
+	if err != nil {
+		return err
+	}
+	if err := writeFileAtomic(dir, uefiVarsFileName, region); err != nil {
+		return fmt.Errorf("write UEFI variable store for %s: %w", instance.ID, err)
+	}
+	seed, err := json.Marshal(uefiSeed{Key: ref.Key, SeededAt: time.Now().UTC()})
+	if err != nil {
+		return fmt.Errorf("encode %s for %s: %w", uefiSeededFileName, instance.ID, err)
+	}
+	if err := writeFileAtomic(dir, uefiSeededFileName, seed); err != nil {
+		return fmt.Errorf("write %s for %s: %w", uefiSeededFileName, instance.ID, err)
+	}
+	recordUEFIStoreSeed("object")
+	slog.InfoContext(ctx, "Seeded UEFI variable store from the object store",
+		"instanceId", instance.ID, "key", ref.Key, "bytes", len(region), "fetch_ms", otelsetup.Millis(time.Since(start)))
+	return nil
+}
+
+// checkUEFIFirmware fails a store whose arch or VARS size differs from this
+// node's, naming both firmwares in the log and the StateReason. A different
+// CODE build boots, because a package update changes it on every node.
+func (m *Manager) checkUEFIFirmware(instance *VM, ref *UEFIVarStoreRef, fw NodeFirmware) error {
+	stored, node := ref.Firmware, fw.Identity
+	var kind string
+	switch {
+	case stored.Arch != node.Arch:
+		kind = "arch"
+	case stored.VarsSize != node.VarsSize:
+		kind = "vars_size"
+	case stored.CodeSHA256 != node.CodeSHA256:
+		recordUEFIFirmwareMismatch("code")
+		slog.Warn("UEFI variable store was written under a different CODE build; booting it",
+			"instanceId", instance.ID, "key", ref.Key, "writerNode", ref.WriterNode,
+			"storeCodeSha256", stored.CodeSHA256, "nodeCodeSha256", node.CodeSHA256, "nodeCodePath", fw.Code)
+		return nil
+	default:
+		return nil
+	}
+
+	recordUEFIFirmwareMismatch(kind)
+	msg := fmt.Sprintf("%s: the UEFI variable store %s was written under firmware arch=%s vars_size=%d code_sha256=%s, and this node runs arch=%s vars_size=%d code_sha256=%s (%s)",
+		uefiStateReasonCode, ref.Key, stored.Arch, stored.VarsSize, stored.CodeSHA256,
+		node.Arch, node.VarsSize, node.CodeSHA256, fw.Code)
+	m.Inspect(instance, func(v *VM) {
+		if v.Instance != nil {
+			v.Instance.StateReason = &ec2.StateReason{Code: aws.String(uefiStateReasonCode), Message: aws.String(msg)}
+		}
+	})
+	slog.Error("UEFI variable store firmware does not match this node; refusing the launch",
+		"instanceId", instance.ID, "key", ref.Key, "writerNode", ref.WriterNode, "mismatch", kind,
+		"storeArch", stored.Arch, "storeVarsSize", stored.VarsSize, "storeCodeSha256", stored.CodeSHA256,
+		"nodeArch", node.Arch, "nodeVarsSize", node.VarsSize, "nodeCodeSha256", node.CodeSHA256, "nodeCodePath", fw.Code)
+	return fmt.Errorf("%w: %s", ErrUEFIFirmwareMismatch, msg)
+}
+
+// reuseUEFIVars reports whether dir already holds the store key names: a
+// seeded.json naming key beside a vars.fd of the region's size.
+func reuseUEFIVars(dir, key string, varsSize int64) bool {
+	data, err := os.ReadFile(filepath.Join(dir, uefiSeededFileName))
+	if err != nil {
+		return false
+	}
+	var seed uefiSeed
+	if err := json.Unmarshal(data, &seed); err != nil || seed.Key != key {
+		return false
+	}
+	info, err := os.Stat(filepath.Join(dir, uefiVarsFileName))
+	if err != nil || info.Size() != varsSize {
+		slog.Warn("UEFI variable store seeded here has no usable vars.fd; fetching it again", "dir", dir, "key", key)
+		return false
+	}
+	return true
+}
+
+// fetchUEFIVars gets key and returns its decompressed region, which must be
+// exactly varsSize bytes. A failed get is retryable; a missing or malformed
+// object is not.
+func fetchUEFIVars(ctx context.Context, deps UEFIVarStoreDeps, key string, varsSize int64) ([]byte, error) {
+	getCtx, cancel := context.WithTimeout(ctx, uefiStoreGetTimeout)
+	defer cancel()
+
+	out, err := deps.Objects.GetObject(getCtx, &s3.GetObjectInput{Bucket: aws.String(deps.Bucket), Key: aws.String(key)})
+	if err != nil {
+		if objectstore.IsNoSuchKeyError(err) {
+			return nil, fmt.Errorf("%w: %s does not exist", ErrUEFIVarStoreUnusable, key)
+		}
+		return nil, fmt.Errorf("get UEFI variable store %s: %w: %w", key, err, ErrMountRetryable)
+	}
+	defer out.Body.Close()
+
+	limit := varsSize + uefiStoreBodySlack
+	raw, err := io.ReadAll(io.LimitReader(out.Body, limit+1))
+	if err != nil {
+		return nil, fmt.Errorf("read UEFI variable store %s: %w: %w", key, err, ErrMountRetryable)
+	}
+	if int64(len(raw)) > limit {
+		return nil, fmt.Errorf("%w: %s is over %d bytes compressed", ErrUEFIVarStoreUnusable, key, limit)
+	}
+
+	gz, err := gzip.NewReader(bytes.NewReader(raw))
+	if err != nil {
+		return nil, fmt.Errorf("%w: %s is not gzip: %w", ErrUEFIVarStoreUnusable, key, err)
+	}
+	region, err := io.ReadAll(io.LimitReader(gz, varsSize+1))
+	if err != nil {
+		return nil, fmt.Errorf("%w: %s does not decompress: %w", ErrUEFIVarStoreUnusable, key, err)
+	}
+	if int64(len(region)) != varsSize {
+		return nil, fmt.Errorf("%w: %s holds a %d-byte region, and this node's VARS template is %d bytes",
+			ErrUEFIVarStoreUnusable, key, len(region), varsSize)
+	}
+	return region, nil
+}
+
+// writeFileAtomic replaces dir/name with data by temporary file and rename,
+// syncing both, so a crash leaves the old file or the new one.
+func writeFileAtomic(dir, name string, data []byte) error {
+	if err := os.MkdirAll(dir, 0o700); err != nil {
+		return err
+	}
+	tmp, err := os.CreateTemp(dir, "."+name+".*")
+	if err != nil {
+		return err
+	}
+	tmpName := tmp.Name()
+	if _, err := tmp.Write(data); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Sync(); err != nil {
+		_ = tmp.Close()
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := tmp.Close(); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	if err := os.Rename(tmpName, filepath.Join(dir, name)); err != nil {
+		_ = os.Remove(tmpName)
+		return err
+	}
+	d, err := os.Open(dir)
+	if err != nil {
+		return err
+	}
+	defer d.Close()
+	return d.Sync()
+}
+
+// uefiStoreMeterName is the instrumentation scope for variable store launches.
+const uefiStoreMeterName = "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm/uefistore"
+
+var (
+	uefiStoreMetricsOnce   sync.Once
+	uefiStoreSeedCounter   metric.Int64Counter
+	uefiFirmwareMismatches metric.Int64Counter
+)
+
+func uefiStoreMetrics() {
+	uefiStoreMetricsOnce.Do(func() {
+		meter := otel.Meter(uefiStoreMeterName)
+		var err error
+		uefiStoreSeedCounter, err = meter.Int64Counter("mulga.vm.uefi_store.seeds",
+			metric.WithDescription("UEFI launches by where their variable store came from: this node's copy or the object store."),
+			metric.WithUnit("{launch}"))
+		if err != nil {
+			otel.Handle(err)
+		}
+		uefiFirmwareMismatches, err = meter.Int64Counter("mulga.vm.uefi_store.firmware_mismatches",
+			metric.WithDescription("UEFI launches whose variable store was written under different firmware, by what differed. Only kind=code boots."),
+			metric.WithUnit("{launch}"))
+		if err != nil {
+			otel.Handle(err)
+		}
+	})
+}
+
+func recordUEFIStoreSeed(source string) {
+	uefiStoreMetrics()
+	if uefiStoreSeedCounter != nil {
+		uefiStoreSeedCounter.Add(context.Background(), 1, metric.WithAttributes(attribute.String("source", source)))
+	}
+}
+
+func recordUEFIFirmwareMismatch(kind string) {
+	uefiStoreMetrics()
+	if uefiFirmwareMismatches != nil {
+		uefiFirmwareMismatches.Add(context.Background(), 1, metric.WithAttributes(attribute.String("kind", kind)))
+	}
+}

spinifex/runtime/compute/vm/uefi_varstore_test.go

@@ -0,0 +1,539 @@
+package vm
+
+import (
+	"bytes"
+	"compress/gzip"
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
+	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/attribute"
+	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
+	"go.opentelemetry.io/otel/sdk/metric/metricdata"
+)
+
+const (
+	testUEFIBucket   = "instances"
+	testUEFIVarsSize = 4096
+	testUEFIInstance = "i-0123456789abcdef0"
+)
+
+var (
+	uefiMetricsReaderOnce sync.Once
+	uefiMetricsReader     *sdkmetric.ManualReader
+)
+
+// uefiCounter returns the sum of the named counter's points whose attribute
+// key has value.
+func uefiCounter(t *testing.T, name, key, value string) int64 {
+	t.Helper()
+	uefiMetricsReaderOnce.Do(func() {
+		uefiMetricsReader = sdkmetric.NewManualReader()
+		otel.SetMeterProvider(sdkmetric.NewMeterProvider(sdkmetric.WithReader(uefiMetricsReader)))
+	})
+	var rm metricdata.ResourceMetrics
+	require.NoError(t, uefiMetricsReader.Collect(context.Background(), &rm))
+	var total int64
+	for _, sm := range rm.ScopeMetrics {
+		for _, m := range sm.Metrics {
+			sum, ok := m.Data.(metricdata.Sum[int64])
+			if m.Name != name || !ok {
+				continue
+			}
+			for _, dp := range sum.DataPoints {
+				if v, ok := dp.Attributes.Value(attribute.Key(key)); ok && v.AsString() == value {
+					total += dp.Value
+				}
+			}
+		}
+	}
+	return total
+}
+
+// uefiFixture is a node with one x86_64 firmware candidate, an in-memory
+// object store and a data directory.
+type uefiFixture struct {
+	m        *Manager
+	mounter  *fakeVolumeMounter
+	objects  *objectstore.MemoryObjectStore
+	dataDir  string
+	codePath string
+	identity uefistore.FirmwareIdentity
+}
+
+func newUEFIFixture(t *testing.T) *uefiFixture {
+	t.Helper()
+	t.Setenv("XDG_RUNTIME_DIR", t.TempDir())
+	fwDir := t.TempDir()
+	code := filepath.Join(fwDir, "OVMF_CODE.fd")
+	vars := filepath.Join(fwDir, "OVMF_VARS.fd")
+	codeBytes := []byte("ovmf code build 1")
+	require.NoError(t, os.WriteFile(code, codeBytes, 0o600))
+	require.NoError(t, os.WriteFile(vars, make([]byte, testUEFIVarsSize), 0o600))
+	orig := FirmwarePathCandidates
+	FirmwarePathCandidates = map[string][]FirmwareCandidate{"x86_64": {{Code: code, VarsTemplate: vars}}}
+	t.Cleanup(func() { FirmwarePathCandidates = orig })
+
+	sum := sha256.Sum256(codeBytes)
+	f := &uefiFixture{
+		mounter:  &fakeVolumeMounter{},
+		objects:  objectstore.NewMemoryObjectStore(),
+		dataDir:  t.TempDir(),
+		codePath: code,
+		identity: uefistore.FirmwareIdentity{Arch: "x86_64", VarsSize: testUEFIVarsSize, CodeSHA256: hex.EncodeToString(sum[:])},
+	}
+	f.m = NewManagerWithDeps(Deps{
+		VolumeMounter: f.mounter,
+		InstanceTypes: fakeInstanceTypeResolver{"t3.nano": {Architecture: "x86_64", VCPUs: 1, MemoryMiB: 512}},
+		UEFIVarStore:  UEFIVarStoreDeps{Objects: f.objects, Bucket: testUEFIBucket, DataDir: f.dataDir},
+	})
+	return f
+}
+
+// region is a VARS region of the fixture's size whose bytes say which store it is.
+func region(tag string) []byte {
+	b := make([]byte, testUEFIVarsSize)
+	copy(b, tag)
+	return b
+}
+
+func gzipped(t *testing.T, data []byte) []byte {
+	t.Helper()
+	var buf bytes.Buffer
+	zw := gzip.NewWriter(&buf)
+	_, err := zw.Write(data)
+	require.NoError(t, err)
+	require.NoError(t, zw.Close())
+	return buf.Bytes()
+}
+
+func (f *uefiFixture) putObject(t *testing.T, key string, body []byte) {
+	t.Helper()
+	_, err := f.objects.PutObject(context.Background(), &s3.PutObjectInput{
+		Bucket: aws.String(testUEFIBucket), Key: aws.String(key), Body: bytes.NewReader(body),
+	})
+	require.NoError(t, err)
+}
+
+func (f *uefiFixture) objectBody(t *testing.T, key string) []byte {
+	t.Helper()
+	out, err := f.objects.GetObject(context.Background(), &s3.GetObjectInput{Bucket: aws.String(testUEFIBucket), Key: aws.String(key)})
+	require.NoError(t, err)
+	defer out.Body.Close()
+	b, err := io.ReadAll(out.Body)
+	require.NoError(t, err)
+	return b
+}
+
+func (f *uefiFixture) instanceDir() string {
+	return filepath.Join(f.dataDir, "uefi", testUEFIInstance)
+}
+
+func (f *uefiFixture) writeLocal(t *testing.T, seededKey string, vars []byte) {
+	t.Helper()
+	require.NoError(t, os.MkdirAll(f.instanceDir(), 0o700))
+	require.NoError(t, os.WriteFile(filepath.Join(f.instanceDir(), "vars.fd"), vars, 0o600))
+	seed, err := json.Marshal(uefiSeed{Key: seededKey, SeededAt: time.Now()})
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(f.instanceDir(), "seeded.json"), seed, 0o600))
+}
+
+func (f *uefiFixture) localVars(t *testing.T) []byte {
+	t.Helper()
+	b, err := os.ReadFile(filepath.Join(f.instanceDir(), "vars.fd"))
+	require.NoError(t, err)
+	return b
+}
+
+func (f *uefiFixture) seededKey(t *testing.T) string {
+	t.Helper()
+	b, err := os.ReadFile(filepath.Join(f.instanceDir(), "seeded.json"))
+	require.NoError(t, err)
+	var seed uefiSeed
+	require.NoError(t, json.Unmarshal(b, &seed))
+	return seed.Key
+}
+
+func (f *uefiFixture) instance(key string, fw uefistore.FirmwareIdentity) *VM {
+	v := &VM{
+		ID: testUEFIInstance, InstanceType: "t3.nano", BootMode: "uefi", Status: StateStopped,
+		Instance: &ec2.Instance{},
+	}
+	if key != "" {
+		v.UEFIVarStore = &UEFIVarStoreRef{Key: key, Firmware: fw, WriterNode: "node-a", WrittenAt: time.Unix(1700000000, 0).UTC()}
+	}
+	f.m.Insert(v)
+	return v
+}
+
+func keyFor(name string) string { return uefistore.KeyPrefix(testUEFIInstance) + name }
+
+func TestPrepareUEFIVarStore_FetchesTheObjectAndRecordsTheSeed(t *testing.T) {
+	f := newUEFIFixture(t)
+	key := keyFor("11111111-1111-1111-1111-111111111111")
+	f.putObject(t, key, gzipped(t, region("converted")))
+	instance := f.instance(key, f.identity)
+	before := uefiCounter(t, "mulga.vm.uefi_store.seeds", "source", "object")
+
+	require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), instance))
+
+	assert.Equal(t, region("converted"), f.localVars(t), "vars.fd must hold the decompressed region")
+	assert.Equal(t, key, f.seededKey(t))
+	assert.Equal(t, before+1, uefiCounter(t, "mulga.vm.uefi_store.seeds", "source", "object"))
+	assert.Equal(t, key, instance.UEFIVarStore.Key, "seeding only reads the pointer")
+}
+
+// A start on the node that seeded the pointer boots that node's copy, which
+// can be newer than the object, and makes no get at all.
+func TestPrepareUEFIVarStore_ReusesTheLocalCopyWhenSeededNamesThePointer(t *testing.T) {
+	f := newUEFIFixture(t)
+	key := keyFor("22222222-2222-2222-2222-222222222222")
+	f.writeLocal(t, key, region("guest wrote this"))
+	instance := f.instance(key, f.identity)
+	before := uefiCounter(t, "mulga.vm.uefi_store.seeds", "source", "local")
+
+	require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), instance), "no object exists, so a get would fail")
+
+	assert.Equal(t, region("guest wrote this"), f.localVars(t))
+	assert.Equal(t, before+1, uefiCounter(t, "mulga.vm.uefi_store.seeds", "source", "local"))
+}
+
+// Once the pointer has moved past what this node seeded, the local copy is
+// stale and the pointed object replaces it.
+func TestPrepareUEFIVarStore_RefetchesWhenTheLocalCopyIsNotThePointer(t *testing.T) {
+	tests := []struct {
+		name  string
+		local func(f *uefiFixture, t *testing.T)
+	}{
+		{name: "seeded from an older pointer", local: func(f *uefiFixture, t *testing.T) {
+			f.writeLocal(t, keyFor("older"), region("stale"))
+		}},
+		{name: "vars.fd of the wrong size", local: func(f *uefiFixture, t *testing.T) {
+			f.writeLocal(t, keyFor("current"), []byte("short"))
+		}},
+		{name: "seeded.json unreadable", local: func(f *uefiFixture, t *testing.T) {
+			f.writeLocal(t, keyFor("current"), region("stale"))
+			require.NoError(t, os.WriteFile(filepath.Join(f.instanceDir(), "seeded.json"), []byte("{"), 0o600))
+		}},
+		{name: "no vars.fd", local: func(f *uefiFixture, t *testing.T) {
+			f.writeLocal(t, keyFor("current"), region("stale"))
+			require.NoError(t, os.Remove(filepath.Join(f.instanceDir(), "vars.fd")))
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := newUEFIFixture(t)
+			key := keyFor("current")
+			tt.local(f, t)
+			f.putObject(t, key, gzipped(t, region("restored")))
+			instance := f.instance(key, f.identity)
+
+			require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), instance))
+
+			assert.Equal(t, region("restored"), f.localVars(t))
+			assert.Equal(t, key, f.seededKey(t))
+		})
+	}
+}
+
+// A store pflash cannot load fails the launch before any volume is mounted,
+// names both firmwares, and leaves the object, the pointer and the local
+// copy as they were.
+func TestRun_UEFIFirmwareMismatchFailsTheLaunchAndChangesNothing(t *testing.T) {
+	tests := []struct {
+		name   string
+		kind   string
+		mutate func(*uefistore.FirmwareIdentity)
+		want   []string
+	}{
+		{name: "arch", kind: "arch", mutate: func(fw *uefistore.FirmwareIdentity) { fw.Arch = "arm64" },
+			want: []string{"arch=arm64", "arch=x86_64"}},
+		{name: "vars size", kind: "vars_size", mutate: func(fw *uefistore.FirmwareIdentity) { fw.VarsSize = 67108864 },
+			want: []string{"vars_size=67108864", "vars_size=4096"}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := newUEFIFixture(t)
+			key := keyFor("mismatch")
+			body := gzipped(t, region("other firmware"))
+			f.putObject(t, key, body)
+			f.writeLocal(t, keyFor("older"), region("local"))
+			stored := f.identity
+			tt.mutate(&stored)
+			instance := f.instance(key, stored)
+			wantRef := *instance.UEFIVarStore
+			before := uefiCounter(t, "mulga.vm.uefi_store.firmware_mismatches", "kind", tt.kind)
+
+			err := f.m.Run(t.Context(), instance)
+
+			require.ErrorIs(t, err, ErrUEFIFirmwareMismatch)
+			require.NotErrorIs(t, err, ErrMountRetryable, "a mismatch does not clear with time")
+			assert.Empty(t, f.mounter.mounted, "nothing is mounted for a store that cannot boot")
+			require.NotNil(t, instance.Instance.StateReason)
+			assert.Equal(t, "Server.InternalError", aws.StringValue(instance.Instance.StateReason.Code))
+			for _, s := range append(tt.want, f.identity.CodeSHA256, f.codePath, key) {
+				assert.Contains(t, aws.StringValue(instance.Instance.StateReason.Message), s)
+			}
+			assert.Equal(t, wantRef, *instance.UEFIVarStore, "the pointer is unchanged")
+			assert.Equal(t, body, f.objectBody(t, key), "the object is unchanged")
+			assert.Equal(t, region("local"), f.localVars(t), "the local copy is unchanged")
+			assert.Equal(t, keyFor("older"), f.seededKey(t))
+			assert.Equal(t, before+1, uefiCounter(t, "mulga.vm.uefi_store.firmware_mismatches", "kind", tt.kind))
+		})
+	}
+}
+
+// An ovmf package update changes the CODE hash on every node, so a store
+// written under another build boots and the difference is counted.
+func TestPrepareUEFIVarStore_DifferentCodeBuildBootsAndIsCounted(t *testing.T) {
+	f := newUEFIFixture(t)
+	key := keyFor("code")
+	f.putObject(t, key, gzipped(t, region("older ovmf")))
+	stored := f.identity
+	stored.CodeSHA256 = strings.Repeat("0", 64)
+	instance := f.instance(key, stored)
+	before := uefiCounter(t, "mulga.vm.uefi_store.firmware_mismatches", "kind", "code")
+
+	require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), instance))
+
+	assert.Equal(t, region("older ovmf"), f.localVars(t))
+	assert.Nil(t, instance.Instance.StateReason)
+	assert.Equal(t, before+1, uefiCounter(t, "mulga.vm.uefi_store.firmware_mismatches", "kind", "code"))
+}
+
+// failingGetStore fails every get, standing in for an object store that
+// does not answer.
+type failingGetStore struct {
+	*objectstore.MemoryObjectStore
+
+	err error
+}
+
+func (s *failingGetStore) GetObject(context.Context, *s3.GetObjectInput) (*s3.GetObjectOutput, error) {
+	return nil, s.err
+}
+
+func TestPrepareUEFIVarStore_FailedGetIsRetryable(t *testing.T) {
+	f := newUEFIFixture(t)
+	f.m.deps.UEFIVarStore.Objects = &failingGetStore{MemoryObjectStore: f.objects, err: errors.New("connection refused")}
+	instance := f.instance(keyFor("down"), f.identity)
+
+	err := f.m.prepareUEFIVarStore(t.Context(), instance)
+
+	require.ErrorIs(t, err, ErrMountRetryable, "restore backs off on a get as it does on a mount")
+	assert.NoFileExists(t, filepath.Join(f.instanceDir(), "seeded.json"))
+}
+
+// An object that is missing or does not decode to this node's region is
+// refused for good, and nothing is written locally.
+func TestPrepareUEFIVarStore_UnusableObjectIsRefused(t *testing.T) {
+	tests := []struct {
+		name string
+		body []byte
+	}{
+		{name: "missing"},
+		{name: "not gzip", body: region("raw, uncompressed")},
+		{name: "short region", body: gzipped(t, make([]byte, testUEFIVarsSize-1))},
+		{name: "long region", body: gzipped(t, make([]byte, testUEFIVarsSize+1))},
+		{name: "truncated gzip", body: gzipped(t, region("cut"))[:20]},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := newUEFIFixture(t)
+			key := keyFor("bad")
+			if tt.body != nil {
+				f.putObject(t, key, tt.body)
+			}
+			instance := f.instance(key, f.identity)
+
+			err := f.m.prepareUEFIVarStore(t.Context(), instance)
+
+			require.ErrorIs(t, err, ErrUEFIVarStoreUnusable)
+			require.NotErrorIs(t, err, ErrMountRetryable)
+			assert.NoFileExists(t, filepath.Join(f.instanceDir(), "vars.fd"))
+			assert.NoFileExists(t, filepath.Join(f.instanceDir(), "seeded.json"))
+		})
+	}
+}
+
+func TestPrepareUEFIVarStore_PointerOutsideTheInstancePrefixIsRefused(t *testing.T) {
+	for _, key := range []string{"uefi/i-0aaaaaaaaaaaaaaaa/other", keyFor("a/b"), uefistore.KeyPrefix(testUEFIInstance)} {
+		t.Run(key, func(t *testing.T) {
+			f := newUEFIFixture(t)
+			f.putObject(t, key, gzipped(t, region("x")))
+			instance := f.instance(key, f.identity)
+
+			require.ErrorIs(t, f.m.prepareUEFIVarStore(t.Context(), instance), ErrUEFIVarStoreUnusable)
+		})
+	}
+}
+
+// A BIOS instance, a microvm and a legacy instance with no pointer touch
+// neither the object store nor the data directory.
+func TestPrepareUEFIVarStore_NoPointerIsANoOp(t *testing.T) {
+	f := newUEFIFixture(t)
+	f.m.deps.UEFIVarStore.Objects = &failingGetStore{MemoryObjectStore: f.objects, err: errors.New("must not be called")}
+	bios := f.instance(keyFor("bios"), f.identity)
+	bios.BootMode = ""
+	require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), bios))
+
+	legacy := f.instance("", f.identity)
+	require.NoError(t, f.m.prepareUEFIVarStore(t.Context(), legacy))
+	assert.NoDirExists(t, filepath.Join(f.dataDir, "uefi"))
+}
+
+// A UEFI instance with no pointer, or two stores, is refused before the
+// launch clears or changes anything on its record.
+func TestRun_UEFIInstanceWithoutExactlyOneStoreIsRefused(t *testing.T) {
+	efiVolume := EBSRequest{Name: "vol-1-efi", EFI: true}
+	tests := []struct {
+		name    string
+		pointer bool
+		efi     bool
+		want    error
+	}{
+		{name: "no pointer and no EFI volume", want: ErrUEFIVarStoreMissing},
+		{name: "a pointer and an EFI volume", pointer: true, efi: true, want: ErrUEFIVarStoreConflict},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := newUEFIFixture(t)
+			key := ""
+			if tt.pointer {
+				key = keyFor("p")
+			}
+			instance := f.instance(key, f.identity)
+			if tt.efi {
+				instance.EBSRequests.Requests = []EBSRequest{efiVolume}
+			}
+			stale := &ec2.StateReason{Code: aws.String("Client.UserInitiatedShutdown"), Message: aws.String("Client.UserInitiatedShutdown: User initiated shutdown")}
+			instance.Instance.StateReason = stale
+			before := instance.Record()
+
+			err := f.m.Run(t.Context(), instance)
+
+			require.ErrorIs(t, err, tt.want)
+			assert.Empty(t, f.mounter.mounted)
+			assert.Equal(t, before, instance.Record(), "the record is unchanged")
+		})
+	}
+}
+
+// Start from StateError transitions to pending before launching; a refused
+// UEFI instance must not take that transition either.
+func TestStart_UEFIInstanceWithoutAPointerIsRefusedBeforeTheTransition(t *testing.T) {
+	f := newUEFIFixture(t)
+	transitions := 0
+	f.m.deps.TransitionState = func(v *VM, s InstanceState) error { transitions++; v.Status = s; return nil }
+	instance := f.instance("", f.identity)
+	instance.Status = StateError
+
+	err := f.m.Start(t.Context(), instance.ID)
+
+	require.ErrorIs(t, err, ErrUEFIVarStoreMissing)
+	assert.Equal(t, 0, transitions)
+	assert.Equal(t, StateError, instance.Status)
+}
+
+// Until RunInstances seeds pointers, an instance whose variables are on an
+// EFI volume still launches through the volume, and a BIOS or microvm
+// guest needs no store at all.
+func TestCheckUEFIVarStore_Accepts(t *testing.T) {
+	tests := []struct {
+		name string
+		v    *VM
+	}{
+		{name: "EFI volume", v: &VM{BootMode: "uefi", EBSRequests: EBSRequests{Requests: []EBSRequest{{Name: "vol-efi", EFI: true}}}}},
+		{name: "pointer", v: &VM{BootMode: "uefi-preferred", UEFIVarStore: &UEFIVarStoreRef{Key: "k"}}},
+		{name: "BIOS", v: &VM{BootMode: "legacy-bios"}},
+		{name: "no boot mode", v: &VM{}},
+		{name: "microvm", v: &VM{BootMode: "uefi", DirectBoot: true}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.NoError(t, CheckUEFIVarStore(tt.v))
+		})
+	}
+}
+
+// A launch from a pointer opens this node's copy as pflash unit 1 with
+// format=raw, after the read-only CODE blob.
+func TestStartQEMU_PointerPassesTheLocalCopyAsPflashUnit1(t *testing.T) {
+	f := newUEFIFixture(t)
+	f.m.deps.NetworkPlumber = &fakeNetworkPlumber{}
+	launches := withFakeQEMU(t)
+	key := keyFor("boot")
+	f.writeLocal(t, key, region("seeded"))
+	instance := f.instance(key, f.identity)
+	instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}}
+
+	require.NoError(t, f.m.startQEMU(instance, false))
+
+	got := launches()
+	require.Len(t, got, 1)
+	vars := filepath.Join(f.instanceDir(), "vars.fd")
+	assert.Equal(t, []string{
+		"file=" + f.codePath + ",format=raw,if=pflash,unit=0,readonly=on",
+		"file=" + vars + ",format=raw,if=pflash,unit=1",
+	}, allArgValues(got[0], "-drive"))
+}
+
+// A reboot reuses the copy the launch seeded; without one there is nothing
+// safe to boot, so QEMU is not started.
+func TestStartQEMU_PointerWithoutALocalCopyIsNotLaunched(t *testing.T) {
+	f := newUEFIFixture(t)
+	f.m.deps.NetworkPlumber = &fakeNetworkPlumber{}
+	launches := withFakeQEMU(t)
+	instance := f.instance(keyFor("boot"), f.identity)
+	instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}}
+
+	err := f.m.startQEMU(instance, false)
+
+	require.ErrorIs(t, err, os.ErrNotExist)
+	assert.Empty(t, launches())
+}
+
+func TestUEFIInstanceDir_RefusesIDsThatAreNotOnePathSegment(t *testing.T) {
+	for _, id := range []string{"", "..", "i-../x", "i-a/b", `i-a\b`, "vol-1"} {
+		_, err := uefiInstanceDir("/data", id)
+		assert.Error(t, err, id)
+	}
+	dir, err := uefiInstanceDir("/data", testUEFIInstance)
+	require.NoError(t, err)
+	assert.Equal(t, "/data/uefi/"+testUEFIInstance, dir)
+}
+
+// The CODE hash is what a store records; a package update that replaces the
+// blob must be hashed again rather than served from the cache.
+func TestNodeFirmwareFor_HashesTheCodeBlobAgainAfterItChanges(t *testing.T) {
+	f := newUEFIFixture(t)
+	first, err := NodeFirmwareFor("x86_64")
+	require.NoError(t, err)
+	assert.Equal(t, f.identity, first.Identity)
+
+	updated := []byte("ovmf code build 2, a longer blob")
+	require.NoError(t, os.WriteFile(f.codePath, updated, 0o600))
+	second, err := NodeFirmwareFor("x86_64")
+	require.NoError(t, err)
+
+	sum := sha256.Sum256(updated)
+	assert.Equal(t, hex.EncodeToString(sum[:]), second.Identity.CodeSHA256)
+	assert.Equal(t, f.codePath, second.Code)
+}

spinifex/runtime/compute/vm/vm.go

@@ -387,7 +387,7 @@ type Device struct {
 }
 
 // Drive is one QEMU -drive entry, used for the pflash firmware drives (the
-// CODE blob and the EFI volume). Each set field becomes a key=value option.
+// CODE blob and the variable store). Each set field becomes a key=value option.
 type Drive struct {
 	File   string `json:"file"`
 	Format string `json:"format"`
@@ -396,7 +396,7 @@ type Drive struct {
 	ID     string `json:"id"`
 	Cache  string `json:"cache,omitempty"`
 	// Unit selects the pflash slot when If=="pflash": 0 is the CODE blob,
-	// 1 is the per-VM VARS volume. Ignored for non-pflash drives.
+	// 1 is the per-VM variable store. Ignored for non-pflash drives.
 	Unit int `json:"unit,omitempty"`
 	// ReadOnly emits readonly=on; used for the pflash CODE blob.
 	ReadOnly bool `json:"readonly,omitempty"`
@@ -588,8 +588,8 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
 			return nil, fwErr
 		}
 		// pflash CODE must come before VARS so QEMU loads firmware into the
-		// readonly slot first; buildDrives emits the VARS drive (unit 1) from
-		// the per-VM EFI viperblock volume.
+		// readonly slot first; startQEMU emits the VARS drive (unit 1) from
+		// the node's copy of the variable store or the EFI volume.
 		drives = append([]Drive{{If: "pflash", Format: "raw", Unit: 0, ReadOnly: true, File: codePath}}, drives...)
 	}
 

spinifex/runtime/daemon/daemon.go

@@ -152,6 +152,7 @@ type Daemon struct {
 	natsConn          *nats.Conn
 	resourceMgr       *ResourceManager
 	instanceService   *ec2instance.InstanceServiceImpl
+	instanceObjects   objectstore.ObjectStore
 	dnsWriter         *dns.Writer
 	dnsReconciler     *dns.Reconciler
 	dnsBaseDomain     string
@@ -2017,6 +2018,7 @@ func (d *Daemon) startCluster() error {
 
 	// Create services before loading/launching instances, since LaunchInstance depends on them
 	store := objectstore.NewS3ObjectStoreFromConfig(netaddr.DialTarget(d.config.Predastore.Host), d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey)
+	d.instanceObjects = store
 	d.instanceService = ec2instance.NewInstanceServiceImpl(d.config, d.resourceMgr.instanceTypes, d.natsConn, store, d.vmMgr, d.resourceMgr, d.jsManager)
 	d.dnsWriter = dns.NewWriter(d.config, d.clusterConfig, d.natsConn)
 	d.dnsReconciler = dns.NewReconciler(d.config, d.clusterConfig, d.natsConn, d.dnsWriter, d.dnsDesiredSet, d.dnsWatchSources()...)
@@ -2211,9 +2213,6 @@ func (d *Daemon) startCluster() error {
 	}
 
 	d.instanceService.SetTerminationDeps(d.volumeService, d.vpcService, ipReleaser, d.tagsService)
-	if d.jsManager != nil && d.jsManager.UEFIStores() != nil {
-		d.instanceService.SetUEFIStores(d.jsManager.UEFIStores())
-	}
 	d.instanceService.SetVolumeCreator(d.volumeService)
 	d.instanceService.SetRunInstancesDeps(d.imageService, d.keyService, &daemonENICreator{d: d}, ipAllocator)
 
@@ -2584,9 +2583,6 @@ func (d *Daemon) startCluster() error {
 		if gpuRec := d.newGPUPoolReconciler(); gpuRec != nil {
 			reapers = append(reapers, gpuRec)
 		}
-		if uefiStores := d.newUEFIStoreReaper(); uefiStores != nil {
-			reapers = append(reapers, uefiStores)
-		}
 		if d.volumeService != nil {
 			reapers = append(reapers, d.volumeService.NewVolumeLeakReaper(d.leakedVolumeInstances))
 			reapers = append(reapers, d.volumeService.NewVolumeReclaimReaper())

spinifex/runtime/daemon/instance_records.go

@@ -3,13 +3,10 @@ package daemon
 import (
 	"context"
 	"errors"
-	"fmt"
 	"log/slog"
-	"strings"
 
 	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
 	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
-	"github.com/nats-io/nats.go/jetstream"
 )
 
 // InstanceRecordPrefix is the key prefix for the per-resource instance record
@@ -218,99 +215,3 @@ func loadRecord[T any](store *kvstore.Store[T], key string) (*T, error) {
 	}
 	return record, nil
 }
-
-// TerminateDecided reports whether instanceID is terminated or terminating, or
-// has no instance record at all, read from the KV store at call time. A read
-// error is returned rather than guessed, so a partition never reads as absent.
-func (m *JetStreamManager) TerminateDecided(instanceID string) (bool, error) {
-	terminated, err := m.LoadTerminatedInstance(instanceID)
-	if err != nil {
-		return false, err
-	}
-	if terminated != nil {
-		return true, nil
-	}
-	record, err := m.LoadInstanceRecord(instanceID)
-	if err != nil {
-		return false, err
-	}
-	if record == nil {
-		return true, nil
-	}
-	switch record.Status.Status {
-	case vm.StateShuttingDown, vm.StateTerminated:
-		return true, nil
-	}
-	return false, nil
-}
-
-// OpenTeardowns lists the terminated instances whose dependency entry dep is
-// recorded and not yet done. A record that has expired is not listed, and a
-// failed listing is returned whole rather than as a partial set.
-func (m *JetStreamManager) OpenTeardowns(dep string) ([]string, error) {
-	terminated, err := m.ListTerminatedInstances()
-	if err != nil {
-		return nil, err
-	}
-	var ids []string
-	for _, v := range terminated {
-		state, ok := v.Teardown[dep]
-		if ok && vm.TeardownState(state) != vm.TeardownDone && v.Status == vm.StateTerminated {
-			ids = append(ids, v.ID)
-		}
-	}
-	return ids, nil
-}
-
-// MarkTeardownDone sets the terminated record's dependency entry dep done. An
-// absent or expired record is not an error: there is no entry left to close.
-func (m *JetStreamManager) MarkTeardownDone(instanceID, dep string) error {
-	terminated, err := m.LoadTerminatedInstance(instanceID)
-	if err != nil {
-		return err
-	}
-	if terminated == nil || vm.TeardownState(terminated.Teardown[dep]) == vm.TeardownDone {
-		return nil
-	}
-	_, err = m.UpdateTerminatedInstance(instanceID, func(v *vm.VM) {
-		if v.Teardown == nil {
-			v.Teardown = make(map[string]string)
-		}
-		v.Teardown[dep] = string(vm.TeardownDone)
-	})
-	if errors.Is(err, kvstore.ErrNotFound) {
-		return nil
-	}
-	return err
-}
-
-// KnownInstanceIDs lists every instance ID the instance-state and terminated
-// buckets hold a record for, in any key space, by key alone so an undecodable
-// record still counts. It attaches to the buckets and fails if one is missing.
-func KnownInstanceIDs(ctx context.Context, js jetstream.JetStream) (map[string]bool, error) {
-	ids := make(map[string]bool)
-	buckets := map[string][]string{
-		InstanceStateBucket:      {InstanceRecordPrefix, StoppedInstancePrefix},
-		TerminatedInstanceBucket: {InstanceRecordPrefix, TerminatedInstancePrefix},
-	}
-	for name, prefixes := range buckets {
-		kv, err := js.KeyValue(ctx, name)
-		if err != nil {
-			return nil, fmt.Errorf("open %s: %w", name, err)
-		}
-		lister, err := kv.ListKeys(ctx)
-		if err != nil {
-			return nil, fmt.Errorf("list %s: %w", name, err)
-		}
-		for key := range lister.Keys() {
-			for _, prefix := range prefixes {
-				if id, ok := strings.CutPrefix(key, prefix); ok {
-					ids[id] = true
-				}
-			}
-		}
-		// The lister closes itself once drained, so Stop only reports that.
-		_ = lister.Stop()
-	}
-	return ids, nil
-}

spinifex/runtime/daemon/local_state.go

@@ -42,10 +42,16 @@ type LocalState struct {
 // LocalStatePath returns the absolute path to the per-node instance state file.
 // Empty dataDir falls back to DefaultDataDir.
 func LocalStatePath(dataDir string) string {
+	return filepath.Join(DataDirOrDefault(dataDir), DefaultLocalStateDir, LocalStateFileName)
+}
+
+// DataDirOrDefault is the daemon's persistent data directory: dataDir, or
+// DefaultDataDir when it is empty.
+func DataDirOrDefault(dataDir string) string {
 	if dataDir == "" {
-		dataDir = DefaultDataDir
+		return DefaultDataDir
 	}
-	return filepath.Join(dataDir, DefaultLocalStateDir, LocalStateFileName)
+	return dataDir
 }
 
 // MarshalLocalState produces the JSON wire form of vms with the lowest schema

spinifex/runtime/daemon/uefi_store_reaper.go

@@ -1,212 +0,0 @@
-package daemon
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"log/slog"
-	"os"
-	"path/filepath"
-	"strings"
-	"time"
-
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/netaddr"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/telemetry"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
-)
-
-// uefiStoreKVTimeout bounds the one record read and write a terminate makes.
-const uefiStoreKVTimeout = 10 * time.Second
-
-// uefiLocalDirName is the directory under the daemon's data directory that
-// holds each instance's local copy of its variable store.
-const uefiLocalDirName = "uefi"
-
-// uefiLocalDir is the directory holding instanceID's local variable store.
-func uefiLocalDir(dataDir, instanceID string) string {
-	if dataDir == "" {
-		dataDir = DefaultDataDir
-	}
-	return filepath.Join(dataDir, uefiLocalDirName, instanceID)
-}
-
-// uefiStoreTerminator satisfies vm.UEFIStoreTerminator over the store-record
-// bucket and this node's data directory.
-type uefiStoreTerminator struct {
-	stores  *uefistore.Store
-	dataDir string
-}
-
-var _ vm.UEFIStoreTerminator = (*uefiStoreTerminator)(nil)
-
-// newUEFIStoreTerminator returns nil, not a typed nil, when the bucket is not
-// open, so the manager sees no terminator rather than one that panics.
-func (d *Daemon) newUEFIStoreTerminator() vm.UEFIStoreTerminator {
-	if d.jsManager == nil || d.jsManager.UEFIStores() == nil {
-		return nil
-	}
-	return &uefiStoreTerminator{stores: d.jsManager.UEFIStores(), dataDir: d.config.DataDir}
-}
-
-func (t *uefiStoreTerminator) MarkUEFIStoreDeleting(instanceID string) (bool, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), uefiStoreKVTimeout)
-	defer cancel()
-	return t.stores.MarkDeleting(ctx, instanceID)
-}
-
-// RemoveLocalUEFIStore refuses anything but a bare instance ID, so a malformed
-// ID cannot point the removal outside the instance's own directory.
-func (t *uefiStoreTerminator) RemoveLocalUEFIStore(instanceID string) error {
-	if !strings.HasPrefix(instanceID, "i-") || strings.ContainsAny(instanceID, `/\`) || strings.Contains(instanceID, "..") {
-		return fmt.Errorf("refusing to remove a local UEFI store for %q", instanceID)
-	}
-	return os.RemoveAll(uefiLocalDir(t.dataDir, instanceID))
-}
-
-// uefiInstanceRecords is what the store reaper asks of the instance domain:
-// whether a terminate is decided, and the terminated records' uefi entries.
-type uefiInstanceRecords interface {
-	TerminateDecided(instanceID string) (bool, error)
-	OpenTeardowns(dep string) ([]string, error)
-	MarkTeardownDone(instanceID, dep string) error
-}
-
-var _ uefiInstanceRecords = (*JetStreamManager)(nil)
-
-// uefiStoreReaper deletes the variable stores whose record is deleting: each
-// object under the instance's prefix, then the record. Terminate, or an
-// operator's forget, sets deleting and returns, so this decides nothing.
-//
-// It also completes a terminate whose mark failed, from the terminated
-// record's pending or failed uefi entry. That record is the terminate itself,
-// so moving the store record to deleting there is still not the reaper's call.
-//
-// Cluster-wide: the store outlives the node that ran the instance, and one
-// sweeper keeps two deletes off one prefix.
-type uefiStoreReaper struct {
-	stores    *uefistore.Store
-	objects   objectstore.ObjectStore
-	bucket    string
-	instances uefiInstanceRecords
-}
-
-var _ vm.Reaper = (*uefiStoreReaper)(nil)
-
-// newUEFIStoreReaper builds the reaper, or nil when this node has no store
-// bucket or object store to reap with.
-func (d *Daemon) newUEFIStoreReaper() *uefiStoreReaper {
-	if d.jsManager == nil || d.jsManager.UEFIStores() == nil || d.config == nil || d.config.Predastore.Bucket == "" {
-		return nil
-	}
-	return &uefiStoreReaper{
-		stores: d.jsManager.UEFIStores(),
-		objects: objectstore.NewS3ObjectStoreFromConfig(netaddr.DialTarget(d.config.Predastore.Host),
-			d.config.Predastore.Region, d.config.Predastore.AccessKey, d.config.Predastore.SecretKey),
-		bucket:    d.config.Predastore.Bucket,
-		instances: d.jsManager,
-	}
-}
-
-func (r *uefiStoreReaper) Class() string         { return "uefi-store" }
-func (r *uefiStoreReaper) Scope() vm.ReaperScope { return vm.ScopeClusterWide }
-
-// Sweep deletes every deleting store it can and reports how many records it
-// retired. A store whose delete fails keeps its record, which has no TTL, and
-// is retried next sweep however long predastore is away.
-func (r *uefiStoreReaper) Sweep(ctx context.Context) (int, error) {
-	r.completeTerminatedMarks(ctx)
-
-	ids, err := r.stores.InstanceIDs(ctx)
-	if err != nil {
-		return 0, fmt.Errorf("list UEFI store records: %w", err)
-	}
-	reaped := 0
-	for _, id := range ids {
-		if err := ctx.Err(); err != nil {
-			return reaped, err
-		}
-		if r.reap(ctx, id) {
-			reaped++
-		}
-	}
-	return reaped, nil
-}
-
-// reap deletes one store if its record is deleting, and reports whether the
-// record was retired.
-func (r *uefiStoreReaper) reap(ctx context.Context, id string) bool {
-	rec, rev, err := r.stores.Get(ctx, id)
-	if err != nil {
-		if !errors.Is(err, kvstore.ErrNotFound) {
-			slog.WarnContext(ctx, "uefi-store: unreadable store record, leaving it", "instanceId", id, "err", err)
-		}
-		return false
-	}
-	if rec.State != uefistore.StateDeleting {
-		return false
-	}
-
-	ok, err := r.instances.TerminateDecided(id)
-	if err != nil {
-		slog.WarnContext(ctx, "uefi-store: cannot read the instance record, retrying next sweep", "instanceId", id, "err", err)
-		return false
-	}
-	if !ok {
-		// Deleting is only ever set by a decided terminate or a forget, so a
-		// live instance here is a defect; its store is kept for an operator.
-		slog.ErrorContext(ctx, "uefi-store: refusing to delete the store of an instance that is not terminating",
-			"instanceId", id, "accountId", rec.AccountID)
-		return false
-	}
-
-	start := time.Now()
-	deleted, err := uefistore.DeleteObjects(ctx, r.objects, r.bucket, id)
-	if err != nil {
-		slog.WarnContext(ctx, "uefi-store: delete incomplete, retrying next sweep",
-			"instanceId", id, "deleted", deleted, "err", err)
-		return false
-	}
-	if err := r.instances.MarkTeardownDone(id, vm.TeardownUEFI); err != nil {
-		slog.WarnContext(ctx, "uefi-store: failed to mark the terminated record, retrying next sweep", "instanceId", id, "err", err)
-		return false
-	}
-	// Removed last: while it exists the obligation is readable, so a sweep
-	// interrupted above resumes rather than leaving objects nobody owns.
-	if err := r.stores.CompareAndDelete(ctx, id, rev); err != nil {
-		slog.WarnContext(ctx, "uefi-store: failed to remove the store record, retrying next sweep", "instanceId", id, "err", err)
-		return false
-	}
-	slog.InfoContext(ctx, "uefi-store: store deleted", "instanceId", id, "accountId", rec.AccountID,
-		"objects", deleted, "elapsed_ms", otelsetup.Millis(time.Since(start)))
-	return true
-}
-
-// completeTerminatedMarks finishes the mark of each terminate that recorded a
-// uefi entry but did not land its store record in deleting.
-func (r *uefiStoreReaper) completeTerminatedMarks(ctx context.Context) {
-	ids, err := r.instances.OpenTeardowns(vm.TeardownUEFI)
-	if err != nil {
-		slog.WarnContext(ctx, "uefi-store: cannot list terminated instances", "err", err)
-		return
-	}
-	for _, id := range ids {
-		rec, _, err := r.stores.Get(ctx, id)
-		switch {
-		case errors.Is(err, kvstore.ErrNotFound):
-			// Nothing left to delete: the store is gone or never existed.
-			if err := r.instances.MarkTeardownDone(id, vm.TeardownUEFI); err != nil {
-				slog.WarnContext(ctx, "uefi-store: failed to mark the terminated record", "instanceId", id, "err", err)
-			}
-		case err != nil:
-			slog.WarnContext(ctx, "uefi-store: unreadable store record", "instanceId", id, "err", err)
-		case rec.State == uefistore.StateLive:
-			if _, err := r.stores.MarkDeleting(ctx, id); err != nil {
-				slog.WarnContext(ctx, "uefi-store: failed to record a terminated instance's store for deletion",
-					"instanceId", id, "err", err)
-			}
-		}
-	}
-}

spinifex/runtime/daemon/uefi_store_reaper_test.go

@@ -1,255 +0,0 @@
-//test:in-package — the reaper and the terminator adapter are unexported daemon types
-package daemon
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"os"
-	"path/filepath"
-	"testing"
-
-	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/aws/aws-sdk-go/service/s3"
-	"github.com/mulgadc/spinifex/spinifex/domains/ec2/instance/uefistore"
-	"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
-	"github.com/mulgadc/spinifex/spinifex/providers/objectstore"
-	"github.com/mulgadc/spinifex/spinifex/runtime/compute/vm"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-const uefiTestBucket = "spinifex"
-
-type uefiReaperFixture struct {
-	m       *JetStreamManager
-	objects *objectstore.MemoryObjectStore
-	r       *uefiStoreReaper
-}
-
-func newUEFIReaperFixture(t *testing.T) *uefiReaperFixture {
-	t.Helper()
-	m := newRecordManagerInPackage(t)
-	require.NoError(t, m.InitUEFIStoreBucket())
-	objects := objectstore.NewMemoryObjectStore()
-	return &uefiReaperFixture{
-		m:       m,
-		objects: objects,
-		r:       &uefiStoreReaper{stores: m.UEFIStores(), objects: objects, bucket: uefiTestBucket, instances: m},
-	}
-}
-
-// seedStore writes a live store record and n objects under its prefix, the
-// state a launch leaves, then marks it deleting if asked, as terminate does.
-func (f *uefiReaperFixture) seedStore(t *testing.T, id string, n int, deleting bool) {
-	t.Helper()
-	_, err := f.m.UEFIStores().Create(t.Context(), &uefistore.UEFIStoreRecord{InstanceID: id, AccountID: "111122223333", State: uefistore.StateLive})
-	require.NoError(t, err)
-	for i := range n {
-		_, err := f.objects.PutObject(t.Context(), &s3.PutObjectInput{
-			Bucket: aws.String(uefiTestBucket),
-			Key:    aws.String(uefistore.KeyPrefix(id) + string(rune('a'+i))),
-			Body:   bytes.NewReader([]byte("vars")),
-		})
-		require.NoError(t, err)
-	}
-	if deleting {
-		_, err := f.m.UEFIStores().MarkDeleting(t.Context(), id)
-		require.NoError(t, err)
-	}
-}
-
-func (f *uefiReaperFixture) writeInstance(t *testing.T, id string, status vm.InstanceState) {
-	t.Helper()
-	v := &vm.VM{ID: id, Status: status, AccountID: "111122223333", Instance: &ec2.Instance{InstanceId: aws.String(id)}}
-	require.NoError(t, f.m.WriteInstanceRecord(id, v.Record()))
-}
-
-func (f *uefiReaperFixture) writeTerminated(t *testing.T, id string, uefi vm.TeardownState) {
-	t.Helper()
-	v := &vm.VM{ID: id, Status: vm.StateTerminated, AccountID: "111122223333", Instance: &ec2.Instance{InstanceId: aws.String(id)},
-		Teardown: map[string]string{vm.TeardownUEFI: string(uefi)}}
-	require.NoError(t, f.m.WriteTerminatedInstance(id, v))
-}
-
-func (f *uefiReaperFixture) objectCount(t *testing.T, id string) int {
-	t.Helper()
-	keys, err := uefistore.ListObjects(t.Context(), f.objects, uefiTestBucket, id)
-	require.NoError(t, err)
-	return len(keys)
-}
-
-func (f *uefiReaperFixture) recordState(t *testing.T, id string) string {
-	t.Helper()
-	rec, _, err := f.m.UEFIStores().Get(t.Context(), id)
-	if errors.Is(err, kvstore.ErrNotFound) {
-		return "absent"
-	}
-	require.NoError(t, err)
-	return rec.State
-}
-
-func (f *uefiReaperFixture) terminatedUEFIMark(t *testing.T, id string) string {
-	t.Helper()
-	v, err := f.m.LoadTerminatedInstance(id)
-	require.NoError(t, err)
-	require.NotNil(t, v)
-	return v.Teardown[vm.TeardownUEFI]
-}
-
-// TestS7_UEFIStoreReaperDeletesOnlyAStoreATerminateMarkedDeleting enforces
-// ADR-0003:S7 for the store reaper: it deletes the objects and record of a
-// store a terminate set deleting, and never a live store, whether its
-// instance runs, is stopped, or has no record the reaper could judge it by.
-func TestS7_UEFIStoreReaperDeletesOnlyAStoreATerminateMarkedDeleting(t *testing.T) {
-	f := newUEFIReaperFixture(t)
-	const (
-		running     = "i-0000000000000running"
-		stopped     = "i-0000000000000stopped"
-		unowned     = "i-0000000000000unowned"
-		terminated  = "i-000000000terminated"
-		terminating = "i-00000000terminating"
-	)
-	f.writeInstance(t, running, vm.StateRunning)
-	f.seedStore(t, running, 3, false)
-	f.writeInstance(t, stopped, vm.StateStopped)
-	f.seedStore(t, stopped, 3, false)
-	f.seedStore(t, unowned, 3, false)
-	f.writeTerminated(t, terminated, vm.TeardownPending)
-	f.seedStore(t, terminated, 3, true)
-	f.writeInstance(t, terminating, vm.StateShuttingDown)
-	f.seedStore(t, terminating, 2, true)
-
-	reaped, err := f.r.Sweep(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, 2, reaped)
-
-	for _, id := range []string{running, stopped, unowned} {
-		assert.Equalf(t, 3, f.objectCount(t, id), "ADR-0003:S7: the reaper deleted objects of the live store %s", id)
-		assert.Equalf(t, uefistore.StateLive, f.recordState(t, id), "ADR-0003:S7: the reaper changed the live store record %s", id)
-	}
-	for _, id := range []string{terminated, terminating} {
-		assert.Zerof(t, f.objectCount(t, id), "a store a terminate marked deleting must be deleted: %s", id)
-		assert.Equalf(t, "absent", f.recordState(t, id), "the record goes once its prefix is gone: %s", id)
-	}
-	assert.Equal(t, string(vm.TeardownDone), f.terminatedUEFIMark(t, terminated))
-}
-
-// TestS7_UEFIStoreReaperRefusesADeletingStoreOfALiveInstance enforces
-// ADR-0003:S7 when the record and the instance disagree: a deleting record
-// whose instance still runs or is stopped is kept, objects and all.
-func TestS7_UEFIStoreReaperRefusesADeletingStoreOfALiveInstance(t *testing.T) {
-	for _, status := range []vm.InstanceState{vm.StateRunning, vm.StateStopped, vm.StatePending} {
-		t.Run(string(status), func(t *testing.T) {
-			f := newUEFIReaperFixture(t)
-			const id = "i-0000000000000000a"
-			f.writeInstance(t, id, status)
-			f.seedStore(t, id, 3, true)
-
-			reaped, err := f.r.Sweep(t.Context())
-			require.NoError(t, err)
-			assert.Zero(t, reaped)
-			assert.Equal(t, 3, f.objectCount(t, id), "ADR-0003:S7: a live instance's store must survive a deleting record")
-			assert.Equal(t, uefistore.StateDeleting, f.recordState(t, id))
-		})
-	}
-}
-
-// failingObjects stands in for predastore while it is down: every list and
-// delete fails.
-type failingObjects struct {
-	objectstore.ObjectStore
-}
-
-func (failingObjects) ListObjectsV2(context.Context, *s3.ListObjectsV2Input) (*s3.ListObjectsV2Output, error) {
-	return nil, errors.New("predastore: connection refused")
-}
-
-func (failingObjects) DeleteObject(context.Context, *s3.DeleteObjectInput) (*s3.DeleteObjectOutput, error) {
-	return nil, errors.New("predastore: connection refused")
-}
-
-// The obligation lives in the store record, which has no TTL, so a terminate
-// still completes when predastore is down for longer than the terminated
-// record lives.
-func TestUEFIStoreReaper_CompletesAfterTheTerminatedRecordExpires(t *testing.T) {
-	f := newUEFIReaperFixture(t)
-	const id = "i-0000000000000000a"
-	f.writeTerminated(t, id, vm.TeardownPending)
-	f.seedStore(t, id, 3, true)
-
-	f.r.objects = failingObjects{f.objects}
-	reaped, err := f.r.Sweep(t.Context())
-	require.NoError(t, err)
-	assert.Zero(t, reaped)
-	assert.Equal(t, uefistore.StateDeleting, f.recordState(t, id), "a failed delete keeps the obligation")
-	assert.Equal(t, string(vm.TeardownPending), f.terminatedUEFIMark(t, id))
-
-	// The bucket's TTL expires the terminated record while predastore is away.
-	require.NoError(t, f.m.DeleteTerminatedInstance(id))
-	f.r.objects = f.objects
-
-	reaped, err = f.r.Sweep(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, 1, reaped)
-	assert.Zero(t, f.objectCount(t, id))
-	assert.Equal(t, "absent", f.recordState(t, id))
-}
-
-// A terminate whose mark failed left the store live and the terminated
-// record's entry failed; the reaper completes that terminate.
-func TestUEFIStoreReaper_CompletesATerminateWhoseMarkFailed(t *testing.T) {
-	f := newUEFIReaperFixture(t)
-	const id = "i-0000000000000000a"
-	f.seedStore(t, id, 3, false)
-	f.writeTerminated(t, id, vm.TeardownFailed)
-
-	reaped, err := f.r.Sweep(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, 1, reaped)
-	assert.Zero(t, f.objectCount(t, id))
-	assert.Equal(t, "absent", f.recordState(t, id))
-	assert.Equal(t, string(vm.TeardownDone), f.terminatedUEFIMark(t, id))
-}
-
-// A pending entry whose store record is already gone has nothing left to
-// delete, so the entry is closed and the terminated record can be purged.
-func TestUEFIStoreReaper_ClosesAPendingEntryWithNoStore(t *testing.T) {
-	f := newUEFIReaperFixture(t)
-	const id = "i-0000000000000000a"
-	f.writeTerminated(t, id, vm.TeardownPending)
-
-	_, err := f.r.Sweep(t.Context())
-	require.NoError(t, err)
-	assert.Equal(t, string(vm.TeardownDone), f.terminatedUEFIMark(t, id))
-}
-
-func TestUEFIStoreTerminator_MarksTheRecordAndRemovesOnlyTheInstanceDirectory(t *testing.T) {
-	f := newUEFIReaperFixture(t)
-	const id = "i-0000000000000000a"
-	f.seedStore(t, id, 1, false)
-	dataDir := t.TempDir()
-	mine := uefiLocalDir(dataDir, id)
-	other := uefiLocalDir(dataDir, "i-0000000000000000b")
-	for _, dir := range []string{mine, other} {
-		require.NoError(t, os.MkdirAll(dir, 0o750))
-		require.NoError(t, os.WriteFile(filepath.Join(dir, "vars.fd"), []byte("vars"), 0o600))
-	}
-	term := &uefiStoreTerminator{stores: f.m.UEFIStores(), dataDir: dataDir}
-
-	has, err := term.MarkUEFIStoreDeleting(id)
-	require.NoError(t, err)
-	assert.True(t, has)
-	assert.Equal(t, uefistore.StateDeleting, f.recordState(t, id))
-
-	require.NoError(t, term.RemoveLocalUEFIStore(id))
-	assert.NoDirExists(t, mine)
-	assert.DirExists(t, other)
-	require.NoError(t, term.RemoveLocalUEFIStore(id), "removal is idempotent")
-
-	for _, bad := range []string{"", "..", "i-0/../..", "i-0000000000000000b/vars.fd", "vol-0123"} {
-		require.Errorf(t, term.RemoveLocalUEFIStore(bad), "%q", bad)
-	}
-	assert.FileExists(t, filepath.Join(other, "vars.fd"))
-}

spinifex/runtime/daemon/vm_adapters.go

@@ -679,7 +679,6 @@ func (d *Daemon) buildVMManagerDeps() vm.Deps {
 		Resources:          newResourceControllerAdapter(d.resourceMgr),
 		VolumeStateUpdater: d.volumeService,
 		InstanceCleaner:    newInstanceCleanerAdapter(d),
-		UEFIStores:         d.newUEFIStoreTerminator(),
 		Hooks: vm.ManagerHooks{
 			OnInstanceUp:           d.onInstanceUpHook(),
 			OnInstanceDown:         d.onInstanceDownHook(),
@@ -697,6 +696,11 @@ func (d *Daemon) buildVMManagerDeps() vm.Deps {
 		DeviceDeletedTimeout:       d.deviceDeletedTimeout,
 		ConsumeCleanShutdownMarker: d.consumeCleanShutdownMarker(),
 		BackingStoreReady:          func() bool { return d.checkPredastoreReady() && d.checkViperblockReady() },
+		UEFIVarStore: vm.UEFIVarStoreDeps{
+			Objects: d.instanceObjects,
+			Bucket:  d.config.Predastore.Bucket,
+			DataDir: DataDirOrDefault(d.config.DataDir),
+		},
 	}
 }
 

spinifex/runtime/daemon/vm_adapters_coverage_test.go

@@ -190,6 +190,24 @@ func TestBuildVMManagerDeps_BackingStoreReady(t *testing.T) {
 		"no predastore configured and JetStream answering is ready")
 }
 
+// Variable stores live in the instance domain's bucket, read through the
+// same object store, with the live copy under the persistent data directory.
+func TestBuildVMManagerDeps_UEFIVarStore(t *testing.T) {
+	objects := objectstore.NewMemoryObjectStore()
+	d := &Daemon{
+		config:          &config.Config{DataDir: "/srv/spinifex", Predastore: config.PredastoreConfig{Bucket: "spinifex-instances"}},
+		vmMgr:           vm.NewManager(),
+		instanceObjects: objects,
+	}
+	got := d.buildVMManagerDeps().UEFIVarStore
+	assert.Same(t, objects, got.Objects)
+	assert.Equal(t, "spinifex-instances", got.Bucket)
+	assert.Equal(t, "/srv/spinifex", got.DataDir)
+
+	d.config.DataDir = ""
+	assert.Equal(t, DefaultDataDir, d.buildVMManagerDeps().UEFIVarStore.DataDir)
+}
+
 // --- volumeMounterAdapter ---
 
 func TestVolumeMounterAdapter_Mount_ClosedConnIsNotRetryable(t *testing.T) {