-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix: Mark nodes that are pending deletion in cluster state for scheduling #2477
Conversation
✅ Deploy Preview for karpenter-docs-prod canceled.
|
e65d361
to
59f4cf2
Compare
This didn't work for me when I pulled this branch and ran it. I have this pdb: apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: inflate-pdb
namespace: default
spec:
maxUnavailable: 5
selector:
matchLabels:
app: inflate and I scaled up 100 inflate pods (the deployment is modified to use 250m CPU each) that landed on a single 8xlarge node and waited until they got running. I deleted the node and we did launch an 8xlarge node at first, but there's some interaction with consolidation that isn't correct as when the node was half full, the new 8xlarge was replaced with a 4xlarge. Original node w/100 pods - ip-192-168-173-189.us-west-2.compute.internal While the pods were slowly moving from the old node to the new node, consolidation tried to replace the new node with a smaller version:
It looks like consolidation isn't counting the draining pods or it would have known that didn't work. |
Yep, that's a complete miss on my side. I updated the PR. Will add a test for your scenario as well |
db8f09d
to
cd75877
Compare
cd75877
to
6447a94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fixes #
Description
How was this change tested?
make test
TEST_FILTER=Integration make e2etests
Does this change impact docs?
Release Note
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.