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

kubelet: make cluster-dns-ip optional #1482

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Apr 13, 2021

Issue number:
#1453

Description of changes:
Outside of EKS, we cannot compute a value for this, and there's not a reasonable default. We also need to account for it being optional for kubelet in standalone mode.

Testing done:
Since Those changes in different version kube-config are an identical change, and it works the same way in all versions, so we just need build aws-k8s-1.19-x86_64 and aws-k8s-1.19-aarch64 images to do test.

With cluster-dns-ip data
bottlerocket-aws-k8s-1.19-x86_64-v1.0.8-b20d9ca6
bottlerocket-aws-k8s-1.19-aarch64-v1.0.8-b20d9ca6

Launching instances to validate if clusterDNS works as expectation.

clusterDNS:
- 10.100.0.10

Without cluster-dns-ip data (remove cluster-dns-ip value in 50-aws-k8s.toml)
bottlerocket-aws-k8s-1.19-x86_64-v1.0.8-ad8c4812-dirty
bottlerocket-aws-k8s-1.19-aarch64-v1.0.8-ad8c4812-dirty

Launching instances to validate if clusterDNS is not in kubelet-config.

clusterDomain: cluster.local
kubeReserved:

(no clusterDNS value there)

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@gthao313 gthao313 added area/kubernetes K8s including EKS, EKS-A, and including VMW priority/p0 labels Apr 13, 2021
@gthao313 gthao313 added this to the next milestone Apr 13, 2021
@gthao313 gthao313 self-assigned this Apr 13, 2021
@gthao313 gthao313 removed their assignment Apr 13, 2021
@bcressey
Copy link
Contributor

Nits on commit message:

  • "kubelet: make cluster-dns-ip optional" would be a better title
  • it would be good to put the description in this PR into the actual commit message

Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

I'm happy once @bcressey 's suggestions re: the commit message are addressed. :)

Were you able to confirm that instances' config was correct with and without the setting? Could you add that to the testing section in the description?

@gthao313 gthao313 changed the title K8s: Make cluster-dns-ip be optional kubelet: make cluster-dns-ip optional Apr 14, 2021
@gthao313 gthao313 requested a review from zmrow April 14, 2021 00:03
Outside of EKS, we cannot compute a value for this, and there's not a
reasonable default. We also need to account for it being optional for
kubelet in standalone mode.
Copy link
Contributor

@zmrow zmrow left a comment

Choose a reason for hiding this comment

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

🔬

@gthao313 gthao313 merged commit adb3b69 into bottlerocket-os:develop Apr 14, 2021
@gthao313 gthao313 removed area/kubernetes K8s including EKS, EKS-A, and including VMW priority/p0 labels Apr 19, 2021
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.

3 participants