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

Add Ability to Specify Kubelet Configuration, specifically clusterDNS #1013

Merged
merged 8 commits into from
Dec 17, 2021
Merged

Add Ability to Specify Kubelet Configuration, specifically clusterDNS #1013

merged 8 commits into from
Dec 17, 2021

Conversation

mikesir87
Copy link
Contributor

@mikesir87 mikesir87 commented Dec 16, 2021

1. Issue, if available:
Resolves #928

2. Description of changes:
Adds the ability to specify kubeletConfiguration, specifically the clusterDNS setting.

3. Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: link to issue
  • No

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

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for karpenter-docs-prod canceled.

🔨 Explore the source changes: 96965e8

🔍 Inspect the deploy log: https://app.netlify.com/sites/karpenter-docs-prod/deploys/61bc05699e363a0007119967

Copy link
Contributor

@ellistarn ellistarn left a comment

Choose a reason for hiding this comment

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

LGTM. I'm a bit tired -- can you get an approval from Suket just in case I'm missing anything?

@ellistarn
Copy link
Contributor

Nice work, btw!

@mikesir87
Copy link
Contributor Author

@suket22 Can you take a look? Thanks!

mikesir87 and others added 7 commits December 16, 2021 19:18
Signed-off-by: Michael Irwin <mikesir87@gmail.com>

Resolves #928
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Co-authored-by: Ellis Tarn <ellistarn@gmail.com>
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@mikesir87
Copy link
Contributor Author

Rebased on top of latest to resolve conflict

@akestner akestner changed the title Add DNS cluster ip support Add Ability to Specify Kubelet Args, specifically clusterDNSIP Dec 17, 2021
ellistarn
ellistarn previously approved these changes Dec 17, 2021
@mikesir87
Copy link
Contributor Author

Just noticed that there was a name mismatch between the JSON object and the Go type. So, fixing that (any chance that could be caught in a lint check?).

And @akestner - I saw the title change. I'm going to tweak it just a tiny bit because the names ended up being adjusted as we did the work and I neglected to go back and update the original description of changes. I've rectified my transgression 😄

@mikesir87 mikesir87 changed the title Add Ability to Specify Kubelet Args, specifically clusterDNSIP Add Ability to Specify Kubelet Configuration, specifically clusterDNS Dec 17, 2021
Signed-off-by: Michael Irwin <mikesir87@gmail.com>
@ellistarn ellistarn merged commit ab0fdf5 into aws:main Dec 17, 2021
@justinsnair
Copy link

justinsnair commented Jan 20, 2022

@mikesir87

I have installed helm-chart v 0.5.5 which should have these changes however when trying to create a provisioner I am getting this error:

error: error validating "provisioner.yaml": error validating data: ValidationError(Provisioner.spec): unknown field "kubeletConfiguration" in sh.karpenter.v1alpha5.Provisioner.spec; if you choose to ignore these errors, turn validation off with --validate=false

Here is my provisioner:

apiVersion: karpenter.sh/v1alpha5
kind: Provisioner
metadata:
  name: default-provisioner
spec:
  kubeletConfiguration: 
    clusterDNS: ["redacted"]
  requirements:
    - key: karpenter.sh/capacity-type
      operator: In
      values: ["spot"]
  limits:
    resources:
      cpu: 500m
  provider:
    instanceProfile: "redacted"
  ttlSecondsAfterEmpty: 30

Am i missing something?

@mikesir87
Copy link
Contributor Author

@justinsnair Did you install the chart as an upgrade to a previous version? If so, you'll need to update the CRD as Helm doesn't do CRD updates.

@mikesir87 mikesir87 deleted the 928-add-dns-cluster-ip-support branch January 20, 2022 21:26
@justinsnair
Copy link

This was it. I knew this but totally slipped my mind.. Thanks for the reminder!

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.

Possibility to set dns-cluster-ip for Karpenter provider
3 participants