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

Implement support for Kubelet Dynamic Configuration #28

Closed
mikedanese opened this issue Nov 22, 2016 · 29 comments · Fixed by kubernetes/kubernetes#55803
Closed

Implement support for Kubelet Dynamic Configuration #28

mikedanese opened this issue Nov 22, 2016 · 29 comments · Fixed by kubernetes/kubernetes#55803
Assignees
Labels
area/UX priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@mikedanese
Copy link
Member

From @luxas on October 5, 2016 19:26

We should use Kubelet Dynamic Settings by pushing values given to kubeadm init to the apiserver when started, and that will make all kubelets in the cluster reload and pick up those configuration values.

That's much better than having to modify every 10-kubeadm.conf systemd dropin file in the whole cluster.

Copied from original issue: kubernetes/kubernetes#34132

@mikedanese
Copy link
Member Author

From @luxas on October 5, 2016 19:28

cc @kubernetes/sig-cluster-lifecycle

@mikedanese
Copy link
Member Author

From @errordeveloper on October 5, 2016 20:25

Is this feature already usable?

@mikedanese
Copy link
Member Author

From @luxas on October 5, 2016 20:29

I don't know actually, but I think an alpha version of it exists

cc @mtaufen @vishh @mikedanese @dchen1107 @yujuhong

@mikedanese
Copy link
Member Author

From @axsuul on October 7, 2016 6:58

Is this related to customizing arguments for the kubernetes API server? I'm trying to customize the secure port it listens on (--secure-port=8081) but I'm not sure how to do it when setting up the cluster with kubeadm.

@mikedanese
Copy link
Member Author

From @errordeveloper on October 7, 2016 7:15

James, no this is not related. To your question, please try editing
/etc/kubernetes/manifests/kube-apiserver.json, but please create another
issue if you have more questions.

On Fri, 7 Oct 2016, 08:59 James Hu, [email protected] wrote:

Is this related to customizing arguments for the kubernetes API server?
I'm trying to customize the secure port it listens on (--secure-port=8081)
but I'm not sure how to do it when setting up the cluster with kubeadm.


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
kubernetes/kubernetes#34132 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAPWSx8wMyYVNJ49UBPb1y69d4nOSja-ks5qxe24gaJpZM4KPMP5
.

@mikedanese
Copy link
Member Author

From @axsuul on October 7, 2016 7:58

@errordeveloper Thanks, I've opened up issue #34307 with more questions

@mikedanese
Copy link
Member Author

From @mtaufen on October 7, 2016 17:3

There is an alpha version of the dynamic settings feature, but it still needs a lot of work. I'm going to be mostly focused on GCI-related things for v1.5, but am planning be giving it high-priority focus for v1.6. I wouldn't recommend building on top of dynamic settings just yet, as it will undergo some significant changes (e.g. we will move to telling a node which configmap to use via annotations rather than creating configmaps with a per-node naming convention).

As it stands you would have to create a configmap for every node in the cluster anyway, and based on @luxas's first comment in this issue, it looks like you are trying to avoid per-node work, so that might be another reason to wait. Granted, this per-node work is still probably less effort than modifying systemd conf across a cluster...

@pires
Copy link
Contributor

pires commented Nov 23, 2016

#64 and #42 are potential use-cases for this.

@luxas
Copy link
Member

luxas commented Nov 23, 2016

We are blocked on this one from other persons/sigs: kubernetes/kubernetes#27980

@jamiehannaford
Copy link
Contributor

This might be possible for v1.8.
Feature issue: kubernetes/enhancements#281
alpha PR in review: kubernetes/kubernetes#46254

@luxas
Copy link
Member

luxas commented Jun 17, 2017

@mtaufen Will the alpha version of Kubelet Dynamic Configuration be easily upgradeable?
I mean, will it be of beta quality but alpha just because of the ladder and policy to not just enable things by default?

kubeadm could be a good way to test these things out on real clusters during the cycle and then we can decide whether we want to enable it or not in v1.8

@luxas luxas added this to the v1.8 milestone Jun 17, 2017
@mtaufen
Copy link

mtaufen commented Jun 19, 2017

It depends on how confident we are that the API types for the Kubelet won't change after 1.8. It's too early to say right now.

@luxas luxas added the area/UX label Nov 9, 2017
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Nov 18, 2017
…config

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add kubeadm support for Kubelet Dynamic Configuration

**What this PR does / why we need it**:
This PR will make kubeadm support for Kubelet Dynamic Configuration. This is still WIP (and the code seems ugly). Creating the PR for now to let reviewers see if I understand the feature correctly and am on the right path and what else I'm missing.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes/kubeadm#28

**Special notes for your reviewer**:
This feature is targeting for 1.9 as alpha.
/cc @luxas @mtaufen @mikedanese 

**Release note**:

```release-note
Kubeadm now supports for Kubelet Dynamic Configuration.
```
@luxas luxas reopened this Nov 19, 2017
@luxas
Copy link
Member

luxas commented Nov 19, 2017

We have some things to fix up yet:

  1. Fix a panic Panic when DynamicKubeletConfig is enabled. #553 (Fix panic when assigning configmap UID of kubelet configuration. kubernetes#56008)
  2. ClusterDNS should be set conditionally based on cfg.Networking.ServiceSubnet (KubeletConfiguration.BaseConfig.ClusterDNS defaults to the tenth address of MasterConfiguration.Networking.ServiceSubnet kubernetes#56013)
  3. Re-engineer the kubeadm join logic to wait for the kubelet to create /etc/kubernetes/kubelet.conf and use those credentials to PATCH the node
  4. Write the the marshalled kubeletconfig object to /var/lib/kubelet/config/init/kubelet so that the kubelet will start up with the right params on init/join. The only params expected in the kubelet command-line after this is kubelet --init-config-dir /var/lib/kubelet/config/init --dynamic-config-dir /var/lib/kubelet/config/dynamic

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Nov 19, 2017
Automatic merge from submit-queue (batch tested with PRs 56008, 56013). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

KubeletConfiguration.BaseConfig.ClusterDNS defaults to the tenth address of MasterConfiguration.Networking.ServiceSubnet

**What this PR does / why we need it**:
If can get DNS IP from MasterConfiguration.Networking.ServiceSubnet, defaults to it. Otherwise defaults to DefaultClusterDNSIP

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes/kubeadm#28 (comment)

**Special notes for your reviewer**:
/cc @luxas 

**Release note**:

```release-note
NONE
```
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Nov 19, 2017
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Re-engineer the kubeadm join logic.

**What this PR does / why we need it**:
- wait for the kubelet to create `/etc/kubernetes/kubelet.conf`
- use those credentials to PATCH the node

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes/kubeadm#28 (comment)

**Special notes for your reviewer**:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews

**Release note**:

```release-note
NONE
```
@mtaufen
Copy link

mtaufen commented Nov 20, 2017

FYI --init-config-dir is likely to change a bit, wrt this thread: kubernetes/kubernetes#55718 (comment)

@luxas
Copy link
Member

luxas commented Nov 21, 2017

Yeah, we'll accommodate accordingly when we move out of alpha with this on the kubeadm side.
Thanks for keeping us updated @mtaufen

@luxas
Copy link
Member

luxas commented Nov 22, 2017

FYI @timothysc ^
We'll do a revised version of https://github.com/kubernetes/kubeadm/blob/master/docs/design/design_v1.8.md soon-ish (with @fabriziopandini?) and include this in there.
Everything here is alpha gated in v1.9, planned for beta in v1.10

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Nov 22, 2017
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

 Write marshalled kubeletconfig object to init-config-dir

**What this PR does / why we need it**:
from @luxas :
>Write the the marshalled kubeletconfig object to /var/lib/kubelet/config/init/kubelet so that the kubelet will start up with the right params on init/join. The only params expected in the kubelet command-line after this is kubelet --init-config-dir /var/lib/kubelet/config/init --dynamic-config-dir /var/lib/kubelet/config/dynamic

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes/kubeadm#28 (comment)

**Special notes for your reviewer**:
/cc @kubernetes/sig-cluster-lifecycle-pr-reviews

**Release note**:

```release-note
NONE
```
vdemeester pushed a commit to vdemeester/kubernetes that referenced this issue Nov 24, 2017
…te-kc

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add validation of kubelet and kube-proxy configuration in kubeadm.

**What this PR does / why we need it**:
kubeadm has implemented the support for Kubelet Dynamic Configuration. This PR adds validation on the kubelet configuration.

kube-proxy validation also added in this PR.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
ref: kubernetes/kubeadm#28

**Special notes for your reviewer**:
/cc @luxas @kubernetes/sig-cluster-lifecycle-pr-reviews

**Release note**:

```release-note
NONE
```
@luxas
Copy link
Member

luxas commented Nov 28, 2017

I think all the tasks needed to be performed were merged last week, so closing this as fixed for an alpha state now. The design of this feature is covered in design doc update of v1.9 #573.

We'll open new issues later for graduating this feature to beta on the kubeadm side.

Thanks @xiangpengzhao for the awesome work on this 🎉!

@bronger
Copy link

bronger commented Jul 25, 2018

Is #83 supposed to work for v1.11? Because I still need to set

KUBELET_EXTRA_ARGS=--cluster-dns=10.67.97.10

in /etc/sysconfig/kubelet so that the IP of the DNS server in the pods is correct. I called:

kubeadm init --apiserver-cert-extra-sans kubmaster,kubmaster.mygreatdomain.de \
    --service-cidr 10.67.97.0/24

Without setting KUBELET_EXTRA_ARGS, the DNS is looked for at 10.96.0.10.

@neolit123
Copy link
Member

@bronger
i cannot comment on the topic if the kubeadm flag (service-cidr) should somehow translate to the DNS IP in the kubelet.

but assuming you have transitioned to the kubeadm config v1alpha2 you can use the following:

nodeRegistration:
  kubeletExtraArgs:
    cluster-dns: 10.67.97.10

this will help you to not have to update the kubelet file.

ref: https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/

@luxas
Copy link
Member

luxas commented Jul 25, 2018

@bronger check the /var/lib/kubelet/config.yaml file, cluster-dns should be set correctly there in v1.11

@bronger
Copy link

bronger commented Jul 26, 2018

@luxas No, it says

clusterDNS:
- 10.96.0.10

@luxas
Copy link
Member

luxas commented Jul 27, 2018

@bronger please open a new issue with all the required information to debug that. Might be a bug.

@zerkms
Copy link

zerkms commented Aug 24, 2018

@bronger have you reported one? could you refer to it please, I can confirm on a latest stable kubeadm kubeadm version: &version.Info{Major:"1", Minor:"11", GitVersion:"v1.11.2", GitCommit:"bb9ffb1654d4a729bb4cec18ff088eacc153c239", GitTreeState:"clean", BuildDate:"2018-08-07T23:14:39Z", GoVersion:"go1.10.3", Compiler:"gc", Platform:"linux/amd64"} the problem still is reproducible.

@bronger
Copy link

bronger commented Aug 24, 2018

No, I haven't so far. Long vacation and business trip came into way. @luxas Does it make sense to just re-open #83?

@zerkms
Copy link

zerkms commented Aug 24, 2018

I think #83 perfectly fits, the new one would be identical

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UX priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants