From f4cb550c94fb8350f6fc19fd7c61c194fcd8ec46 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 1 Oct 2026 08:33:41 +1000 Subject: [PATCH] refactor: localize GP3 volume policy --- docs/PACKAGE_BOUNDARY_MIGRATION.md | 1 + spinifex/daemon/shutdown_test.go | 5 +++-- spinifex/domains/ec2/ebs/policy/gp3.go | 18 ++++++++++++++++ spinifex/domains/ec2/ebs/policy/gp3_test.go | 9 ++++++++ .../ec2/instance/provider_root_volume_test.go | 12 +++++------ .../handlers/ec2/instance/service_impl.go | 9 ++++---- .../volume/provider_snapshot_source_test.go | 8 +++---- spinifex/handlers/ec2/volume/service_impl.go | 21 ++++++++++--------- spinifex/types/ebs.go | 16 -------------- 9 files changed, 57 insertions(+), 42 deletions(-) create mode 100644 spinifex/domains/ec2/ebs/policy/gp3.go create mode 100644 spinifex/domains/ec2/ebs/policy/gp3_test.go diff --git a/docs/PACKAGE_BOUNDARY_MIGRATION.md b/docs/PACKAGE_BOUNDARY_MIGRATION.md index 7e5239330..570d9ea25 100644 --- a/docs/PACKAGE_BOUNDARY_MIGRATION.md +++ b/docs/PACKAGE_BOUNDARY_MIGRATION.md @@ -188,6 +188,7 @@ while the branch is in flight; they do not describe the umbrella repository's | `53a614717` | `spinifex/types/cluster.go` (`SharedClusterData`) | `spinifex/daemon/config_hash.go` | Shared-config hash input is daemon-local implementation data; focused config-hash tests preserve deterministic, shared-only semantics. | | `78fd017e0` | `spinifex/types/telemetry.go` (`GuestTelemetryMeta`) | `contracts/telemetry/v1/guest_meta.go` | QMP collector sidecar JSON is a versioned file contract between VM runtime and telemetry role; `guest_meta_test.go` pins its filename and payload. | | `4b35a2a33` | `spinifex/types/node.go` (`NodeVMsResponse`, `VMInfo`, `VMGPUInfo`) | `contracts/cluster/v1/node_vms.go` | VM inventory is a versioned cluster fan-out payload used by SPX CLI and EKS host lookup; `node_vms_test.go` pins its subject and JSON shape. | +| `08bd335f0` | `spinifex/types/node.go` (`NodeStatusResponse`, capacity and GPU inventory) | `contracts/cluster/v1/status.go` | Node status is a versioned fan-out observation contract for operator inspection and EC2/EKS scheduling consumers; `status_test.go` pins its subject and JSON shape. | The EC2 contract row is intentionally different from the directory moves: it creates a compatibility boundary. `ec2.cmd.*` retains its deployed one-token NATS diff --git a/spinifex/daemon/shutdown_test.go b/spinifex/daemon/shutdown_test.go index a7425e590..7e974483a 100644 --- a/spinifex/daemon/shutdown_test.go +++ b/spinifex/daemon/shutdown_test.go @@ -11,9 +11,10 @@ import ( "testing" "time" + ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy" + "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/mulgadc/spinifex/spinifex/types" "github.com/mulgadc/spinifex/spinifex/utils" - "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -704,7 +705,7 @@ func stubEBSUnmount(t *testing.T, d *Daemon, resp types.EBSUnMountResponse) func func drainRunningInstance(id, volume string) *vm.VM { instance := &vm.VM{ID: id, Status: vm.StateRunning, AccountID: "111122223333"} instance.EBSRequests.Requests = []types.EBSRequest{ - {Name: volume, VolType: types.VolumeTypeGP3, Boot: true}, + {Name: volume, VolType: ebspolicy.VolumeTypeGP3, Boot: true}, } return instance } diff --git a/spinifex/domains/ec2/ebs/policy/gp3.go b/spinifex/domains/ec2/ebs/policy/gp3.go new file mode 100644 index 000000000..bae8cdbce --- /dev/null +++ b/spinifex/domains/ec2/ebs/policy/gp3.go @@ -0,0 +1,18 @@ +// Package ebspolicy owns EC2 EBS product policy that the control plane exposes. +package ebspolicy + +// VolumeTypeGP3 is the only EBS volume type this platform currently serves. +const VolumeTypeGP3 = "gp3" + +const ( + // GP3 IOPS envelope (AWS): 3000 baseline on any size, up to 500 IOPS/GiB, + // capped at 16000. + 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 +) diff --git a/spinifex/domains/ec2/ebs/policy/gp3_test.go b/spinifex/domains/ec2/ebs/policy/gp3_test.go new file mode 100644 index 000000000..c40477389 --- /dev/null +++ b/spinifex/domains/ec2/ebs/policy/gp3_test.go @@ -0,0 +1,9 @@ +package ebspolicy + +import "testing" + +func TestGP3Envelope(t *testing.T) { + if VolumeTypeGP3 != "gp3" || DefaultGP3IOPS != 3000 || MaxGP3IOPS != 16000 || GP3IOPSPerGiB != 500 || DefaultGP3Throughput != 125 || MaxGP3Throughput != 1000 { + t.Fatal("unexpected GP3 policy envelope") + } +} diff --git a/spinifex/handlers/ec2/instance/provider_root_volume_test.go b/spinifex/handlers/ec2/instance/provider_root_volume_test.go index 68bcac8b1..fac123304 100644 --- a/spinifex/handlers/ec2/instance/provider_root_volume_test.go +++ b/spinifex/handlers/ec2/instance/provider_root_volume_test.go @@ -9,12 +9,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/s3" - "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata" - "github.com/mulgadc/spinifex/spinifex/providers/ebs" + ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/mulgadc/spinifex/spinifex/objectstore" - spxtypes "github.com/mulgadc/spinifex/spinifex/types" + "github.com/mulgadc/spinifex/spinifex/providers/ebs" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -159,9 +159,9 @@ func TestPrepareRootVolume_Provider_WritesMetadataDocument(t *testing.T) { "attach flips the volume to in-use later; creation must not pre-empt it") assert.False(t, doc.CreatedAt.IsZero()) assert.Equal(t, testRootAZ, doc.AvailabilityZone) - assert.Equal(t, spxtypes.VolumeTypeGP3, doc.VolumeType) - assert.Equal(t, spxtypes.DefaultGP3IOPS, doc.IOPS) - assert.Equal(t, spxtypes.DefaultGP3Throughput, doc.Throughput) + assert.Equal(t, ebspolicy.VolumeTypeGP3, doc.VolumeType) + assert.Equal(t, ebspolicy.DefaultGP3IOPS, doc.IOPS) + assert.Equal(t, ebspolicy.DefaultGP3Throughput, doc.Throughput) assert.Equal(t, "snap-source", doc.SnapshotID) assert.True(t, doc.DeleteOnTermination) assert.False(t, doc.Encrypted, "no encryption key is configured in this service") diff --git a/spinifex/handlers/ec2/instance/service_impl.go b/spinifex/handlers/ec2/instance/service_impl.go index c31b84c50..e76966a1a 100644 --- a/spinifex/handlers/ec2/instance/service_impl.go +++ b/spinifex/handlers/ec2/instance/service_impl.go @@ -20,6 +20,7 @@ import ( "github.com/mulgadc/spinifex/contracts/ec2/v1" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata" + ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy" "github.com/mulgadc/spinifex/spinifex/domains/ec2/instancetypes" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" awsfilters "github.com/mulgadc/spinifex/spinifex/foundation/aws/filters" @@ -1660,7 +1661,7 @@ func (s *InstanceServiceImpl) createDataVolume(ctx context.Context, input *ec2.R create := &ec2.CreateVolumeInput{ AvailabilityZone: aws.String(s.config.AZ), Size: aws.Int64(p.sizeGiB), - VolumeType: aws.String(spxtypes.VolumeTypeGP3), + VolumeType: aws.String(ebspolicy.VolumeTypeGP3), TagSpecifications: volumeTagSpecifications(input.TagSpecifications), } if p.iops > 0 { @@ -1826,8 +1827,8 @@ func (s *InstanceServiceImpl) createRootVolumeViaProvider(ctx context.Context, s CapacityGiB: safecast.IntToUint64(spec.sizeBytes / bytesPerGiB), State: string(ebsprovider.VolumeStateAvailable), CreatedAt: time.Now(), AvailabilityZone: s.config.AZ, - VolumeType: spxtypes.VolumeTypeGP3, IOPS: rootVolumeIOPS(spec.iops), - Throughput: spxtypes.DefaultGP3Throughput, SnapshotID: amiConfig.SnapshotID, + VolumeType: ebspolicy.VolumeTypeGP3, IOPS: rootVolumeIOPS(spec.iops), + Throughput: ebspolicy.DefaultGP3Throughput, SnapshotID: amiConfig.SnapshotID, DeleteOnTermination: spec.deleteOnTermination, Encrypted: mkey != nil, Tags: spec.tags, ProviderHandle: created.Handle, @@ -1847,7 +1848,7 @@ func (s *InstanceServiceImpl) createRootVolumeViaProvider(ctx context.Context, s // no Iops, so DescribeVolumes never reports a root volume as having zero. func rootVolumeIOPS(requested int) int { if requested <= 0 { - return spxtypes.DefaultGP3IOPS + return ebspolicy.DefaultGP3IOPS } return requested } diff --git a/spinifex/handlers/ec2/volume/provider_snapshot_source_test.go b/spinifex/handlers/ec2/volume/provider_snapshot_source_test.go index cc32aa258..90ac716db 100644 --- a/spinifex/handlers/ec2/volume/provider_snapshot_source_test.go +++ b/spinifex/handlers/ec2/volume/provider_snapshot_source_test.go @@ -9,10 +9,10 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata" - "github.com/mulgadc/spinifex/spinifex/providers/ebs" + ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy" handlers_ec2_instance "github.com/mulgadc/spinifex/spinifex/handlers/ec2/instance" "github.com/mulgadc/spinifex/spinifex/objectstore" - spxtypes "github.com/mulgadc/spinifex/spinifex/types" + "github.com/mulgadc/spinifex/spinifex/providers/ebs" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -161,7 +161,7 @@ func TestDescribeVolumes_Provider_SeesInstanceRootVolume(t *testing.T) { assert.Equal(t, rootVolumeID, aws.StringValue(got.VolumeId)) assert.Equal(t, "available", aws.StringValue(got.State)) assert.Equal(t, providerTestAZ, aws.StringValue(got.AvailabilityZone)) - assert.Equal(t, spxtypes.VolumeTypeGP3, aws.StringValue(got.VolumeType)) - assert.Equal(t, int64(spxtypes.DefaultGP3IOPS), aws.Int64Value(got.Iops)) + assert.Equal(t, ebspolicy.VolumeTypeGP3, aws.StringValue(got.VolumeType)) + assert.Equal(t, int64(ebspolicy.DefaultGP3IOPS), aws.Int64Value(got.Iops)) assert.Equal(t, "snap-src", aws.StringValue(got.SnapshotId)) } diff --git a/spinifex/handlers/ec2/volume/service_impl.go b/spinifex/handlers/ec2/volume/service_impl.go index 8756ddf51..13184e670 100644 --- a/spinifex/handlers/ec2/volume/service_impl.go +++ b/spinifex/handlers/ec2/volume/service_impl.go @@ -15,16 +15,17 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/bluebottle/pkg/safecast" - "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/mulgadc/spinifex/spinifex/config" "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/metadata" - "github.com/mulgadc/spinifex/spinifex/providers/ebs" + ebspolicy "github.com/mulgadc/spinifex/spinifex/domains/ec2/ebs/policy" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" awsfilters "github.com/mulgadc/spinifex/spinifex/foundation/aws/filters" handlers_ec2_instance "github.com/mulgadc/spinifex/spinifex/handlers/ec2/instance" "github.com/mulgadc/spinifex/spinifex/objectstore" + "github.com/mulgadc/spinifex/spinifex/providers/ebs" + "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/mulgadc/spinifex/spinifex/types" "github.com/mulgadc/spinifex/spinifex/utils" - "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" ) @@ -125,10 +126,10 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV } // Validate volume type: only gp3 supported (or empty defaults to gp3) - if input.VolumeType != nil && *input.VolumeType != "" && *input.VolumeType != types.VolumeTypeGP3 { + if input.VolumeType != nil && *input.VolumeType != "" && *input.VolumeType != ebspolicy.VolumeTypeGP3 { return nil, errors.New(awserrors.ErrorUnknownVolumeType) } - volumeType := types.VolumeTypeGP3 + volumeType := ebspolicy.VolumeTypeGP3 // Validate availability zone matches this node's AZ if input.AvailabilityZone == nil || *input.AvailabilityZone == "" { @@ -180,22 +181,22 @@ func (s *VolumeServiceImpl) CreateVolume(ctx context.Context, input *ec2.CreateV // Honor caller-supplied Iops for gp3, else the 3000 baseline. The ceiling is // min(16000, 500*size) but never below the free baseline, so small volumes // still get 3000. - iops := types.DefaultGP3IOPS + iops := ebspolicy.DefaultGP3IOPS if input.Iops != nil { iops = int(*input.Iops) } - maxIOPS := min(max(int(size)*types.GP3IOPSPerGiB, types.DefaultGP3IOPS), types.MaxGP3IOPS) - if iops < types.DefaultGP3IOPS || iops > maxIOPS { + maxIOPS := min(max(int(size)*ebspolicy.GP3IOPSPerGiB, ebspolicy.DefaultGP3IOPS), ebspolicy.MaxGP3IOPS) + if iops < ebspolicy.DefaultGP3IOPS || iops > maxIOPS { 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 := types.DefaultGP3Throughput + throughput := ebspolicy.DefaultGP3Throughput if input.Throughput != nil { throughput = int(*input.Throughput) } - if throughput < types.DefaultGP3Throughput || throughput > types.MaxGP3Throughput { + if throughput < ebspolicy.DefaultGP3Throughput || throughput > ebspolicy.MaxGP3Throughput { return nil, errors.New(awserrors.ErrorInvalidParameterValue) } diff --git a/spinifex/types/ebs.go b/spinifex/types/ebs.go index 2020ddb0b..804928a75 100644 --- a/spinifex/types/ebs.go +++ b/spinifex/types/ebs.go @@ -24,22 +24,6 @@ type EBSRequest struct { HotplugPort int `json:"HotplugPort,omitempty"` } -// VolumeTypeGP3 is the only EBS volume type this platform serves. -const VolumeTypeGP3 = "gp3" - -const ( - // gp3 IOPS envelope (AWS): 3000 baseline on any size, up to 500 IOPS/GiB, - // capped at 16000. - 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 -) - // NBDTransport defines the transport type for NBD connections. type NBDTransport string