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

feat: accept and surface gp3 Throughput on CreateVolume/DescribeVolumes (#658)

Browse Source 
* 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
This commit is contained in:
Josh Siviour
2026-08-03 00:25:49 +10:00
committed by GitHub
parent bbf1ca7f0b
commit 8ae8d2fec4
5 changed files with 107 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/COMMANDS.md
+1 -1
View File
@@ -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** |
go.mod
+1 -1
View File
@@ -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
go.sum
+2 -2
View File
@@ -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=
spinifex/handlers/ec2/volume/service_impl.go
+23
View File
@@ -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)
}
spinifex/handlers/ec2/volume/service_impl_test.go
+80
View File
@@ -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)