Compare commits

..

5 Commits

Author SHA1 Message Date
brynmailer-mdc 2961d73d03 test(compute): cover an unnamed boot device with no medium in the device map 2026-11-02 14:25:25 +10:00
brynmailer-mdc 042666b6ad refactor(compute): share the NBD node options and export dispatch
The data and boot -blockdev builders share one helper for the nbd server,
export and reconnect-delay options, buildDrives dispatches both branches
through one export helper, and hot attach parses its URI with the shared
parser. Emitted arguments are unchanged. The BlkDeviceMapped doc now states
when it falls back to the scalar form.
2026-11-02 14:25:25 +10:00
brynmailer-mdc 3a19e76f6b fix(compute): refuse a disk-boot launch with no boot volume
Data-volume -blockdev nodes satisfy Execute's drive check on their own, so
a disk-boot request list without a boot volume would start a QEMU with
nothing to boot. The disk-boot path now refuses it before launch.
2026-11-02 14:24:18 +10:00
brynmailer-mdc 03cb800f4c test(viperblock): pin the boot -blockdev shape in the nbdkit orphan reaper
The boot volume's NBD server options now carry a file. prefix, which the
referencer scan matches only because it compares by substring. Cover the
unix and TCP boot strings so a stricter match cannot reap a live root
export, and stop describing the joined host:port form as a boot drive.
2026-11-02 14:23:24 +10:00
brynmailer-mdc 6aa989e1ec feat(compute): boot the root volume from a -blockdev node that takes NBD or host_device
The boot volume moves from -drive if=none to a -blockdev node named os, so
a block export reaches QEMU as host_device as data volumes already do. The
NBD boot shape keeps its cache, reconnect, iothread and bootindex settings;
werror and rerror move onto the virtio-blk device. A block EFI volume is
still refused because pflash has no host_device form.
2026-11-02 14:14:07 +10:00
20 changed files with 341 additions and 669 deletions
+3 -33
View File
@@ -5,7 +5,6 @@ package config
import (
"fmt"
"log/slog"
"maps"
"net/netip"
"os"
"path/filepath"
@@ -356,11 +355,6 @@ type EBSConfig struct {
CacheSizeMB *int `json:"CacheSizeMB" mapstructure:"cache_size_mb"`
}
// SupportedEBSProvider reports whether name is a provider a node may run.
func SupportedEBSProvider(name string) bool {
return name == EBSProviderViperblockd || name == EBSProviderQEMUNBD
}
// ResolvedProvider normalizes an empty Provider to EBSProviderViperblockd,
// the default provider.
func (c EBSConfig) ResolvedProvider() string {
@@ -807,27 +801,6 @@ func validateVultrPool(cc *ClusterConfig, p ExternalPool) error {
return nil
}
// validateSingleEBSProvider refuses nodes whose explicit [nodes.<name>.ebs]
// providers differ. A peer stanza without one is unknown, not the default: the
// generated config omits it, and the cluster's KV lock is the authoritative check.
func validateSingleEBSProvider(cc *ClusterConfig) error {
var first, want string
for _, name := range slices.Sorted(maps.Keys(cc.Nodes)) {
if !viper.IsSet("nodes." + name + ".ebs.provider") {
continue
}
got := cc.Nodes[name].EBS.ResolvedProvider()
if first == "" {
first, want = name, got
continue
}
if got != want {
return fmt.Errorf("config: [nodes.%s.ebs] provider resolves to %q but [nodes.%s.ebs] resolves to %q; every node in a cluster must use the same EBS provider", first, want, name, got)
}
}
return nil
}
func validateClusterConfig(cc *ClusterConfig) error {
if viper.IsSet("network.external_dhcp") {
return fmt.Errorf("config: [network] external_dhcp is no longer supported; remove the key (static WAN-pool allocation only)")
@@ -836,9 +809,9 @@ func validateClusterConfig(cc *ClusterConfig) error {
if viper.IsSet("nodes." + nodeName + ".vpcd.dhcp_bind_bridge") {
return fmt.Errorf("config: [nodes.%s.vpcd] dhcp_bind_bridge is no longer supported; remove the key (vpcd no longer runs a DHCP client)", nodeName)
}
switch {
case nodeCfg.EBS.Provider == "", SupportedEBSProvider(nodeCfg.EBS.Provider):
case nodeCfg.EBS.Provider == EBSProviderEmbedded:
switch nodeCfg.EBS.Provider {
case "", EBSProviderViperblockd, EBSProviderQEMUNBD:
case EBSProviderEmbedded:
// Refuse rather than silently upgrade: the embedded engine is gone,
// and starting anyway would migrate this node's volumes to
// ebsmetadata one-way without the operator ever asking for it.
@@ -856,9 +829,6 @@ func validateClusterConfig(cc *ClusterConfig) error {
return fmt.Errorf("config: [nodes.%s.ebs] cache_size_mb=%d must not be negative; use 0 to disable the cache, or remove the key for the default of %d", nodeName, *c, DefaultCacheSizeMB)
}
}
if err := validateSingleEBSProvider(cc); err != nil {
return err
}
if err := validateNorthstarCluster(cc); err != nil {
return err
-39
View File
@@ -1108,45 +1108,6 @@ provider = "bogus"
assert.Contains(t, err.Error(), `provider="bogus"`)
}
func TestLoadConfig_EBSProvider_SingleProviderPerCluster(t *testing.T) {
stanza := func(node, provider string) string {
return fmt.Sprintf("[nodes.%s.ebs]\nprovider = %q\n", node, provider)
}
cases := []struct {
name string
ebs []string
wantErr string
}{
{"all unset", []string{"", "", ""}, ""},
{"explicit viperblockd beside unset peers", []string{"", stanza("n2", EBSProviderViperblockd), ""}, ""},
{"explicit qemunbd beside unset peers", []string{stanza("n1", EBSProviderQEMUNBD), "", ""}, ""},
{"all explicit qemunbd", []string{stanza("n1", EBSProviderQEMUNBD), stanza("n2", EBSProviderQEMUNBD), stanza("n3", EBSProviderQEMUNBD)}, ""},
{"two explicit providers differ", []string{stanza("n1", EBSProviderQEMUNBD), stanza("n2", EBSProviderViperblockd), ""},
`[nodes.n1.ebs] provider resolves to "qemunbd" but [nodes.n2.ebs] resolves to "viperblockd"`},
{"only the third node differs", []string{stanza("n1", EBSProviderViperblockd), stanza("n2", EBSProviderViperblockd), stanza("n3", EBSProviderQEMUNBD)},
`[nodes.n1.ebs] provider resolves to "viperblockd" but [nodes.n3.ebs] resolves to "qemunbd"`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
body := "node = \"n1\"\n"
for i, ebs := range tc.ebs {
body += fmt.Sprintf("\n[nodes.n%d]\nregion = \"us-east-1\"\n%s", i+1, ebs)
}
resetViper(t)
path := filepath.Join(t.TempDir(), "spinifex.toml")
require.NoError(t, os.WriteFile(path, []byte(body), 0600))
_, err := LoadConfig(path)
if tc.wantErr == "" {
require.NoError(t, err)
return
}
require.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErr)
})
}
}
func TestEBSConfig_ResolvedTunablesDefault(t *testing.T) {
var c EBSConfig
assert.Equal(t, DefaultNBDKitThreads, c.ResolvedThreads())
-28
View File
@@ -1748,31 +1748,3 @@ func TestApplyNetworkConfig_PropagatesPoolDNSServersWithoutSelf(t *testing.T) {
require.Len(t, settings.Pools, 1)
assert.Equal(t, []string{"1.1.1.1", "10.0.0.6", "8.8.8.8"}, settings.Pools[0].DNSServers)
}
// A generated config writes no [nodes.<peer>.ebs] stanza, so a node switched to
// a non-default provider must still load: an absent peer provider is unknown.
func TestSpinifexTomlTemplate_NonDefaultProviderLoadsBesidePeers(t *testing.T) {
allNodes := map[string]formation.NodeInfo{
"node1": {Name: "node1", BindIP: "10.0.0.1", AdvertiseIP: "192.168.1.31"},
"node2": {Name: "node2", BindIP: "10.0.0.2", AdvertiseIP: "192.168.1.32"},
"node3": {Name: "node3", BindIP: "10.0.0.3", AdvertiseIP: "192.168.1.33"},
}
settings := multinodeNorthstarSettings("/etc/spinifex/northstar/northstar.toml")
settings.RemoteNodes = buildRemoteNodes(allNodes, "node1", settings.NorthstarConfigPath)
require.Len(t, settings.RemoteNodes, 2)
path := filepath.Join(t.TempDir(), "spinifex.toml")
require.NoError(t, admin.GenerateConfigFile(path, spinifexTomlTemplate, settings))
rendered, err := os.ReadFile(path)
require.NoError(t, err)
const defaultLine = "provider = \"viperblockd\"\n"
require.Equal(t, 1, strings.Count(string(rendered), defaultLine), "the template must set the provider for its own node only")
switched := strings.Replace(string(rendered), defaultLine, "provider = \"qemunbd\"\n", 1)
require.NoError(t, os.WriteFile(path, []byte(switched), 0o600))
cfg, err := config.LoadConfig(path)
require.NoError(t, err)
assert.Equal(t, config.EBSProviderQEMUNBD, cfg.Nodes["node1"].EBS.ResolvedProvider())
assert.Contains(t, cfg.Nodes, "node2")
assert.Contains(t, cfg.Nodes, "node3")
}
@@ -136,9 +136,6 @@ encryption_key_file = "{{.EncryptionKeyFile}}"
{{- end}}
[nodes.{{.Node}}.ebs]
# provider: every node in a cluster must name the same one, and the daemon
# refuses to start on a provider other than the one the cluster recorded.
# Changing it needs the cluster wiped and reformed.
provider = "viperblockd"
# Per-volume export tunables. Both are applied per nbdkit process and there is
# one of those per volume, so the host cost of each is the value times the
-1
View File
@@ -21,7 +21,6 @@ Single source of truth for the current schema version of every config file Spini
|---|---|---|
| `spinifex-instance-state` | `5` | `daemon.InstanceStateBucketVersion` |
| `spinifex-cluster-state` | `1` | `daemon.ClusterStateBucketVersion` |
| `spinifex-cluster-config` | `1` | `daemon.ClusterConfigBucketVersion` |
| `spinifex-terminated-instances` | `3` | `daemon.TerminatedInstanceBucketVersion` |
## Registered migrations
@@ -363,11 +363,11 @@ func endpointReferencer(procRoot string, disc discoveredNbdkit) (pid int, found
// cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
// disc's endpoint. A socket mount's path appears verbatim in whichever
// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its
// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair
// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or
// split across server.host=/server.port= within the same comma-joined
// -blockdev argument, so both shapes are checked.
// argument named it (an EFI -drive's file= or a -blockdev's server.path=). A
// TCP mount's 127.0.0.1:<port> pair may appear joined in one argument (an EFI
// pflash drive's nbd://host:port) or split across server.host=/server.port=
// within one -blockdev argument. The match is by substring, so the boot
// volume's file.server.path=/file.server.host=/file.server.port= also match.
func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
@@ -415,7 +415,7 @@ func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
// TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
// endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
// splits host and port into server.host=/server.port= within one argument,
// not just the joined host:port form a boot/EFI drive uses.
// not just the joined host:port form an EFI pflash drive uses.
func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
@@ -428,6 +428,46 @@ func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *test
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a TCP endpoint split across server.host=/server.port= must still be recognized as referenced")
}
// TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning pins the boot
// volume's -blockdev, whose server options carry a file. prefix: a live
// guest's root nbdkit named only that way must never be signalled.
func TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning(t *testing.T) {
socket := filepath.Join(t.TempDir(), "vol-boot-root.sock")
tests := []struct {
name string
arg string
disc func(pid int) discoveredNbdkit
}{
{
name: "unix socket",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=" + socket + ",file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-unix1", Socket: socket}
},
},
{
name: "tcp",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=10897,file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-tcp1", Port: 10897}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
const referencerPID = 909092
writeReferencerFixture(t, procRoot, referencerPID, tt.arg)
logs := captureLogs(t)
assert.False(t, reapOrphanedNbdkit(procRoot, tt.disc(pid), reapReasonUnadoptable), "a boot volume's nbdkit must never be signalled")
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a boot volume's nbdkit must never be signalled")
assert.Contains(t, logs.String(), fmt.Sprintf("referencing_pid=%d", referencerPID))
})
}
}
// unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
// strips all permissions from the directory itself, so os.ReadFile on the
// cmdline path inside it fails with a permission error rather than
@@ -84,6 +84,10 @@ func buildDeviceMap(devices []qmp.BlockDevice) map[string]string {
name := dev.Device
pciIndex := extractPCIIndex(dev.QDev)
// A -blockdev boot node has no BlockBackend name; its node-name is "os".
if name == "" && pciIndex >= 0 && dev.Inserted != nil {
name = dev.Inserted.NodeName
}
if pciIndex < 0 {
// Hot-plugged device: extract name and port from QDev path.
@@ -127,6 +127,23 @@ func TestBuildDeviceMap(t *testing.T) {
},
wantLen: 4,
},
{
// A -blockdev boot node leaves the BlockBackend unnamed, so the
// entry is keyed by its node-name instead of "".
name: "blockdev boot device is named by its node-name",
devices: []qmp.BlockDevice{
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "os"}, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{NodeName: "#block123"}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "nbd-vol-abc"}, QDev: "/machine/peripheral/vdisk-vol-abc/virtio-backend"},
},
want: map[string]string{
"os": "/dev/vda",
"cloudinit": "/dev/vdb",
"vdisk-vol-abc": "/dev/vdc",
},
absent: []string{"", "nbd-vol-abc"},
wantLen: 3,
},
{
// Boot devices returned out of order: lowest PCI index wins /dev/vda;
// hot-plugged devices sort after boot devices.
@@ -426,3 +443,15 @@ func TestLogGuestDeviceMap_LeavesBlockDeviceMappingsAlone(t *testing.T) {
assert.Equal(t, "/dev/sdf", aws.StringValue(v.Instance.BlockDeviceMappings[1].DeviceName),
"a data mapping must still name the device the caller asked for, not /dev/vdb")
}
// TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium covers an unnamed
// boot-time BlockBackend that reports no inserted medium.
func TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium(t *testing.T) {
devices := []qmp.BlockDevice{
{Device: "", Inserted: nil, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
}
var got map[string]string
require.NotPanics(t, func() { got = buildDeviceMap(devices) })
assert.Equal(t, "/dev/vdb", got["cloudinit"], "the unnamed device still holds its PCI slot")
}
+61 -11
View File
@@ -6,6 +6,8 @@ import (
"slices"
"strconv"
"strings"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
)
// IsMicrovm reports whether machineType is the QEMU microvm machine, with or
@@ -162,12 +164,14 @@ type blkDeviceJSON struct {
NumQueues int `json:"num-queues,omitempty"`
BootIndex int `json:"bootindex,omitempty"`
Mapping []iothreadVQMapping `json:"iothread-vq-mapping"`
Werror string `json:"werror"`
Rerror string `json:"rerror"`
}
// BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues
// across a pool of poolSize IOThreads, round-robin. Falls back to BlkDevice's
// scalar iothread= form when poolSize <= 1, so the common small-guest case is
// byte-for-byte what it was before this existed.
// scalar iothread= form when poolSize <= 1, when there is one queue, or on an
// MMIO machine.
//
// Every queue is mapped exactly once and the scalar iothread= property is
// absent: QEMU treats the two as alternative assignments and rejects both.
@@ -197,6 +201,8 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
NumQueues: queues,
BootIndex: bootIdx,
Mapping: mapping,
Werror: "stop",
Rerror: "stop",
})
if err != nil {
return Device{}, fmt.Errorf("marshal virtio-blk iothread-vq-mapping: %w", err)
@@ -204,9 +210,9 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
return Device{Value: string(b)}, nil
}
// BlkDevice returns the appropriate QEMU virtio-blk device string for
// machineType. iothread and queues are omitted when empty/zero. bootIdx is
// only emitted for PCI machines (MMIO has no bootindex concept).
// BlkDevice returns the virtio-blk device string for machineType, omitting an
// empty iothread or zero queues; bootIdx is PCI-only. It sets werror/rerror=stop
// (see VolumeBlkDeviceArgs), as a -blockdev node has no BlockBackend to hold it.
func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device {
var b strings.Builder
if IsMMIO(machineType) {
@@ -224,6 +230,7 @@ func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Dev
if !IsMMIO(machineType) && bootIdx > 0 {
fmt.Fprintf(&b, ",bootindex=%d", bootIdx)
}
b.WriteString(",werror=stop,rerror=stop")
return Device{Value: b.String()}
}
@@ -282,9 +289,9 @@ func HotplugEBSBus(port int) string {
// (rendered as a joined command-line string) so the two block-graph shapes
// cannot diverge.
//
// werror/rerror mirror the boot drive's on-error policy (see Drive.Werror):
// a data volume has no -drive-backed BlockBackend to carry it, so the qdev
// device itself sets werror=stop,rerror=stop.
// werror=stop,rerror=stop pauses the VM and holds a failed request, so a
// transient backend loss is resumed with cont instead of reaching the guest as
// EIO. A -blockdev node has no BlockBackend to carry it, so the device sets it.
func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string {
return []string{
fmt.Sprintf("id=%s", VolumeDeviceID(volumeID)),
@@ -383,11 +390,54 @@ func (o NBDServerOpts) CommandLineArgs() []string {
// monitor-owned node that blockdev-del can later remove (unlike -drive,
// which only creates a BlockBackend with an auto-generated node-name).
func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, server.CommandLineArgs()...)
opts = append(opts, "export=", fmt.Sprintf("reconnect-delay=%d", NBDReconnectDelaySeconds))
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, nbdNodeOpts("", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// nbdNodeOpts returns an nbd node's server, export and reconnect-delay options,
// each prefixed with prefix: "" on the nbd node itself, "file." under raw.
func nbdNodeOpts(prefix string, server NBDServerOpts) []string {
var opts []string
for _, arg := range server.CommandLineArgs() {
opts = append(opts, prefix+arg)
}
return append(opts, prefix+"export=", fmt.Sprintf("%sreconnect-delay=%d", prefix, NBDReconnectDelaySeconds))
}
// bootNodeName is the boot volume's block-node name, which its -device names
// with drive=.
const bootNodeName = "os"
// nbdServerOpts parses an NBD URI into the server options a blockdev takes.
func nbdServerOpts(uri string) (NBDServerOpts, error) {
serverType, socketPath, host, port, err := nbd.ParseNBDURI(uri)
if err != nil {
return NBDServerOpts{}, err
}
return NBDServerOpts{Type: serverType, Path: socketPath, Host: host, Port: port}, nil
}
// bootNBDBlockdev renders the boot volume's -blockdev for an NBD export: a raw
// node over an nbd node, the graph -drive format=raw,cache=none built, with the
// same O_DIRECT setting and reconnect delay.
func bootNBDBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=raw", fmt.Sprintf("node-name=%s", nodeName), "cache.direct=on", "file.driver=nbd"}, nbdNodeOpts("file.", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// exportBlockdev renders v's -blockdev as nodeName: a host_device node for a
// block export, otherwise the NBD node nbdBlockdev builds from its URI.
func exportBlockdev(v EBSRequest, kind volumeExportKind, nodeName string, nbdBlockdev func(string, NBDServerOpts) Blockdev) (Blockdev, error) {
if kind == volumeExportBlock {
return VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes}), nil
}
server, err := nbdServerOpts(v.NBDURI)
if err != nil {
return Blockdev{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
return nbdBlockdev(nodeName, server), nil
}
// HostDeviceOpts is a host block device export. QEMU opens Path through the
// host_device protocol driver with O_DIRECT and native AIO, and a raw format
// node over it exposes only the first SizeBytes to the guest.
@@ -413,7 +463,7 @@ func (o HostDeviceOpts) QMPArgs(nodeName string) map[string]any {
}
// VolumeHostDeviceBlockdev renders the -blockdev equivalent of QMPArgs for a
// data volume backed by a host block device.
// volume backed by a host block device.
func VolumeHostDeviceBlockdev(nodeName string, dev HostDeviceOpts) Blockdev {
return Blockdev{Value: fmt.Sprintf(
"driver=raw,node-name=%s,size=%d,file.driver=host_device,file.filename=%s,file.aio=native,file.cache.direct=on",
+5 -5
View File
@@ -130,28 +130,28 @@ func TestNICQueues_ClampedWhenEnabled(t *testing.T) {
func TestBlkDevice_PCI(t *testing.T) {
d := BlkDevice("q35", "os", "ioth-os", 4, 1)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoBootIdx(t *testing.T) {
d := BlkDevice("q35", "data", "ioth-data", 2, 0)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoIOThread(t *testing.T) {
d := BlkDevice("q35", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-pci,drive=data", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO(t *testing.T) {
d := BlkDevice("microvm", "os", "ioth-os", 4, 1)
// MMIO has no bootindex
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4", d.Value)
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO_NoIOThread(t *testing.T) {
d := BlkDevice("microvm,x-option-roms=off", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-device,drive=data", d.Value)
assert.Equal(t, "virtio-blk-device,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestRngDevice_PCI(t *testing.T) {
+27 -42
View File
@@ -315,7 +315,7 @@ func (m *Manager) startQEMU(instance *VM, appendConsole bool) error {
m.initENIRequests(instance)
instance.EBSRequests.Mu.Lock()
driveCfg, err := buildDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
driveCfg, err := buildDiskBootDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
instance.EBSRequests.Mu.Unlock()
if err != nil {
return err
@@ -1398,16 +1398,24 @@ func directBootLaunchConfig(base Config, requests []EBSRequest, cpuCount int) (C
return cfg, nil
}
// buildDiskBootDrives is buildDrives for a guest that boots from a volume: a
// request list with no boot volume is refused, as QEMU would start with
// nothing to boot from and the guest would never come up.
func buildDiskBootDrives(requests []EBSRequest, cpuCount int, machineType string) (driveConfig, error) {
if !slices.ContainsFunc(requests, func(r EBSRequest) bool { return r.Boot }) {
return driveConfig{}, errors.New("no boot volume among the instance's EBS requests")
}
return buildDrives(requests, cpuCount, machineType)
}
// buildDrives converts EBS requests into QEMU drive/iothread/device/blockdev
// configs. Returns an error if any volume has no valid export (see
// classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while
// allocating one for a data volume.
//
// Boot volumes keep the existing anonymous if=none -drive shape exactly as
// before — they are not detachable (see ErrVolumeNotDetachable), so the
// anonymous-node problem this function otherwise fixes does not apply, and
// changing the boot drive risks boot order. EFI volumes emit pflash unit=1;
// the readonly CODE blob (unit=0) is added by Config.Execute.
// 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).
//
// 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
@@ -1432,41 +1440,29 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
}
switch {
case kind == volumeExportBlock && (v.Boot || v.EFI):
// Boot and EFI keep the legacy -drive shape, which has no
// host_device form here; only data volumes take a block export.
return driveConfig{}, fmt.Errorf("volume %s: block device export is only supported for data volumes", v.Name)
case kind == volumeExportBlock && v.EFI:
// The VARS store is a pflash -drive, which has no host_device form.
return driveConfig{}, fmt.Errorf("volume %s: block device export is not supported for an EFI volume", v.Name)
case v.Boot:
drive := Drive{
File: v.NBDURI,
Format: "raw",
If: "none",
Media: "disk",
ID: "os",
Cache: "none",
// Pause the VM and hold the failed request rather than
// delivering EIO to the guest, which aborts its journal. A
// transient backend loss is then recovered with cont.
Werror: "stop",
Rerror: "stop",
ReconnectDelay: NBDReconnectDelaySeconds,
blockdev, err := exportBlockdev(*v, kind, bootNodeName, bootNBDBlockdev)
if err != nil {
return driveConfig{}, err
}
// The boot disk's virtqueues are spread across a pool rather than
// all served by one host thread. A pool of 1 emits exactly the
// command line this used to, which is every guest below 4 vCPUs.
// all served by one host thread. A pool of 1 keeps the scalar
// iothread= form, which is every guest below 4 vCPUs.
pool := BlkIOThreadPoolSize(cpuCount)
for i := range pool {
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: BlkIOThreadID("ioth-os", i, pool)})
}
dev, err := BlkDeviceMapped(machineType, drive.ID, "ioth-os", cpuCount, 1, pool)
dev, err := BlkDeviceMapped(machineType, bootNodeName, "ioth-os", cpuCount, 1, pool)
if err != nil {
return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err)
}
cfg.Blockdevs = append(cfg.Blockdevs, blockdev)
cfg.Devices = append(cfg.Devices, dev)
cfg.Drives = append(cfg.Drives, drive)
case v.EFI:
cfg.Drives = append(cfg.Drives, Drive{
@@ -1495,20 +1491,9 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
nodeName := VolumeNodeName(v.Name)
iothreadID := VolumeIOThreadID(v.Name)
var blockdev Blockdev
if kind == volumeExportBlock {
blockdev = VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes})
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(v.NBDURI)
if err != nil {
return driveConfig{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
blockdev = VolumeBlockdev(nodeName, NBDServerOpts{
Type: serverType,
Path: socketPath,
Host: nbdHost,
Port: nbdPort,
})
blockdev, err := exportBlockdev(*v, kind, nodeName, VolumeBlockdev)
if err != nil {
return driveConfig{}, err
}
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: iothreadID})
+142 -18
View File
@@ -132,12 +132,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
@@ -148,12 +148,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 4,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}]}`},
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
@@ -249,7 +249,7 @@ func TestBuildDrives(t *testing.T) {
wantErr: "no free EBS hot-plug port for volume vol-overflow",
},
{
name: "mixed boot + EFI + data: boot and EFI keep the legacy -drive shape",
name: "mixed boot + EFI + data: boot is a -blockdev node and EFI keeps the pflash -drive",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-efi", NBDURI: "nbd:unix:/tmp/efi.sock", EFI: true},
@@ -260,7 +260,6 @@ func TestBuildDrives(t *testing.T) {
// unaffected by the pool either way — they keep one IOThread each.
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
{File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
},
wantIOThreads: []IOThread{
@@ -268,10 +267,11 @@ func TestBuildDrives(t *testing.T) {
{ID: "ioth-vol-data-a"},
},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=nbd,node-name=nbd-vol-data-a,server.type=unix,server.path=/tmp/data-a.sock,export=,reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-data-a,drive=nbd-vol-data-a,iothread=ioth-vol-data-a,serial=voldataa,bus=hotplug-ebs3,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 0, 3},
@@ -348,31 +348,72 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=raw,node-name=nbd-vol-blk,size=1048576,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-blk,drive=nbd-vol-blk,iothread=ioth-vol-blk,serial=volblk,bus=hotplug-ebs1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 1},
},
{
name: "block boot volume is rejected",
requests: []EBSRequest{{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, Boot: true}},
name: "block boot volume is a raw node over host_device named os under the bootindex device",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0},
},
{
name: "block boot volume with four vCPUs keeps the mapped device and error policy",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 4,
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
name: "NBD boot volume over TCP renders inet server options under file.",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd://127.0.0.1:44801", Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=44801,file.export=,file.reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
name: "unparseable NBD boot URI is rejected",
requests: []EBSRequest{{Name: "vol-boot", NBDURI: "http://example/boot", Boot: true}},
cpuCount: 2,
wantErr: "volume vol-boot: block device export is only supported for data volumes",
wantErr: "parse NBDURI for volume vol-boot",
},
{
name: "block EFI volume is rejected",
requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}},
cpuCount: 2,
wantErr: "volume vol-efi: block device export is only supported for data volumes",
wantErr: "volume vol-efi: block device export is not supported for an EFI volume",
},
{
name: "both NBDURI and BlockDevice set is rejected",
@@ -1109,6 +1150,7 @@ func TestStartQEMU_DiskBoot_MgmtTapCreated(t *testing.T) {
InstanceType: "t3.nano",
MgmtMAC: "02:aa:bb:cc:dd:ee",
}
instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: "nbd:unix:/tmp/root.sock", Boot: true}}
err := m.startQEMU(instance, false)
@@ -2168,6 +2210,88 @@ func TestStartQEMU_DiskBoot_DataVolumeStaysOnThePCIPath(t *testing.T) {
assert.Equal(t, 1, instance.EBSRequests.Requests[1].HotplugPort)
}
// TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode launches a disk-boot guest
// from each export kind and checks the boot volume reaches QEMU as the "os"
// -blockdev node behind the bootindex=1 device, with no -drive left for it.
func TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode(t *testing.T) {
tests := []struct {
name string
req func(t *testing.T) EBSRequest
wantBlockdev func(req EBSRequest) string
}{
{
name: "nbd export",
req: func(t *testing.T) EBSRequest {
return EBSRequest{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
hostPort := strings.TrimPrefix(req.NBDURI, "nbd://")
host, port, _ := strings.Cut(hostPort, ":")
return "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=" + host +
",file.server.port=" + port + ",file.export=,file.reconnect-delay=30"
},
},
{
name: "block export",
req: func(t *testing.T) EBSRequest {
dev := filepath.Join(t.TempDir(), "sdx")
require.NoError(t, os.WriteFile(dev, nil, 0o600))
return EBSRequest{Name: "vol-root", BlockDevice: dev, BlockSizeBytes: 8 << 30, Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
return "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=" + req.BlockDevice +
",file.aio=native,file.cache.direct=on"
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
req := tt.req(t)
instance := &VM{ID: "i-boot-blockdev", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = []EBSRequest{req}
require.NoError(t, m.startQEMU(instance, false))
got := launches()
require.Len(t, got, 1)
assert.Equal(t, []string{tt.wantBlockdev(req)}, allArgValues(got[0], "-blockdev"))
assert.Contains(t, allArgValues(got[0], "-device"), "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=1,bootindex=1,werror=stop,rerror=stop")
for _, d := range allArgValues(got[0], "-drive") {
assert.NotContains(t, d, "id=os", "the boot volume must not also be a -drive")
}
})
}
}
// TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched covers a disk-boot guest
// whose only volumes are data volumes: their -blockdev nodes would satisfy
// Execute, but QEMU would have nothing to boot from.
func TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched(t *testing.T) {
tests := []struct {
name string
requests []EBSRequest
}{
{name: "data volume only", requests: []EBSRequest{{Name: "vol-data-a", NBDURI: "nbd:unix:/tmp/data-a.sock"}}},
{name: "no volumes", requests: nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
instance := &VM{ID: "i-no-boot", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = tt.requests
err := m.startQEMU(instance, false)
require.Error(t, err)
assert.Contains(t, err.Error(), "no boot volume among the instance's EBS requests")
assert.Empty(t, launches(), "QEMU must not start")
})
}
}
func deviceValues(devs []Device) []string {
out := make([]string, len(devs))
for i, d := range devs {
+7 -28
View File
@@ -371,8 +371,8 @@ type Device struct {
Value string `json:"value"`
}
// Drive is one QEMU -drive entry, used for boot, EFI and pflash firmware
// drives. Each set field becomes a key=value option.
// 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.
type Drive struct {
File string `json:"file"`
Format string `json:"format"`
@@ -380,15 +380,6 @@ type Drive struct {
Media string `json:"media"`
ID string `json:"id"`
Cache string `json:"cache,omitempty"`
// Werror and Rerror set QEMU's block on-error policy. "stop" pauses the VM
// and holds the failed request, so a transient backend loss is resumed with
// cont instead of reaching the guest as EIO and corrupting its filesystem.
Werror string `json:"werror,omitempty"`
Rerror string `json:"rerror,omitempty"`
// ReconnectDelay emits file.reconnect-delay, the seconds an NBD disconnect
// pauses requests before failing them. The file. prefix is required: the
// bare name reaches the format driver, which has no such option.
ReconnectDelay int `json:"reconnect_delay,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.
Unit int `json:"unit,omitempty"`
@@ -405,9 +396,9 @@ type IOThread struct {
// Blockdev is a QEMU -blockdev entry: the command-line equivalent of the
// blockdev-add QMP command. Unlike a -drive with if=none, it produces a
// monitor-owned node addressable by node-name, which is what makes
// blockdev-del able to remove it later. Used for data (non-boot, non-EFI)
// volumes so a cold-booted one has the same detachable block-graph shape as
// a hot-attached one; boot/EFI volumes keep the legacy -drive shape.
// blockdev-del able to remove it later. Used for boot and data volumes, so a
// cold-booted data volume has the same detachable block-graph shape as a
// hot-attached one; EFI volumes keep the pflash -drive shape.
type Blockdev struct {
Value string `json:"value"`
}
@@ -571,8 +562,8 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
args = append(args, "-object", fmt.Sprintf("iothread,id=%s", iot.ID))
}
if len(cfg.Drives) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive or a kernel image is required")
if len(cfg.Drives) == 0 && len(cfg.Blockdevs) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive, blockdev or a kernel image is required")
}
drives := cfg.Drives
@@ -622,18 +613,6 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
opts = append(opts, fmt.Sprintf("cache=%s", drive.Cache))
}
if drive.Werror != "" {
opts = append(opts, fmt.Sprintf("werror=%s", drive.Werror))
}
if drive.Rerror != "" {
opts = append(opts, fmt.Sprintf("rerror=%s", drive.Rerror))
}
if drive.ReconnectDelay > 0 {
opts = append(opts, fmt.Sprintf("file.reconnect-delay=%d", drive.ReconnectDelay))
}
args = append(args, "-drive", strings.Join(opts, ","))
}
+10 -2
View File
@@ -39,7 +39,7 @@ func TestExecute(t *testing.T) {
// Expect error, at least one drive or kernel image required
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image is required")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image is required")
assert.Nil(t, cmd)
cfg.Drives = []Drive{
@@ -347,6 +347,14 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
assert.NotNil(t, cmd)
})
t.Run("blockdevs-only is OK", func(t *testing.T) {
cfg := base
cfg.Blockdevs = []Blockdev{{Value: "driver=raw,node-name=os,file.driver=nbd,file.server.type=unix,file.server.path=/run/os.sock"}}
cmd, err := cfg.Execute()
assert.NoError(t, err)
assert.NotNil(t, cmd)
})
t.Run("kernel-only is OK", func(t *testing.T) {
cfg := base
cfg.KernelImage = "/boot/vmlinuz"
@@ -359,7 +367,7 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
cfg := base
cmd, err := cfg.Execute()
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image")
assert.Nil(t, cmd)
})
}
+2 -3
View File
@@ -11,7 +11,6 @@ import (
"time"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/qmp"
)
@@ -143,7 +142,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
if kind == volumeExportBlock {
blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName)
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(ebsRequest.NBDURI)
server, err := nbdServerOpts(ebsRequest.NBDURI)
if err != nil {
slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err)
_ = m.rollbackUnmount(ctx, instance.AccountID, ebsRequest)
@@ -152,7 +151,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
blockdevArgs = map[string]any{
"node-name": nodeName,
"driver": "nbd",
"server": NBDServerOpts{Type: serverType, Path: socketPath, Host: nbdHost, Port: nbdPort}.QMPArg(),
"server": server.QMPArg(),
"export": "",
"read-only": false,
"reconnect-delay": NBDReconnectDelaySeconds,
-42
View File
@@ -1943,17 +1943,6 @@ func (d *Daemon) startCluster() error {
return fmt.Errorf("initialize JetStream: %w", err)
}
// Before any service is built: a node on a different provider from the
// cluster's would address existing volumes through the wrong implementation.
// A refusal exits so the unit shows failed rather than a healthy idle daemon.
if err := d.claimEBSProvider(); err != nil {
if errors.Is(err, errStartupPermanent) {
slog.Error("Refusing to start: EBS provider lock", "err", err)
d.exitFunc(1)
}
return fmt.Errorf("EBS provider lock: %w", err)
}
// Remove the obsolete spinifex-dhcp-leases bucket (idempotent).
if js, jsErr := jetstream.New(d.natsConn); jsErr == nil {
if err := kvutil.DeleteBucketIfExists(d.ctx, js, "spinifex-dhcp-leases"); err != nil {
@@ -2720,10 +2709,6 @@ func (d *Daemon) initJetStream() error {
err = d.jsManager.InitTerminatedInstanceBucket()
}
if err == nil {
err = d.jsManager.InitClusterConfigBucket()
}
if err == nil {
d.jsManager.SetSyncObserver(d)
// Replicas cannot be undeclared here: bucket creation refuses
@@ -2772,29 +2757,6 @@ func (d *Daemon) upgradeJetStreamReplicas() {
}
}
// claimEBSProvider records this node's EBS provider as the cluster's, or
// refuses when the cluster already records a different one.
func (d *Daemon) claimEBSProvider() error {
provider := d.config.EBS.ResolvedProvider()
rev, err := initServiceWithRetry("EBS provider lock", func() (uint64, error) {
ctx, cancel := context.WithTimeout(context.Background(), ebsProviderClaimTimeout)
defer cancel()
return d.jsManager.ClaimEBSProvider(ctx, d.node, provider)
})
if err != nil {
return err
}
slog.Info("EBS provider matches the cluster's", "provider", provider, "revision", rev)
return nil
}
// ebsProviderClaimTimeout bounds one attempt at the provider lock's KV calls.
const ebsProviderClaimTimeout = 10 * time.Second
// errStartupPermanent marks an error that a startup retry loop cannot clear,
// so the loop returns it at once rather than waiting out its budget.
var errStartupPermanent = errors.New("permanent startup refusal")
// initRetrySleep is the sleep seam used by initServiceWithRetry; tests override it.
var initRetrySleep = time.Sleep
@@ -2815,10 +2777,6 @@ func initServiceWithRetry[T any](name string, initFn func() (T, error)) (T, erro
}
return result, nil
}
if errors.Is(err, errStartupPermanent) {
var zero T
return zero, err
}
elapsed := time.Since(start)
if elapsed >= maxWait {
@@ -1,103 +0,0 @@
package daemon
import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
"github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore"
"github.com/nats-io/nats.go/jetstream"
)
// EBSProviderKey is the ClusterConfigBucket key recording the one EBS
// provider the cluster runs.
const EBSProviderKey = "ebs-provider"
// EBSProviderRecord is the value at EBSProviderKey.
type EBSProviderRecord struct {
Provider string `json:"provider"`
// RecordedBy names the node whose Create won, for the refusal message.
RecordedBy string `json:"recorded_by"`
}
// EBSProviderMismatchError is the refusal a daemon returns when its configured
// provider is not the one the cluster recorded. Retrying cannot clear it.
type EBSProviderMismatchError struct {
Node string
Configured string
Recorded EBSProviderRecord
}
func (e *EBSProviderMismatchError) Error() string {
return fmt.Sprintf("EBS provider mismatch: node %s is configured with provider %q, but this cluster runs %q (recorded by node %s in KV bucket %s, key %s); "+
"set [nodes.%s.ebs] provider = %q to join it, or change the cluster's provider by wiping and reforming the cluster, since volumes record no provider to migrate by",
e.Node, e.Configured, e.Recorded.Provider, e.Recorded.RecordedBy, ClusterConfigBucket, EBSProviderKey, e.Node, e.Recorded.Provider)
}
func (e *EBSProviderMismatchError) Unwrap() error { return errStartupPermanent }
// EBSProviderRecordInvalidError reports a recorded value that does not decode
// or names no supported provider, so there is nothing to compare against.
type EBSProviderRecordInvalidError struct {
Raw []byte
Reason string
}
func (e *EBSProviderRecordInvalidError) Error() string {
return fmt.Sprintf("EBS provider lock unreadable: KV bucket %s, key %s holds %q (%s); "+
"correct the value to {\"provider\":\"<provider>\"} naming the provider the cluster's volumes were created on, or delete the key so the next daemon records its own",
ClusterConfigBucket, EBSProviderKey, e.Raw, e.Reason)
}
func (e *EBSProviderRecordInvalidError) Unwrap() error { return errStartupPermanent }
// ClaimEBSProvider records provider as the cluster's EBS provider if none is
// recorded, and otherwise checks it against the recorded one. Create decides
// a race, so one provider is ever recorded; it returns that record's revision.
func (m *JetStreamManager) ClaimEBSProvider(ctx context.Context, node, provider string) (uint64, error) {
if m.clusterConfig == nil {
return 0, errors.New("cluster config KV not initialized")
}
claim := &EBSProviderRecord{Provider: provider, RecordedBy: node}
rev, err := kvstore.On[EBSProviderRecord](m.clusterConfig).Create(ctx, EBSProviderKey, claim)
if err == nil {
return rev, nil
}
if !errors.Is(err, kvstore.ErrExists) {
return 0, fmt.Errorf("record EBS provider: %w", err)
}
recorded, rev, err := m.readEBSProvider(ctx)
if err != nil {
return 0, err
}
if recorded.Provider != provider {
return 0, &EBSProviderMismatchError{Node: node, Configured: provider, Recorded: *recorded}
}
return rev, nil
}
// readEBSProvider reads the raw value rather than through the store's codec,
// so a malformed one can be quoted in the refusal.
func (m *JetStreamManager) readEBSProvider(ctx context.Context) (*EBSProviderRecord, uint64, error) {
kv, err := m.clusterConfig.KV(ctx)
if err != nil {
return nil, 0, err
}
entry, err := kv.Get(ctx, EBSProviderKey)
if err != nil {
if errors.Is(err, jetstream.ErrKeyNotFound) {
return nil, 0, fmt.Errorf("recorded EBS provider vanished after Create lost: %w", err)
}
return nil, 0, fmt.Errorf("read recorded EBS provider: %w", err)
}
var rec EBSProviderRecord
if err := json.Unmarshal(entry.Value(), &rec); err != nil {
return nil, 0, &EBSProviderRecordInvalidError{Raw: entry.Value(), Reason: "not a JSON record: " + err.Error()}
}
if !config.SupportedEBSProvider(rec.Provider) {
return nil, 0, &EBSProviderRecordInvalidError{Raw: entry.Value(), Reason: fmt.Sprintf("provider %q is not a supported provider", rec.Provider)}
}
return &rec, entry.Revision(), nil
}
@@ -1,272 +0,0 @@
package daemon
import (
"context"
"encoding/json"
"fmt"
"slices"
"strconv"
"sync"
"testing"
"time"
testutil "github.com/mulgadc/spinifex/internal/testkit"
"github.com/mulgadc/spinifex/spinifex/bootstrap/config"
"github.com/mulgadc/spinifex/spinifex/foundation/state/kvutil"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// newLockManager opens the cluster-config bucket over its own connection, as
// a separate daemon would.
func newLockManager(t *testing.T, url string) *JetStreamManager {
t.Helper()
nc, err := nats.Connect(url)
require.NoError(t, err)
t.Cleanup(nc.Close)
m, err := NewJetStreamManager(nc)
require.NoError(t, err)
require.NoError(t, m.InitClusterConfigBucket())
return m
}
func recordedEBSProvider(t *testing.T, js jetstream.JetStream) (EBSProviderRecord, uint64) {
t.Helper()
kv, err := js.KeyValue(t.Context(), ClusterConfigBucket)
require.NoError(t, err)
entry, err := kv.Get(t.Context(), EBSProviderKey)
require.NoError(t, err)
var rec EBSProviderRecord
require.NoError(t, json.Unmarshal(entry.Value(), &rec))
return rec, entry.Revision()
}
func TestClusterConfigBucket_NoTTLAndSchemaVersion(t *testing.T) {
ns, _, js := testutil.StartTestJetStream(t)
newLockManager(t, ns.ClientURL())
kv, err := js.KeyValue(t.Context(), ClusterConfigBucket)
require.NoError(t, err)
status, err := kv.Status(t.Context())
require.NoError(t, err)
assert.Equal(t, int64(1), status.History())
assert.Zero(t, status.TTL(), "a TTL would expire the lock and let a second provider in")
v, err := kvutil.ReadVersion(t.Context(), kv)
require.NoError(t, err)
assert.Equal(t, 1, v)
}
func TestClaimEBSProvider_FirstClaimRecordsAndMatchingNodesJoin(t *testing.T) {
ns, _, js := testutil.StartTestJetStream(t)
first := newLockManager(t, ns.ClientURL())
second := newLockManager(t, ns.ClientURL())
rev, err := first.ClaimEBSProvider(t.Context(), "node1", config.EBSProviderQEMUNBD)
require.NoError(t, err)
joined, err := second.ClaimEBSProvider(t.Context(), "node2", config.EBSProviderQEMUNBD)
require.NoError(t, err)
assert.Equal(t, rev, joined)
restarted, err := first.ClaimEBSProvider(t.Context(), "node1", config.EBSProviderQEMUNBD)
require.NoError(t, err, "a restart on the same provider must start")
assert.Equal(t, rev, restarted)
rec, revAfter := recordedEBSProvider(t, js)
assert.Equal(t, EBSProviderRecord{Provider: config.EBSProviderQEMUNBD, RecordedBy: "node1"}, rec)
assert.Equal(t, rev, revAfter, "a matching claim must not rewrite the key")
}
func TestClaimEBSProvider_DifferentProviderRefused(t *testing.T) {
ns, _, js := testutil.StartTestJetStream(t)
first := newLockManager(t, ns.ClientURL())
second := newLockManager(t, ns.ClientURL())
_, err := first.ClaimEBSProvider(t.Context(), "node1", config.EBSProviderViperblockd)
require.NoError(t, err)
_, err = second.ClaimEBSProvider(t.Context(), "node2", config.EBSProviderQEMUNBD)
var mismatch *EBSProviderMismatchError
require.ErrorAs(t, err, &mismatch)
assert.ErrorIs(t, err, errStartupPermanent)
assert.Equal(t, "node2", mismatch.Node)
assert.Equal(t, config.EBSProviderQEMUNBD, mismatch.Configured)
assert.Equal(t, EBSProviderRecord{Provider: config.EBSProviderViperblockd, RecordedBy: "node1"}, mismatch.Recorded)
for _, want := range []string{`"qemunbd"`, `"viperblockd"`, "node1", ClusterConfigBucket, EBSProviderKey, `[nodes.node2.ebs] provider = "viperblockd"`, "wiping and reforming"} {
assert.Contains(t, err.Error(), want)
}
rec, _ := recordedEBSProvider(t, js)
assert.Equal(t, config.EBSProviderViperblockd, rec.Provider, "a refused claim must leave the recorded provider in place")
}
// A value that does not decode, or names no supported provider, is refused as
// unreadable rather than as a mismatch, and quotes what the key holds.
func TestClaimEBSProvider_InvalidRecordRefused(t *testing.T) {
cases := []struct {
name string
raw string
reason string
}{
{"not JSON", "qemunbd", "not a JSON record"},
{"empty provider", `{"provider":"","recorded_by":"node1"}`, `provider "" is not a supported provider`},
{"unknown provider", `{"provider":"cephd","recorded_by":"node1"}`, `provider "cephd" is not a supported provider`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ns, _, js := testutil.StartTestJetStream(t)
m := newLockManager(t, ns.ClientURL())
kv, err := js.KeyValue(t.Context(), ClusterConfigBucket)
require.NoError(t, err)
_, err = kv.Put(t.Context(), EBSProviderKey, []byte(tc.raw))
require.NoError(t, err)
_, err = m.ClaimEBSProvider(t.Context(), "node2", config.EBSProviderViperblockd)
var invalid *EBSProviderRecordInvalidError
require.ErrorAs(t, err, &invalid)
assert.ErrorIs(t, err, errStartupPermanent)
assert.NotErrorAs(t, err, new(*EBSProviderMismatchError))
for _, want := range []string{ClusterConfigBucket, EBSProviderKey, strconv.Quote(tc.raw), tc.reason} {
assert.Contains(t, err.Error(), want)
}
})
}
}
// Two daemons on different providers starting together: whichever Create wins
// is recorded, and the other is refused.
func TestClaimEBSProvider_ConcurrentDifferentProvidersLeaveOneKeyAndOneRefusal(t *testing.T) {
for range 20 {
ns, _, js := testutil.StartTestJetStream(t)
managers := []*JetStreamManager{newLockManager(t, ns.ClientURL()), newLockManager(t, ns.ClientURL())}
providers := []string{config.EBSProviderViperblockd, config.EBSProviderQEMUNBD}
nodes := []string{"node1", "node2"}
errs := make([]error, 2)
revs := make([]uint64, 2)
start := make(chan struct{})
var wg sync.WaitGroup
for i := range managers {
wg.Go(func() {
<-start
revs[i], errs[i] = managers[i].ClaimEBSProvider(context.Background(), nodes[i], providers[i])
})
}
close(start)
wg.Wait()
rec, rev := recordedEBSProvider(t, js)
winner := slices.Index(providers, rec.Provider)
require.GreaterOrEqual(t, winner, 0, "recorded provider %q is neither claimant's", rec.Provider)
assert.Equal(t, nodes[winner], rec.RecordedBy)
require.NoError(t, errs[winner])
assert.Equal(t, revs[winner], rev, "the key must still be the winner's Create")
var mismatch *EBSProviderMismatchError
require.ErrorAs(t, errs[1-winner], &mismatch, "loser must be refused")
assert.Equal(t, rec.Provider, mismatch.Recorded.Provider)
}
}
// A permanent refusal returns on the first attempt rather than being retried
// for the startup budget.
func TestInitServiceWithRetry_PermanentErrorNotRetried(t *testing.T) {
stubInitRetrySleep(t)
calls := 0
_, err := initServiceWithRetry("svc", func() (int, error) {
calls++
if calls > 1 {
t.Fatal("a permanent error was retried")
}
return 0, fmt.Errorf("refused: %w", errStartupPermanent)
})
require.ErrorIs(t, err, errStartupPermanent)
assert.Equal(t, 1, calls)
}
// An unset provider resolves to viperblockd for the check, and the daemon
// refuses on the first mismatch: a retry would need a sleep first.
func TestDaemonClaimEBSProvider_MismatchRefusesWithoutRetry(t *testing.T) {
ns, _, _ := testutil.StartTestJetStream(t)
prev := initRetrySleep
initRetrySleep = func(time.Duration) { t.Fatal("a mismatch was retried") }
t.Cleanup(func() { initRetrySleep = prev })
first := &Daemon{node: "node1", config: &config.Config{EBS: config.EBSConfig{Provider: config.EBSProviderQEMUNBD}}, jsManager: newLockManager(t, ns.ClientURL())}
require.NoError(t, first.claimEBSProvider())
second := &Daemon{node: "node2", config: &config.Config{}, jsManager: newLockManager(t, ns.ClientURL())}
err := second.claimEBSProvider()
var mismatch *EBSProviderMismatchError
require.ErrorAs(t, err, &mismatch)
assert.Equal(t, config.EBSProviderViperblockd, mismatch.Configured)
}
// A transient KV failure is retried until the bucket answers.
func TestDaemonClaimEBSProvider_RetriesUntilBucketOpens(t *testing.T) {
ns, _, js := testutil.StartTestJetStream(t)
m := newLockManager(t, ns.ClientURL())
bucket := m.clusterConfig
m.clusterConfig = nil
sleeps := 0
prev := initRetrySleep
initRetrySleep = func(time.Duration) {
sleeps++
m.clusterConfig = bucket
}
t.Cleanup(func() { initRetrySleep = prev })
d := &Daemon{node: "node1", config: &config.Config{}, jsManager: m}
require.NoError(t, d.claimEBSProvider())
assert.Equal(t, 1, sleeps)
rec, _ := recordedEBSProvider(t, js)
assert.Equal(t, config.EBSProviderViperblockd, rec.Provider)
}
// A daemon whose provider differs from the recorded one exits 1 before any
// cluster service is built or any instance restored.
func TestStartCluster_EBSProviderMismatchExits(t *testing.T) {
t.Setenv("SPINIFEX_REQUIRE_NATS", "")
ns, _, js := testutil.StartTestJetStream(t)
seed := newLockManager(t, ns.ClientURL())
_, err := seed.ClaimEBSProvider(t.Context(), "node-2", config.EBSProviderQEMUNBD)
require.NoError(t, err)
tmpDir := t.TempDir()
cfg := &config.ClusterConfig{
Node: "node-1",
Nodes: map[string]config.Config{
"node-1": {BaseDir: tmpDir, DataDir: tmpDir, NATS: config.NATSConfig{Host: ns.ClientURL()}},
},
}
d, err := NewDaemon(cfg)
require.NoError(t, err)
t.Cleanup(d.cancel)
exitCh := make(chan int, 1)
d.exitFunc = func(code int) { exitCh <- code }
err = d.startCluster()
var mismatch *EBSProviderMismatchError
require.ErrorAs(t, err, &mismatch)
assert.Equal(t, config.EBSProviderViperblockd, mismatch.Configured)
select {
case code := <-exitCh:
assert.Equal(t, 1, code)
default:
t.Fatal("a provider mismatch must request exit(1)")
}
assert.Nil(t, d.instanceService, "no cluster service may be built after the refusal")
assert.Nil(t, d.imageService)
assert.Nil(t, d.snapshotService)
assert.Nil(t, d.volumeService)
assert.Nil(t, d.vpcService)
assert.Nil(t, d.eksService)
assert.Zero(t, d.vmMgr.Count(), "no instance may be restored after the refusal")
rec, _ := recordedEBSProvider(t, js)
assert.Equal(t, config.EBSProviderQEMUNBD, rec.Provider)
}
+5 -33
View File
@@ -25,9 +25,6 @@ const (
InstanceStateBucket = "spinifex-instance-state"
// ClusterStateBucket is the name of the KV bucket for cluster state (heartbeats, shutdown markers, service maps).
ClusterStateBucket = clusterv1.ClusterStateBucket
// ClusterConfigBucket holds settings fixed when a cluster forms. Unlike
// ClusterStateBucket it has no TTL, so a key written once stays written.
ClusterConfigBucket = "spinifex-cluster-config"
// InstanceStatePrefix is the key prefix for the per-node running-set blobs
// the record space replaces. Nothing writes it after the cutover; the
// migration reads it and a rolled-back node still finds what it left.
@@ -56,7 +53,6 @@ const (
// on a bucket stamped 5 rather than reading the keys it no longer owns.
InstanceStateBucketVersion = 5
ClusterStateBucketVersion = 1
ClusterConfigBucketVersion = 1
TerminatedInstanceBucketVersion = 3
)
@@ -84,12 +80,11 @@ type JetStreamManager struct {
// The per-resource key space the three views above are moving onto. Each
// is a third view over a bucket one of them already holds, so the two
// spaces share a handle and a recovery driven through either repairs both.
records *kvstore.Store[vm.InstanceRecord] // i.<id>, instance-state bucket
termRecords *kvstore.Store[vm.InstanceRecord] // i.<id>, terminated bucket
clusterKV jetstream.KeyValue // spinifex-cluster-state
clusterConfig *kvstore.Bucket // spinifex-cluster-config
obs KVSyncObserver
running runningSetState
records *kvstore.Store[vm.InstanceRecord] // i.<id>, instance-state bucket
termRecords *kvstore.Store[vm.InstanceRecord] // i.<id>, terminated bucket
clusterKV jetstream.KeyValue // spinifex-cluster-state
obs KVSyncObserver
running runningSetState
}
// checkNodeStateVersion reports whether a node record read from KV is one this
@@ -209,29 +204,6 @@ func (m *JetStreamManager) InitClusterStateBucket() error {
return nil
}
// clusterConfigConfig describes the cluster-config bucket. RecreateIfMissing
// stays false: a lost lock is not reconstructible by recovery, only by the
// next daemon start, which claims it again.
func clusterConfigConfig() kvstore.Config {
return kvstore.Config{
Name: ClusterConfigBucket,
Description: "Spinifex cluster configuration fixed at formation (EBS provider)",
History: 1,
OnOpen: func(ctx context.Context, kv jetstream.KeyValue) error {
return migrate.DefaultRegistry.RunKV(ctx, ClusterConfigBucket, kv, ClusterConfigBucketVersion)
},
Missing: "cluster config KV bucket not initialized",
}
}
// InitClusterConfigBucket initializes the cluster-config KV bucket, creating it
// if it doesn't exist.
func (m *JetStreamManager) InitClusterConfigBucket() error {
m.clusterConfig = kvstore.NewBucket(m.js, clusterConfigConfig())
_, err := m.clusterConfig.KV(context.Background())
return err
}
// InitTerminatedInstanceBucket initializes the terminated-instances KV bucket with a 1-hour TTL.
// JetStream automatically purges keys after 1 hour, matching AWS behavior for terminated instances.
func (m *JetStreamManager) InitTerminatedInstanceBucket() error {