docs: Fix description of how autoscaling min and max are set in services

[new-guy]

Description

This PR updates the docs to accurately reflect how the desired_count variable affects the autoscaling definition in the services module. I am not sure of the ramifications of changing the functionality to reflect the docs, so I instead changed the docs to reflect the functionality.

Motivation and Context

I was trying to get the task count configured in some stuff I'm working on, and I found that the docs state that desired_count is ignored, however that variable is used when setting the min_capacity and max_capacity values in the aws_appautoscaling_target resource. I spent a good chunk of time trying to figure out what I was doing wrong before I dug through the source code and found the issue.

Breaking Changes

None

How Has This Been Tested?

No functional changes - just documentation

[bryantbiggs]

This is not correct - what issue are you running into?

[new-guy]

It could be that I'm doing something wrong with the way I'm configuring things, but it is unfortunately not true that desired_count is always ignored. See here: https://github.com/terraform-aws-modules/terraform-aws-ecs/blob/master/modules/service/main.tf#L1199-L1200

The min and max autoscaling targets are set to the min and max of their respective variables vs desired_count. If you set the autoscaling_min_capacity variable to 2, and desired_count remains at 1, then the value of autoscaling_min_capacity in the aws_appautoscaling_target resource will be set to 1.

[new-guy]

Maybe instead the autoscaling_min_capacity attribute in aws_appautoscaling_target should be the max of desired_count and var.autoscaling_min_capacity?

[bryantbiggs]

If you set the autoscaling_min_capacity variable to 2, and desired_count remains at 1, then the value of autoscaling_min_capacity in the aws_appautoscaling_target resource will be set to 1.

Conversely, if you set this on create, it will fail. While I agree its not a perfect scenario, there are always rough edges when two points of modification are interacting - the intent is still the same. The desired value must stay within the bounds of min and mix such that min >= desired >= max

[new-guy]

Conversely, if you set this on create, it will fail.

I am not sure I understand specifically what you mean by this. If you set what on create, what will fail?

Explaining the requirement that desired is between min and max was the objective of the documentation changes I made, though my changes could be worded better. I wasn't able to find this requirement in the existing documentation and had to go into the source code to discover this requirement. The docs currently state that the desired value is always ignored, which is misleading. Do you have ideas on how the docs should instead be modified to make this requirement more clear? Would love for others to benefit from my suffering, haha

[new-guy]

Maybe I could update an example to include the min, max, and desired? And update the docs to say what you said about desired count being bound by min and max rather than saying desired count is ignored?

[new-guy]

Any thoughts on this? It seems like it should be a relatively easy bit of detail to add

[bryantbiggs]

It's not clear what problem we are trying to solve, if there is even a problem. It seems like we're just trying to get a commit added, but I could be wrong

[thattommyhall]

this confused me, changing desired_count to 2 did nothing and adding autoscaling_{min|max}_capacity it was taking a minimum of 1 anyway, had to do all 3 to get what I wanted

  desired_count            = 2
  autoscaling_min_capacity = 2
  autoscaling_max_capacity = 8

[bryantbiggs]

Can you elaborate a bit more - what did you initially have, what did you want to change and how did you try changing it, etc

[thattommyhall]

sounds like what @new-guy described.

I set

desired_count            = 2

but nothing happend

Then I set

autoscaling_min_capacity = 2
autoscaling_max_capacity = 8

and nothing happened, only when all 3 were set did it go to 2

[bryantbiggs]

But thats the intended behavior we've described in the docs https://github.com/terraform-aws-modules/terraform-aws-ecs/tree/master/docs#service-1

desired_count is ignored because autoscaling is used. When autoscaling is used, it manages the value in desired_count which is why we need to ignore it and avoid Terraform from reverting changes (i.e - de-scale during an event because there was a Terraform apply). There are various ways to change the desired_count, one of which we have provided in the docs (see link above)

So it sounds like just lack of understanding - not sure how else we can make this behavior clear in the docs but its all detailed there

[thattommyhall]

Nice that the docs also mention ignore_task_definition_changes 👍 (that's just what I am coming up to now)

[bryantbiggs]

closing this out for now since I believe everything is covered in the current documentation

[github-actions]

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.