diff --git a/cmd/spinifex/cmd/service.go b/cmd/spinifex/cmd/service.go index 76cfba5e0..bcc99e53f 100644 --- a/cmd/spinifex/cmd/service.go +++ b/cmd/spinifex/cmd/service.go @@ -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, }) diff --git a/go.mod b/go.mod index 2c6611199..686585f99 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index d6b07cfd1..bbdc34c9a 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/spinifex/config/config.go b/spinifex/config/config.go index af4678dcc..98814361e 100644 --- a/spinifex/config/config.go +++ b/spinifex/config/config.go @@ -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. diff --git a/spinifex/config/config_test.go b/spinifex/config/config_test.go index 756308309..6c9cae5df 100644 --- a/spinifex/config/config_test.go +++ b/spinifex/config/config_test.go @@ -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 diff --git a/spinifex/handlers/ec2/volume/service_impl.go b/spinifex/handlers/ec2/volume/service_impl.go index ebc9040a0..9f9792daa 100644 --- a/spinifex/handlers/ec2/volume/service_impl.go +++ b/spinifex/handlers/ec2/volume/service_impl.go @@ -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 diff --git a/spinifex/nbd/nbd.go b/spinifex/nbd/nbd.go index a38c226b5..896c27a29 100644 --- a/spinifex/nbd/nbd.go +++ b/spinifex/nbd/nbd.go @@ -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. diff --git a/spinifex/nbd/nbd_test.go b/spinifex/nbd/nbd_test.go index 932f02b60..09ec96a32 100644 --- a/spinifex/nbd/nbd_test.go +++ b/spinifex/nbd/nbd_test.go @@ -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) { diff --git a/spinifex/services/viperblockd/viperblockd.go b/spinifex/services/viperblockd/viperblockd.go index bac233e9b..b6923bd08 100644 --- a/spinifex/services/viperblockd/viperblockd.go +++ b/spinifex/services/viperblockd/viperblockd.go @@ -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, } diff --git a/spinifex/vm/lifecycle.go b/spinifex/vm/lifecycle.go index b54ed5615..3883db517 100644 --- a/spinifex/vm/lifecycle.go +++ b/spinifex/vm/lifecycle.go @@ -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)) diff --git a/spinifex/vm/lifecycle_test.go b/spinifex/vm/lifecycle_test.go index b88243fc1..41c5b03b4 100644 --- a/spinifex/vm/lifecycle_test.go +++ b/spinifex/vm/lifecycle_test.go @@ -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"}}, diff --git a/spinifex/vm/vm.go b/spinifex/vm/vm.go index caf8a5a39..705e7f4b7 100644 --- a/spinifex/vm/vm.go +++ b/spinifex/vm/vm.go @@ -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, ",")) } diff --git a/tests/e2e/storagegrowth/image_pull_test.go b/tests/e2e/storagegrowth/image_pull_test.go index 63e3075d6..52c6a0874 100644 --- a/tests/e2e/storagegrowth/image_pull_test.go +++ b/tests/e2e/storagegrowth/image_pull_test.go @@ -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