Skip to content

r/aws_appautoscaling_scheduled_action: Fix bugs#8777

Merged
gdavison merged 8 commits into
hashicorp:mainfrom
stefansundin:appautoscaling_scheduled_action-capacity-fix
Mar 8, 2021
Merged

r/aws_appautoscaling_scheduled_action: Fix bugs#8777
gdavison merged 8 commits into
hashicorp:mainfrom
stefansundin:appautoscaling_scheduled_action-capacity-fix

Conversation

@stefansundin

Copy link
Copy Markdown
Contributor

Do not default max_capacity and min_capacity to 0. Allow them to be left undefined to allow either one to be used individually.

Basically, it is currently impossible to use Terraform to create the following:

$ aws application-autoscaling put-scheduled-action --service-namespace rds --scalable-dimension rds:cluster:ReadReplicaCount --resource-id cluster:xxx --scheduled-action-name scale-down --schedule "cron(0 22 ? * MON-FRI *)" --scalable-target-action MinCapacity=0

$ aws application-autoscaling describe-scheduled-actions --service-namespace rds
{
    "ScheduledActions": [
        {
            "ScheduledActionName": "scale-down",
            "ScheduledActionARN": "arn:aws:autoscaling:xxxxxxx",
            "ServiceNamespace": "rds",
            "Schedule": "cron(0 22 ? * MON-FRI *)",
            "ResourceId": "cluster:xxx",
            "ScalableDimension": "rds:cluster:ReadReplicaCount",
            "ScalableTargetAction": {
                "MinCapacity": 0
            }
        }
    ]
}

Basically, this provider would set max_capacity = 0, which would very likely trigger the following error:

Error: ValidationException: Maximum capacity cannot be less than minimum capacity

This change allows for specifying only min_capacity, or only max_capacity.

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

- resource/aws_appautoscaling_scheduled_action: Do not default `max_capacity` and `min_capacity` to `0`, allow either to be used on its own.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAppautoscalingScheduledAction_dynamo'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSAppautoscalingScheduledAction_dynamo -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAppautoscalingScheduledAction_dynamo
=== PAUSE TestAccAWSAppautoscalingScheduledAction_dynamo
=== CONT  TestAccAWSAppautoscalingScheduledAction_dynamo
--- PASS: TestAccAWSAppautoscalingScheduledAction_dynamo (127.86s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	127.916s

With only the test change, the following happened:

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSAppautoscalingScheduledAction_dynamo -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAppautoscalingScheduledAction_dynamo
=== PAUSE TestAccAWSAppautoscalingScheduledAction_dynamo
=== CONT  TestAccAWSAppautoscalingScheduledAction_dynamo
--- FAIL: TestAccAWSAppautoscalingScheduledAction_dynamo (112.68s)
    testing.go:568: Step 0 error: errors during apply:

        Error: ValidationException: Maximum capacity cannot be less than minimum capacity
        	status code: 400, request id: xxxxxx-xxxx-xxxx-xxxx-xxxxxxxxx

          on /var/folders/h6/r59_wnd905g_8s6x2mt7w0ch0000gn/T/tf-test995279429/main.tf line 22:
          (source code not available)


FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	112.733s
make: *** [testacc] Error 1

…eft undefined so that they can be used individually.
@ghost ghost added size/XS Managed by automation to categorize the size of a PR. service/applicationautoscaling tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels May 25, 2019
@stefansundin

Copy link
Copy Markdown
Contributor Author

@bflad If you have time, this one also needs a review.

@stefansundin

Copy link
Copy Markdown
Contributor Author

I have found more bugs in this resource.. For example, it does not detect changes (e.g. updated schedule), so it's currently only usable for the initial resource creation since it can't update any changes made.

Do you want me to add more fixes to this PR, or open two new smaller PRs?

…ate 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.
…updating these attributes, we can just put and it will overwrite the existing resource.
@ghost ghost added size/M Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels May 30, 2019
@stefansundin stefansundin changed the title r/aws_appautoscaling_scheduled_action: Do not default max_capacity and min_capacity to 0 r/aws_appautoscaling_scheduled_action: Fix bugs May 30, 2019
@stefansundin

Copy link
Copy Markdown
Contributor Author

Okay, I decided to put it all in here since it's all kind of dependent on each other. I tried to be as descriptive as possible in my commit messages. Let me know what needs tweaking.

And the test is still passing!

$ make testacc TESTARGS='-run=TestAccAWSAppautoscalingScheduledAction_dynamo'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -parallel 20 -run=TestAccAWSAppautoscalingScheduledAction_dynamo -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSAppautoscalingScheduledAction_dynamo
=== PAUSE TestAccAWSAppautoscalingScheduledAction_dynamo
=== CONT  TestAccAWSAppautoscalingScheduledAction_dynamo
--- PASS: TestAccAWSAppautoscalingScheduledAction_dynamo (41.27s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	41.327s

@aeschright aeschright requested a review from a team June 28, 2019 18:15
@stefansundin

Copy link
Copy Markdown
Contributor Author

@ewbankkit @bflad This PR needs some love a review, please. :)

I think it's done and from what I remember everything was working when I created this PR (over 250 days ago!).

Base automatically changed from master to main January 23, 2021 00:56
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:56
@gdavison gdavison self-assigned this Mar 5, 2021

@gdavison gdavison left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @stefansundin. I've made some updates to the acceptance tests and made the Update call only update parameters that have changed 🚀

@ghost

ghost commented Mar 12, 2021

Copy link
Copy Markdown

This has been released in version 3.32.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost

ghost commented Apr 8, 2021

Copy link
Copy Markdown

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants