Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spot fleet request not terminating instances on destroy #17268

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
98635c2
Fix spot fleet request not terminating instances
pmalek Jan 24, 2021
23cbd86
Revert "Fix spot fleet request not terminating instances"
ewbankkit Apr 22, 2022
723cc43
Merge branch 'main' into HEAD
ewbankkit Apr 22, 2022
8868654
r/aws_spot_fleet_request: Alphabetize attributes.
ewbankkit Apr 22, 2022
def711a
r/aws_spot_fleet_request: Reorder functions.
ewbankkit Apr 22, 2022
394075b
Add 'FindSpotFleetRequestByID' and friends.
ewbankkit Apr 22, 2022
bbd42fc
Add paginators for 'DescribeSpotFleetInstances' and 'DescribeSpotFlee…
ewbankkit Apr 22, 2022
24e0047
Add 'FindSpotFleetInstancesByID' and friends.
ewbankkit Apr 22, 2022
b2249f6
Add 'FindSpotFleetRequestHistoryRecords'.
ewbankkit Apr 22, 2022
015e98a
Reorder arguments to 'deleteSpotFleetRequest'.
ewbankkit Apr 22, 2022
7e83dbc
Add and use 'CancelSpotFleetRequestsError'.
ewbankkit Apr 22, 2022
fd2bfaa
r/aws_spot_fleet_request: Tidy up resource Delete.
ewbankkit Apr 22, 2022
c9da87f
r/aws_spot_fleet_request: Ensure that launched instances are tagged i…
ewbankkit Apr 22, 2022
c8ea588
Acceptance test output:
ewbankkit Apr 22, 2022
8aa9b42
r/aws_spot_fleet_request: Tidy up resource Create.
ewbankkit Apr 25, 2022
3e3edf3
r/aws_spot_fleet_request: Tidy up resource Read.
ewbankkit Apr 25, 2022
a5da2e9
r/aws_spot_fleet_request: Tidy up resource Update.
ewbankkit Apr 25, 2022
f7a4eb3
Add 'FindImageByID' and friends.
ewbankkit Apr 25, 2022
ba53afb
r/aws_spot_fleet_request: Add 'terminate_instances_on_delete' argument.
ewbankkit Apr 25, 2022
7b7feeb
Fix terrafmt errors.
ewbankkit Apr 25, 2022
60fc241
Add 'ImageId' to test data to pass eventual consistency check.
ewbankkit Apr 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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