Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asg size changes should be ignored. #34

Closed
1 task
ozbillwang opened this issue Jun 26, 2018 · 16 comments
Closed
1 task

asg size changes should be ignored. #34

ozbillwang opened this issue Jun 26, 2018 · 16 comments

Comments

@ozbillwang
Copy link
Contributor

ozbillwang commented Jun 26, 2018

I have issues

asg size changes should be ignored.

I'm submitting a

  • feature request

What is the current behavior

Updated asg sized after deploy, terraform apply detects the changes, which should be ignored.

At least change indesired_capacity should be ignored.

  ~ module.eks.aws_autoscaling_group.workers
      desired_capacity:         "2" => "1"
      max_size:                 "5" => "3"
      min_size:                 "2" => "1"

What's the expected behaviour

Ignore the changes, since we don't want the running system to be re-sized.

Environment

  • Affected module version: 1.0.0
  • OS: ubuntu
  • Terraform version: 0.11.7

Other relevant info

If you are fine to ignore change in desired_capacity, I can raise PR for this feature, please confirm.

@brandonjbjelland
Copy link
Contributor

Hey @ozbillwang 👋

So if I understand this correctly, the only value you're wanting to ignore is the desired_capacity, correct? The other values (min & max) rightly had a delta that terraform planned to change. I don't think those need to be ignored since they aren't dynamic values.

This sounds reasonable to me. Looking forward to your PR. Thanks for going through the process! 🏅

ozbillwang added a commit to ozbillwang/terraform-aws-eks that referenced this issue Jun 26, 2018
@ozbillwang
Copy link
Contributor Author

PR has been raised.

@hatemosphere
Copy link
Contributor

so i guess with this PR merged we can experiment with external node autoscaler and in the end it will be compatible with terraform

@ozbillwang
Copy link
Contributor Author

@hatemosphere

This is no compatible problem. The benefit for this feature is, if you have running system with higher desired capacity, the change applied without reduce running worker nodes

brandonjbjelland added a commit that referenced this issue Jun 26, 2018
#34 - asg size changes should be ignored - desired_capacity
@brandonjbjelland
Copy link
Contributor

Good observations. Yea this whole space is tricky, trying to determine which declarative system should own which resources, but I think in this case it makes good sense to defer to the k8s autoscaler. 👍

@dev-e
Copy link

dev-e commented Feb 1, 2019

Hi! Why did you implement this rule? Does not it drive to configuration drift? Anyway it should be an opportunity to make any changes through terraform configuration, not by manual click in AWS console!

@max-rocket-internet
Copy link
Contributor

@dev-e no, because in k8s you need to use the cluster-autoscaler: https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/docs/autoscaling.md

@dev-e
Copy link

dev-e commented Feb 1, 2019

@max-rocket-internet use of cluster-autoscaler is not mandatory. What if you want to control number of nodes by yourself?

@max-rocket-internet
Copy link
Contributor

@dev-e sure but then you make the change manually.

@dev-e
Copy link

dev-e commented Feb 1, 2019

@max-rocket-internet, and so we get a configuration drift.

@max-rocket-internet
Copy link
Contributor

Sure. But most people use the cluster-autoscaler which is the proper way of handling scaling up and down. So what can we do? Lifecycle rules don't support interpolation so we can't make it easily configurable either.

@dev-e
Copy link

dev-e commented Feb 1, 2019

@max-rocket-internet, thanks, I understood your argumentation. Actually I meant the minimum number of nodes that cluster-autoscaler cannot bring down. With autoDiscovery.enabled=true we still can operate with the minimum capacity by modifying asg_min_size parameter. Maybe it will be useful to add somewhere to docs about cluster-autoscaler.

@chancez
Copy link

chancez commented Mar 18, 2020

Sure. But most people use the cluster-autoscaler which is the proper way of handling scaling up and down. So what can we do? Lifecycle rules don't support interpolation so we can't make it easily configurable either.

This is just blatantly false.

@chancez
Copy link

chancez commented Mar 18, 2020

Especially given that less and less support for the autoscaler is built in, it seems like this should be configurable. I will likely open a PR for this.

@chancez
Copy link

chancez commented Mar 18, 2020

Or I guess it has to be a static list. This is really unfortunate.

@github-actions
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants