Skip to content

Use Kubelet config #86

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

Merged
merged 1 commit into from
Nov 6, 2018
Merged

Use Kubelet config #86

merged 1 commit into from
Nov 6, 2018

Conversation

ernoaapa
Copy link
Contributor

@ernoaapa ernoaapa commented Nov 2, 2018

To remove deprecated kubelet flags, and make --cluster-dns
configurable at boot time, updated kubelet to use the --config
from userdata directory.

closes #71

dog jump

@ernoaapa ernoaapa force-pushed the kubelet-config branch 6 times, most recently from 0f7e3fa to 9912b8c Compare November 3, 2018 01:00
Copy link
Contributor

@ijc ijc left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks for this @ernoaapa!.

I'm a little bit unsure about pushing the responsibility for making kubelet-config.json all the way back to boot.sh, that is intended as a quick demo/getting started/example but we (or at least I) sort of expect that in production people will wrapthe boot with their own scaffolding/supervisor etc rather than using that script directly and this puts a lot more responsibility on them than they previously had compared with setting a few semantically quite high level options.

OTOH being able to provide the whole config file is pretty much the apex of flexibility...

At the risk of layering on complexity, how about we do both? IOW if /run/config/kubelet-config.json exists then kubelet.sh uses it, but if it doesn't then it will construct one out of various things which can be written into /etc/kubelet.sh.conf (including $KUBE_CLUSTER_DNS). I think that mostly just means moving the code you already added to boot.sh into kubelet.sh and adjusting the scaffolding a little to look for /run/config/kubelet-config.json first.

WDYT?

(BTW, I like the using read to do a multiline/complex assignment with substitutions! I've not seen that pattern before)

@ernoaapa
Copy link
Contributor Author

ernoaapa commented Nov 6, 2018

@ijc Supporting both sounds like a good idea! I'll update this.

To remove deprecated kubelet flags, and make `--cluster-dns`
configurable at boot time, updated kubelet to use the `--config`
from userdata directory.

Signed-off-by: Erno Aapa <erno.aapa@gmail.com>
@ernoaapa
Copy link
Contributor Author

ernoaapa commented Nov 6, 2018

Thanks for the feedback @ijc! Updated the PR based on your feedback.

Copy link
Contributor

@ijc ijc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

It'd be nice to document the various things which can be written in /etc/kubelet.sh.conf (now including KUBE_CLUSTER_DNS) but we don't actually doc the current set so not something I think we should block this PR on.

@ijc ijc merged commit 7622bd4 into linuxkit:master Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch from cli configuration to kubelet.conf
3 participants