From 7e769318b2d9e285f1702d941cbcd3f5c222fd14 Mon Sep 17 00:00:00 2001 From: Elie Date: Thu, 15 Apr 2021 12:06:09 +0200 Subject: [PATCH 1/9] Add instance shutdown behavior retrieval --- aws/resource_aws_instance.go | 23 +++++++++++++++++++++++ aws/resource_aws_instance_test.go | 1 + 2 files changed, 24 insertions(+) diff --git a/aws/resource_aws_instance.go b/aws/resource_aws_instance.go index 712e905e27d..5fe02c8ac11 100644 --- a/aws/resource_aws_instance.go +++ b/aws/resource_aws_instance.go @@ -273,6 +273,7 @@ func resourceAwsInstance() *schema.Resource { "instance_initiated_shutdown_behavior": { Type: schema.TypeString, Optional: true, + Computed: true, }, "instance_state": { Type: schema.TypeString, @@ -940,6 +941,11 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { return err } + // Retrieve instance shutdown behavior + if err := readInstanceShutdownBehavior(d, conn); err != nil { + return err + } + if err := readBlockDevices(d, instance, conn); err != nil { return err } @@ -2188,6 +2194,23 @@ func readSecurityGroups(d *schema.ResourceData, instance *ec2.Instance, conn *ec return nil } +func readInstanceShutdownBehavior(d *schema.ResourceData, conn *ec2.EC2) error { + out, err := conn.DescribeInstanceAttribute(&ec2.DescribeInstanceAttributeInput{ + InstanceId: aws.String(d.Id()), + Attribute: aws.String("instanceInitiatedShutdownBehavior"), + }) + + if err != nil { + return err + } + + if err = d.Set("instance_initiated_shutdown_behavior", out.InstanceInitiatedShutdownBehavior.Value); err != nil { + return err + } + + return nil +} + func getAwsEc2InstancePasswordData(instanceID string, conn *ec2.EC2) (string, error) { log.Printf("[INFO] Reading password data for instance %s", instanceID) diff --git a/aws/resource_aws_instance_test.go b/aws/resource_aws_instance_test.go index 5a41699cc20..6ca9497ad65 100644 --- a/aws/resource_aws_instance_test.go +++ b/aws/resource_aws_instance_test.go @@ -258,6 +258,7 @@ func TestAccAWSInstance_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckInstanceExists(resourceName, &v), testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`instance/i-[a-z0-9]+`)), + resource.TestCheckResourceAttr(resourceName, "instance_initiated_shutdown_behavior", "stop"), ), }, { From 358e199dd6e3809331f2642f52995896736594db Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 22:54:37 -0400 Subject: [PATCH 2/9] r/instance: Add changelog --- .changelog/18880.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/18880.txt diff --git a/.changelog/18880.txt b/.changelog/18880.txt new file mode 100644 index 00000000000..4dcdbba5d44 --- /dev/null +++ b/.changelog/18880.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_instance: Make `instance_initiated_shutdown_behavior` also computed to read value +``` From 421206e79718d714c6250bfb20bdcf7b6f4138f8 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 22:55:40 -0400 Subject: [PATCH 3/9] r/instance: Minor clean up --- aws/resource_aws_instance.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_instance.go b/aws/resource_aws_instance.go index 5fe02c8ac11..b08a6abff36 100644 --- a/aws/resource_aws_instance.go +++ b/aws/resource_aws_instance.go @@ -1542,6 +1542,21 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { func resourceAwsInstanceDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn + // false = enable api termination + // true = disable api termination (protected) + if !d.Get("disable_api_termination").(bool) { + _, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(d.Id()), + DisableApiTermination: &ec2.AttributeBooleanValue{ + Value: aws.Bool(d.Get("disable_api_termination").(bool)), + }, + }) + + if err != nil { + log.Printf("[WARN] attempting to terminate EC2 instance (%s) despite error enabling API termination: %s", d.Id(), err) + } + } + err := awsTerminateInstance(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) if err != nil { @@ -2195,17 +2210,17 @@ func readSecurityGroups(d *schema.ResourceData, instance *ec2.Instance, conn *ec } func readInstanceShutdownBehavior(d *schema.ResourceData, conn *ec2.EC2) error { - out, err := conn.DescribeInstanceAttribute(&ec2.DescribeInstanceAttributeInput{ + output, err := conn.DescribeInstanceAttribute(&ec2.DescribeInstanceAttributeInput{ InstanceId: aws.String(d.Id()), - Attribute: aws.String("instanceInitiatedShutdownBehavior"), + Attribute: aws.String(ec2.InstanceAttributeNameInstanceInitiatedShutdownBehavior), }) if err != nil { - return err + return fmt.Errorf("error while describing instance (%s) attribute (%s): %w", d.Id(), ec2.InstanceAttributeNameInstanceInitiatedShutdownBehavior, err) } - if err = d.Set("instance_initiated_shutdown_behavior", out.InstanceInitiatedShutdownBehavior.Value); err != nil { - return err + if output != nil && output.InstanceInitiatedShutdownBehavior != nil { + d.Set("instance_initiated_shutdown_behavior", output.InstanceInitiatedShutdownBehavior.Value) } return nil From e7a4f9c323b1f39c5466550be9d76d85b02b15b6 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 22:56:37 -0400 Subject: [PATCH 4/9] tests/r/instance: Add sweeper concurrency --- aws/resource_aws_instance_test.go | 59 +++++++++++++++---------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/aws/resource_aws_instance_test.go b/aws/resource_aws_instance_test.go index 6ca9497ad65..d887eb19b69 100644 --- a/aws/resource_aws_instance_test.go +++ b/aws/resource_aws_instance_test.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ec2" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -42,18 +43,25 @@ func testAccErrorCheckSkipEC2(t *testing.T) resource.ErrorCheckFunc { func testSweepInstances(region string) error { client, err := sharedClientForRegion(region) + if err != nil { return fmt.Errorf("error getting client: %s", err) } + conn := client.(*AWSClient).ec2conn + sweepResources := make([]*testSweepResource, 0) + var errs *multierror.Error err = conn.DescribeInstancesPages(&ec2.DescribeInstancesInput{}, func(page *ec2.DescribeInstancesOutput, lastPage bool) bool { - if len(page.Reservations) == 0 { - log.Print("[DEBUG] No EC2 Instances to sweep") - return false + if page == nil { + return !lastPage } for _, reservation := range page.Reservations { + if reservation == nil { + continue + } + for _, instance := range reservation.Instances { id := aws.StringValue(instance.InstanceId) @@ -62,42 +70,31 @@ func testSweepInstances(region string) error { continue } - log.Printf("[INFO] Terminating EC2 Instance: %s", id) - err := awsTerminateInstance(conn, id, 5*time.Minute) - - if isAWSErr(err, "OperationNotPermitted", "Modify its 'disableApiTermination' instance attribute and try again.") { - log.Printf("[INFO] Enabling API Termination on EC2 Instance: %s", id) + r := resourceAwsInstance() + d := r.Data(nil) + d.SetId(id) + d.Set("disable_api_termination", false) - input := &ec2.ModifyInstanceAttributeInput{ - InstanceId: instance.InstanceId, - DisableApiTermination: &ec2.AttributeBooleanValue{ - Value: aws.Bool(false), - }, - } - - _, err = conn.ModifyInstanceAttribute(input) - - if err == nil { - err = awsTerminateInstance(conn, id, 5*time.Minute) - } - } - - if err != nil { - log.Printf("[ERROR] Error terminating EC2 Instance (%s): %s", id, err) - } + sweepResources = append(sweepResources, NewTestSweepResource(r, d, client)) } } return !lastPage }) + if err != nil { - if testSweepSkipSweepError(err) { - log.Printf("[WARN] Skipping EC2 Instance sweep for %s: %s", region, err) - return nil - } - return fmt.Errorf("Error retrieving EC2 Instances: %s", err) + errs = multierror.Append(errs, fmt.Errorf("error describing EC2 Instances for %s: %w", region, err)) } - return nil + if err = testSweepResourceOrchestrator(sweepResources); err != nil { + errs = multierror.Append(errs, fmt.Errorf("error sweeping EC2 Instances for %s: %w", region, err)) + } + + if testSweepSkipSweepError(errs.ErrorOrNil()) { + log.Printf("[WARN] Skipping EC2 Instance sweep for %s: %s", region, errs) + return nil + } + + return errs.ErrorOrNil() } func TestFetchRootDevice(t *testing.T) { From 82d7a5a69bbac523bb99c1f3ba10d77fbae210dd Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 22:59:22 -0400 Subject: [PATCH 5/9] r/instance: Update changelog --- .changelog/18880.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/18880.txt b/.changelog/18880.txt index 4dcdbba5d44..3299d7e032c 100644 --- a/.changelog/18880.txt +++ b/.changelog/18880.txt @@ -1,3 +1,3 @@ ```release-note:enhancement -resource/aws_instance: Make `instance_initiated_shutdown_behavior` also computed to read value +resource/aws_instance: Make `instance_initiated_shutdown_behavior` also computed, allowing value to be read ``` From 0a116d70949241082f36833676962bc16395d70d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 23:36:46 -0400 Subject: [PATCH 6/9] r/instance: Clean up disableAPITermination attribute update --- aws/resource_aws_instance.go | 50 +++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/aws/resource_aws_instance.go b/aws/resource_aws_instance.go index b08a6abff36..0942c55faf3 100644 --- a/aws/resource_aws_instance.go +++ b/aws/resource_aws_instance.go @@ -1324,14 +1324,10 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { } if d.HasChange("disable_api_termination") && !d.IsNewResource() { - _, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ - InstanceId: aws.String(d.Id()), - DisableApiTermination: &ec2.AttributeBooleanValue{ - Value: aws.Bool(d.Get("disable_api_termination").(bool)), - }, - }) + err := resourceAwsInstanceDisableAPITermination(conn, d.Id(), d.Get("disable_api_termination").(bool)) + if err != nil { - return err + return fmt.Errorf("error modifying instance (%s) attribute (%s): %w", d.Id(), ec2.InstanceAttributeNameDisableApiTermination, err) } } @@ -1542,22 +1538,13 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { func resourceAwsInstanceDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).ec2conn - // false = enable api termination - // true = disable api termination (protected) - if !d.Get("disable_api_termination").(bool) { - _, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ - InstanceId: aws.String(d.Id()), - DisableApiTermination: &ec2.AttributeBooleanValue{ - Value: aws.Bool(d.Get("disable_api_termination").(bool)), - }, - }) + err := resourceAwsInstanceDisableAPITermination(conn, d.Id(), d.Get("disable_api_termination").(bool)) - if err != nil { - log.Printf("[WARN] attempting to terminate EC2 instance (%s) despite error enabling API termination: %s", d.Id(), err) - } + if err != nil { + log.Printf("[WARN] attempting to terminate EC2 instance (%s) despite error enabling API termination: %s", d.Id(), err) } - err := awsTerminateInstance(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) + err = awsTerminateInstance(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) if err != nil { return fmt.Errorf("error terminating EC2 Instance (%s): %s", d.Id(), err) @@ -1566,6 +1553,29 @@ func resourceAwsInstanceDelete(d *schema.ResourceData, meta interface{}) error { return nil } +func resourceAwsInstanceDisableAPITermination(conn *ec2.EC2, id string, disableAPITermination bool) error { + // false = enable api termination + // true = disable api termination (protected) + + _, err := conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(id), + DisableApiTermination: &ec2.AttributeBooleanValue{ + Value: aws.Bool(disableAPITermination), + }, + }) + + if tfawserr.ErrMessageContains(err, "UnsupportedOperation", "not supported for spot instances") { + log.Printf("[WARN] failed to modify instance (%s) attribute (%s): %s", id, ec2.InstanceAttributeNameDisableApiTermination, err) + return nil + } + + if err != nil { + return fmt.Errorf("error modify instance (%s) attribute (%s) to value %t: %w", id, ec2.InstanceAttributeNameDisableApiTermination, disableAPITermination, err) + } + + return nil +} + // InstanceStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch // an EC2 instance. func InstanceStateRefreshFunc(conn *ec2.EC2, instanceID string, failStates []string) resource.StateRefreshFunc { From 0942715b638a8a356a032c35ccc1e75f5522fb67 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 23:37:10 -0400 Subject: [PATCH 7/9] tests/r/spot_fleet_request: Add sweeper concurrency --- aws/resource_aws_spot_fleet_request_test.go | 40 +++++++++++++++------ 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/aws/resource_aws_spot_fleet_request_test.go b/aws/resource_aws_spot_fleet_request_test.go index 1fdd0a4eed2..30138bb5fc5 100644 --- a/aws/resource_aws_spot_fleet_request_test.go +++ b/aws/resource_aws_spot_fleet_request_test.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + multierror "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" @@ -24,12 +25,20 @@ func init() { func testSweepSpotFleetRequests(region string) error { client, err := sharedClientForRegion(region) + if err != nil { return fmt.Errorf("error getting client: %s", err) } + conn := client.(*AWSClient).ec2conn + sweepResources := make([]*testSweepResource, 0) + var errs *multierror.Error err = conn.DescribeSpotFleetRequestsPages(&ec2.DescribeSpotFleetRequestsInput{}, func(page *ec2.DescribeSpotFleetRequestsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + if len(page.SpotFleetRequestConfigs) == 0 { log.Print("[DEBUG] No Spot Fleet Requests to sweep") return false @@ -38,22 +47,31 @@ func testSweepSpotFleetRequests(region string) error { for _, config := range page.SpotFleetRequestConfigs { id := aws.StringValue(config.SpotFleetRequestId) - log.Printf("[INFO] Deleting Spot Fleet Request: %s", id) - err := deleteSpotFleetRequest(id, true, 15*time.Minute, conn) - if err != nil { - log.Printf("[ERROR] Failed to delete Spot Fleet Request (%s): %s", id, err) - } + r := resourceAwsSpotFleetRequest() + d := r.Data(nil) + d.SetId(id) + d.Set("terminate_instances_with_expiration", true) + + sweepResources = append(sweepResources, NewTestSweepResource(r, d, client)) } + return !lastPage }) + if err != nil { - if testSweepSkipSweepError(err) { - log.Printf("[WARN] Skipping EC2 Spot Fleet Requests sweep for %s: %s", region, err) - return nil - } - return fmt.Errorf("Error retrieving EC2 Spot Fleet Requests: %s", err) + errs = multierror.Append(errs, fmt.Errorf("error describing EC2 Spot Fleet Requests for %s: %w", region, err)) } - return nil + + if err = testSweepResourceOrchestrator(sweepResources); err != nil { + errs = multierror.Append(errs, fmt.Errorf("error sweeping EC2 Spot Fleet Requests for %s: %w", region, err)) + } + + if testSweepSkipSweepError(errs.ErrorOrNil()) { + log.Printf("[WARN] Skipping EC2 Spot Fleet Requests sweep for %s: %s", region, errs) + return nil + } + + return errs.ErrorOrNil() } func TestAccAWSSpotFleetRequest_basic(t *testing.T) { From acaa7282a861e6fe4ca44bb3568912fc7241d32d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 23:39:27 -0400 Subject: [PATCH 8/9] r/instance: Correct log message --- aws/resource_aws_instance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_instance.go b/aws/resource_aws_instance.go index 0942c55faf3..0f30e3cd4ec 100644 --- a/aws/resource_aws_instance.go +++ b/aws/resource_aws_instance.go @@ -1541,7 +1541,7 @@ func resourceAwsInstanceDelete(d *schema.ResourceData, meta interface{}) error { err := resourceAwsInstanceDisableAPITermination(conn, d.Id(), d.Get("disable_api_termination").(bool)) if err != nil { - log.Printf("[WARN] attempting to terminate EC2 instance (%s) despite error enabling API termination: %s", d.Id(), err) + log.Printf("[WARN] attempting to terminate EC2 instance (%s) despite error modifying attribute (%s): %s", d.Id(), ec2.InstanceAttributeNameDisableApiTermination, err) } err = awsTerminateInstance(conn, d.Id(), d.Timeout(schema.TimeoutDelete)) From bbaeb26d7469d7753a1af942d688afe5ebb46e2f Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 16 Apr 2021 23:52:07 -0400 Subject: [PATCH 9/9] tests/r/instance: Adjust instance types for basic test --- aws/resource_aws_instance_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_instance_test.go b/aws/resource_aws_instance_test.go index d887eb19b69..48d4a54046c 100644 --- a/aws/resource_aws_instance_test.go +++ b/aws/resource_aws_instance_test.go @@ -3711,7 +3711,7 @@ func testAccInstanceConfigBasic() string { return composeConfig( testAccLatestAmazonLinuxHvmEbsAmiConfig(), // https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-classic-platform.html#ec2-classic-instance-types - testAccAvailableEc2InstanceTypeForRegion("t1.micro", "m1.small", "t3.micro", "t2.micro"), + testAccAvailableEc2InstanceTypeForRegion("t3.micro", "t2.micro", "t1.micro", "m1.small"), ` resource "aws_instance" "test" { ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id