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.

Sign up for GitHub

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

Jump to bottom

Graduate kubelet config to beta #571

Closed
fabriziopandini opened this issue Nov 24, 2017 · 11 comments · Fixed by kubernetes/kubernetes#63887
Closed

Graduate kubelet config to beta #571

fabriziopandini opened this issue Nov 24, 2017 · 11 comments · Fixed by kubernetes/kubernetes#63887
Assignees
@luxas
Labels
lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone
v1.11

Comments

@fabriziopandini
Copy link
Member

FEATURE REQUEST

Kubelet config was introduced in kubeadm init as alpha feature behind feature gates, but it is planned to graduate it as soon as possible.

It should be necessary to introduce corresponding phases as well before graduation.
@luxas luxas added this to the v1.10 milestone Nov 24, 2017
@luxas
Copy link
Member

luxas commented Nov 24, 2017

/assign @xiangpengzhao
this is a goal for v1.10, but the PR can be sent ASAP. It just can't merge before the v1.9 freeze lifts.

@luxas luxas added kind/enhancement priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Nov 24, 2017
@xiangpengzhao
Copy link

xiangpengzhao commented Nov 28, 2017 •
edited
Loading

TODO list:

if err := kubeletphase.WriteInitKubeletConfigToDiskOnMaster(i.cfg)
	// KubeletBaseConfigurationConfigMapKey specifies in what ConfigMap key the initial remote configuration of kubelet should be stored
	// TODO: Use the constant ("kubelet.config.k8s.io") defined in pkg/kubelet/kubeletconfig/util/keys/keys.go
	// after https://github.com/kubernetes/kubernetes/pull/53833 being merged.
	KubeletBaseConfigurationConfigMapKey = "kubelet"

UPDATE:
#53833 still uses kubelet as the configmap key: https://github.com/mtaufen/kubernetes/blob/9ebaf5e7d27451ef525b3373c46db29221af9cc0/pkg/kubelet/kubeletconfig/checkpoint/configmap.go#L30

The 3, 4, 5 should wait for the associated PR merged.

Anything else am I missing or anything incorrect from above list? @luxas @fabriziopandini

@fabriziopandini
Copy link
Member Author

@xiangpengzhao, with "introduce corresponding phases" I was thinking at kubeadm alpha phases, where we should provide a set of subcommands that allow user to invoke atomically all the steps executed during kubeadm init.

@xiangpengzhao, @luxas, if you agree I will rename this issue into "Graduate kubelet config to beta“

@xiangpengzhao
Copy link

@fabriziopandini thanks for clarifying !
+1 from me on renaming the issue title.

@fabriziopandini fabriziopandini changed the title Implement phases for kubelet config Graduate kubelet config to beta Nov 28, 2017
@luxas
Copy link
Member

luxas commented Nov 28, 2017

Anything else should do here?

@xiangpengzhao it should be exposed as commands in, for example:

  • kubeadm phase kubelet config init write
  • kubeadm phase kubelet config dynamic enable-for-node
  • kubeadm phase kubelet config dynamic upload

@xiangpengzhao xiangpengzhao mentioned this issue Dec 15, 2017
Merged
@xiangpengzhao
Copy link

phase added in kubernetes/kubernetes#57224. The graduation should wait for the associated PRs merged.

@luxas
Copy link
Member

luxas commented Dec 25, 2017

FWIW; we need a phase / something command that points a kubelet to a new ConfigMap as well, for the kubelet upgrade scenario.

e.g. I've just upgraded the kubelet (which is associated with the v1.9 configmap) to v1.10, I now need to repoint it to the v1.10 config... maybe we can just document to use kubeadm phase kubelet config dynamic enable-for-node and make an alias for it as well?

@fabriziopandini
Copy link
Member Author

@luxas please check discussion on #57244
currently enable-dynamic-config is the only phase subcommand that implements a step of the kubeadm join sequence (everything else is related to kubeadm init).

IMO we should move this somewhere else, and considering the use case, I suggest under kubeadm upgrade. What about kubeadm upgrade dynamic-kubelet-config --node-name=myNode?

@xiangpengzhao
Copy link

I was thinking phase preflight node to be a step of kubeadm join (isn't it?) when I implemented enable-dynamic-config. So I also made enable-dynamic-config a step of kubeadm join.

Agree to document to make them more clear.

@luxas
Copy link
Member

luxas commented Dec 26, 2017

I was thinking phase preflight node to be a step of kubeadm join (isn't it?)

It is, yes

So I also made enable-dynamic-config a step of kubeadm join.

👍

kubeadm upgrade dynamic-kubelet-config --node-name=myNode?

Something like that probably makes sense... What do people think would be the most productive naming?

kubeadm upgrade kubelet-config [myNode]
kubeadm upgrade node config [myNode]
...etc?

@xiangpengzhao xiangpengzhao mentioned this issue Feb 23, 2018
Closed
@xiangpengzhao
Copy link

@timothysc wrote in kubernetes/kubernetes#53084 (comment) (Graduate the kubeletconfig API to beta)

@ kubernetes/sig-cluster-lifecycle-misc (we should move to enable asap).

I updated the TODO list #571 (comment) as per the final implementation of kubernetes/kubernetes#53833. The only bullet in the TODO list is switch feature gate from alpha to beta and default to true. @timothysc are there anything else you were thinking of in your comment?

This was referenced Feb 26, 2018
Closed
Merged
@timothysc timothysc modified the milestones: v1.10, v1.11 Mar 5, 2018
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Apr 13, 2018
Merge pull request #60476 from xiangpengzhao/remove-todos
ac57bbe 
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>.

Remove invalid TODOs in kubeadm constants.

**What this PR does / why we need it**:
These TODOs are invalid now.

**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#571 (comment)

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Apr 17, 2018
Merge pull request #57224 from xiangpengzhao/kubeadm-phase-kubelet
923f6c5 
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 phase command for dynamic kubelet configuration in kubeadm.

**What this PR does / why we need it**:
As the title says.

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

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

**Release note**:

```release-note
Phase `kubeadm alpha phase kubelet` is added to support dynamic kubelet configuration in kubeadm.
```
@luxas luxas assigned luxas and unassigned xiangpengzhao May 22, 2018
@luxas luxas added lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. and removed triaged labels May 22, 2018
@luxas luxas mentioned this issue May 22, 2018
Merged
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue May 23, 2018
Merge pull request #63887 from luxas/kubeadm_kubelet_integration
773ced1 
Automatic merge from submit-queue (batch tested with PRs 63914, 63887, 64116, 64026, 62933). 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>.

kubeadm: Write kubelet config file to disk and persist in-cluster

**What this PR does / why we need it**:
In order to make configuration flow from the cluster level to node level, we need a way for kubeadm to tell the kubelet what config to use. As of v1.10 (I think) the kubelet can read `--config` using the kubelet Beta ComponentConfiguration API, so now we have an interface to talk to the kubelet properly.

This PR:
 - Writes the kubelet ComponentConfig to `/var/lib/kubelet/config.yaml` on init and join
 - Writes an environment file to source in the kubelet systemd dropin `/var/lib/kubelet/kubeadm-flags.env`. This file contain runtime flags that should be passed to the kubelet.
 - Uploads a ConfigMap with the name `kubelet-config-1.X`
 - Patches the node object so that it starts using the ConfigMap with updates using Dynamic Kubelet Configuration, **only if the feature gate is set** (currently alpha and off by default, not intended to be switched on in v1.11)
 - Updates the phase commands to reflect this new flow

The kubelet dropin file I used now looks like this:
```
# v1.11.x dropin as-is at HEAD
# /etc/systemd/system/kubelet.service.d/10-kubeadm.conf
---
[Service]
Environment="KUBELET_KUBECONFIG_ARGS=--bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --kubeconfig=/etc/kubernetes/kubelet.conf"
Environment="KUBELET_CONFIG_ARGS=--config=/var/lib/kubelet/config.yaml"
EnvironmentFile-=/var/lib/kubelet/kubeadm-flags.env
# Should default to 0 in v1.11: #63881, and hence not be here in the real v1.11 manifest
Environment="KUBELET_CADVISOR_ARGS=--cadvisor-port=0"
# Should be configurable via the config file: #63878, and hence be configured using the file in v1.11
Environment="KUBELET_CERTIFICATE_ARGS=--rotate-certificates=true"
ExecStart=
ExecStart=/usr/bin/kubelet $KUBELET_KUBECONFIG_ARGS $KUBELET_CONFIG_ARGS $KUBELET_KUBEADM_ARGS $KUBELET_CADVISOR_ARGS $KUBELET_CERTIFICATE_ARGS $KUBELET_EXTRA_ARGS
---
# v1.11.x dropin end goal
# /etc/systemd/system/kubelet.service.d/10-kubeadm.conf
---
[Service]
Environment="KUBELET_KUBECONFIG_ARGS=--bootstrap-kubeconfig=/etc/kubernetes/bootstrap-kubelet.conf --kubeconfig=/etc/kubernetes/kubelet.conf"
Environment="KUBELET_CONFIG_ARGS=--config=/var/lib/kubelet/config.yaml"
EnvironmentFile-=/var/lib/kubelet/kubeadm-flags.env
ExecStart=
ExecStart=/usr/bin/kubelet $KUBELET_KUBECONFIG_ARGS $KUBELET_CONFIG_ARGS $KUBELET_KUBEADM_ARGS $KUBELET_EXTRA_ARGS
---
# Environment file dynamically created at runtime by "kubeadm init"
# /var/lib/kubelet/kubeadm-flags.env
KUBELET_KUBEADM_ARGS=--cni-bin-dir=/opt/cni/bin --cni-conf-dir=/etc/cni/net.d --network-plugin=cni
```

**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#822
Fixes kubernetes/kubeadm#571

**Special notes for your reviewer**:

**Release note**:

```release-note
"kubeadm init" now writes a structured and versioned kubelet ComponentConfiguration file to `/var/lib/kubelet/config.yaml` and an environment file with runtime flags (you can source this file in the systemd kubelet dropin) to `/var/lib/kubelet/kubeadm-flags.env`.
```
@kubernetes/sig-cluster-lifecycle-pr-reviews @mtaufen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@luxas luxas

Labels
lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Milestone
v1.11
Development

Successfully merging a pull request may close this issue.

kubeadm: Write kubelet config file to disk and persist in-cluster luxas/kubernetes
4 participants
@timothysc @fabriziopandini @luxas @xiangpengzhao