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 evictionHard, kubeReserved and podsPerCore to Provisioner #2444

Merged
merged 13 commits into from
Sep 12, 2022

Conversation

jonathan-innis
Copy link
Contributor

Fixes #2129

Description

  • Adds the ability to pass evictionHard, kubeReserved and podsPerCore in Provisioner spec
apiVersion: karpenter.sh/v1alpha5
kind: Provisioner
metadata:
  name: default
spec:
  ...
  kubeletConfiguration:
    podsPerCore: 3
    kubeReserved:
      cpu: 100m
      memory: 50Mi
    evictionHard:
      memory.available: 10%

How was this change tested?

  • make test
  • TEST_FILTER="integration" make e2etests
  • Manual validation

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note

Added `evictionHard`, `kubeReserved` and `podsPerCore` to Provisioner

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

Sorry, something went wrong.

@jonathan-innis jonathan-innis requested a review from a team as a code owner September 1, 2022 19:20
@netlify
Copy link

netlify bot commented Sep 1, 2022

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit cfe75cb
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/631bc94ce9bf6f0008449c8b

if e.KubeletConfig != nil && e.KubeletConfig.MaxPods != nil {
userData.WriteString(" \\\n--use-max-pods false")
kubeletExtraArgs += fmt.Sprintf(" --max-pods=%d", ptr.Int32Value(e.KubeletConfig.MaxPods))
} else if !e.AWSENILimitedPodDensity {
userData.WriteString(" \\\n--use-max-pods false")
kubeletExtraArgs += " --max-pods=110"
}
if e.KubeletConfig != nil && e.KubeletConfig.PodsPerCore != nil {
kubeletExtraArgs += fmt.Sprintf(" --pods-per-core=%d", ptr.Int32Value(e.KubeletConfig.PodsPerCore))
Copy link
Contributor

Choose a reason for hiding this comment

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

@bwagner5 Wasn't there some weird issue earlier this year with the eks bootstrap script and setting values using an = sign?

E.g.
--pods-per-core 50 vs --pods-per-core=50

Copy link
Contributor

Choose a reason for hiding this comment

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

Found the old issue, #1520 , it's only the bootstrap args that it causes problems with.

Copy link
Contributor

Choose a reason for hiding this comment

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

^^ yep, kubelet args will work fine w/ =

tzneal
tzneal previously approved these changes Sep 9, 2022
Copy link
Contributor

@tzneal tzneal left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

nice! A few comments but mainly one issue in the podsPerCore calc.

@bwagner5
Copy link
Contributor

bwagner5 commented Sep 9, 2022

Does it make sense to support soft eviction thresholds as well?

@jonathan-innis
Copy link
Contributor Author

Does it make sense to support soft eviction thresholds as well?

It probably does, I figured we could decouple these for now, but we should probably follow-up this PR with support for evictionSoft and respect the higher of evictionHard and evictionSoft for scheduling

@jonathan-innis jonathan-innis force-pushed the added-kubelet-config branch 2 times, most recently from 6a2077b to 0d9cf51 Compare September 9, 2022 21:15
bwagner5
bwagner5 previously approved these changes Sep 9, 2022
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

lgtm

@jonathan-innis jonathan-innis force-pushed the added-kubelet-config branch 2 times, most recently from 2b5e0d9 to 6766f31 Compare September 9, 2022 23:16
Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Allow passing --kube-reserved/system-reserved/eviction-hard flags
3 participants