gsadmin/spinifex
1
0
Fork 0
mirror of https://github.com/mulgadc/spinifex.git synced 2026-10-16 01:22:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: deliver backend ENOSPC to the guest and enable chunk GC in production (#549)

Browse Source 
* feat: plumb GCEnabled config through to every VB construction site

viperblock chunk GC is gated behind VB.GCEnabled, but spinifex never
set it, so every deployed node ran with GC off by construction
regardless of which viperblock binary shipped. Add a
[viperblock] gc_enabled config key (default false, mirrors shardwal)
and thread it through to the three places spinifex builds a VB: the
nbdkit plugin it spawns per mounted volume (via a new gc_enabled
plugin param), viperblockd's detached-volume opens, and the volume
service's CreateVolume.

* chore: repin viperblock to GC-enable lineage

The gc-enable plumbing references viperblock.VB.GCEnabled, which is
absent from the previously pinned v1.12.1. Repin to the feat/nbd-gc-enable
commit so a standalone (GOWORK=off) build — the path the ansible deploy
uses — resolves the field, and spx plus the nbdkit plugin build from the
same GC-bearing viperblock lineage.

* fix(storagegrowth): stop launching a nano for image-pull repro

ensureImagePullInstance shared harness.DiscoverNanoInstanceType with the
synthetic npass suite. A 512MB guest OOM-thrashes containerd before any
disk-driven ENOSPC and conflates the storage leak with guest-memory
pressure. Add a package-local imagePullInstanceType helper: an exact
override via SPINIFEX_STORAGEGROWTH_INSTANCE_TYPE, otherwise the smallest
advertised type at or above SPINIFEX_STORAGEGROWTH_MIN_MEM_MIB (default
16384 MiB). npass keeps using the nano selector unchanged.

* feat: report guest block write errors so backend ENOSPC reaches the guest

Set werror=report and rerror=report on the guest boot drive so a viperblock
ENOSPC (or any other backend write failure) is surfaced to the guest as a
real I/O error instead of QEMU silently pausing the VM. Without this the
guest kernel never learns its disk is out of space, so it can't fail
writes cleanly or let an operator react.

* chore: repin predastore and viperblock to the storage-capacity-safety lineage

Drop the local-path replace for predastore (a test shortcut) and pin both
predastore and viperblock to their feat/storage-capacity-safety commits, so
a standalone (GOWORK=off) build resolves the free-space watermark, ENOSPC
classification, and backpressure plumbing that spinifex's werror/rerror
change depends on.

* chore: bump viperblock to 054a44a3f0f5

Realigns spx's linked viperblock with the submodule working tree, which the
build role's source-drift gate compares against.

* chore: repin predastore and viperblock to their dev lineage

Both were pinned to pseudo-versions off feat/storage-capacity-safety. That work
has now landed on dev in each repo, and both PRs were squash-merged, so the
branch commits those pins named are unreachable from any branch.

Repin to the dev commits carrying the same code: predastore 5699329 and
viperblock 35dd9f2.
This commit is contained in:
Josh Siviour
2026-07-21 15:39:14 +10:00
committed by GitHub
parent 08fc5e4c58
commit a01e58d90c
13 changed files with 213 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/spinifex/cmd/service.go
+9
View File
@@ -297,6 +297,14 @@ var viperblockStartCmd = &cobra.Command{
shardWAL = *nodeConfig.Viperblock.ShardWAL
}
// Resolve chunk GC setting: default false unless explicitly set to true.
// GC is not a per-volume toggle — flipping it on applies to every VB this
// node constructs (nbdkit plugin + volume service).
gcEnabled := false
if nodeConfig.Viperblock.GCEnabled != nil {
gcEnabled = *nodeConfig.Viperblock.GCEnabled
}
encryptionKeyFile := nodeConfig.Viperblock.EncryptionKeyFile
if envKey := viper.GetString("viperblock-encryption-key-file"); envKey != "" {
encryptionKeyFile = envKey
@@ -317,6 +325,7 @@ var viperblockStartCmd = &cobra.Command{
BaseDir: nodeConfig.Predastore.BaseDir,
NodeName: clusterConfig.Node,
ShardWAL: shardWAL,
GCEnabled: gcEnabled,
EncryptionKeyFile: encryptionKeyFile,
})
go.mod
+2 -2
View File
@@ -21,8 +21,8 @@ require (
github.com/klauspost/cpuid/v2 v2.4.0
github.com/miekg/dns v1.1.72
github.com/mulgadc/northstar v1.12.1-0.20260716014846-f85933fe2910
github.com/mulgadc/predastore v1.12.1
github.com/mulgadc/viperblock v1.12.1
github.com/mulgadc/predastore v1.12.2-0.20260721042232-56993298a996
github.com/mulgadc/viperblock v1.12.2-0.20260721051906-35dd9f2c1b9e
github.com/nats-io/nats-server/v2 v2.14.3
github.com/nats-io/nats.go v1.52.0
github.com/opencontainers/runtime-spec v1.3.0
go.sum
+4 -4
View File
@@ -1319,10 +1319,10 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc
github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
github.com/mulgadc/northstar v1.12.1-0.20260716014846-f85933fe2910 h1:em4NMKrCbjCFI+nuQ06Vcy+OK8JDimCf1tV2lCnX31k=
github.com/mulgadc/northstar v1.12.1-0.20260716014846-f85933fe2910/go.mod h1:wSbEv6trLTyLJM4jdyS9JQabx52FQOfQ9CtHMBo/A64=
github.com/mulgadc/predastore v1.12.1 h1:mkJqmUbxfI3kBRmtBecjvBsRLFpnKARGWtmx4b9XKKg=
github.com/mulgadc/predastore v1.12.1/go.mod h1:kgTncuCaB11e3eCQeZF0ydEFG0biuOlrZ4DlHNzwvZY=
github.com/mulgadc/viperblock v1.12.1 h1:DWMbzPhIkYzdhOy4sn/D0/BJWJ5uhBoKWbVMfS7DMl4=
github.com/mulgadc/viperblock v1.12.1/go.mod h1:5MGciKEvqpwLv2pdjk9j7PR8NHtXPs4pJzJi4UJojeU=
github.com/mulgadc/predastore v1.12.2-0.20260721042232-56993298a996 h1:trj4crGlgNiAGTn0dWOscXibPd1ldCMTmw7P2OnF734=
github.com/mulgadc/predastore v1.12.2-0.20260721042232-56993298a996/go.mod h1:kgTncuCaB11e3eCQeZF0ydEFG0biuOlrZ4DlHNzwvZY=
github.com/mulgadc/viperblock v1.12.2-0.20260721051906-35dd9f2c1b9e h1:mY6m8KwRqd4rK/gHx12rh9AOd/zKLOQuUCJaJsw0r5A=
github.com/mulgadc/viperblock v1.12.2-0.20260721051906-35dd9f2c1b9e/go.mod h1:UGIWumMNLiV++B5HJ1VFnlbEuJBm/b0pyC/Sfi4Oslo=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
spinifex/config/config.go
+7
View File
@@ -124,6 +124,13 @@ type ViperblockConfig struct {
// EncryptionKeyFile is the path to the shared 32-byte AES-256 master key for viperblock at-rest encryption.
// Empty means cleartext. When set, all VB instances must load it via masterkey.LoadShared.
EncryptionKeyFile string `json:"EncryptionKeyFile" mapstructure:"encryption_key_file"`
// GCEnabled turns on viperblock chunk garbage collection (mark-and-sweep,
// snapshot-ancestry gated) on every VB this node constructs: the nbdkit
// plugin backing a mounted volume, and the short-lived VBs used by the
// volume service. Default false when nil so existing deployments keep
// today's behavior until explicitly opted in.
GCEnabled *bool `json:"GCEnabled" mapstructure:"gc_enabled"`
}
// VPCDConfig holds the VPC daemon (vpcd) configuration.
spinifex/config/config_test.go
+44
View File
@@ -785,6 +785,50 @@ shardwal = true
assert.True(t, *n.Viperblock.ShardWAL)
}
func TestLoadConfig_ViperblockGCEnabled_DefaultNil(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[nodes.n1]
region = "us-east-1"
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.NoError(t, err)
n := cfg.Nodes["n1"]
assert.Nil(t, n.Viperblock.GCEnabled, "GCEnabled should be nil when not configured (defaults to false in service)")
}
func TestLoadConfig_ViperblockGCEnabled_True(t *testing.T) {
resetViper(t)
dir := t.TempDir()
path := filepath.Join(dir, "spinifex.toml")
toml := `
node = "n1"
[nodes.n1]
region = "us-east-1"
[nodes.n1.viperblock]
gc_enabled = true
`
require.NoError(t, os.WriteFile(path, []byte(toml), 0600))
cfg, err := LoadConfig(path)
require.NoError(t, err)
n := cfg.Nodes["n1"]
require.NotNil(t, n.Viperblock.GCEnabled, "GCEnabled should be set when explicitly configured")
assert.True(t, *n.Viperblock.GCEnabled)
}
func TestParseEndpoints(t *testing.T) {
cases := []struct {
name string
spinifex/handlers/ec2/volume/service_impl.go
+5
View File
@@ -202,6 +202,10 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV
return nil, errors.New(awserrors.ErrorServerInternal)
}
// GCEnabled: default false unless explicitly set to true, matching the
// nbdkit plugin and viperblockd resolution of the same config field.
gcEnabled := s.config.Viperblock.GCEnabled != nil && *s.config.Viperblock.GCEnabled
vbconfig := viperblock.VB{
VolumeName: volumeID,
VolumeSize: volumeSizeBytes,
@@ -210,6 +214,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV
VolumeConfig: volumeConfig,
MasterKey: mkey,
EncryptionEnabled: mkey != nil,
GCEnabled: gcEnabled,
}
// If created from a snapshot, set the snapshot fields so viperblock's
spinifex/nbd/nbd.go
+6
View File
@@ -30,6 +30,11 @@ type NBDKitConfig struct {
// it is forwarded to the viperblock plugin so encrypted volumes open with
// matching encryption state. Empty → plugin opens in cleartext mode.
EncryptionKeyFile string `json:"encryption_key_file"`
// GCEnabled forwards spinifex's [viperblock] gc_enabled toggle to the
// plugin's gc_enabled param, gating chunk garbage collection on the VB
// this nbdkit process constructs. Default false, matching ShardWAL.
GCEnabled bool `json:"gc_enabled"`
}
// buildArgs constructs the nbdkit command-line arguments from the config.
@@ -69,6 +74,7 @@ func (cfg *NBDKitConfig) buildArgs() ([]string, error) {
fmt.Sprintf("host=%s", cfg.Host),
fmt.Sprintf("cache_size=%d", cfg.CacheSize),
fmt.Sprintf("shardwal=%t", cfg.ShardWAL),
fmt.Sprintf("gc_enabled=%t", cfg.GCEnabled),
}
// Only forward the key when configured; an empty value would explicitly
// set the plugin to cleartext and override its ENCRYPTION_KEY_FILE fallback.
spinifex/nbd/nbd_test.go
+38
View File
@@ -44,6 +44,7 @@ func TestBuildArgs_TCPTransport(t *testing.T) {
"host=localhost:9000",
"cache_size=256",
"shardwal=true",
"gc_enabled=false",
}
assertArgs(t, expected, args)
@@ -65,6 +66,7 @@ func TestBuildArgs_UnixSocketTransport(t *testing.T) {
Host: "10.0.0.1:9000",
CacheSize: 128,
ShardWAL: false,
GCEnabled: true,
}
args, err := cfg.buildArgs()
@@ -87,6 +89,7 @@ func TestBuildArgs_UnixSocketTransport(t *testing.T) {
"host=10.0.0.1:9000",
"cache_size=128",
"shardwal=false",
"gc_enabled=true",
}
assertArgs(t, expected, args)
@@ -272,6 +275,41 @@ func TestBuildArgs_EncryptionKeyFileOmittedWhenEmpty(t *testing.T) {
}
}
func TestBuildArgs_GCEnabledForwarded(t *testing.T) {
cfg := &NBDKitConfig{
Socket: "/tmp/nbd.sock",
PidFile: "/tmp/nbd.pid",
PluginPath: "/plugin.so",
GCEnabled: true,
}
args, err := cfg.buildArgs()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if slices.Index(args, "gc_enabled=true") < 0 {
t.Errorf("expected gc_enabled=true in args, got: %v", args)
}
}
func TestBuildArgs_GCEnabledDefaultFalse(t *testing.T) {
cfg := &NBDKitConfig{
Socket: "/tmp/nbd.sock",
PidFile: "/tmp/nbd.pid",
PluginPath: "/plugin.so",
}
args, err := cfg.buildArgs()
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if slices.Index(args, "gc_enabled=false") < 0 {
t.Errorf("expected gc_enabled=false (default off) in args, got: %v", args)
}
}
func assertArgs(t *testing.T, expected, got []string) {
t.Helper()
if len(expected) != len(got) {
spinifex/services/viperblockd/viperblockd.go
+9 -1
View File
@@ -120,6 +120,12 @@ type Config struct {
// ShardWAL enables sharded WAL for mounted volumes (default false)
ShardWAL bool
// GCEnabled turns on viperblock chunk garbage collection for every VB this
// service constructs: the nbdkit plugin backing each mounted volume, and
// the short-lived detached VBs opened for config updates and sealing.
// Default false, matching ShardWAL.
GCEnabled bool
// EncryptionKeyFile is the path to the shared AES-256 master key for at-rest
// encryption. Empty → cleartext mode (legacy).
EncryptionKeyFile string
@@ -208,6 +214,7 @@ func openVolumeVB(cfg *Config, volumeName string) (*viperblock.VB, error) {
VolumeConfig: viperblock.VolumeConfig{},
MasterKey: cfg.masterKey,
EncryptionEnabled: cfg.masterKey != nil,
GCEnabled: cfg.GCEnabled,
}
vb, err := viperblock.New(&vbconfig, "s3", s3cfg)
if err != nil {
@@ -365,7 +372,7 @@ func launchService(cfg *Config) (err error) {
slog.Warn("Viperblock at-rest encryption disabled (no EncryptionKeyFile configured)")
}
slog.Info("Viperblock config", "shardwal", cfg.ShardWAL)
slog.Info("Viperblock config", "shardwal", cfg.ShardWAL, "gc_enabled", cfg.GCEnabled)
if cfg.NodeName != "" {
slog.Info("Waiting for EBS events", "node", cfg.NodeName)
@@ -854,6 +861,7 @@ func launchService(cfg *Config) (err error) {
SecretKey: cfg.SecretKey,
CacheSize: nbdCacheSize,
ShardWAL: cfg.ShardWAL,
GCEnabled: cfg.GCEnabled,
EncryptionKeyFile: cfg.EncryptionKeyFile,
}
spinifex/vm/lifecycle.go
+7
View File
@@ -1124,6 +1124,13 @@ func buildDrives(requests []types.EBSRequest, cpuCount int, machineType string)
drive.ID = "os"
drive.Cache = "none"
// Report backend write errors to the guest rather than letting
// QEMU's default werror=enospc pause the VM: when the storage pool
// exhausts, the guest fs then returns a clean ENOSPC to userspace
// and the VM stays reachable instead of freezing.
drive.Werror = "report"
drive.Rerror = "report"
iothreadID := "ioth-os"
iothreads = append(iothreads, IOThread{ID: iothreadID})
devices = append(devices, BlkDevice(machineType, drive.ID, iothreadID, cpuCount, 1))
spinifex/vm/lifecycle_test.go
+2 -2
View File
@@ -122,7 +122,7 @@ func TestBuildDrives(t *testing.T) {
},
cpuCount: 4,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none"},
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "report", Rerror: "report"},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantDevices: []Device{
@@ -163,7 +163,7 @@ func TestBuildDrives(t *testing.T) {
},
cpuCount: 4,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none"},
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "report", Rerror: "report"},
{File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}},
spinifex/vm/vm.go
+17
View File
@@ -204,6 +204,15 @@ 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. Left empty, QEMU
// applies its default werror=enospc, which pauses the whole VM on an
// out-of-space write rather than reporting it — so a guest never sees the
// backend's ENOSPC. Setting "report" delivers the error to the guest,
// letting its filesystem surface a clean out-of-space to userspace and stay
// alive. Set on the drive: with if=none the virtio-blk device inherits the
// BlockBackend's on-error policy.
Werror string `json:"werror,omitempty"`
Rerror string `json:"rerror,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"`
@@ -397,6 +406,14 @@ 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))
}
args = append(args, "-drive", strings.Join(opts, ","))
}
tests/e2e/storagegrowth/image_pull_test.go
+63 -1
View File
@@ -568,7 +568,7 @@ func TestImagePullGrowth(t *testing.T) {
// fix.AWS.EC2.RunInstances directly instead (launchImagePullRootInstance).
func ensureImagePullInstance(t *testing.T, fix *Fixture, launch func(ami, instType, keyName, subnetID, sgID string) string) (instanceID string, tgt harness.SSHTarget, inst *ec2.Instance) {
t.Helper()
instType, arch := harness.DiscoverNanoInstanceType(t, fix.Harness)
instType, arch := imagePullInstanceType(t, fix)
ami := harness.DiscoverUbuntuAMI(t, fix.Harness, arch)
keyName, keyPath := harness.EnsureKeyPair(t, fix.Harness, fix.ArtifactDir(t))
@@ -635,6 +635,68 @@ func imagePullDefaultSG(t *testing.T, c *harness.AWSClient, vpcID string) string
return aws.StringValue(out.SecurityGroups[0].GroupId)
}
// imagePullInstanceType selects the guest for the image-pull repro. The
// synthetic npass suite uses a nano (512MB) to pack many suites onto one node,
// but a real multi-GB image pull onto 512MB OOM-thrashes containerd long before
// any disk-driven ENOSPC, and 512MB against a concurrent-write pull is exactly
// the guest-memory-pressure regime that produces the separate siv-482 lost-write
// corruption — conflating two faults in one measurement. This mode instead picks
// an instance with headroom well above the pull's working set, so the run
// measures the storage leak and not memory pressure.
//
// SPINIFEX_STORAGEGROWTH_INSTANCE_TYPE overrides the choice outright.
// Otherwise the smallest advertised type at or above SPINIFEX_STORAGEGROWTH_MIN_MEM_MIB
// (default 16384, matching the ~16GB EKS GPU worker the incident was reported on)
// is used — smallest-above-floor so it gets headroom without grabbing a type the
// node cannot seat.
func imagePullInstanceType(t *testing.T, fix *Fixture) (instanceType, arch string) {
t.Helper()
out, err := fix.AWS.EC2.DescribeInstanceTypes(&ec2.DescribeInstanceTypesInput{})
require.NoError(t, err, "DescribeInstanceTypes")
archOf := func(it *ec2.InstanceTypeInfo) string {
if it.ProcessorInfo == nil || len(it.ProcessorInfo.SupportedArchitectures) == 0 {
return ""
}
return aws.StringValue(it.ProcessorInfo.SupportedArchitectures[0])
}
if override := os.Getenv("SPINIFEX_STORAGEGROWTH_INSTANCE_TYPE"); override != "" {
for _, it := range out.InstanceTypes {
if aws.StringValue(it.InstanceType) == override {
a := archOf(it)
require.NotEmptyf(t, a, "instance type %s advertises no architecture", override)
return override, a
}
}
t.Fatalf("SPINIFEX_STORAGEGROWTH_INSTANCE_TYPE=%s not advertised by this cluster", override)
}
minMemMiB := int64(16384)
if v := os.Getenv("SPINIFEX_STORAGEGROWTH_MIN_MEM_MIB"); v != "" {
parsed, perr := strconv.ParseInt(v, 10, 64)
require.NoErrorf(t, perr, "parse SPINIFEX_STORAGEGROWTH_MIN_MEM_MIB=%q", v)
minMemMiB = parsed
}
var best *ec2.InstanceTypeInfo
var bestMem int64
for _, it := range out.InstanceTypes {
if it.MemoryInfo == nil || it.MemoryInfo.SizeInMiB == nil || archOf(it) == "" {
continue
}
mem := aws.Int64Value(it.MemoryInfo.SizeInMiB)
if mem < minMemMiB {
continue
}
if best == nil || mem < bestMem {
best, bestMem = it, mem
}
}
require.NotNilf(t, best, "no advertised instance type has >= %d MiB memory", minMemMiB)
return aws.StringValue(best.InstanceType), archOf(best)
}
// createImagePullVolume creates the volume the pull runs against, sized by
// sizeGiB (unlike npass's fixed 1 GiB extent, this workload's footprint is
// the whole point of the measurement, so its size must be large enough to