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: added disrupting candidates for nodepool status #634

Closed
wants to merge 1 commit into from

Conversation

sadath-12
Copy link
Contributor

Fixes #493 ( NodesDisrupting part )

Description

Whenever karpenter has decided to disrupt some nodes it will be updated in status field NodesDisrupting of nodepool and when that's done NodesDisrupting will become empty again

How was this change tested?
I ran few tests and saw in the console if the status were updating

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sadath-12 sadath-12 requested a review from a team as a code owner October 24, 2023 08:11
@njtran njtran added the blocked Unable to make progress due to some dependency label Oct 25, 2023
@jonathan-innis
Copy link
Member

This is another PR that adds API surface and I think needs a design before we can consider taking this into the project. There's some things that we need to consider here like the value of the API surface to users, are there other API avenues that we could surface the same information, how many times will this cause us to update the Provisioner and will that cause too many APIServer calls?

This is just a subset of questions that I have, but given these are questions that have large impact, I think a short design that's proposed with the changes could be helpful here.

@sadath-12
Copy link
Contributor Author

sure @jonathan-innis

@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2023
@sadath-12 sadath-12 closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Unable to make progress due to some dependency needs-design needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Surface health conditions of Karpenter resources as status conditions
4 participants