From 8ae8d2fec43815cef2f4fcfe3dda412c5e8042f1 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Mon, 3 Aug 2026 00:25:49 +1000 Subject: [PATCH] feat: accept and surface gp3 Throughput on CreateVolume/DescribeVolumes (#658) * feat: accept and surface gp3 Throughput on CreateVolume/DescribeVolumes Honors input.Throughput (125-1000 MiB/s, default 125 when omitted) on CreateVolume, persists it to VolumeMetadata, and surfaces it via DescribeVolumes (getVolumeByID), mirroring the Iops handling shipped in mulga-siv-380. Pins viperblock to the commit that adds the VolumeMetadata.Throughput field. Zero Throughput on a read volume means it predates this field; it is omitted from the response rather than surfaced as a misleading 0 MiB/s, matching the existing IOPS convention. * chore: pin viperblock to the merged gp3 Throughput commit --- docs/COMMANDS.md | 2 +- go.mod | 2 +- go.sum | 4 +- spinifex/handlers/ec2/volume/service_impl.go | 23 ++++++ .../handlers/ec2/volume/service_impl_test.go | 80 +++++++++++++++++++ 5 files changed, 107 insertions(+), 4 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index ab6edd683..cfe52e822 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -254,7 +254,7 @@ Spot Instance Requests (SIRs) are a **mock** over the on-demand `run-instances` | Command | Implemented Flags | Missing Flags | Status | |---------|-------------------|---------------|--------| | `describe-volumes` | `--volume-ids`, `--filters` (volume-id, status, size, volume-type, attachment.instance-id, attachment.status, attachment.device, availability-zone, tag:*), persisted `DeleteOnTermination` | `--max-results`, `--next-token`, `--dry-run` | **DONE** | -| `create-volume` | `--size`, `--availability-zone`, `--volume-type` (gp3), `--snapshot-id`, `--tag-specifications` | `--iops` (hardcoded 3000), `--encrypted` (hardcoded false), `--throughput` | **DONE** | +| `create-volume` | `--size`, `--availability-zone`, `--volume-type` (gp3), `--snapshot-id`, `--tag-specifications`, `--iops`, `--throughput` | `--encrypted` (hardcoded false) | **DONE** | | `delete-volume` | `--volume-id` | `--dry-run` | **DONE** | | `modify-volume` | `--volume-id`, `--size`, `--volume-type`, `--iops` | `--throughput`, `--dry-run`, `--multi-attach-enabled` | **DONE** | | `attach-volume` | `--volume-id`, `--instance-id`, `--device` (auto-assigns `/dev/sd[f-p]`) | `--dry-run` | **DONE** | diff --git a/go.mod b/go.mod index f2ce0e318..cdb15b0f6 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/miekg/dns v1.1.72 github.com/mulgadc/northstar v1.14.0 github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4 - github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0 + github.com/mulgadc/viperblock v1.14.1-0.20260802141430-3d448922ab52 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 837017fdb..9319a5eeb 100644 --- a/go.sum +++ b/go.sum @@ -1321,8 +1321,8 @@ github.com/mulgadc/northstar v1.14.0 h1:8JzDcS4k3SN5PF3jBdB8x9FOWC24eITCClmOdwpo github.com/mulgadc/northstar v1.14.0/go.mod h1:6EArbWIQUrCFQ+XZ5ZJrGS/ep/u/rF6l+dKZh5vVIRs= github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4 h1:Q4/NEryYTMiALkFwbPPOrKpNgLir+d7T2hbyRpAFnWc= github.com/mulgadc/predastore v1.14.1-0.20260730221229-608939ef2bb4/go.mod h1:na0aNXABZF03/HfFWCrAPHrId/2FIEIUQfB82vlu1xg= -github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0 h1:etjtOa6Sq21CtX+8RU4vYb5JBz+dExPLLym6xpXD4Mw= -github.com/mulgadc/viperblock v1.14.1-0.20260802084514-ed52f6a0e4c0/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858= +github.com/mulgadc/viperblock v1.14.1-0.20260802141430-3d448922ab52 h1:XFgNGFofRtqv7A85F1i9WxYK8UxpzRP5X2tv6yL/zdY= +github.com/mulgadc/viperblock v1.14.1-0.20260802141430-3d448922ab52/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858= 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/handlers/ec2/volume/service_impl.go b/spinifex/handlers/ec2/volume/service_impl.go index 0eade1318..6348f61a8 100644 --- a/spinifex/handlers/ec2/volume/service_impl.go +++ b/spinifex/handlers/ec2/volume/service_impl.go @@ -39,6 +39,11 @@ const ( defaultGP3IOPS = 3000 maxGP3IOPS = 16000 gp3IOPSPerGiB = 500 + + // gp3 Throughput envelope (AWS): 125 MiB/s baseline, 1000 MiB/s ceiling, + // flat range independent of volume size. + defaultGP3Throughput = 125 + maxGP3Throughput = 1000 ) // Ensure VolumeServiceImpl implements VolumeService. @@ -169,6 +174,16 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV return nil, errors.New(awserrors.ErrorInvalidParameterValue) } + // Honor caller-supplied Throughput for gp3, else the 125 MiB/s baseline. + // Range is flat (125-1000), unlike Iops it does not scale with size. + throughput := defaultGP3Throughput + if input.Throughput != nil { + throughput = int(*input.Throughput) + } + if throughput < defaultGP3Throughput || throughput > maxGP3Throughput { + return nil, errors.New(awserrors.ErrorInvalidParameterValue) + } + slog.InfoContext(ctx, "CreateVolume", "volumeId", volumeID, "size", size, "type", volumeType, "az", *input.AvailabilityZone, "snapshotId", snapshotID) @@ -189,6 +204,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV AvailabilityZone: *input.AvailabilityZone, VolumeType: volumeType, IOPS: iops, + Throughput: throughput, SnapshotID: snapshotID, Tags: tags, }, @@ -247,6 +263,7 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV AvailabilityZone: input.AvailabilityZone, CreateTime: aws.Time(now), Iops: aws.Int64(int64(iops)), + Throughput: aws.Int64(int64(throughput)), Encrypted: aws.Bool(mkey != nil), Tags: utils.MapToEC2Tags(tags), } @@ -1000,6 +1017,12 @@ func (s *VolumeServiceImpl) getVolumeByID(ctx context.Context, volumeID string) volume.Iops = aws.Int64(int64(volMeta.IOPS)) } + // Zero means unset (volume created before the Throughput field existed); + // omit rather than surface a misleading 0 MiB/s. + if volMeta.Throughput > 0 { + volume.Throughput = aws.Int64(int64(volMeta.Throughput)) + } + if volMeta.SnapshotID != "" { volume.SnapshotId = aws.String(volMeta.SnapshotID) } diff --git a/spinifex/handlers/ec2/volume/service_impl_test.go b/spinifex/handlers/ec2/volume/service_impl_test.go index 5f550849f..1f2cad9f9 100644 --- a/spinifex/handlers/ec2/volume/service_impl_test.go +++ b/spinifex/handlers/ec2/volume/service_impl_test.go @@ -147,6 +147,26 @@ func TestCreateVolume_Validation(t *testing.T) { }, wantErr: awserrors.ErrorInvalidParameterValue, }, + { + name: "Throughput_BelowBaseline", + az: "ap-southeast-2a", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + Throughput: aws.Int64(124), + }, + wantErr: awserrors.ErrorInvalidParameterValue, + }, + { + name: "Throughput_AboveCeiling", + az: "ap-southeast-2a", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + Throughput: aws.Int64(1001), + }, + wantErr: awserrors.ErrorInvalidParameterValue, + }, { name: "MismatchedAZ", az: "ap-southeast-2a", @@ -187,6 +207,10 @@ func TestCreateVolume_Validation(t *testing.T) { // TestCreateVolume_PassesValidation verifies that valid inputs pass validation // and only fail at the viperblock/S3 layer (no S3 backend in unit tests). +// ThroughputOmitted_DefaultsToBaseline doubles as a default-value regression +// check: the zero value of the throughput int is 0, which fails the >=125 +// range check, so this case only passes if CreateVolume actually assigns the +// 125 baseline when input.Throughput is nil. func TestCreateVolume_PassesValidation(t *testing.T) { tests := []struct { name string @@ -221,6 +245,37 @@ func TestCreateVolume_PassesValidation(t *testing.T) { Iops: aws.Int64(8000), }, }, + { + name: "ThroughputOmitted_DefaultsToBaseline", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + }, + }, + { + name: "ExplicitThroughputAtBaseline", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + Throughput: aws.Int64(125), + }, + }, + { + name: "ExplicitThroughputAtCeiling", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + Throughput: aws.Int64(1000), + }, + }, + { + name: "ExplicitThroughputMidRange", + input: &ec2.CreateVolumeInput{ + Size: aws.Int64(80), + AvailabilityZone: aws.String("ap-southeast-2a"), + Throughput: aws.Int64(500), + }, + }, } for _, tt := range tests { @@ -659,6 +714,7 @@ func TestGetVolumeByID_FullMetadata(t *testing.T) { AvailabilityZone: "ap-southeast-2a", VolumeType: "gp3", IOPS: 5000, + Throughput: 250, SnapshotID: "snap-abc", AttachedInstance: "i-12345", DeviceName: "/dev/nbd0", @@ -693,6 +749,7 @@ func TestGetVolumeByID_FullMetadata(t *testing.T) { assert.Equal(t, "in-use", *vol.State) assert.Equal(t, "gp3", *vol.VolumeType) assert.Equal(t, int64(5000), *vol.Iops) + assert.Equal(t, int64(250), *vol.Throughput) assert.Equal(t, "snap-abc", *vol.SnapshotId) assert.True(t, *vol.Encrypted) assert.Equal(t, "ap-southeast-2a", *vol.AvailabilityZone) @@ -747,6 +804,29 @@ func TestGetVolumeByID_DefaultStateAndType(t *testing.T) { assert.Equal(t, "gp3", *result.volume.VolumeType) } +// TestGetVolumeByID_ThroughputOmitted_PreFieldVolume covers a volume written +// before Throughput existed on VolumeMetadata: json.Unmarshal leaves the new +// int field at its zero value, and getVolumeByID must omit Throughput from +// the response rather than surface a misleading 0 MiB/s. +func TestGetVolumeByID_ThroughputOmitted_PreFieldVolume(t *testing.T) { + store := objectstore.NewMemoryObjectStore() + svc := newTestVolumeServiceWithStore("ap-southeast-2a", store) + + // Simulate a pre-field volume by writing raw JSON with no Throughput key. + rawConfig := `{"VolumeConfig":{"VolumeMetadata":{"VolumeID":"vol-prefield","SizeGiB":5,"State":"available","VolumeType":"gp3","IOPS":3000}}}` + _, err := store.PutObject(context.Background(), &s3.PutObjectInput{ + Bucket: aws.String("test-bucket"), + Key: aws.String("vol-prefield/config.json"), + Body: strings.NewReader(rawConfig), + }) + require.NoError(t, err) + + result, err := svc.getVolumeByID(context.Background(), "vol-prefield") + require.NoError(t, err) + + assert.Nil(t, result.volume.Throughput) +} + func TestGetVolumeByID_EmptyVolumeID(t *testing.T) { store := objectstore.NewMemoryObjectStore() svc := newTestVolumeServiceWithStore("ap-southeast-2a", store)