From 56df82ead2e1250a13806d13a1af4db02868a783 Mon Sep 17 00:00:00 2001 From: colinfish-toast Date: Mon, 13 Nov 2023 22:58:48 -0500 Subject: [PATCH 1/8] Fix FindScheduledAction in aws_appautoscaling to distinguish same-named schedules by scalable dimension --- internal/service/appautoscaling/find.go | 4 ++-- internal/service/appautoscaling/scheduled_action.go | 2 +- internal/service/appautoscaling/scheduled_action_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/service/appautoscaling/find.go b/internal/service/appautoscaling/find.go index bbc317915c6..15fb6b4a01a 100644 --- a/internal/service/appautoscaling/find.go +++ b/internal/service/appautoscaling/find.go @@ -11,7 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" ) -func FindScheduledAction(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, name, serviceNamespace, resourceId string) (*applicationautoscaling.ScheduledAction, error) { +func FindScheduledAction(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, name, serviceNamespace, resourceId, scalableDimension string) (*applicationautoscaling.ScheduledAction, error) { var result *applicationautoscaling.ScheduledAction input := &applicationautoscaling.DescribeScheduledActionsInput{ @@ -29,7 +29,7 @@ func FindScheduledAction(ctx context.Context, conn *applicationautoscaling.Appli continue } - if name == aws.StringValue(item.ScheduledActionName) { + if name == aws.StringValue(item.ScheduledActionName) && scalableDimension == aws.StringValue(item.ScalableDimension) { result = item return false } diff --git a/internal/service/appautoscaling/scheduled_action.go b/internal/service/appautoscaling/scheduled_action.go index 73bfed9f874..a3869766698 100644 --- a/internal/service/appautoscaling/scheduled_action.go +++ b/internal/service/appautoscaling/scheduled_action.go @@ -208,7 +208,7 @@ func resourceScheduledActionRead(ctx context.Context, d *schema.ResourceData, me var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - scheduledAction, err := FindScheduledAction(ctx, conn, d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string)) + scheduledAction, err := FindScheduledAction(ctx, conn, d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string), d.Get("scalable_dimension").(string)) if tfresource.NotFound(err) && !d.IsNewResource() { log.Printf("[WARN] Application Auto Scaling Scheduled Action (%s) not found, removing from state", d.Id()) d.SetId("") diff --git a/internal/service/appautoscaling/scheduled_action_test.go b/internal/service/appautoscaling/scheduled_action_test.go index ee6afdec26f..ee6be646b72 100644 --- a/internal/service/appautoscaling/scheduled_action_test.go +++ b/internal/service/appautoscaling/scheduled_action_test.go @@ -606,7 +606,7 @@ func testAccCheckScheduledActionExists(ctx context.Context, name string, obj *ap conn := acctest.Provider.Meta().(*conns.AWSClient).AppAutoScalingConn(ctx) - sa, err := tfappautoscaling.FindScheduledAction(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"]) + sa, err := tfappautoscaling.FindScheduledAction(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) if err != nil { return err } From b72b6779b3d5fdaf7a56a1db1edc0a9dca05bf7a Mon Sep 17 00:00:00 2001 From: colinfish-toast Date: Mon, 13 Nov 2023 23:42:07 -0500 Subject: [PATCH 2/8] Add changelog file --- .changelog/34382.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/34382.txt diff --git a/.changelog/34382.txt b/.changelog/34382.txt new file mode 100644 index 00000000000..9303f76565e --- /dev/null +++ b/.changelog/34382.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_appautoscaling_find: Fix FindScheduledAction to distinguish same-named schedules by scalable dimension +``` From a93947403e6910e1097f41bf400a4b60a621b185 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 07:52:21 -0400 Subject: [PATCH 3/8] Tweak CHANGELOG entry. --- .changelog/34382.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/34382.txt b/.changelog/34382.txt index 9303f76565e..a8177c50759 100644 --- a/.changelog/34382.txt +++ b/.changelog/34382.txt @@ -1,3 +1,3 @@ ```release-note:bug -resource/aws_appautoscaling_find: Fix FindScheduledAction to distinguish same-named schedules by scalable dimension +resource/aws_appautoscaling_scheduled_action: Read correct resource by using `scalable_dimension` as an additional filter ``` From cfe936d179c50fc86deadc0bb9e00b3101c70cd1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 08:19:00 -0400 Subject: [PATCH 4/8] r/aws_appautoscaling_scheduled_action: Tidy up. --- .../service/appautoscaling/exports_test.go | 11 + internal/service/appautoscaling/find.go | 51 ---- .../appautoscaling/scheduled_action.go | 230 ++++++++++-------- .../appautoscaling/scheduled_action_test.go | 2 +- .../appautoscaling/service_package_gen.go | 2 +- 5 files changed, 138 insertions(+), 158 deletions(-) create mode 100644 internal/service/appautoscaling/exports_test.go delete mode 100644 internal/service/appautoscaling/find.go diff --git a/internal/service/appautoscaling/exports_test.go b/internal/service/appautoscaling/exports_test.go new file mode 100644 index 00000000000..468e37e8a00 --- /dev/null +++ b/internal/service/appautoscaling/exports_test.go @@ -0,0 +1,11 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package appautoscaling + +// Exports for use in tests only. +var ( + ResourceScheduledAction = resourceScheduledAction + + FindScheduledActionByFourPartKey = findScheduledActionByFourPartKey +) diff --git a/internal/service/appautoscaling/find.go b/internal/service/appautoscaling/find.go deleted file mode 100644 index 15fb6b4a01a..00000000000 --- a/internal/service/appautoscaling/find.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package appautoscaling - -import ( - "context" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/applicationautoscaling" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" -) - -func FindScheduledAction(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, name, serviceNamespace, resourceId, scalableDimension string) (*applicationautoscaling.ScheduledAction, error) { - var result *applicationautoscaling.ScheduledAction - - input := &applicationautoscaling.DescribeScheduledActionsInput{ - ScheduledActionNames: []*string{aws.String(name)}, - ServiceNamespace: aws.String(serviceNamespace), - ResourceId: aws.String(resourceId), - } - err := conn.DescribeScheduledActionsPagesWithContext(ctx, input, func(page *applicationautoscaling.DescribeScheduledActionsOutput, lastPage bool) bool { - if page == nil { - return !lastPage - } - - for _, item := range page.ScheduledActions { - if item == nil { - continue - } - - if name == aws.StringValue(item.ScheduledActionName) && scalableDimension == aws.StringValue(item.ScalableDimension) { - result = item - return false - } - } - - return !lastPage - }) - if err != nil { - return nil, err - } - - if result == nil { - return nil, &retry.NotFoundError{ - LastRequest: input, - } - } - - return result, nil -} diff --git a/internal/service/appautoscaling/scheduled_action.go b/internal/service/appautoscaling/scheduled_action.go index a3869766698..3d6184bf225 100644 --- a/internal/service/appautoscaling/scheduled_action.go +++ b/internal/service/appautoscaling/scheduled_action.go @@ -7,23 +7,24 @@ import ( "context" "log" "strconv" + "strings" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/applicationautoscaling" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/types/nullable" ) -// @SDKResource("aws_appautoscaling_scheduled_action") -func ResourceScheduledAction() *schema.Resource { +// @SDKResource("aws_appautoscaling_scheduled_action", namae="Scheduled Action") +func resourceScheduledAction() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceScheduledActionPut, ReadWithoutTimeout: resourceScheduledActionRead, @@ -31,12 +32,17 @@ func ResourceScheduledAction() *schema.Resource { DeleteWithoutTimeout: resourceScheduledActionDelete, Schema: map[string]*schema.Schema{ - "name": { + "arn": { Type: schema.TypeString, - Required: true, - ForceNew: true, + Computed: true, }, - "service_namespace": { + "end_time": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.IsRFC3339Time, + DiffSuppressFunc: suppressEquivalentTime, + }, + "name": { Type: schema.TypeString, Required: true, ForceNew: true, @@ -82,6 +88,11 @@ func ResourceScheduledAction() *schema.Resource { Type: schema.TypeString, Required: true, }, + "service_namespace": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, // The AWS API normalizes start_time and end_time to UTC. Uses // suppressEquivalentTime to allow any timezone to be used. "start_time": { @@ -90,21 +101,11 @@ func ResourceScheduledAction() *schema.Resource { ValidateFunc: validation.IsRFC3339Time, DiffSuppressFunc: suppressEquivalentTime, }, - "end_time": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.IsRFC3339Time, - DiffSuppressFunc: suppressEquivalentTime, - }, "timezone": { Type: schema.TypeString, Optional: true, Default: "UTC", }, - "arn": { - Type: schema.TypeString, - Computed: true, - }, }, } } @@ -113,124 +114,97 @@ func resourceScheduledActionPut(ctx context.Context, d *schema.ResourceData, met var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) + name, serviceNamespace, resourceID := d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string) + id := strings.Join([]string{name, serviceNamespace, resourceID}, "-") input := &applicationautoscaling.PutScheduledActionInput{ - ScheduledActionName: aws.String(d.Get("name").(string)), - ServiceNamespace: aws.String(d.Get("service_namespace").(string)), - ResourceId: aws.String(d.Get("resource_id").(string)), + ResourceId: aws.String(resourceID), ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), + ScheduledActionName: aws.String(name), + ServiceNamespace: aws.String(serviceNamespace), } - needsPut := true if d.IsNewResource() { - scheduledActionPopulateInputForCreate(input, d) + if v, ok := d.GetOk("end_time"); ok { + t, _ := time.Parse(time.RFC3339, v.(string)) + input.EndTime = aws.Time(t) + } + input.ScalableTargetAction = expandScalableTargetAction(d.Get("scalable_target_action").([]interface{})) + input.Schedule = aws.String(d.Get("schedule").(string)) + if v, ok := d.GetOk("start_time"); ok { + t, _ := time.Parse(time.RFC3339, v.(string)) + input.StartTime = aws.Time(t) + } + input.Timezone = aws.String(d.Get("timezone").(string)) } else { - needsPut = scheduledActionPopulateInputForUpdate(input, d) - } - - if needsPut { - err := retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { - _, err := conn.PutScheduledActionWithContext(ctx, input) - if err != nil { - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { - return retry.RetryableError(err) - } - return retry.NonRetryableError(err) + if d.HasChange("end_time") { + if v, ok := d.GetOk("end_time"); ok { + t, _ := time.Parse(time.RFC3339, v.(string)) + input.EndTime = aws.Time(t) } - return nil - }) - if tfresource.TimedOut(err) { - _, err = conn.PutScheduledActionWithContext(ctx, input) } - if err != nil { - return sdkdiag.AppendErrorf(diags, "putting Application Auto Scaling scheduled action: %s", err) + if d.HasChange("scalable_target_action") { + input.ScalableTargetAction = expandScalableTargetAction(d.Get("scalable_target_action").([]interface{})) } - - if d.IsNewResource() { - d.SetId(d.Get("name").(string) + "-" + d.Get("service_namespace").(string) + "-" + d.Get("resource_id").(string)) + if d.HasChange("schedule") { + input.Schedule = aws.String(d.Get("schedule").(string)) + } + if d.HasChange("start_time") { + if v, ok := d.GetOk("start_time"); ok { + t, _ := time.Parse(time.RFC3339, v.(string)) + input.StartTime = aws.Time(t) + } + } + if d.HasChange("timezone") { + input.Timezone = aws.String(d.Get("timezone").(string)) } } - return append(diags, resourceScheduledActionRead(ctx, d, meta)...) -} - -func scheduledActionPopulateInputForCreate(input *applicationautoscaling.PutScheduledActionInput, d *schema.ResourceData) { - input.Schedule = aws.String(d.Get("schedule").(string)) - input.ScalableTargetAction = expandScalableTargetAction(d.Get("scalable_target_action").([]interface{})) - input.Timezone = aws.String(d.Get("timezone").(string)) - - if v, ok := d.GetOk("start_time"); ok { - t, _ := time.Parse(time.RFC3339, v.(string)) - input.StartTime = aws.Time(t) - } - if v, ok := d.GetOk("end_time"); ok { - t, _ := time.Parse(time.RFC3339, v.(string)) - input.EndTime = aws.Time(t) - } -} - -func scheduledActionPopulateInputForUpdate(input *applicationautoscaling.PutScheduledActionInput, d *schema.ResourceData) bool { - hasChange := false - - if d.HasChange("schedule") { - input.Schedule = aws.String(d.Get("schedule").(string)) - hasChange = true - } - - if d.HasChange("scalable_target_action") { - input.ScalableTargetAction = expandScalableTargetAction(d.Get("scalable_target_action").([]interface{})) - hasChange = true - } + const ( + timeout = 5 * time.Minute + ) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, timeout, func() (interface{}, error) { + return conn.PutScheduledActionWithContext(ctx, input) + }, applicationautoscaling.ErrCodeObjectNotFoundException) - if d.HasChange("timezone") { - input.Timezone = aws.String(d.Get("timezone").(string)) - hasChange = true + if err != nil { + return sdkdiag.AppendErrorf(diags, "putting Application Auto Scaling Scheduled Action (%s): %s", id, err) } - if d.HasChange("start_time") { - if v, ok := d.GetOk("start_time"); ok { - t, _ := time.Parse(time.RFC3339, v.(string)) - input.StartTime = aws.Time(t) - hasChange = true - } - } - if d.HasChange("end_time") { - if v, ok := d.GetOk("end_time"); ok { - t, _ := time.Parse(time.RFC3339, v.(string)) - input.EndTime = aws.Time(t) - hasChange = true - } + if d.IsNewResource() { + d.SetId(id) } - return hasChange + return append(diags, resourceScheduledActionRead(ctx, d, meta)...) } func resourceScheduledActionRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - scheduledAction, err := FindScheduledAction(ctx, conn, d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string), d.Get("scalable_dimension").(string)) + scheduledAction, err := findScheduledActionByFourPartKey(ctx, conn, d.Get("name").(string), d.Get("service_namespace").(string), d.Get("resource_id").(string), d.Get("scalable_dimension").(string)) + if tfresource.NotFound(err) && !d.IsNewResource() { log.Printf("[WARN] Application Auto Scaling Scheduled Action (%s) not found, removing from state", d.Id()) d.SetId("") return diags } + if err != nil { - return sdkdiag.AppendErrorf(diags, "describing Application Auto Scaling Scheduled Action (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "reading Application Auto Scaling Scheduled Action (%s): %s", d.Id(), err) } + d.Set("arn", scheduledAction.ScheduledActionARN) + if scheduledAction.EndTime != nil { + d.Set("end_time", scheduledAction.EndTime.Format(time.RFC3339)) + } if err := d.Set("scalable_target_action", flattenScalableTargetAction(scheduledAction.ScalableTargetAction)); err != nil { return sdkdiag.AppendErrorf(diags, "setting scalable_target_action: %s", err) } - d.Set("schedule", scheduledAction.Schedule) if scheduledAction.StartTime != nil { d.Set("start_time", scheduledAction.StartTime.Format(time.RFC3339)) } - if scheduledAction.EndTime != nil { - d.Set("end_time", scheduledAction.EndTime.Format(time.RFC3339)) - } d.Set("timezone", scheduledAction.Timezone) - d.Set("arn", scheduledAction.ScheduledActionARN) return diags } @@ -239,25 +213,71 @@ func resourceScheduledActionDelete(ctx context.Context, d *schema.ResourceData, var diags diag.Diagnostics conn := meta.(*conns.AWSClient).AppAutoScalingConn(ctx) - input := &applicationautoscaling.DeleteScheduledActionInput{ + log.Printf("[DEBUG] Deleting Application Auto Scaling Scheduled Action: %s", d.Id()) + _, err := conn.DeleteScheduledActionWithContext(ctx, &applicationautoscaling.DeleteScheduledActionInput{ + ResourceId: aws.String(d.Get("resource_id").(string)), + ScalableDimension: aws.String(d.Get("scalable_dimension").(string)), ScheduledActionName: aws.String(d.Get("name").(string)), ServiceNamespace: aws.String(d.Get("service_namespace").(string)), - ResourceId: aws.String(d.Get("resource_id").(string)), - } - if v, ok := d.GetOk("scalable_dimension"); ok { - input.ScalableDimension = aws.String(v.(string)) + }) + + if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { + return diags } - _, err := conn.DeleteScheduledActionWithContext(ctx, input) + if err != nil { - if tfawserr.ErrCodeEquals(err, applicationautoscaling.ErrCodeObjectNotFoundException) { - return diags - } return sdkdiag.AppendErrorf(diags, "deleting Application Auto Scaling Scheduled Action (%s): %s", d.Id(), err) } return diags } +func findScheduledActionByFourPartKey(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, name, serviceNamespace, resourceID, scalableDimension string) (*applicationautoscaling.ScheduledAction, error) { + input := &applicationautoscaling.DescribeScheduledActionsInput{ + ResourceId: aws.String(resourceID), + ScheduledActionNames: aws.StringSlice([]string{name}), + ServiceNamespace: aws.String(serviceNamespace), + } + + return findScheduledAction(ctx, conn, input, func(v *applicationautoscaling.ScheduledAction) bool { + return aws.StringValue(v.ScheduledActionName) == name && aws.StringValue(v.ScalableDimension) == scalableDimension + }) +} + +func findScheduledAction(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, input *applicationautoscaling.DescribeScheduledActionsInput, filter tfslices.Predicate[*applicationautoscaling.ScheduledAction]) (*applicationautoscaling.ScheduledAction, error) { + output, err := findScheduledActions(ctx, conn, input, filter) + + if err != nil { + return nil, err + } + + return tfresource.AssertSinglePtrResult(output) +} + +func findScheduledActions(ctx context.Context, conn *applicationautoscaling.ApplicationAutoScaling, input *applicationautoscaling.DescribeScheduledActionsInput, filter tfslices.Predicate[*applicationautoscaling.ScheduledAction]) ([]*applicationautoscaling.ScheduledAction, error) { + var output []*applicationautoscaling.ScheduledAction + + err := conn.DescribeScheduledActionsPagesWithContext(ctx, input, func(page *applicationautoscaling.DescribeScheduledActionsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.ScheduledActions { + if v != nil && filter(v) { + output = append(output, v) + } + } + + return !lastPage + }) + + if err != nil { + return nil, err + } + + return output, nil +} + func expandScalableTargetAction(l []interface{}) *applicationautoscaling.ScalableTargetAction { if len(l) == 0 || l[0] == nil { return nil diff --git a/internal/service/appautoscaling/scheduled_action_test.go b/internal/service/appautoscaling/scheduled_action_test.go index c51487a704c..8ced78327f5 100644 --- a/internal/service/appautoscaling/scheduled_action_test.go +++ b/internal/service/appautoscaling/scheduled_action_test.go @@ -607,7 +607,7 @@ func testAccCheckScheduledActionExists(ctx context.Context, name string, obj *ap conn := acctest.Provider.Meta().(*conns.AWSClient).AppAutoScalingConn(ctx) - sa, err := tfappautoscaling.FindScheduledAction(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + sa, err := tfappautoscaling.FindScheduledActionByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) if err != nil { return err } diff --git a/internal/service/appautoscaling/service_package_gen.go b/internal/service/appautoscaling/service_package_gen.go index b3afc60b206..3ef20f9379a 100644 --- a/internal/service/appautoscaling/service_package_gen.go +++ b/internal/service/appautoscaling/service_package_gen.go @@ -34,7 +34,7 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka TypeName: "aws_appautoscaling_policy", }, { - Factory: ResourceScheduledAction, + Factory: resourceScheduledAction, TypeName: "aws_appautoscaling_scheduled_action", }, { From 13b930a7a0d15fef989d6ecf1484788979b7d1c0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 08:30:16 -0400 Subject: [PATCH 5/8] r/aws_appautoscaling_scheduled_action: Tidy up acceptance tests. --- .../appautoscaling/scheduled_action_test.go | 148 ++++++++---------- 1 file changed, 65 insertions(+), 83 deletions(-) diff --git a/internal/service/appautoscaling/scheduled_action_test.go b/internal/service/appautoscaling/scheduled_action_test.go index 8ced78327f5..6c6bf4d71c4 100644 --- a/internal/service/appautoscaling/scheduled_action_test.go +++ b/internal/service/appautoscaling/scheduled_action_test.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfappautoscaling "github.com/hashicorp/terraform-provider-aws/internal/service/appautoscaling" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" ) @@ -577,42 +578,39 @@ func testAccCheckScheduledActionDestroy(ctx context.Context) resource.TestCheckF continue } - input := &applicationautoscaling.DescribeScheduledActionsInput{ - ResourceId: aws.String(rs.Primary.Attributes["resource_id"]), - ScheduledActionNames: []*string{aws.String(rs.Primary.Attributes["name"])}, - ServiceNamespace: aws.String(rs.Primary.Attributes["service_namespace"]), + _, err := tfappautoscaling.FindScheduledActionByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + + if tfresource.NotFound(err) { + continue } - resp, err := conn.DescribeScheduledActionsWithContext(ctx, input) + if err != nil { return err } - if len(resp.ScheduledActions) > 0 { - return fmt.Errorf("Appautoscaling Scheduled Action (%s) not deleted", rs.Primary.Attributes["name"]) - } + + return fmt.Errorf("Application Auto Scaling Scheduled Action %s still exists", rs.Primary.ID) } + return nil } } -func testAccCheckScheduledActionExists(ctx context.Context, name string, obj *applicationautoscaling.ScheduledAction) resource.TestCheckFunc { +func testAccCheckScheduledActionExists(ctx context.Context, n string, v *applicationautoscaling.ScheduledAction) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", name) - } - - if rs.Primary.ID == "" { - return fmt.Errorf("Application Autoscaling scheduled action (%s) ID not set", name) + return fmt.Errorf("Not found: %s", n) } conn := acctest.Provider.Meta().(*conns.AWSClient).AppAutoScalingConn(ctx) - sa, err := tfappautoscaling.FindScheduledActionByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + output, err := tfappautoscaling.FindScheduledActionByFourPartKey(ctx, conn, rs.Primary.Attributes["name"], rs.Primary.Attributes["service_namespace"], rs.Primary.Attributes["resource_id"], rs.Primary.Attributes["scalable_dimension"]) + if err != nil { return err } - *obj = *sa + *v = *output return nil } @@ -621,7 +619,7 @@ func testAccCheckScheduledActionExists(ctx context.Context, name string, obj *ap func testAccCheckScheduledActionNotRecreated(i, j *applicationautoscaling.ScheduledAction) resource.TestCheckFunc { return func(s *terraform.State) error { if !aws.TimeValue(i.CreationTime).Equal(aws.TimeValue(j.CreationTime)) { - return fmt.Errorf("Application Auto Scaling scheduled action recreated") + return fmt.Errorf("Application Auto Scaling Scheduled Action recreated") } return nil @@ -761,7 +759,7 @@ resource "aws_ecs_service" "test" { } func testAccScheduledActionConfig_emr(rName, ts string) string { - return fmt.Sprintf(` + return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptInDefaultExclude(), fmt.Sprintf(` resource "aws_appautoscaling_scheduled_action" "test" { name = %[1]q service_namespace = aws_appautoscaling_target.test.service_namespace @@ -784,17 +782,6 @@ resource "aws_appautoscaling_target" "test" { max_capacity = 5 } -data "aws_availability_zones" "available" { - # The requested instance type c4.large is not supported in the requested availability zone. - exclude_zone_ids = ["usw2-az4"] - state = "available" - - filter { - name = "opt-in-status" - values = ["opt-in-not-required"] - } -} - data "aws_partition" "current" {} resource "aws_emr_cluster" "test" { @@ -847,58 +834,64 @@ resource "aws_emr_instance_group" "test" { instance_type = "c4.large" } +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + enable_dns_hostnames = true + + tags = { + Name = %[1]q + } +} + +resource "aws_internet_gateway" "test" { + vpc_id = aws_vpc.test.id + + tags = { + Name = %[1]q + } +} + resource "aws_security_group" "test" { - name = %[1]q - description = "Allow all inbound traffic" - vpc_id = aws_vpc.test.id + name = %[1]q + vpc_id = aws_vpc.test.id ingress { from_port = 0 - to_port = 0 protocol = "-1" self = true + to_port = 0 } egress { + cidr_blocks = ["0.0.0.0/0"] from_port = 0 - to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + to_port = 0 } - depends_on = [aws_subnet.test] - - lifecycle { - ignore_changes = [ - ingress, - egress, - ] + tags = { + Name = %[1]q + for-use-with-amazon-emr-managed-policies = true } -} -resource "aws_vpc" "test" { - cidr_block = "168.31.0.0/16" - enable_dns_hostnames = true - - tags = { - Name = "terraform-testacc-appautoscaling-scheduled-action-emr" + # EMR will modify ingress rules + lifecycle { + ignore_changes = [ingress] } } resource "aws_subnet" "test" { - availability_zone = data.aws_availability_zones.available.names[0] - cidr_block = "168.31.0.0/20" - vpc_id = aws_vpc.test.id + availability_zone = data.aws_availability_zones.available.names[0] + cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 8, 0) + map_public_ip_on_launch = true + vpc_id = aws_vpc.test.id tags = { - Name = "tf-acc-appautoscaling-scheduled-action" + Name = %[1]q + for-use-with-amazon-emr-managed-policies = true } } -resource "aws_internet_gateway" "test" { - vpc_id = aws_vpc.test.id -} - resource "aws_route_table" "test" { vpc_id = aws_vpc.test.id @@ -906,11 +899,15 @@ resource "aws_route_table" "test" { cidr_block = "0.0.0.0/0" gateway_id = aws_internet_gateway.test.id } + + tags = { + Name = %[1]q + } } -resource "aws_main_route_table_association" "test" { - vpc_id = aws_vpc.test.id +resource "aws_route_table_association" "test" { route_table_id = aws_route_table.test.id + subnet_id = aws_subnet.test.id } resource "aws_iam_role" "emr_role" { @@ -1089,7 +1086,7 @@ resource "aws_iam_role_policy_attachment" "autoscale_role" { role = aws_iam_role.autoscale_role.name policy_arn = "arn:${data.aws_partition.current.partition}:iam::aws:policy/service-role/AmazonElasticMapReduceforAutoScalingRole" } -`, rName, ts) +`, rName, ts)) } func testAccScheduledActionConfig_duplicateName(rName string) string { @@ -1163,7 +1160,7 @@ resource "aws_appautoscaling_scheduled_action" "test" { } func testAccScheduledActionConfig_spotFleet(rName, ts, validUntil string) string { - return fmt.Sprintf(` + return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinux2HVMEBSX8664AMI(), fmt.Sprintf(` resource "aws_appautoscaling_scheduled_action" "test" { name = %[1]q service_namespace = aws_appautoscaling_target.test.service_namespace @@ -1185,24 +1182,9 @@ resource "aws_appautoscaling_target" "test" { max_capacity = 3 } -data "aws_ami" "amzn-ami-minimal-hvm-ebs" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-minimal-hvm-*"] - } - - filter { - name = "root-device-type" - values = ["ebs"] - } -} - data "aws_partition" "current" {} -resource "aws_iam_role" "fleet_role" { +resource "aws_iam_role" "test" { assume_role_policy = < Date: Mon, 18 Mar 2024 08:32:48 -0400 Subject: [PATCH 6/8] Reduce visibility. --- internal/service/appautoscaling/exports_test.go | 2 ++ internal/service/appautoscaling/policy.go | 4 ++-- internal/service/appautoscaling/service_package_gen.go | 5 +++-- internal/service/appautoscaling/target.go | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/service/appautoscaling/exports_test.go b/internal/service/appautoscaling/exports_test.go index 468e37e8a00..e766811783c 100644 --- a/internal/service/appautoscaling/exports_test.go +++ b/internal/service/appautoscaling/exports_test.go @@ -5,7 +5,9 @@ package appautoscaling // Exports for use in tests only. var ( + ResourcePolicy = resourcePolicy ResourceScheduledAction = resourceScheduledAction + ResourceTarget = resourceTarget FindScheduledActionByFourPartKey = findScheduledActionByFourPartKey ) diff --git a/internal/service/appautoscaling/policy.go b/internal/service/appautoscaling/policy.go index 1d29c938bad..f73f514a0ca 100644 --- a/internal/service/appautoscaling/policy.go +++ b/internal/service/appautoscaling/policy.go @@ -29,8 +29,8 @@ const ( ResNamePolicy = "Policy" ) -// @SDKResource("aws_appautoscaling_policy") -func ResourcePolicy() *schema.Resource { +// @SDKResource("aws_appautoscaling_policy", name="Scaling Policy") +func resourcePolicy() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourcePolicyCreate, ReadWithoutTimeout: resourcePolicyRead, diff --git a/internal/service/appautoscaling/service_package_gen.go b/internal/service/appautoscaling/service_package_gen.go index 3ef20f9379a..3d5aebfed24 100644 --- a/internal/service/appautoscaling/service_package_gen.go +++ b/internal/service/appautoscaling/service_package_gen.go @@ -30,15 +30,16 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePackageSDKResource { return []*types.ServicePackageSDKResource{ { - Factory: ResourcePolicy, + Factory: resourcePolicy, TypeName: "aws_appautoscaling_policy", + Name: "Scaling Policy", }, { Factory: resourceScheduledAction, TypeName: "aws_appautoscaling_scheduled_action", }, { - Factory: ResourceTarget, + Factory: resourceTarget, TypeName: "aws_appautoscaling_target", Name: "Target", Tags: &types.ServicePackageResourceTags{ diff --git a/internal/service/appautoscaling/target.go b/internal/service/appautoscaling/target.go index 5fa630c3e3b..5f8aed4cf4d 100644 --- a/internal/service/appautoscaling/target.go +++ b/internal/service/appautoscaling/target.go @@ -26,7 +26,7 @@ import ( // @SDKResource("aws_appautoscaling_target", name="Target") // @Tags(identifierAttribute="arn") -func ResourceTarget() *schema.Resource { +func resourceTarget() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceTargetCreate, ReadWithoutTimeout: resourceTargetRead, From 02a02e54dc0de56279c96d5f6614e27c162d845c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 08:34:20 -0400 Subject: [PATCH 7/8] Add 'TestAccAppAutoScalingScheduledAction_disappears'. --- .../appautoscaling/scheduled_action_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/internal/service/appautoscaling/scheduled_action_test.go b/internal/service/appautoscaling/scheduled_action_test.go index 6c6bf4d71c4..9d9eba33948 100644 --- a/internal/service/appautoscaling/scheduled_action_test.go +++ b/internal/service/appautoscaling/scheduled_action_test.go @@ -79,6 +79,32 @@ func TestAccAppAutoScalingScheduledAction_dynamoDB(t *testing.T) { }) } +func TestAccAppAutoScalingScheduledAction_disappears(t *testing.T) { + ctx := acctest.Context(t) + var sa applicationautoscaling.ScheduledAction + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + schedule := time.Now().AddDate(0, 0, 1).Format("2006-01-02T15:04:05") + + resourceName := "aws_appautoscaling_scheduled_action.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.AppAutoScalingServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckScheduledActionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccScheduledActionConfig_dynamoDB(rName, schedule), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckScheduledActionExists(ctx, resourceName, &sa), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfappautoscaling.ResourceTarget(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccAppAutoScalingScheduledAction_ecs(t *testing.T) { ctx := acctest.Context(t) var sa applicationautoscaling.ScheduledAction From ac57a54336c53b31c44827c9c0d8b1e483dfab69 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 18 Mar 2024 08:39:36 -0400 Subject: [PATCH 8/8] r/aws_appautoscaling_scheduled_action: Fix acceptance tests. --- internal/service/appautoscaling/scheduled_action_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/appautoscaling/scheduled_action_test.go b/internal/service/appautoscaling/scheduled_action_test.go index 9d9eba33948..0a3c2049cda 100644 --- a/internal/service/appautoscaling/scheduled_action_test.go +++ b/internal/service/appautoscaling/scheduled_action_test.go @@ -97,7 +97,7 @@ func TestAccAppAutoScalingScheduledAction_disappears(t *testing.T) { Config: testAccScheduledActionConfig_dynamoDB(rName, schedule), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckScheduledActionExists(ctx, resourceName, &sa), - acctest.CheckResourceDisappears(ctx, acctest.Provider, tfappautoscaling.ResourceTarget(), resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfappautoscaling.ResourceScheduledAction(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -848,7 +848,7 @@ resource "aws_emr_cluster" "test" { configurations = "test-fixtures/emr_configurations.json" - depends_on = [aws_main_route_table_association.test] + depends_on = [aws_route_table_association.test] service_role = aws_iam_role.emr_role.arn autoscaling_role = aws_iam_role.autoscale_role.arn