Skip to content

Commit

Permalink
Merge pull request #17268 from pmalek/spot-fleet-request-terminate-in…
Browse files Browse the repository at this point in the history
…stances

Fix spot fleet request not terminating instances on destroy
  • Loading branch information
ewbankkit authored Apr 25, 2022
2 parents b9d9b25 + 60fc241 commit 032bf0c
Show file tree
Hide file tree
Showing 12 changed files with 1,542 additions and 1,100 deletions.
3 changes: 3 additions & 0 deletions .changelog/17268.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_spot_fleet_request: Add `terminate_instances_on_delete` argument
```
22 changes: 22 additions & 0 deletions internal/service/ec2/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ const (
ErrCodeInvalidSnapshotInUse = "InvalidSnapshot.InUse"
ErrCodeInvalidSnapshotNotFound = "InvalidSnapshot.NotFound"
ErrCodeInvalidSpotDatafeedNotFound = "InvalidSpotDatafeed.NotFound"
ErrCodeInvalidSpotFleetRequestConfig = "InvalidSpotFleetRequestConfig"
ErrCodeInvalidSpotFleetRequestIdNotFound = "InvalidSpotFleetRequestId.NotFound"
ErrCodeInvalidSpotInstanceRequestIDNotFound = "InvalidSpotInstanceRequestID.NotFound"
ErrCodeInvalidSubnetCidrReservationIDNotFound = "InvalidSubnetCidrReservationID.NotFound"
ErrCodeInvalidSubnetIDNotFound = "InvalidSubnetID.NotFound"
Expand All @@ -84,6 +86,26 @@ const (
ErrCodeUnsupportedOperation = "UnsupportedOperation"
)

func CancelSpotFleetRequestError(apiObject *ec2.CancelSpotFleetRequestsErrorItem) error {
if apiObject == nil || apiObject.Error == nil {
return nil
}

return awserr.New(aws.StringValue(apiObject.Error.Code), aws.StringValue(apiObject.Error.Message), nil)
}

func CancelSpotFleetRequestsError(apiObjects []*ec2.CancelSpotFleetRequestsErrorItem) error {
var errors *multierror.Error

for _, apiObject := range apiObjects {
if err := CancelSpotFleetRequestError(apiObject); err != nil {
errors = multierror.Append(errors, fmt.Errorf("%s: %w", aws.StringValue(apiObject.SpotFleetRequestId), err))
}
}

return errors.ErrorOrNil()
}

func UnsuccessfulItemError(apiObject *ec2.UnsuccessfulItemError) error {
if apiObject == nil {
return nil
Expand Down
192 changes: 192 additions & 0 deletions internal/service/ec2/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,59 @@ func FindHost(conn *ec2.EC2, input *ec2.DescribeHostsInput) (*ec2.Host, error) {
return host, nil
}

func FindImage(conn *ec2.EC2, input *ec2.DescribeImagesInput) (*ec2.Image, error) {
output, err := conn.DescribeImages(input)

if tfawserr.ErrCodeEquals(err, ErrCodeInvalidAMIIDNotFound) {
return nil, &resource.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

if output == nil || len(output.Images) == 0 || output.Images[0] == nil {
return nil, tfresource.NewEmptyResultError(input)
}

if count := len(output.Images); count > 1 {
return nil, tfresource.NewTooManyResultsError(count, input)
}

return output.Images[0], nil
}

func FindImageByID(conn *ec2.EC2, id string) (*ec2.Image, error) {
input := &ec2.DescribeImagesInput{
ImageIds: aws.StringSlice([]string{id}),
}

output, err := FindImage(conn, input)

if err != nil {
return nil, err
}

if state := aws.StringValue(output.State); state == ec2.ImageStateDeregistered {
return nil, &resource.NotFoundError{
Message: state,
LastRequest: input,
}
}

// Eventual consistency check.
if aws.StringValue(output.ImageId) != id {
return nil, &resource.NotFoundError{
LastRequest: input,
}
}

return output, nil
}

func FindImageAttribute(ctx context.Context, conn *ec2.EC2, input *ec2.DescribeImageAttributeInput) (*ec2.DescribeImageAttributeOutput, error) {
output, err := conn.DescribeImageAttributeWithContext(ctx, input)

Expand Down Expand Up @@ -1551,6 +1604,145 @@ func FindSecurityGroups(conn *ec2.EC2, input *ec2.DescribeSecurityGroupsInput) (
return output, nil
}

func FindSpotFleetInstances(conn *ec2.EC2, input *ec2.DescribeSpotFleetInstancesInput) ([]*ec2.ActiveInstance, error) {
var output []*ec2.ActiveInstance

err := describeSpotFleetInstancesPages(conn, input, func(page *ec2.DescribeSpotFleetInstancesOutput, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, v := range page.ActiveInstances {
if v != nil {
output = append(output, v)
}
}

return !lastPage
})

if tfawserr.ErrCodeEquals(err, ErrCodeInvalidSpotFleetRequestIdNotFound) {
return nil, &resource.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

return output, nil
}

func FindSpotFleetRequests(conn *ec2.EC2, input *ec2.DescribeSpotFleetRequestsInput) ([]*ec2.SpotFleetRequestConfig, error) {
var output []*ec2.SpotFleetRequestConfig

err := conn.DescribeSpotFleetRequestsPages(input, func(page *ec2.DescribeSpotFleetRequestsOutput, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, v := range page.SpotFleetRequestConfigs {
if v != nil {
output = append(output, v)
}
}

return !lastPage
})

if tfawserr.ErrCodeEquals(err, ErrCodeInvalidSpotFleetRequestIdNotFound) {
return nil, &resource.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

return output, nil
}

func FindSpotFleetRequest(conn *ec2.EC2, input *ec2.DescribeSpotFleetRequestsInput) (*ec2.SpotFleetRequestConfig, error) {
output, err := FindSpotFleetRequests(conn, input)

if err != nil {
return nil, err
}

if len(output) == 0 || output[0] == nil || output[0].SpotFleetRequestConfig == nil {
return nil, tfresource.NewEmptyResultError(input)
}

if count := len(output); count > 1 {
return nil, tfresource.NewTooManyResultsError(count, input)
}

return output[0], nil
}

func FindSpotFleetRequestByID(conn *ec2.EC2, id string) (*ec2.SpotFleetRequestConfig, error) {
input := &ec2.DescribeSpotFleetRequestsInput{
SpotFleetRequestIds: aws.StringSlice([]string{id}),
}

output, err := FindSpotFleetRequest(conn, input)

if err != nil {
return nil, err
}

if state := aws.StringValue(output.SpotFleetRequestState); state == ec2.BatchStateCancelled || state == ec2.BatchStateCancelledRunning || state == ec2.BatchStateCancelledTerminating {
return nil, &resource.NotFoundError{
Message: state,
LastRequest: input,
}
}

// Eventual consistency check.
if aws.StringValue(output.SpotFleetRequestId) != id {
return nil, &resource.NotFoundError{
LastRequest: input,
}
}

return output, nil
}

func FindSpotFleetRequestHistoryRecords(conn *ec2.EC2, input *ec2.DescribeSpotFleetRequestHistoryInput) ([]*ec2.HistoryRecord, error) {
var output []*ec2.HistoryRecord

err := describeSpotFleetRequestHistoryPages(conn, input, func(page *ec2.DescribeSpotFleetRequestHistoryOutput, lastPage bool) bool {
if page == nil {
return !lastPage
}

for _, v := range page.HistoryRecords {
if v != nil {
output = append(output, v)
}
}

return !lastPage
})

if tfawserr.ErrCodeEquals(err, ErrCodeInvalidSpotFleetRequestIdNotFound) {
return nil, &resource.NotFoundError{
LastError: err,
LastRequest: input,
}
}

if err != nil {
return nil, err
}

return output, nil
}

func FindSpotInstanceRequests(conn *ec2.EC2, input *ec2.DescribeSpotInstanceRequestsInput) ([]*ec2.SpotInstanceRequest, error) {
var output []*ec2.SpotInstanceRequest

Expand Down
1 change: 1 addition & 0 deletions internal/service/ec2/generate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//go:generate go run ../../generate/tagresource/main.go -IDAttribName=resource_id
//go:generate go run ../../generate/tags/main.go -GetTag -ListTags -ListTagsOp=DescribeTags -ListTagsInFiltIDName=resource-id -ListTagsInIDElem=Resources -ServiceTagsSlice -TagOp=CreateTags -TagInIDElem=Resources -TagInIDNeedSlice=yes -TagType2=TagDescription -UntagOp=DeleteTags -UntagInNeedTagType -UntagInTagsElem=Tags -UpdateTags
//go:generate go run generate/createtags/main.go
//go:generate go run ../../generate/listpages/main.go -ListOps=DescribeSpotFleetInstances,DescribeSpotFleetRequestHistory
// ONLY generate directives and package declaration! Do not add anything else to this file.

package ec2
30 changes: 11 additions & 19 deletions internal/service/ec2/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -2001,25 +2001,17 @@ func fetchLaunchTemplateAmi(specs []interface{}, conn *ec2.EC2) (string, error)
return "", nil
}

func FetchRootDeviceName(ami string, conn *ec2.EC2) (*string, error) {
if ami == "" {
func FetchRootDeviceName(conn *ec2.EC2, amiID string) (*string, error) {
if amiID == "" {
return nil, errors.New("Cannot fetch root device name for blank AMI ID.")
}

log.Printf("[DEBUG] Describing AMI %q to get root block device name", ami)
res, err := conn.DescribeImages(&ec2.DescribeImagesInput{
ImageIds: []*string{aws.String(ami)},
})
image, err := FindImageByID(conn, amiID)

if err != nil {
return nil, err
}

// For a bad image, we just return nil so we don't block a refresh
if len(res.Images) == 0 {
return nil, nil
}

image := res.Images[0]
rootDeviceName := image.RootDeviceName

// Instance store backed AMIs do not provide a root device name.
Expand Down Expand Up @@ -2050,7 +2042,7 @@ func FetchRootDeviceName(ami string, conn *ec2.EC2) (*string, error) {
}

if rootDeviceName == nil {
return nil, fmt.Errorf("Error finding Root Device Name for AMI (%s)", ami)
return nil, fmt.Errorf("Error finding Root Device Name for AMI (%s)", amiID)
}

return rootDeviceName, nil
Expand Down Expand Up @@ -2255,29 +2247,29 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([
}
}

var ami string
var amiID string
if v, ok := d.GetOk("launch_template"); ok {
var err error
ami, err = fetchLaunchTemplateAmi(v.([]interface{}), conn)
amiID, err = fetchLaunchTemplateAmi(v.([]interface{}), conn)
if err != nil {
return nil, err
}
}

// AMI id from attributes overrides ami from launch template
if v, ok := d.GetOk("ami"); ok {
ami = v.(string)
amiID = v.(string)
}

if ami == "" {
if amiID == "" {
return nil, errors.New("`ami` must be set or provided via launch template")
}

if dn, err := FetchRootDeviceName(ami, conn); err == nil {
if dn, err := FetchRootDeviceName(conn, amiID); err == nil {
if dn == nil {
return nil, fmt.Errorf(
"Expected 1 AMI for ID: %s, got none",
ami)
amiID)
}

blockDevices = append(blockDevices, &ec2.BlockDeviceMapping{
Expand Down
4 changes: 3 additions & 1 deletion internal/service/ec2/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestFetchRootDevice(t *testing.T) {
{
"device name in mappings",
[]*ec2.Image{{
ImageId: aws.String("ami-123"),
RootDeviceType: aws.String("ebs"),
RootDeviceName: aws.String("/dev/xvda"),
BlockDeviceMappings: []*ec2.BlockDeviceMapping{
Expand All @@ -58,6 +59,7 @@ func TestFetchRootDevice(t *testing.T) {
{
"device name not in mappings",
[]*ec2.Image{{
ImageId: aws.String("ami-123"),
RootDeviceType: aws.String("ebs"),
RootDeviceName: aws.String("/dev/xvda"),
BlockDeviceMappings: []*ec2.BlockDeviceMapping{
Expand Down Expand Up @@ -88,7 +90,7 @@ func TestFetchRootDevice(t *testing.T) {
data := r.Data.(*ec2.DescribeImagesOutput)
data.Images = tc.images
})
name, _ := tfec2.FetchRootDeviceName("ami-123", conn)
name, _ := tfec2.FetchRootDeviceName(conn, "ami-123")
if tc.name != aws.StringValue(name) {
t.Errorf("Expected name %s, got %s", tc.name, aws.StringValue(name))
}
Expand Down
Loading

0 comments on commit 032bf0c

Please sign in to comment.