From c6cfae0cbee29dbf185e4ec8a47c5c6d02a66b56 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Fri, 24 May 2019 19:33:08 -0700 Subject: [PATCH 1/7] Do not default max_capacity and min_capacity to 0. Allow them to be left undefined so that they can be used individually. --- aws/resource_aws_appautoscaling_scheduled_action.go | 7 +++---- aws/resource_aws_appautoscaling_scheduled_action_test.go | 8 +++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index 03ac1156b92..4d2cb0aec4c 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -97,13 +97,12 @@ func resourceAwsAppautoscalingScheduledActionPut(d *schema.ResourceData, meta in if v, ok := d.GetOk("schedule"); ok { input.Schedule = aws.String(v.(string)) } - if v, ok := d.GetOk("scalable_target_action"); ok { + if _, ok := d.GetOk("scalable_target_action"); ok { sta := &applicationautoscaling.ScalableTargetAction{} - raw := v.([]interface{})[0].(map[string]interface{}) - if max, ok := raw["max_capacity"]; ok { + if max, ok := d.GetOkExists("scalable_target_action.0.max_capacity"); ok { sta.MaxCapacity = aws.Int64(int64(max.(int))) } - if min, ok := raw["min_capacity"]; ok { + if min, ok := d.GetOkExists("scalable_target_action.0.min_capacity"); ok { sta.MinCapacity = aws.Int64(int64(min.(int))) } input.ScalableTargetAction = sta diff --git a/aws/resource_aws_appautoscaling_scheduled_action_test.go b/aws/resource_aws_appautoscaling_scheduled_action_test.go index 0edefa9567f..5ff58cfda73 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action_test.go +++ b/aws/resource_aws_appautoscaling_scheduled_action_test.go @@ -22,7 +22,7 @@ func TestAccAWSAppautoscalingScheduledAction_dynamo(t *testing.T) { { Config: testAccAppautoscalingScheduledActionConfig_DynamoDB(acctest.RandString(5), ts), Check: resource.ComposeTestCheckFunc( - testAccCheckAwsAppautoscalingScheduledActionExists("aws_appautoscaling_scheduled_action.hoge"), + testAccCheckAwsAppautoscalingScheduledActionExists("aws_appautoscaling_scheduled_action.scale_down"), ), }, }, @@ -135,7 +135,7 @@ resource "aws_appautoscaling_target" "read" { max_capacity = 10 } -resource "aws_appautoscaling_scheduled_action" "hoge" { +resource "aws_appautoscaling_scheduled_action" "scale_down" { name = "tf-appauto-%s" service_namespace = "${aws_appautoscaling_target.read.service_namespace}" resource_id = "${aws_appautoscaling_target.read.resource_id}" @@ -144,7 +144,9 @@ resource "aws_appautoscaling_scheduled_action" "hoge" { scalable_target_action { min_capacity = 1 - max_capacity = 10 + # max_capacity is omitted and shall thus be omitted from the request + # if it is not omitted and instead sent with the value 0, then the following error would be returned: + # "ValidationException: Maximum capacity cannot be less than minimum capacity" } } `, rName, rName, ts) From 2c09217c73235c84ed8709680f34ece1b1de4fa0 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 18:50:05 -0700 Subject: [PATCH 2/7] Use ResourceId when reading the scheduled action, because you can create multiple scheduled actions with the same name if they target different resources. For example, you can have multiple scheduled actions named "scale-down" that target different Aurora clusters. --- aws/resource_aws_appautoscaling_scheduled_action.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index 4d2cb0aec4c..71a7577f29f 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -145,14 +145,17 @@ func resourceAwsAppautoscalingScheduledActionRead(d *schema.ResourceData, meta i conn := meta.(*AWSClient).appautoscalingconn saName := d.Get("name").(string) + saResourceId := d.Get("resource_id").(string) input := &applicationautoscaling.DescribeScheduledActionsInput{ ScheduledActionNames: []*string{aws.String(saName)}, ServiceNamespace: aws.String(d.Get("service_namespace").(string)), + ResourceId: aws.String(saResourceId), } resp, err := conn.DescribeScheduledActions(input) if err != nil { return err } + if len(resp.ScheduledActions) < 1 { log.Printf("[WARN] Application Autoscaling Scheduled Action (%s) not found, removing from state", d.Id()) d.SetId("") From 20825f390faa8028e309ff018efb0ea1b0e2db88 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 18:51:33 -0700 Subject: [PATCH 3/7] Set more attributes than just the arn when reading the resource. This allows us to update these attributes. --- ...rce_aws_appautoscaling_scheduled_action.go | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index 71a7577f29f..b1a4d9f7a53 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -164,10 +164,18 @@ func resourceAwsAppautoscalingScheduledActionRead(d *schema.ResourceData, meta i if len(resp.ScheduledActions) != 1 { return fmt.Errorf("Expected 1 scheduled action under %s, found %d", saName, len(resp.ScheduledActions)) } - if *resp.ScheduledActions[0].ScheduledActionName != saName { + sa := resp.ScheduledActions[0] + if *sa.ScheduledActionName != saName { return fmt.Errorf("Scheduled Action (%s) not found", saName) } - d.Set("arn", resp.ScheduledActions[0].ScheduledActionARN) + + if err := d.Set("scalable_target_action", flattenScalableTargetActionConfiguration(sa.ScalableTargetAction)); err != nil { + return fmt.Errorf("error setting scalable_target_action: %s", err) + } + d.Set("schedule", sa.Schedule) + d.Set("start_time", sa.StartTime) + d.Set("end_time", sa.EndTime) + d.Set("arn", sa.ScheduledActionARN) return nil } @@ -193,3 +201,19 @@ func resourceAwsAppautoscalingScheduledActionDelete(d *schema.ResourceData, meta return nil } + +func flattenScalableTargetActionConfiguration(cfg *applicationautoscaling.ScalableTargetAction) []interface{} { + if cfg == nil { + return []interface{}{} + } + + m := make(map[string]interface{}) + if cfg.MaxCapacity != nil { + m["max_capacity"] = *cfg.MaxCapacity + } + if cfg.MinCapacity != nil { + m["min_capacity"] = *cfg.MinCapacity + } + + return []interface{}{m} +} From ac9de91139ebb79c2e2ee106c593ae15fbedf60a Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 18:52:35 -0700 Subject: [PATCH 4/7] It is not necessary to delete and recreate the scheduled action when updating these attributes, we can just put and it will overwrite the existing resource. --- aws/resource_aws_appautoscaling_scheduled_action.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index b1a4d9f7a53..3b4a55742ab 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -17,6 +17,7 @@ func resourceAwsAppautoscalingScheduledAction() *schema.Resource { return &schema.Resource{ Create: resourceAwsAppautoscalingScheduledActionPut, Read: resourceAwsAppautoscalingScheduledActionRead, + Update: resourceAwsAppautoscalingScheduledActionPut, Delete: resourceAwsAppautoscalingScheduledActionDelete, Schema: map[string]*schema.Schema{ @@ -43,19 +44,19 @@ func resourceAwsAppautoscalingScheduledAction() *schema.Resource { "scalable_target_action": { Type: schema.TypeList, Optional: true, - ForceNew: true, + ForceNew: false, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "max_capacity": { Type: schema.TypeInt, Optional: true, - ForceNew: true, + ForceNew: false, }, "min_capacity": { Type: schema.TypeInt, Optional: true, - ForceNew: true, + ForceNew: false, }, }, }, @@ -63,17 +64,17 @@ func resourceAwsAppautoscalingScheduledAction() *schema.Resource { "schedule": { Type: schema.TypeString, Optional: true, - ForceNew: true, + ForceNew: false, }, "start_time": { Type: schema.TypeString, Optional: true, - ForceNew: true, + ForceNew: false, }, "end_time": { Type: schema.TypeString, Optional: true, - ForceNew: true, + ForceNew: false, }, "arn": { Type: schema.TypeString, From 0ca43a8dd77fd84426a936dafb1337f4c135a0f1 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 19:52:35 -0700 Subject: [PATCH 5/7] Ugh.. use TypeString to allow unspecified value. Based on similar code in resource_aws_launch_template.go. --- ...rce_aws_appautoscaling_scheduled_action.go | 42 +++++++++++++------ aws/validators.go | 20 +++++++++ 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index 3b4a55742ab..6924702148d 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "strconv" "time" "github.com/aws/aws-sdk-go/aws" @@ -49,14 +50,20 @@ func resourceAwsAppautoscalingScheduledAction() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "max_capacity": { - Type: schema.TypeInt, - Optional: true, - ForceNew: false, + // Use TypeString to allow an "unspecified" value, + // since TypeInt only has allows numbers with 0 as default. + Type: schema.TypeString, + Optional: true, + ForceNew: false, + ValidateFunc: validateTypeStringNullableInteger, }, "min_capacity": { - Type: schema.TypeInt, - Optional: true, - ForceNew: false, + // Use TypeString to allow an "unspecified" value, + // since TypeInt only has allows numbers with 0 as default. + Type: schema.TypeString, + Optional: true, + ForceNew: false, + ValidateFunc: validateTypeStringNullableInteger, }, }, }, @@ -98,13 +105,22 @@ func resourceAwsAppautoscalingScheduledActionPut(d *schema.ResourceData, meta in if v, ok := d.GetOk("schedule"); ok { input.Schedule = aws.String(v.(string)) } - if _, ok := d.GetOk("scalable_target_action"); ok { + if v, ok := d.GetOk("scalable_target_action"); ok { sta := &applicationautoscaling.ScalableTargetAction{} - if max, ok := d.GetOkExists("scalable_target_action.0.max_capacity"); ok { - sta.MaxCapacity = aws.Int64(int64(max.(int))) + raw := v.([]interface{})[0].(map[string]interface{}) + if max, ok := raw["max_capacity"]; ok && max.(string) != "" { + maxInt, err := strconv.ParseInt(max.(string), 10, 64) + if err != nil { + return fmt.Errorf("error converting max_capacity %q from string to integer: %s", v.(string), err) + } + sta.MaxCapacity = aws.Int64(maxInt) } - if min, ok := d.GetOkExists("scalable_target_action.0.min_capacity"); ok { - sta.MinCapacity = aws.Int64(int64(min.(int))) + if min, ok := raw["min_capacity"]; ok && min.(string) != "" { + minInt, err := strconv.ParseInt(min.(string), 10, 64) + if err != nil { + return fmt.Errorf("error converting min_capacity %q from string to integer: %s", v.(string), err) + } + sta.MinCapacity = aws.Int64(minInt) } input.ScalableTargetAction = sta } @@ -210,10 +226,10 @@ func flattenScalableTargetActionConfiguration(cfg *applicationautoscaling.Scalab m := make(map[string]interface{}) if cfg.MaxCapacity != nil { - m["max_capacity"] = *cfg.MaxCapacity + m["max_capacity"] = strconv.FormatInt(aws.Int64Value(cfg.MaxCapacity), 10) } if cfg.MinCapacity != nil { - m["min_capacity"] = *cfg.MinCapacity + m["min_capacity"] = strconv.FormatInt(aws.Int64Value(cfg.MinCapacity), 10) } return []interface{}{m} diff --git a/aws/validators.go b/aws/validators.go index dab1c72f66c..5e83698f6be 100644 --- a/aws/validators.go +++ b/aws/validators.go @@ -82,6 +82,26 @@ func validateTypeStringNullableFloat(v interface{}, k string) (ws []string, es [ return } +// validateTypeStringNullableInteger provides custom error messaging for TypeString integers +// Some arguments require an integer value or an unspecified, empty field. +func validateTypeStringNullableInteger(v interface{}, k string) (ws []string, es []error) { + value, ok := v.(string) + if !ok { + es = append(es, fmt.Errorf("expected type of %s to be string", k)) + return + } + + if value == "" { + return + } + + if _, err := strconv.ParseInt(value, 10, 64); err != nil { + es = append(es, fmt.Errorf("%s: cannot parse '%s' as int: %s", k, value, err)) + } + + return +} + func validateTransferServerID(v interface{}, k string) (ws []string, errors []error) { value := v.(string) From 403c1572b95a54f9f613f912cd83b1dde40982b8 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 19:55:30 -0700 Subject: [PATCH 6/7] Update test to also update the scheduled action. --- ...aws_appautoscaling_scheduled_action_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action_test.go b/aws/resource_aws_appautoscaling_scheduled_action_test.go index ed05cbf916c..7772582b346 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action_test.go +++ b/aws/resource_aws_appautoscaling_scheduled_action_test.go @@ -13,18 +13,26 @@ import ( ) func TestAccAWSAppautoscalingScheduledAction_dynamo(t *testing.T) { - ts := time.Now().AddDate(0, 0, 1).Format("2006-01-02T15:04:05") + rName := acctest.RandString(5) + schedule1 := fmt.Sprintf("at(%s)", time.Now().AddDate(0, 0, 1).Format("2006-01-02T15:04:05")) + schedule2 := fmt.Sprintf("at(%s)", time.Now().AddDate(0, 0, 2).Format("2006-01-02T15:04:05")) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAwsAppautoscalingScheduledActionDestroy, Steps: []resource.TestStep{ { - Config: testAccAppautoscalingScheduledActionConfig_DynamoDB(acctest.RandString(5), ts), + Config: testAccAppautoscalingScheduledActionConfig_DynamoDB(rName, schedule1), Check: resource.ComposeTestCheckFunc( testAccCheckAwsAppautoscalingScheduledActionExists("aws_appautoscaling_scheduled_action.scale_down"), ), }, + { + Config: testAccAppautoscalingScheduledActionConfig_DynamoDB(rName, schedule2), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("aws_appautoscaling_scheduled_action.scale_down", "schedule", schedule2), + ), + }, }, }) } @@ -113,7 +121,7 @@ func testAccCheckAwsAppautoscalingScheduledActionExists(name string) resource.Te } } -func testAccAppautoscalingScheduledActionConfig_DynamoDB(rName, ts string) string { +func testAccAppautoscalingScheduledActionConfig_DynamoDB(rName, schedule string) string { return fmt.Sprintf(` resource "aws_dynamodb_table" "hoge" { name = "tf-ddb-%s" @@ -140,7 +148,7 @@ resource "aws_appautoscaling_scheduled_action" "scale_down" { service_namespace = "${aws_appautoscaling_target.read.service_namespace}" resource_id = "${aws_appautoscaling_target.read.resource_id}" scalable_dimension = "${aws_appautoscaling_target.read.scalable_dimension}" - schedule = "at(%s)" + schedule = "%s" scalable_target_action { min_capacity = 1 @@ -149,7 +157,7 @@ resource "aws_appautoscaling_scheduled_action" "scale_down" { # "ValidationException: Maximum capacity cannot be less than minimum capacity" } } -`, rName, rName, ts) +`, rName, rName, schedule) } func testAccAppautoscalingScheduledActionConfig_ECS(rName, ts string) string { From a0d686e74685cffec9d88a7d459ec3ff55f088a0 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 29 May 2019 19:59:16 -0700 Subject: [PATCH 7/7] Remove unnecessary variable. --- aws/resource_aws_appautoscaling_scheduled_action.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/aws/resource_aws_appautoscaling_scheduled_action.go b/aws/resource_aws_appautoscaling_scheduled_action.go index 6924702148d..9a7d26d9500 100644 --- a/aws/resource_aws_appautoscaling_scheduled_action.go +++ b/aws/resource_aws_appautoscaling_scheduled_action.go @@ -162,11 +162,10 @@ func resourceAwsAppautoscalingScheduledActionRead(d *schema.ResourceData, meta i conn := meta.(*AWSClient).appautoscalingconn saName := d.Get("name").(string) - saResourceId := d.Get("resource_id").(string) input := &applicationautoscaling.DescribeScheduledActionsInput{ ScheduledActionNames: []*string{aws.String(saName)}, ServiceNamespace: aws.String(d.Get("service_namespace").(string)), - ResourceId: aws.String(saResourceId), + ResourceId: aws.String(d.Get("resource_id").(string)), } resp, err := conn.DescribeScheduledActions(input) if err != nil {