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: taints and labels per autoscaler nodepool #1012

Merged

Conversation

Silvest89
Copy link
Contributor

@Silvest89 Silvest89 commented Oct 10, 2023

I found that the autoscaler support for taints and labels was very very lacking! The autoscaler predicate didn't know before hand the taints and labels of the nodepools . And there was no support of taints/labels per nodepool.
This also makes it so that multi arch node pools are possible! I have been running this a month or so~ and no quirks 👯

This PR goes hand in hand with a PR in autoscaler. eg breaking-change

kubernetes/autoscaler#6184

@mysticaltech mysticaltech changed the base branch from master to staging October 11, 2023 13:04
@mysticaltech
Copy link
Collaborator

@Silvest89 This is great, super needed indeed. As soon as the autoscaler PR merges, we will merge this one. Please let us know. Thanks!

@Silvest89 Silvest89 force-pushed the autoscaler-taints-labels branch 2 times, most recently from e03bb94 to 0996a05 Compare October 12, 2023 21:29
@Silvest89 Silvest89 force-pushed the autoscaler-taints-labels branch 2 times, most recently from 5130e7b to a80fa62 Compare October 16, 2023 19:42
@Silvest89 Silvest89 force-pushed the autoscaler-taints-labels branch from a80fa62 to 62ac1a4 Compare October 18, 2023 17:04
@Silvest89 Silvest89 marked this pull request as draft October 18, 2023 17:27
@Silvest89 Silvest89 marked this pull request as ready for review October 20, 2023 05:52
@Silvest89
Copy link
Contributor Author

@mysticaltech The autoscaler PR has been approved ;], so will see when it merges.

@mysticaltech
Copy link
Collaborator

@mysticaltech The autoscaler PR has been approved ;], so will see when it merges.

Wonderful news! 🙏💙

@Silvest89
Copy link
Contributor Author

@mysticaltech
The pr is merged

@mysticaltech
Copy link
Collaborator

Sorry @Silvest89, I did not have time to reply today. Super good job 🙏🙏

This weekend I merged all PRs to staging but there was an error when trying to do a test deloy of staging. I need to debug this and then we can merge this.

Will try to debug tonight.

@mysticaltech
Copy link
Collaborator

mysticaltech commented Oct 23, 2023

Staging now worked! During the week-end when I tried, it was a network error while fetching k3s, maybe one of that service was down at the moment at the very moment I gave it a shot. So merging this for now. We can add more features later! 🚀

@mysticaltech
Copy link
Collaborator

mysticaltech commented Oct 24, 2023

@Silvest89 Now staging deployed but getting this error:

Screenshot from 2023-10-24 01-47-29

Screenshot from 2023-10-24 03-09-19

So probably templates/autoscaler.yaml.tpl needs tweaking. A fix would be super appreciated! 🙏

@mysticaltech
Copy link
Collaborator

The changes remains on staging, but I just deployed all previous PRs in v2.8.3; we will release v2.9.0 along with the fix 🚀

@Silvest89
Copy link
Contributor Author

Ah right, this is because you still run with the old autoscaler 🤭 will push the fix later today

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.

2 participants