aws / karpenter-provider-aws

Karpenter is a Kubernetes Node Autoscaler built for flexibility, performance, and simplicity.
https://karpenter.sh
Apache License 2.0
6.84k stars 963 forks source link

fix: Fix pointer mutation causing incorrect maxPods values #7020

Closed jonathan-innis closed 2 months ago

jonathan-innis commented 2 months ago

Fixes #7011 Fixes #6890 Fixes #6979

Description

This PR fixes a bug where both GetKubeletConfigurationWithNodeClaim and GetKubeletConfigurationWithNodePool were returning the nodeClass.Spec.Kubelet back to the caller when Kubelet was specified and there was no Kubelet compatibility annotation.

This would be fine; however, this call returned back the original pointer for the Kubelet data -- meaning that any caller could then mutate the original data for other callers without knowing it. In this case, the launch template resolver was inadvertently mutating the maxPods data across instance type combinations, leading to incorrect maxPods values being specified sometimes.

How was this change tested?

Does this change impact docs?

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

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 10887984482

Details


Totals Coverage Status
Change from base Build 10838271515: 0.02%
Covered Lines: 5942
Relevant Lines: 7484

💛 - Coveralls
github-actions[bot] commented 2 months ago

Snapshot successfully published to oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter:0-cd04d65077eaed45e212e2140c0081768f3de547. To install you must login to the ECR repo with an AWS account:

aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 021119463062.dkr.ecr.us-east-1.amazonaws.com

helm upgrade --install karpenter oci://021119463062.dkr.ecr.us-east-1.amazonaws.com/karpenter/snapshot/karpenter --version "0-cd04d65077eaed45e212e2140c0081768f3de547" --namespace "kube-system" --create-namespace \
  --set "settings.clusterName=${CLUSTER_NAME}" \
  --set "settings.interruptionQueue=${CLUSTER_NAME}" \
  --set controller.resources.requests.cpu=1 \
  --set controller.resources.requests.memory=1Gi \
  --set controller.resources.limits.cpu=1 \
  --set controller.resources.limits.memory=1Gi \
  --wait
michaelkorofiverr commented 2 months ago

In which release can we expect this fix?