From 341af8b424e1fb007d5c5510e425404d409f1749 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 21 Jul 2020 00:00:11 -0400 Subject: [PATCH] resource/aws_appautoscaling_target: Remove DeregisterScalableTarget retries on all errors and add disappears test Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/13409 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/13826 Output from acceptance testing: ``` --- PASS: TestAccAWSAppautoScalingTarget_multipleTargets (20.68s) --- PASS: TestAccAWSAppautoScalingTarget_optionalRoleArn (25.17s) --- PASS: TestAccAWSAppautoScalingTarget_basic (43.13s) --- PASS: TestAccAWSAppautoScalingTarget_spotFleetRequest (57.42s) --- PASS: TestAccAWSAppautoScalingTarget_disappears (71.79s) --- PASS: TestAccAWSAppautoScalingTarget_emrCluster (840.33s) --- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameName (24.97s) --- PASS: TestAccAWSAppautoScalingPolicy_dynamodb_table (26.58s) --- PASS: TestAccAWSAppautoScalingPolicy_multiplePoliciesSameResource (28.13s) --- PASS: TestAccAWSAppautoScalingPolicy_dynamodb_index (37.07s) --- PASS: TestAccAWSAppautoScalingPolicy_spotFleetRequest (71.64s) --- PASS: TestAccAWSAppautoScalingPolicy_disappears (75.45s) --- PASS: TestAccAWSAppautoScalingPolicy_basic (77.30s) --- PASS: TestAccAWSAppautoScalingPolicy_scaleOutAndIn (79.17s) --- PASS: TestAccAWSAppautoScalingPolicy_ResourceId_ForceNew (83.72s) ``` --- aws/resource_aws_appautoscaling_target.go | 51 ++++++------------- ...resource_aws_appautoscaling_target_test.go | 22 ++++++++ 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/aws/resource_aws_appautoscaling_target.go b/aws/resource_aws_appautoscaling_target.go index 80b4e125a01..b589aa602c8 100644 --- a/aws/resource_aws_appautoscaling_target.go +++ b/aws/resource_aws_appautoscaling_target.go @@ -3,15 +3,13 @@ package aws import ( "fmt" "log" + "strings" "time" - "github.com/hashicorp/terraform-plugin-sdk/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/applicationautoscaling" - "strings" + "github.com/hashicorp/terraform-plugin-sdk/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" ) func resourceAwsAppautoscalingTarget() *schema.Resource { @@ -148,46 +146,29 @@ func resourceAwsAppautoscalingTargetRead(d *schema.ResourceData, meta interface{ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).appautoscalingconn - namespace := d.Get("service_namespace").(string) - dimension := d.Get("scalable_dimension").(string) - - t, err := getAwsAppautoscalingTarget(d.Id(), namespace, dimension, conn) - if err != nil { - return err - } - if t == nil { - log.Printf("[INFO] Application AutoScaling Target %q not found", d.Id()) - return nil - } - - log.Printf("[DEBUG] Application AutoScaling Target destroy: %#v", d.Id()) - deleteOpts := applicationautoscaling.DeregisterScalableTargetInput{ + input := &applicationautoscaling.DeregisterScalableTargetInput{ ResourceId: aws.String(d.Get("resource_id").(string)), ServiceNamespace: aws.String(d.Get("service_namespace").(string)), ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), } - err = resource.Retry(5*time.Minute, func() *resource.RetryError { - if _, err := conn.DeregisterScalableTarget(&deleteOpts); err != nil { - if awserr, ok := err.(awserr.Error); ok { - // @TODO: We should do stuff here depending on the actual error returned - return resource.RetryableError(awserr) - } - // Non recognized error, no retry. - return resource.NonRetryableError(err) - } - // Successful delete - return nil - }) + _, err := conn.DeregisterScalableTarget(input) + if err != nil { - return err + return fmt.Errorf("error deleting Application AutoScaling Target (%s): %w", d.Id(), err) } return resource.Retry(5*time.Minute, func() *resource.RetryError { - if t, _ = getAwsAppautoscalingTarget(d.Id(), namespace, dimension, conn); t != nil { - return resource.RetryableError( - fmt.Errorf("Application AutoScaling Target still exists")) + t, err := getAwsAppautoscalingTarget(d.Get("resource_id").(string), d.Get("service_namespace").(string), d.Get("scalable_dimension").(string), conn) + + if err != nil { + return resource.NonRetryableError(err) } + + if t != nil { + return resource.RetryableError(fmt.Errorf("Application AutoScaling Target (%s) still exists", d.Id())) + } + return nil }) } diff --git a/aws/resource_aws_appautoscaling_target_test.go b/aws/resource_aws_appautoscaling_target_test.go index ce5927d83cb..346f9f9d57e 100644 --- a/aws/resource_aws_appautoscaling_target_test.go +++ b/aws/resource_aws_appautoscaling_target_test.go @@ -53,6 +53,28 @@ func TestAccAWSAppautoScalingTarget_basic(t *testing.T) { }) } +func TestAccAWSAppautoScalingTarget_disappears(t *testing.T) { + var target applicationautoscaling.ScalableTarget + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_appautoscaling_target.bar" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAppautoscalingTargetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSAppautoscalingTargetConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAppautoscalingTargetExists(resourceName, &target), + testAccCheckResourceDisappears(testAccProvider, resourceAwsAppautoscalingTarget(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccAWSAppautoScalingTarget_spotFleetRequest(t *testing.T) { var target applicationautoscaling.ScalableTarget validUntil := time.Now().UTC().Add(24 * time.Hour).Format(time.RFC3339)