feat: Add support for aws_autoscaling_policy

Sudokamikaze commented 2 years ago

Description

This PR adds support for aws_autoscaling_policy resource

Support is fully complete and covers all settings mentioned in documentation(https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/autoscaling_policy)

Motivation and Context

I decided to implement support for this resource, as it's a part of ASGs, brings (important in my opinion) feature to scale-in / scale out based on CPU load metric (predefined_metric_type = ASGAverageCPUUtilization)

How Has This Been Tested?

[ ] I have tested and validated these changes using one or more of the provided `examples/` projects I tested this PR creating ASG with addition of following block of code:

create_scaling_policy = true
scaling_policies = {
was-avg-cpu-policy-greater-than-xx = {
  policy_type               = "TargetTrackingScaling"
  estimated_instance_warmup = 1200
  target_tracking_configuration = {
    predefined_metric_specification = {
      predefined_metric_type = "ASGAverageCPUUtilization"
    }
    target_value = 50.0
  }
}
}

In terraform plan I see following result:

# module.workers-ami-test.module.autoscaling.aws_autoscaling_policy.this["was-avg-cpu-policy-greater-than-xx"] will be created"aws_autoscaling_policy" "this" {
  + arn                       = (known after apply)
  + autoscaling_group_name    = "workers-ami-test"
  + estimated_instance_warmup = 1200
  + id                        = (known after apply)
  + metric_aggregation_type   = (known after apply)
  + name                      = "was-avg-cpu-policy-greater-than-xx"
  + policy_type               = "TargetTrackingScaling"

  + target_tracking_configuration {
      + disable_scale_in = false
      + target_value     = 50

      + predefined_metric_specification {
          + predefined_metric_type = "ASGAverageCPUUtilization"
        }
    }
}

I also made a test without it, as variables have default value of false and null, it does not break anything

Sudokamikaze commented 2 years ago

Good start. Please update the code in the examples and docs in README.

Agreed, I'll update them and will let you know once it's done, thank you :)

Qanop commented 2 years ago

Great message! I was just looking for this solution today while modifying the code.

Sudokamikaze commented 2 years ago

@antonbabenko I've added example based on average CPU load and added missing outputs

Sudokamikaze commented 2 years ago

@bryantbiggs Hey, TYVM for the review & changes

I've implemented them, slightly tested by plan & updated documentation slightly

Sudokamikaze commented 2 years ago

Looks pretty good. There are few remaining comments, also please update outputs in examples, and make CI green&passing. Thanks!

BTW, I'm seeing First-time contributors need a maintainer to approve running workflows. Learn more. each time, it could be much better if those linters could run without approval, so I could fix it before asking you to re-review

Also I forgot to mention, in previous review round, I implemented target_value as required argument for both blocks (to align more with official documentation)

antonbabenko commented 2 years ago

This is how GitHub is working for us. You don't need to wait for GitHub Actions to run after you commit, because you can run pre-commit run -a to make all required changes locally before committing, and GitHub Actions will do the same and it will be green afterward. Also, you can run this once - pre-commit install --install-hooks and then run git commit normally.

I've just run it locally.

Thank you very much for this PR! I am going to merge it now.

antonbabenko commented 2 years ago

This PR is included in version 4.11.0 :tada:

Sudokamikaze commented 2 years ago

This is how GitHub is working for us. You don't need to wait for GitHub Actions to run after you commit, because you can run pre-commit run -a to make all required changes locally before committing, and GitHub Actions will do the same and it will be green afterward. Also, you can run this once - pre-commit install --install-hooks and then run git commit normally.

I've just run it locally.

Thank you very much for this PR! I am going to merge it now.

Huge thank you! In this PR I learned good practices and looking forward to contribute to your modules, next time it'll be much less to review ;)

antonbabenko commented 2 years ago

You are more than welcome! We have plenty of work to do in terraform-aws-modules.

github-actions[bot] commented 1 year ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-autoscaling