From 61b6e698fab411795602a9604bfb15a38391af21 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 4 Oct 2017 12:40:33 +0100 Subject: [PATCH 1/4] r/appautoscaling_policy: Improve err messages --- aws/resource_aws_appautoscaling_policy.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_appautoscaling_policy.go b/aws/resource_aws_appautoscaling_policy.go index 673026f5a29..9b1fc7d6804 100644 --- a/aws/resource_aws_appautoscaling_policy.go +++ b/aws/resource_aws_appautoscaling_policy.go @@ -270,7 +270,7 @@ func resourceAwsAppautoscalingPolicyCreate(d *schema.ResourceData, meta interfac return nil }) if err != nil { - return err + return fmt.Errorf("Failed to create scaling policy: %s", err) } d.Set("arn", resp.PolicyARN) @@ -317,7 +317,7 @@ func resourceAwsAppautoscalingPolicyUpdate(d *schema.ResourceData, meta interfac log.Printf("[DEBUG] Application Autoscaling Update Scaling Policy: %#v", params) _, err := conn.PutScalingPolicy(¶ms) if err != nil { - return err + return fmt.Errorf("Failed to update scaling policy: %s", err) } return resourceAwsAppautoscalingPolicyRead(d, meta) @@ -341,7 +341,7 @@ func resourceAwsAppautoscalingPolicyDelete(d *schema.ResourceData, meta interfac } log.Printf("[DEBUG] Deleting Application AutoScaling Policy opts: %#v", params) if _, err := conn.DeleteScalingPolicy(¶ms); err != nil { - return fmt.Errorf("Application AutoScaling Policy: %s", err) + return fmt.Errorf("Failed to delete autoscaling policy: %s", err) } d.SetId("") @@ -578,7 +578,6 @@ func getAwsAppautoscalingPolicy(d *schema.ResourceData, meta interface{}) (*appl } } - // policy not found return nil, nil } From c14f9ef3417fb667fe37184872b7d8cc120c611c Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 4 Oct 2017 12:42:20 +0100 Subject: [PATCH 2/4] r/appautoscaling_policy: Use dimension when looking up policy --- aws/resource_aws_appautoscaling_policy.go | 5 +- ...resource_aws_appautoscaling_policy_test.go | 140 ++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_appautoscaling_policy.go b/aws/resource_aws_appautoscaling_policy.go index 9b1fc7d6804..d90d2fa259f 100644 --- a/aws/resource_aws_appautoscaling_policy.go +++ b/aws/resource_aws_appautoscaling_policy.go @@ -571,9 +571,10 @@ func getAwsAppautoscalingPolicy(d *schema.ResourceData, meta interface{}) (*appl } // find scaling policy - name := d.Get("name") + name := d.Get("name").(string) + dimension := d.Get("scalable_dimension").(string) for idx, sp := range resp.ScalingPolicies { - if *sp.PolicyName == name { + if *sp.PolicyName == name && *sp.ScalableDimension == dimension { return resp.ScalingPolicies[idx], nil } } diff --git a/aws/resource_aws_appautoscaling_policy_test.go b/aws/resource_aws_appautoscaling_policy_test.go index 440e398427b..b039a0a6f75 100644 --- a/aws/resource_aws_appautoscaling_policy_test.go +++ b/aws/resource_aws_appautoscaling_policy_test.go @@ -122,6 +122,36 @@ func TestAccAWSAppautoScalingPolicy_dynamoDb(t *testing.T) { }) } +func TestAccAWSAppautoScalingPolicy_multiplePolicies(t *testing.T) { + var readPolicy applicationautoscaling.ScalingPolicy + var writePolicy applicationautoscaling.ScalingPolicy + + tableName := fmt.Sprintf("tf-autoscaled-table-%s", acctest.RandString(5)) + namePrefix := fmt.Sprintf("tf-appautoscaling-policy-%s", acctest.RandString(5)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAppautoscalingPolicyDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccAWSAppautoscalingPolicy_multiplePolicies(tableName, namePrefix), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAppautoscalingPolicyExists("aws_appautoscaling_policy.read", &readPolicy), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.read", "name", namePrefix+"-read"), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.read", "service_namespace", "dynamodb"), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.read", "scalable_dimension", "dynamodb:table:ReadCapacityUnits"), + + testAccCheckAWSAppautoscalingPolicyExists("aws_appautoscaling_policy.write", &writePolicy), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.write", "name", namePrefix+"-write"), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.write", "service_namespace", "dynamodb"), + resource.TestCheckResourceAttr("aws_appautoscaling_policy.write", "scalable_dimension", "dynamodb:table:WriteCapacityUnits"), + ), + }, + }, + }) +} + func testAccCheckAWSAppautoscalingPolicyExists(n string, policy *applicationautoscaling.ScalingPolicy) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -525,3 +555,113 @@ resource "aws_appautoscaling_policy" "dynamo_test" { } `, randPolicyName, randPolicyName) } + +func testAccAWSAppautoscalingPolicy_multiplePolicies(tableName, namePrefix string) string { + return fmt.Sprintf(` +resource "aws_dynamodb_table" "dynamodb_table_test" { + name = "%s" + read_capacity = 5 + write_capacity = 5 + hash_key = "FooKey" + attribute { + name = "FooKey" + type = "S" + } +} + +resource "aws_iam_role" "autoscale_role" { + assume_role_policy = < Date: Wed, 4 Oct 2017 12:45:00 +0100 Subject: [PATCH 3/4] r/appautoscaling_target: Refactoring --- aws/resource_aws_appautoscaling_target.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_appautoscaling_target.go b/aws/resource_aws_appautoscaling_target.go index 2490f4d2dea..5666b71359c 100644 --- a/aws/resource_aws_appautoscaling_target.go +++ b/aws/resource_aws_appautoscaling_target.go @@ -96,7 +96,8 @@ func resourceAwsAppautoscalingTargetCreate(d *schema.ResourceData, meta interfac func resourceAwsAppautoscalingTargetRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).appautoscalingconn - t, err := getAwsAppautoscalingTarget(d, conn) + namespace := d.Get("service_namespace").(string) + t, err := getAwsAppautoscalingTarget(d.Id(), namespace, conn) if err != nil { return err } @@ -119,7 +120,9 @@ func resourceAwsAppautoscalingTargetRead(d *schema.ResourceData, meta interface{ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).appautoscalingconn - t, err := getAwsAppautoscalingTarget(d, conn) + namespace := d.Get("service_namespace").(string) + + t, err := getAwsAppautoscalingTarget(d.Id(), namespace, conn) if err != nil { return err } @@ -153,7 +156,7 @@ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interfac } return resource.Retry(5*time.Minute, func() *resource.RetryError { - if t, _ = getAwsAppautoscalingTarget(d, conn); t != nil { + if t, _ = getAwsAppautoscalingTarget(d.Id(), namespace, conn); t != nil { return resource.RetryableError( fmt.Errorf("Application AutoScaling Target still exists")) } @@ -161,14 +164,12 @@ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interfac }) } -func getAwsAppautoscalingTarget( - d *schema.ResourceData, +func getAwsAppautoscalingTarget(resourceId, namespace string, conn *applicationautoscaling.ApplicationAutoScaling) (*applicationautoscaling.ScalableTarget, error) { - tgtName := d.Id() describeOpts := applicationautoscaling.DescribeScalableTargetsInput{ - ResourceIds: []*string{aws.String(tgtName)}, - ServiceNamespace: aws.String(d.Get("service_namespace").(string)), + ResourceIds: []*string{aws.String(resourceId)}, + ServiceNamespace: aws.String(namespace), } log.Printf("[DEBUG] Application AutoScaling Target describe configuration: %#v", describeOpts) @@ -181,7 +182,7 @@ func getAwsAppautoscalingTarget( } for idx, tgt := range describeTargets.ScalableTargets { - if *tgt.ResourceId == tgtName { + if *tgt.ResourceId == resourceId { return describeTargets.ScalableTargets[idx], nil } } From ab5f66ac0e38b71ff0e8e8ed5f97afd63b3b62da Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 4 Oct 2017 12:45:55 +0100 Subject: [PATCH 4/4] r/appautoscaling_target: Use dimension when looking up target --- aws/resource_aws_appautoscaling_target.go | 12 +- ...resource_aws_appautoscaling_target_test.go | 125 ++++++++++++++++-- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/aws/resource_aws_appautoscaling_target.go b/aws/resource_aws_appautoscaling_target.go index 5666b71359c..8734d869f70 100644 --- a/aws/resource_aws_appautoscaling_target.go +++ b/aws/resource_aws_appautoscaling_target.go @@ -97,7 +97,8 @@ func resourceAwsAppautoscalingTargetRead(d *schema.ResourceData, meta interface{ conn := meta.(*AWSClient).appautoscalingconn namespace := d.Get("service_namespace").(string) - t, err := getAwsAppautoscalingTarget(d.Id(), namespace, conn) + dimension := d.Get("scalable_dimension").(string) + t, err := getAwsAppautoscalingTarget(d.Id(), namespace, dimension, conn) if err != nil { return err } @@ -121,8 +122,9 @@ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interfac conn := meta.(*AWSClient).appautoscalingconn namespace := d.Get("service_namespace").(string) + dimension := d.Get("scalable_dimension").(string) - t, err := getAwsAppautoscalingTarget(d.Id(), namespace, conn) + t, err := getAwsAppautoscalingTarget(d.Id(), namespace, dimension, conn) if err != nil { return err } @@ -156,7 +158,7 @@ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interfac } return resource.Retry(5*time.Minute, func() *resource.RetryError { - if t, _ = getAwsAppautoscalingTarget(d.Id(), namespace, conn); t != nil { + if t, _ = getAwsAppautoscalingTarget(d.Id(), namespace, dimension, conn); t != nil { return resource.RetryableError( fmt.Errorf("Application AutoScaling Target still exists")) } @@ -164,7 +166,7 @@ func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interfac }) } -func getAwsAppautoscalingTarget(resourceId, namespace string, +func getAwsAppautoscalingTarget(resourceId, namespace, dimension string, conn *applicationautoscaling.ApplicationAutoScaling) (*applicationautoscaling.ScalableTarget, error) { describeOpts := applicationautoscaling.DescribeScalableTargetsInput{ @@ -182,7 +184,7 @@ func getAwsAppautoscalingTarget(resourceId, namespace string, } for idx, tgt := range describeTargets.ScalableTargets { - if *tgt.ResourceId == resourceId { + if *tgt.ResourceId == resourceId && *tgt.ScalableDimension == dimension { return describeTargets.ScalableTargets[idx], nil } } diff --git a/aws/resource_aws_appautoscaling_target_test.go b/aws/resource_aws_appautoscaling_target_test.go index f61484d19b4..0f6b3977b32 100644 --- a/aws/resource_aws_appautoscaling_target_test.go +++ b/aws/resource_aws_appautoscaling_target_test.go @@ -88,6 +88,40 @@ func TestAccAWSAppautoScalingTarget_emrCluster(t *testing.T) { }) } +func TestAccAWSAppautoScalingTarget_multipleTargets(t *testing.T) { + var writeTarget applicationautoscaling.ScalableTarget + var readTarget applicationautoscaling.ScalableTarget + + rInt := acctest.RandInt() + tableName := fmt.Sprintf("tf_acc_test_table_%d", rInt) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAppautoscalingTargetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSAppautoscalingTarget_multipleTargets(tableName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAppautoscalingTargetExists("aws_appautoscaling_target.write", &writeTarget), + resource.TestCheckResourceAttr("aws_appautoscaling_target.write", "service_namespace", "dynamodb"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.write", "resource_id", "table/"+tableName), + resource.TestCheckResourceAttr("aws_appautoscaling_target.write", "scalable_dimension", "dynamodb:table:WriteCapacityUnits"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.write", "min_capacity", "1"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.write", "max_capacity", "10"), + + testAccCheckAWSAppautoscalingTargetExists("aws_appautoscaling_target.read", &readTarget), + resource.TestCheckResourceAttr("aws_appautoscaling_target.read", "service_namespace", "dynamodb"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.read", "resource_id", "table/"+tableName), + resource.TestCheckResourceAttr("aws_appautoscaling_target.read", "scalable_dimension", "dynamodb:table:ReadCapacityUnits"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.read", "min_capacity", "2"), + resource.TestCheckResourceAttr("aws_appautoscaling_target.read", "max_capacity", "15"), + ), + }, + }, + }) +} + func testAccCheckAWSAppautoscalingTargetDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).appautoscalingconn @@ -137,22 +171,19 @@ func testAccCheckAWSAppautoscalingTargetExists(n string, target *applicationauto conn := testAccProvider.Meta().(*AWSClient).appautoscalingconn - describeTargets, err := conn.DescribeScalableTargets( - &applicationautoscaling.DescribeScalableTargetsInput{ - ResourceIds: []*string{aws.String(rs.Primary.ID)}, - ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]), - }, - ) + namespace := rs.Primary.Attributes["service_namespace"] + dimension := rs.Primary.Attributes["scalable_dimension"] + tgt, err := getAwsAppautoscalingTarget(rs.Primary.ID, namespace, dimension, conn) if err != nil { return err } - - if len(describeTargets.ScalableTargets) != 1 || *describeTargets.ScalableTargets[0].ResourceId != rs.Primary.ID { - return fmt.Errorf("Application AutoScaling ResourceId not found") + if tgt == nil { + return fmt.Errorf("Scalable target for %q (%s/%s) not found", + rs.Primary.ID, namespace, dimension) } - target = describeTargets.ScalableTargets[0] + *target = *tgt return nil } @@ -734,3 +765,77 @@ resource "aws_appautoscaling_target" "test" { max_capacity = 3 } `) + +func testAccAWSAppautoscalingTarget_multipleTargets(tableName string) string { + return fmt.Sprintf(` +resource "aws_dynamodb_table" "dynamodb_table_test" { + name = "%s" + read_capacity = 5 + write_capacity = 5 + hash_key = "FooKey" + attribute { + name = "FooKey" + type = "S" + } +} + +resource "aws_iam_role" "autoscale_role" { + assume_role_policy = <