From fc36a37db87e6aa4b6de88f14e6d4915dd86f9c3 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 24 Mar 2020 08:44:20 -0700 Subject: [PATCH 1/2] only set NoDevice if NoEphemeral is set; otherwise, legit block device mappings get destroyed --- .../amazon/common/step_run_spot_instance.go | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/builder/amazon/common/step_run_spot_instance.go b/builder/amazon/common/step_run_spot_instance.go index a50da505f..0f9e65aaf 100644 --- a/builder/amazon/common/step_run_spot_instance.go +++ b/builder/amazon/common/step_run_spot_instance.go @@ -49,24 +49,6 @@ type StepRunSpotInstance struct { func (s *StepRunSpotInstance) CreateTemplateData(userData *string, az string, state multistep.StateBag, marketOptions *ec2.LaunchTemplateInstanceMarketOptionsRequest) *ec2.RequestLaunchTemplateData { blockDeviceMappings := s.LaunchMappings.BuildEC2BlockDeviceMappings() - if s.NoEphemeral { - // This is only relevant for windows guests. Ephemeral drives by - // default are assigned to drive names xvdca-xvdcz. - // When vms are launched from the AWS console, they're automatically - // removed from the block devices if the user hasn't said to use them, - // but the SDK does not perform this cleanup. The following code just - // manually removes the ephemeral drives from the mapping so that they - // don't clutter up console views and cause confusion. - log.Printf("no_ephemeral was set, so creating drives xvdca-xvdcz as empty mappings") - DefaultEphemeralDeviceLetters := "abcdefghijklmnopqrstuvwxyz" - for _, letter := range DefaultEphemeralDeviceLetters { - bd := &ec2.BlockDeviceMapping{ - DeviceName: aws.String("xvdc" + string(letter)), - NoDevice: aws.String(""), - } - blockDeviceMappings = append(blockDeviceMappings, bd) - } - } // Convert the BlockDeviceMapping into a // LaunchTemplateBlockDeviceMappingRequest. These structs are identical, // except for the EBS field -- on one, that field contains a @@ -80,11 +62,29 @@ func (s *StepRunSpotInstance) CreateTemplateData(userData *string, az string, launchRequest := &ec2.LaunchTemplateBlockDeviceMappingRequest{ DeviceName: mapping.DeviceName, Ebs: (*ec2.LaunchTemplateEbsBlockDeviceRequest)(mapping.Ebs), - NoDevice: mapping.NoDevice, VirtualName: mapping.VirtualName, } launchMappingRequests = append(launchMappingRequests, launchRequest) } + if s.NoEphemeral { + // This is only relevant for windows guests. Ephemeral drives by + // default are assigned to drive names xvdca-xvdcz. + // When vms are launched from the AWS console, they're automatically + // removed from the block devices if the user hasn't said to use them, + // but the SDK does not perform this cleanup. The following code just + // manually removes the ephemeral drives from the mapping so that they + // don't clutter up console views and cause confusion. + log.Printf("no_ephemeral was set, so creating drives xvdca-xvdcz as empty mappings") + DefaultEphemeralDeviceLetters := "abcdefghijklmnopqrstuvwxyz" + for _, letter := range DefaultEphemeralDeviceLetters { + launchRequest := &ec2.LaunchTemplateBlockDeviceMappingRequest{ + DeviceName: aws.String("xvdc" + string(letter)), + NoDevice: aws.String(""), + } + launchMappingRequests = append(launchMappingRequests, launchRequest) + } + + } iamInstanceProfile := aws.String(state.Get("iamInstanceProfile").(string)) From 8ae6256c03c123a239f5585d3892f3265fc2bd3b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 24 Mar 2020 09:10:31 -0700 Subject: [PATCH 2/2] add test of NoEphemeral --- .../common/step_run_spot_instance_test.go | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/builder/amazon/common/step_run_spot_instance_test.go b/builder/amazon/common/step_run_spot_instance_test.go index 9d4fa8a0d..7ecef5d3a 100644 --- a/builder/amazon/common/step_run_spot_instance_test.go +++ b/builder/amazon/common/step_run_spot_instance_test.go @@ -94,3 +94,42 @@ func TestCreateTemplateData(t *testing.T) { t.Fatalf("Template shouldn't contain instance profile if iamInstanceProfile is unset.") } } + +func TestCreateTemplateData_NoEphemeral(t *testing.T) { + state := tStateSpot() + stepRunSpotInstance := getBasicStep() + stepRunSpotInstance.NoEphemeral = true + template := stepRunSpotInstance.CreateTemplateData(aws.String("userdata"), "az", state, + &ec2.LaunchTemplateInstanceMarketOptionsRequest{}) + if len(template.BlockDeviceMappings) != 26 { + t.Fatalf("Should have created 26 mappings to keep ephemeral drives from appearing.") + } + + // Now check that noEphemeral doesn't mess with the mappings in real life. + // state = tStateSpot() + // stepRunSpotInstance = getBasicStep() + // stepRunSpotInstance.NoEphemeral = true + // mappings := []*ec2.InstanceBlockDeviceMapping{ + // &ec2.InstanceBlockDeviceMapping{ + // DeviceName: "xvda", + // Ebs: { + // DeleteOnTermination: true, + // Status: "attaching", + // VolumeId: "vol-044cd49c330f21c05", + // }, + // }, + // &ec2.InstanceBlockDeviceMapping{ + // DeviceName: "/dev/xvdf", + // Ebs: { + // DeleteOnTermination: false, + // Status: "attaching", + // VolumeId: "vol-0eefaf2d6ae35827e", + // }, + // }, + // } + // template = stepRunSpotInstance.CreateTemplateData(aws.String("userdata"), "az", state, + // &ec2.LaunchTemplateInstanceMarketOptionsRequest{}) + // if len(*template.BlockDeviceMappings) != 26 { + // t.Fatalf("Should have created 26 mappings to keep ephemeral drives from appearing.") + // } +}