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

fix: Fix support for --cloudprovider=external #375

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

jonathan-innis
Copy link
Member

@jonathan-innis jonathan-innis commented Jun 20, 2023

Fixes #

Description

EKS 1.26 adds in the support for --cloudprovider=external. Adding this value to the kubelet arguments causes the kubelet registration process to register the Node with the node.cloudprovider.kubernetes.io/uninitialized. Karpenter was unaware of this change, which caused it to overlaunch capacity when these taints were added to the node on startup.

How was this change tested?

Saw Karpenter over-scale when running a version prior to this change. After upgrade to the snapshot version that included this change, Karpenter did not over-scale and went to the proper number of nodes.

Prior to this change

karpenter git:(main) while true; do sleep 1; k get machines | wc -l; done
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     245
     261
     261
     261
     261
     261
     261
     261
     261
     261
     261
     261
     261
     261
     260
     260
     260
     260
     260
     260
     260
     260
     260
     260
     239
     239
     201
     201
     201
     201
     201

With this change

karpenter git:(main) while true; do sleep 1; k get machines | wc -l; done
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201
     201

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

@jonathan-innis jonathan-innis requested a review from a team as a code owner June 20, 2023 07:29
@jonathan-innis jonathan-innis changed the title chore: Fix externeal cloudprovider taint add in 1.26 chore: Fix external cloudprovider taint add in 1.26 Jun 20, 2023
@jonathan-innis jonathan-innis changed the title chore: Fix external cloudprovider taint add in 1.26 chore: Fix support for --cloudprovider=external Jun 20, 2023
@jonathan-innis jonathan-innis enabled auto-merge (squash) June 20, 2023 07:32
@jonathan-innis jonathan-innis merged commit b303101 into kubernetes-sigs:main Jun 20, 2023
@jonathan-innis jonathan-innis changed the title chore: Fix support for --cloudprovider=external fix: Fix support for --cloudprovider=external Jun 22, 2023
ellistarn pushed a commit to ellistarn/karpenter that referenced this pull request Jun 22, 2023
ellistarn pushed a commit to ellistarn/karpenter that referenced this pull request Jun 22, 2023
ellistarn pushed a commit to ellistarn/karpenter that referenced this pull request Jun 22, 2023
ellistarn added a commit that referenced this pull request Jun 22, 2023
Co-authored-by: Jonathan Innis <jonathan.innis.ji@gmail.com>
@dabmajor
Copy link

dabmajor commented Jul 6, 2023

Is it known if this bug impacts all releases prior to 0.28.0? For example, 0.27.5. I ask, because the issue is defined as being introduced by an external change in EKS 1.26

@tzneal
Copy link
Contributor

tzneal commented Jul 7, 2023

Prior to v0.28, Karpenter created the node object. In that case, kubelet wouldn't add the taint so the problem only existed in v0.28.0.

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.

4 participants