From 0370b339288141c1cd4623f3d60d52cb4f7f1495 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 29 Mar 2021 19:42:26 -0400 Subject: [PATCH 1/2] resource/aws_spot_instance_request: Handle read-after-create eventual consistency Reference: https://github.com/hashicorp/terraform-provider-aws/issues/12679 Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16796 Output from acceptance testing in AWS Commercial: ``` --- PASS: TestAccAWSSpotInstanceRequest_basic (65.29s) --- PASS: TestAccAWSSpotInstanceRequest_vpc (75.75s) --- PASS: TestAccAWSSpotInstanceRequest_InterruptStop (96.45s) --- PASS: TestAccAWSSpotInstanceRequest_withBlockDuration (97.68s) --- PASS: TestAccAWSSpotInstanceRequest_withLaunchGroup (108.30s) --- PASS: TestAccAWSSpotInstanceRequest_SubnetAndSGAndPublicIpAddress (119.75s) --- PASS: TestAccAWSSpotInstanceRequest_InterruptHibernate (125.90s) --- PASS: TestAccAWSSpotInstanceRequest_NetworkInterfaceAttributes (140.51s) --- PASS: TestAccAWSSpotInstanceRequest_tags (165.36s) --- PASS: TestAccAWSSpotInstanceRequest_getPasswordData (187.44s) --- PASS: TestAccAWSSpotInstanceRequest_disappears (320.26s) --- PASS: TestAccAWSSpotInstanceRequest_withoutSpotPrice (328.46s) --- PASS: TestAccAWSSpotInstanceRequest_validUntil (338.82s) ``` Output from acceptance testing in AWS GovCloud (US): ``` --- PASS: TestAccAWSSpotInstanceRequest_withoutSpotPrice (66.13s) --- PASS: TestAccAWSSpotInstanceRequest_basic (66.24s) --- PASS: TestAccAWSSpotInstanceRequest_withBlockDuration (66.32s) --- PASS: TestAccAWSSpotInstanceRequest_InterruptHibernate (74.19s) --- PASS: TestAccAWSSpotInstanceRequest_vpc (95.40s) --- PASS: TestAccAWSSpotInstanceRequest_withLaunchGroup (107.94s) --- PASS: TestAccAWSSpotInstanceRequest_validUntil (107.98s) --- PASS: TestAccAWSSpotInstanceRequest_InterruptStop (127.16s) --- PASS: TestAccAWSSpotInstanceRequest_tags (155.38s) --- PASS: TestAccAWSSpotInstanceRequest_SubnetAndSGAndPublicIpAddress (160.34s) --- PASS: TestAccAWSSpotInstanceRequest_NetworkInterfaceAttributes (161.00s) --- PASS: TestAccAWSSpotInstanceRequest_getPasswordData (209.51s) --- PASS: TestAccAWSSpotInstanceRequest_disappears (332.58s) ``` --- .changelog/pending.txt | 3 ++ aws/internal/service/ec2/errors.go | 4 ++ aws/internal/service/ec2/finder/finder.go | 31 +++++++++++ aws/resource_aws_spot_instance_request.go | 66 ++++++++++++++++------- 4 files changed, 86 insertions(+), 18 deletions(-) create mode 100644 .changelog/pending.txt diff --git a/.changelog/pending.txt b/.changelog/pending.txt new file mode 100644 index 00000000000..c5ad43b2ef9 --- /dev/null +++ b/.changelog/pending.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_spot_instance_request: Handle read-after-create eventual consistency +``` diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go index 4414e732b06..4bb9c3030b8 100644 --- a/aws/internal/service/ec2/errors.go +++ b/aws/internal/service/ec2/errors.go @@ -46,6 +46,10 @@ const ( InvalidGroupNotFound = "InvalidGroup.NotFound" ) +const ( + ErrCodeInvalidSpotInstanceRequestIDNotFound = "InvalidSpotInstanceRequestID.NotFound" +) + const ( ErrCodeInvalidSubnetIDNotFound = "InvalidSubnetID.NotFound" ) diff --git a/aws/internal/service/ec2/finder/finder.go b/aws/internal/service/ec2/finder/finder.go index cbc272faeb1..9e7ee3fdec3 100644 --- a/aws/internal/service/ec2/finder/finder.go +++ b/aws/internal/service/ec2/finder/finder.go @@ -288,6 +288,37 @@ func SecurityGroupByID(conn *ec2.EC2, id string) (*ec2.SecurityGroup, error) { return result.SecurityGroups[0], nil } +// SpotInstanceRequestByID looks up a SpotInstanceRequest by ID. When not found, returns nil and potentially an API error. +func SpotInstanceRequestByID(conn *ec2.EC2, id string) (*ec2.SpotInstanceRequest, error) { + input := &ec2.DescribeSpotInstanceRequestsInput{ + SpotInstanceRequestIds: aws.StringSlice([]string{id}), + } + + output, err := conn.DescribeSpotInstanceRequests(input) + + if err != nil { + return nil, err + } + + if output == nil { + return nil, nil + } + + for _, spotInstanceRequest := range output.SpotInstanceRequests { + if spotInstanceRequest == nil { + continue + } + + if aws.StringValue(spotInstanceRequest.SpotInstanceRequestId) != id { + continue + } + + return spotInstanceRequest, nil + } + + return nil, nil +} + // SubnetByID looks up a Subnet by ID. When not found, returns nil and potentially an API error. func SubnetByID(conn *ec2.EC2, id string) (*ec2.Subnet, error) { input := &ec2.DescribeSubnetsInput{ diff --git a/aws/resource_aws_spot_instance_request.go b/aws/resource_aws_spot_instance_request.go index dbd94290211..907832b1d0d 100644 --- a/aws/resource_aws_spot_instance_request.go +++ b/aws/resource_aws_spot_instance_request.go @@ -9,11 +9,16 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" iamwaiter "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/iam/waiter" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) func resourceAwsSpotInstanceRequest() *schema.Resource { @@ -250,35 +255,60 @@ func resourceAwsSpotInstanceRequestRead(d *schema.ResourceData, meta interface{} conn := meta.(*AWSClient).ec2conn ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig - req := &ec2.DescribeSpotInstanceRequestsInput{ - SpotInstanceRequestIds: []*string{aws.String(d.Id())}, - } - resp, err := conn.DescribeSpotInstanceRequests(req) + var request *ec2.SpotInstanceRequest - if err != nil { - // If the spot request was not found, return nil so that we can show - // that it is gone. - if isAWSErr(err, "InvalidSpotInstanceRequestID.NotFound", "") { - log.Printf("[WARN] EC2 Spot Instance Request (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil + err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError { + var err error + + request, err = finder.SpotInstanceRequestByID(conn, d.Id()) + + if d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidSpotInstanceRequestIDNotFound) { + return resource.RetryableError(err) } - // Some other error, report it - return err + if err != nil { + return resource.NonRetryableError(err) + } + + if d.IsNewResource() && request == nil { + return resource.RetryableError(&resource.NotFoundError{ + LastError: fmt.Errorf("EC2 Spot Instance Request (%s) not found", d.Id()), + }) + } + + return nil + }) + + if tfresource.TimedOut(err) { + request, err = finder.SpotInstanceRequestByID(conn, d.Id()) } - // If nothing was found, then return no state - if len(resp.SpotInstanceRequests) == 0 { + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidSpotInstanceRequestIDNotFound) { + log.Printf("[WARN] EC2 Spot Instance Request (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return fmt.Errorf("error reading EC2 Spot Instance Request (%s): %w", d.Id(), err) + } + + if request == nil { + if d.IsNewResource() { + return fmt.Errorf("error reading EC2 Spot Instance Request (%s): not found after creation", d.Id()) + } + log.Printf("[WARN] EC2 Spot Instance Request (%s) not found, removing from state", d.Id()) d.SetId("") return nil } - request := resp.SpotInstanceRequests[0] + if aws.StringValue(request.State) == ec2.SpotInstanceStateCancelled || aws.StringValue(request.State) == ec2.SpotInstanceStateClosed { + if d.IsNewResource() { + return fmt.Errorf("error reading EC2 Spot Instance Request (%s): %s after creation", d.Id(), aws.StringValue(request.State)) + } - // if the request is cancelled or closed, then it is gone - if *request.State == ec2.SpotInstanceStateCancelled || *request.State == ec2.SpotInstanceStateClosed { + log.Printf("[WARN] EC2 Spot Instance Request (%s) %s, removing from state", d.Id(), aws.StringValue(request.State)) d.SetId("") return nil } From afd238115052fdc5e76af35f2b30a9636a2c1bea Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 29 Mar 2021 19:48:36 -0400 Subject: [PATCH 2/2] Update CHANGELOG for #18473 --- .changelog/{pending.txt => 18473.txt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .changelog/{pending.txt => 18473.txt} (100%) diff --git a/.changelog/pending.txt b/.changelog/18473.txt similarity index 100% rename from .changelog/pending.txt rename to .changelog/18473.txt