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

feat: WIP: allow user to over-scale a buffer of instances in an ASG #100

Closed
wants to merge 3 commits into from
Closed

feat: WIP: allow user to over-scale a buffer of instances in an ASG #100

wants to merge 3 commits into from

Conversation

js-timbirkett
Copy link
Contributor

Hello 👋 - A few weeks ago I opened #96 but promptly closed it as I could have solved the problem with a different tool. After looking back at this, I think it'd be simpler if solved in eks-rolling-update directly.

This PR adds a new env variable: ASG_BUFFER_INSTANCES which allows an arbitrary number to be given to eks_rolling_update.py and will cause each ASG to be over-scaled by that number.

But why?

The past few rolling upgrades I've done have resulted in some things like workloads with PV/PVC getting stuck in pending as other pods had started, scaleout of HPAs causing pods to get stuck in pending, deployments during rollout causing issues...

Since I've been pre-scaling each ASG by a few instances it hasn't been an issue and cluster-autoscaler takes care of scaling in unused compute after rollout.

As always, open to any feedback or ideas 😸

Thanks for an awesome tool!

@pysysops
Copy link

I wonder if this would help with #84 🤔

@js-timbirkett js-timbirkett changed the title feat: allow user to over-scale a buffer of instances in an ASG feat: WIP: allow user to over-scale a buffer of instances in an ASG Mar 18, 2021
@js-timbirkett
Copy link
Contributor Author

After looking at this again, I need to understand the behaviours of scaling in various cases first. I don't think it's as simple as the changes that have been made in this PR... I will look more closely at this 🔜

@adkafka
Copy link

adkafka commented Sep 1, 2021

Any update on this thread? We could use this extra parameter as well.

@js-timbirkett
Copy link
Contributor Author

Closing due to lack of time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants