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

Add CAPI cluster autoscaling warning #976

Merged

Conversation

anders-elastisys
Copy link
Contributor

@anders-elastisys anders-elastisys commented Oct 17, 2024

⚠️ IMPORTANT ⚠️: This is a public repository. Make sure to not disclose:

  • personal data beyond what is necessary for interacting with this Pull Request;
  • business confidential information, such as customer names.

Quality gates:

  • I'm aware of the Contributor Guide and did my best to follow the guidelines.
  • I'm aware of the Glossary and did my best to use those terms.

Application Developer applications could potentially block the cluster autoscaler from downscaling nodes. This PR adds a warning notice about this in the Cluster API documentation.

Related to https://github.com/elastisys/ck8s-issue-tracker/issues/313

@anders-elastisys anders-elastisys requested a review from a team October 17, 2024 12:23
@anders-elastisys anders-elastisys requested a review from a team as a code owner October 17, 2024 12:23
Copy link
Collaborator

@cristiklein cristiklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition and something I learned today. Thanks!

Copy link
Contributor

@Xartos Xartos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that you can set emptyDir medium to memory 🤯

Question: do you know if all this can be fixed by setting "cluster-autoscaler.kubernetes.io/safe-to-evict": "true"? I'm just thinking if we should also add that here

@anders-elastisys
Copy link
Contributor Author

Question: do you know if all this can be fixed by setting "cluster-autoscaler.kubernetes.io/safe-to-evict": "true"? I'm just thinking if we should also add that here

I have not tested this myself but according to the docs that seems to fix it for problematic pods The link added in the warning notice would inform about using the label, but I guess it cant hurt to make it more visible by adding it directly in this document as well.

@Xartos
Copy link
Contributor

Xartos commented Oct 18, 2024

Question: do you know if all this can be fixed by setting "cluster-autoscaler.kubernetes.io/safe-to-evict": "true"? I'm just thinking if we should also add that here

I have not tested this myself but according to the docs that seems to fix it for problematic pods The link added in the warning notice would inform about using the label, but I guess it cant hurt to make it more visible by adding it directly in this document as well.

I guess it might be good to "force" them to go through the list to maybe fix it properly (Like fix the emptyDir or whatever) and not just incentivize using this "catch-all" solution. Because as you say, they should be able to find that themselves pretty quickly anyway if they look

@anders-elastisys
Copy link
Contributor Author

@Xartos then I will merge this as it is, I also went ahead and created an issue for possibly adding gatekeeper constraints that can help prevent issues with this in the future: elastisys/compliantkubernetes-apps#2318

@anders-elastisys anders-elastisys merged commit c466abb into main Oct 18, 2024
1 check passed
@anders-elastisys anders-elastisys deleted the anders-elastisys/add-capi-autoscaling-warning branch October 18, 2024 09:57
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.

5 participants