diff --git a/spinifex/handlers/ec2/instance/provider_root_volume_test.go b/spinifex/handlers/ec2/instance/provider_root_volume_test.go index 687821061..3b4dc52d8 100644 --- a/spinifex/handlers/ec2/instance/provider_root_volume_test.go +++ b/spinifex/handlers/ec2/instance/provider_root_volume_test.go @@ -88,7 +88,7 @@ func TestPrepareRootVolume_Provider_CreatesFromAMISnapshot(t *testing.T) { instance := &vm.VM{AccountID: testRootAccount} input := &ec2.RunInstancesInput{ImageId: aws.String("ami-1")} - err := svc.prepareRootVolume(context.Background(), input, "vol-root", testRootVolumeBytes, 0, + err := svc.prepareRootVolume(context.Background(), input, "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, false) require.NoError(t, err) @@ -100,10 +100,39 @@ func TestPrepareRootVolume_Provider_CreatesFromAMISnapshot(t *testing.T) { require.Len(t, instance.EBSRequests.Requests, 1) assert.Equal(t, "vol-root", instance.EBSRequests.Requests[0].Name) + assert.Equal(t, ebsmetadata.RootDeviceName, instance.EBSRequests.Requests[0].DeviceName, + "the launcher writes this to the volume's metadata, which DescribeVolumes reports as the attachment device") assert.True(t, instance.EBSRequests.Requests[0].Boot, "the root volume must be marked bootable") assert.False(t, instance.EBSRequests.Requests[0].DeleteOnTermination) } +// TestPrepareRootVolume_Provider_AppliesVolumeTags locks that a launch-time +// volume TagSpecification reaches the root volume's metadata document, which is +// the only place DescribeVolumes reads tags from. +func TestPrepareRootVolume_Provider_AppliesVolumeTags(t *testing.T) { + defer goleak.VerifyNone(t) + + provider := ebsprovider.NewMemoryProvider(ebsprovider.Capabilities{}) + seedProviderSnapshot(t, provider, "vol-origin", "snap-source") + store := objectstore.NewMemoryObjectStore() + svc := providerRootVolumeService(t, provider, rootVolumeAMILoader(), store) + + input := &ec2.RunInstancesInput{ + ImageId: aws.String("ami-1"), + TagSpecifications: []*ec2.TagSpecification{ + {ResourceType: aws.String("instance"), Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("node")}}}, + {ResourceType: aws.String("volume"), Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("root-disk")}}}, + }, + } + err := svc.prepareRootVolume(context.Background(), input, "vol-root", ebsmetadata.RootDeviceName, + testRootVolumeBytes, 0, &vm.VM{AccountID: testRootAccount}, true) + require.NoError(t, err) + + doc, err := ebsmetadata.NewStore(store, testRootBucket).GetVolume(context.Background(), testRootAccount, "vol-root") + require.NoError(t, err) + assert.Equal(t, map[string]string{"Name": "root-disk"}, doc.Tags) +} + // TestPrepareRootVolume_Provider_WritesMetadataDocument locks the control-plane // record of the root volume. Without it the volume has neither a document nor a // legacy config.json, so DescribeVolumes cannot see it at all. @@ -117,7 +146,7 @@ func TestPrepareRootVolume_Provider_WritesMetadataDocument(t *testing.T) { instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.NoError(t, err) doc, err := ebsmetadata.NewStore(store, testRootBucket).GetVolume(context.Background(), testRootAccount, "vol-root") @@ -151,7 +180,7 @@ func TestPrepareRootVolume_Provider_HonoursRequestedIOPS(t *testing.T) { svc := providerRootVolumeService(t, provider, rootVolumeAMILoader(), store) err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 5000, &vm.VM{AccountID: testRootAccount}, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 5000, &vm.VM{AccountID: testRootAccount}, true) require.NoError(t, err) doc, err := ebsmetadata.NewStore(store, testRootBucket).GetVolume(context.Background(), testRootAccount, "vol-root") @@ -190,7 +219,7 @@ func TestPrepareRootVolume_Provider_RollbackOnMetadataWriteFailure(t *testing.T) instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.Error(t, err) assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) assert.Empty(t, instance.EBSRequests.Requests, "a failed launch must not claim a boot volume") @@ -214,7 +243,7 @@ func TestPrepareRootVolume_Provider_RepeatIsIdempotent(t *testing.T) { for range 2 { instance := &vm.VM{AccountID: testRootAccount} - err := svc.prepareRootVolume(context.Background(), input, "vol-root", testRootVolumeBytes, 0, + err := svc.prepareRootVolume(context.Background(), input, "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.NoError(t, err) require.Len(t, instance.EBSRequests.Requests, 1) @@ -233,7 +262,7 @@ func TestPrepareRootVolume_Provider_AMIWithoutSnapshotFails(t *testing.T) { instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.Error(t, err) assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) assert.Empty(t, instance.EBSRequests.Requests, "a failed launch must not claim a boot volume") @@ -254,7 +283,7 @@ func TestPrepareRootVolume_Provider_UnknownAMIFails(t *testing.T) { instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-gone")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.Error(t, err) assert.Equal(t, awserrors.ErrorInvalidAMIIDNotFound, err.Error()) assert.Empty(t, instance.EBSRequests.Requests) @@ -275,7 +304,7 @@ func TestPrepareRootVolume_Provider_SourceVolumeUnresolvableFails(t *testing.T) instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.Error(t, err) assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) assert.Empty(t, instance.EBSRequests.Requests) @@ -300,7 +329,7 @@ func TestPrepareRootVolume_Provider_CreateFailureIsFatal(t *testing.T) { instance := &vm.VM{AccountID: testRootAccount} err := svc.prepareRootVolume(context.Background(), &ec2.RunInstancesInput{ImageId: aws.String("ami-1")}, - "vol-root", testRootVolumeBytes, 0, instance, true) + "vol-root", ebsmetadata.RootDeviceName, testRootVolumeBytes, 0, instance, true) require.Error(t, err) assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) assert.Empty(t, instance.EBSRequests.Requests) diff --git a/spinifex/handlers/ec2/instance/service_impl.go b/spinifex/handlers/ec2/instance/service_impl.go index ad0e35888..df9e50211 100644 --- a/spinifex/handlers/ec2/instance/service_impl.go +++ b/spinifex/handlers/ec2/instance/service_impl.go @@ -303,6 +303,12 @@ func (s *InstanceServiceImpl) RunInstance(input *ec2.RunInstancesInput) (*vm.VM, ec2Instance.SetArchitecture(arch) } + // Every image this platform serves boots from an EBS root at the same + // device, and DescribeImages already reports both. DescribeInstances left + // them null, so a caller comparing the two read a disagreement. + ec2Instance.SetRootDeviceName(ebsmetadata.RootDeviceName) + ec2Instance.SetRootDeviceType(ec2.DeviceTypeEbs) + // Stamp the metadata-options block so DescribeInstances reports the posture; // the hop limit and the IMDSv1 opt-in are request-driven. var hopLimit *int64 @@ -343,7 +349,7 @@ func (s *InstanceServiceImpl) RunInstance(input *ec2.RunInstancesInput) (*vm.VM, // instanceTagsFromSpec extracts the instance-scoped tags from a RunInstances // TagSpecifications list. Only ResourceType "instance" applies to the launched -// instance; volume/network-interface specs are handled elsewhere. +// instance; the volume specs go to the root volume's metadata. func instanceTagsFromSpec(specs []*ec2.TagSpecification) []*ec2.Tag { var tags []*ec2.Tag for _, spec := range specs { @@ -360,6 +366,28 @@ func instanceTagsFromSpec(specs []*ec2.TagSpecification) []*ec2.Tag { return tags } +// volumeTagsFromSpec extracts the volume-scoped tags from a RunInstances +// TagSpecifications list, in the shape ebsmetadata records them. Only the root +// volume exists at launch, so they all land on it. +func volumeTagsFromSpec(specs []*ec2.TagSpecification) map[string]string { + var tags map[string]string + for _, spec := range specs { + if spec == nil || aws.StringValue(spec.ResourceType) != ec2.ResourceTypeVolume { + continue + } + for _, t := range spec.Tags { + if t == nil || aws.StringValue(t.Key) == "" { + continue + } + if tags == nil { + tags = make(map[string]string, len(spec.Tags)) + } + tags[aws.StringValue(t.Key)] = aws.StringValue(t.Value) + } + } + return tags +} + // ApplyInstanceTagMutation applies a set or remove tag mutation to a tag list, // mirroring the central tag store's merge, value-match delete, and clear-all // semantics, and returns the resulting list sorted by key. @@ -1475,7 +1503,7 @@ func (s *InstanceServiceImpl) GenerateVolumes(ctx context.Context, input *ec2.Ru deleteOnTermination := p.deleteOnTermination // Step 1: Create or validate root volume - err := s.prepareRootVolume(ctx, input, imageId, size, p.iops, instance, deleteOnTermination) + err := s.prepareRootVolume(ctx, input, imageId, deviceName, size, p.iops, instance, deleteOnTermination) if err != nil { return nil, err } @@ -1505,7 +1533,7 @@ func (s *InstanceServiceImpl) GenerateVolumes(ctx context.Context, input *ec2.Ru // prepareRootVolume allocates the root volume through the EBS provider and // records the boot volume the launcher must attach. -func (s *InstanceServiceImpl) prepareRootVolume(ctx context.Context, input *ec2.RunInstancesInput, imageId string, size, iops int, instance *vm.VM, deleteOnTermination bool) error { +func (s *InstanceServiceImpl) prepareRootVolume(ctx context.Context, input *ec2.RunInstancesInput, imageId, deviceName string, size, iops int, instance *vm.VM, deleteOnTermination bool) error { if s.ebsProvider == nil { slog.ErrorContext(ctx, "no EBS provider configured", "op", "prepareRootVolume") return errors.New(awserrors.ErrorServerInternal) @@ -1517,18 +1545,22 @@ func (s *InstanceServiceImpl) prepareRootVolume(ctx context.Context, input *ec2. sizeBytes: size, iops: iops, deleteOnTermination: deleteOnTermination, + tags: volumeTagsFromSpec(input.TagSpecifications), }); err != nil { return err } - appendRootEBSRequest(instance, imageId, deleteOnTermination) + appendRootEBSRequest(instance, imageId, deviceName, deleteOnTermination) return nil } -// appendRootEBSRequest records the boot volume the launcher must attach. -func appendRootEBSRequest(instance *vm.VM, volumeID string, deleteOnTermination bool) { +// appendRootEBSRequest records the boot volume the launcher must attach. The +// device name travels with it because the launcher writes it to the volume's +// metadata, which is what DescribeVolumes reports as Attachments[].Device. +func appendRootEBSRequest(instance *vm.VM, volumeID, deviceName string, deleteOnTermination bool) { instance.EBSRequests.Mu.Lock() instance.EBSRequests.Requests = append(instance.EBSRequests.Requests, spxtypes.EBSRequest{ Name: volumeID, + DeviceName: deviceName, Boot: true, DeleteOnTermination: deleteOnTermination, }) @@ -1544,6 +1576,7 @@ type rootVolumeSpec struct { sizeBytes int iops int deleteOnTermination bool + tags map[string]string } // createRootVolumeViaProvider allocates the root volume through the injected @@ -1608,6 +1641,7 @@ func (s *InstanceServiceImpl) createRootVolumeViaProvider(ctx context.Context, s VolumeType: spxtypes.VolumeTypeGP3, IOPS: rootVolumeIOPS(spec.iops), Throughput: spxtypes.DefaultGP3Throughput, SnapshotID: amiConfig.SnapshotID, DeleteOnTermination: spec.deleteOnTermination, Encrypted: mkey != nil, + Tags: spec.tags, ProviderHandle: created.Handle, }); err != nil { slog.ErrorContext(ctx, "Failed to persist root volume metadata", "volumeId", spec.volumeID, "err", err) diff --git a/spinifex/handlers/ec2/instance/service_impl_test.go b/spinifex/handlers/ec2/instance/service_impl_test.go index 473c2dfb8..ad1ff2640 100644 --- a/spinifex/handlers/ec2/instance/service_impl_test.go +++ b/spinifex/handlers/ec2/instance/service_impl_test.go @@ -106,6 +106,47 @@ func TestRunInstance_ArchitecturePopulated(t *testing.T) { } } +func TestRunInstance_RootDeviceFieldsPopulated(t *testing.T) { + svc := &InstanceServiceImpl{ + instanceTypes: map[string]*ec2.InstanceTypeInfo{"t3.micro": {InstanceType: aws.String("t3.micro")}}, + } + + _, ec2Instance, err := svc.RunInstance(&ec2.RunInstancesInput{ + ImageId: aws.String("ami-0abcdef1234567890"), + InstanceType: aws.String("t3.micro"), + }) + require.NoError(t, err) + assert.Equal(t, ebsmetadata.RootDeviceName, aws.StringValue(ec2Instance.RootDeviceName)) + assert.Equal(t, ec2.DeviceTypeEbs, aws.StringValue(ec2Instance.RootDeviceType)) +} + +func TestVolumeTagsFromSpec(t *testing.T) { + specs := []*ec2.TagSpecification{ + {ResourceType: aws.String("instance"), Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("node")}}}, + {ResourceType: aws.String("volume"), Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("root")}, + {Key: aws.String("env"), Value: aws.String("test")}, + {Key: aws.String(""), Value: aws.String("dropped")}, + }}, + } + + assert.Equal(t, map[string]string{"Name": "root", "env": "test"}, volumeTagsFromSpec(specs)) + assert.Nil(t, volumeTagsFromSpec(nil)) + assert.Nil(t, volumeTagsFromSpec([]*ec2.TagSpecification{specs[0]})) +} + +func TestAppendRootEBSRequest_RecordsDeviceName(t *testing.T) { + instance := &vm.VM{} + appendRootEBSRequest(instance, "vol-1", ebsmetadata.RootDeviceName, true) + + require.Len(t, instance.EBSRequests.Requests, 1) + req := instance.EBSRequests.Requests[0] + assert.Equal(t, "vol-1", req.Name) + assert.Equal(t, ebsmetadata.RootDeviceName, req.DeviceName) + assert.True(t, req.Boot) + assert.True(t, req.DeleteOnTermination) +} + func TestRunInstance_WithIamInstanceProfile(t *testing.T) { const profileARN = "arn:aws:iam::111122223333:instance-profile/app-profile" svc := &InstanceServiceImpl{