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

Allow overwrite KubeletRunDirectory when init/join #2104

Closed
pytimer opened this issue Apr 9, 2020 · 37 comments
Closed

Allow overwrite KubeletRunDirectory when init/join #2104

pytimer opened this issue Apr 9, 2020 · 37 comments
Assignees
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence.
Milestone

Comments

@pytimer
Copy link

pytimer commented Apr 9, 2020

Is this a BUG REPORT or FEATURE REQUEST?

FEATURE REQUEST

Versions

kubeadm version (use kubeadm version): v1.17.2

Environment:

  • Kubernetes version (use kubectl version): v1.17.2
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Others:

What happened?

I want to change the kubelet run directory when init or join nodes, setting root-dir in kubeadm kubeletExtraArgs field of nodeRegistration object. But kubeadm also write the kubelet config.yaml to /var/lib/kubelet and this behavior make init fail.

What you expected to happen?

I hope kubeadm can setting KubeletRunDirectory. I see this variable is constant now.

The code: https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/constants/constants.go#L244

I want to know my idea wether correct, i see a relate issue: #1478 . If needs, i will try to do it.

How to reproduce it (as minimally and precisely as possible)?

Anything else we need to know?

@neolit123
Copy link
Member

see here:
#1478 (comment)

you can solve this with a symlink.
last time we spoke about this we didn't want to enable the override on the side of kubeadm.

/priority awaiting-more-evidence
/kind feature

@k8s-ci-robot k8s-ci-robot added priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. kind/feature Categorizes issue or PR as related to a new feature. labels Apr 9, 2020
@pytimer
Copy link
Author

pytimer commented Apr 10, 2020

@neolit123 Thanks for reply.

Use symlink can solve my problem. But in my case, i use ansible to deploy some clusters, some of them use /var/lib/kubelet, but another use other directory, if use symlink, i should compare with custom kubelet run directory and /var/lib/kubelet, and decide wether use symlink. This adds a little complexity, so if kubeadm support, it make me easy to do it.

@neolit123
Copy link
Member

potentially, instead of writing the config always to the default kubelet directory, we can check if the user has passed a KubeletConfiguration and use the value from there.
unfortunately this means on joining nodes we have to use flags only as kubeadm join currently does not support KubeletConfiguration.
so that's a much larger complexity on the kubeadm side.

maybe the kubeadm API should support the field (e.g. in NodeRegistrationOptions), instead of making assumption based on KubeletConfiguration.

@neolit123
Copy link
Member

/kind design

@k8s-ci-robot k8s-ci-robot added the kind/design Categorizes issue or PR as related to design. label Apr 10, 2020
@fabriziopandini
Copy link
Member

I'm +1 to defer this to when the design for node specific settings in component config is complete.
We are trying to avoid to create/maintain knobs for specific flags

@pytimer
Copy link
Author

pytimer commented Apr 13, 2020

If the issue #1682 solved, i think this issue also solved.

@neolit123
Copy link
Member

related KEP kubernetes/enhancements#1439

@neolit123 neolit123 added this to the Next milestone Apr 15, 2020
@neolit123 neolit123 added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/awaiting-more-evidence Lowest priority. Possibly useful, but not yet enough support to actually get it done. labels Apr 15, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 14, 2020
@neolit123
Copy link
Member

/remove-lifecycle stale
let's re-evaluate again after kubernetes/enhancements#1439

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 20, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 18, 2020
@fabriziopandini
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 19, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 17, 2021
@neolit123
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 17, 2021
@neolit123 neolit123 added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jan 17, 2021
@neolit123 neolit123 modified the milestones: Next, v1.22 Apr 14, 2021
@neolit123 neolit123 mentioned this issue Apr 14, 2021
16 tasks
@pacoxu
Copy link
Member

pacoxu commented May 19, 2021

  • P3: customize /var/lib/kubelet

I think we can add KubeletRootDir in v1beta3 like CertificatesDir. I would work on it if there is no objection.
/assign

@neolit123
Copy link
Member

neolit123 commented May 25, 2021 via email

@pacoxu
Copy link
Member

pacoxu commented Jun 7, 2021

/unassign
since not yet decided.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 5, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 5, 2021
@neolit123
Copy link
Member

neolit123 commented Oct 5, 2021 via email

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Oct 5, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 3, 2022
@neolit123 neolit123 added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 11, 2022
@chendave chendave mentioned this issue Jun 9, 2023
26 tasks
@chendave
Copy link
Member

Early investigation for this issue, might not accurate enough, correct me pls.

  • the original requirement for this issue is the support of the overwrite the kubelet run directory, and this is still hard-coded today,
	// KubeletRunDirectory specifies the directory where the kubelet runtime information is stored.
	KubeletRunDirectory = "/var/lib/kubelet"

So, this issue still stands, and one tradeoff is making KubeletRunDirectory a config item in the clusterConfiguration, I know this was pushed back because there was node-specific config feature considered at that time.

@neolit123 @pacoxu @SataQiu Can we include this in v1beta4? one day the node-specific config is implemented, we can pull this out from clusterConfiguration then, WDYT?

@neolit123
Copy link
Member

+1 to remove from cluster scope. it can be a feature in noderegistrationoptions, but i think the demand is relatively low. users can use symlinks too, i think.

@pacoxu
Copy link
Member

pacoxu commented Jun 13, 2023

We need to be careful with it.

As #2104 (comment) pointed out, kubelet.conf file will place based on the kubelet root-dir. Both config.yaml and kubeadm-flags.env. This should be included in the docs.

BTW, kubelet cert-dir is not using the root-dir, the user should specify it as well to change the cert-dir when needed.

--cert-dir string     Default: /var/lib/kubelet/pki

+1 to remove from cluster scope. it can be a feature in noderegistrationoptions, but I think the demand is relatively low. users can use symlinks too, i think.

https://kubernetes.io/docs/reference/config-api/kubeadm-config.v1beta3/#kubeadm-k8s-io-v1beta3-NodeRegistrationOptions

A question when I implement kubernetes/enhancements#3930: use kubeletExtraArgs will be combined with the cirSocket to save into the kubelet env file.

https://github.com/kubernetes/kubernetes/blob/01b6bb41b0b912535025e82028d0f180a0416e02/cmd/kubeadm/app/phases/kubelet/flags.go#L60-L73

If we add root-dir support, it would be similar to the criSocket. (not best practice IMO, though this may have a historical reason that we want to be consistent to the --cri-socket flag.) EDITED

@neolit123
Copy link
Member

We need to be careful with it.

agreed, i think we should just drop it from the cluster configuration as a start. implementation / piping the option in kubeadm feels google doc proposal worthy.

@neolit123
Copy link
Member

neolit123 commented Jun 13, 2023

A question when I implement kubernetes/enhancements#3930: use kubeletExtraArgs will be combined with the cirSocket to save into the kubelet env file.

the kep in question should be using the kubeletconfiguration to set a local criSocket, maybe by storing a patch file in the kubelet dir, no?
(i think i commented about this idea already there)

i don't think we should use kubelet flags for new features.

@SataQiu
Copy link
Member

SataQiu commented Jun 13, 2023

If we support a particular kubelet flag, maybe users want kubeadm to support more, and that would be disastrous.

i don't think we should use kubelet flags for new features.

+1 We should try to avoid to maintain specific flags

@chendave
Copy link
Member

chendave commented Jun 13, 2023

/var/lib/kubelet is used during upgrade to write the kubelet.conf file on disk that contains the KubeletConfiguration.

kubelet.conf file will place based on the kubelet root-dir. Both config.yaml and kubeadm-flags.env

I am trying to understand those statements, for what I can see,

kubelet.conf is created in this location,

# ls /etc/kubernetes/kubelet.conf
/etc/kubernetes/kubelet.conf

and config.yaml and kubeadm-flags.env are hosted here,

# ls /var/lib/kubelet/config.yaml
/var/lib/kubelet/config.yaml
# ls /var/lib/kubelet/kubeadm-flags.env
/var/lib/kubelet/kubeadm-flags.env

And it's config.yaml we should take care of, right?

@chendave
Copy link
Member

what we have from @neolit123 @pacoxu @SataQiu 's comments,

  • this is low priority feature for 1.29, as symlinks can be used instead.
  • KubeletRunDirectory could be configured by the nodeRegistration from InitConfiguration or JoinConfiguration
  • this might need a doc for review, I think if the change is relative simple, we might discuss in the pr instead?

Not sure if anyone would like to take this one.

cc @ruquanzhao who might offer some help along the way.

@ruquanzhao
Copy link
Member

/assign

@pacoxu
Copy link
Member

pacoxu commented Jun 15, 2023

And it's config.yaml we should take care of, right?

--root-dir should be in kubeadm-flags.env as it is not supported in Kubelet Configuration (v1beta1).

@ruquanzhao
Copy link
Member

ruquanzhao commented Jun 15, 2023

Environment="KUBELET_CONFIG_ARGS=--config=/var/lib/kubelet/config.yaml"
EnvironmentFile=-/var/lib/kubelet/kubeadm-flags.env

/var/lib/kubelet is also hardcoded in 10-kubeadm.conf when kubeadm installed by apt.

In my view, it is not appropriate to touch this file, since it's destributed by package manager, and we have to consider restore it when reset or any other commands.

So, should we keep using paths /var/lib/kubelet/config.yaml and /var/lib/kubelet/kubeadm-flags.env?

@neolit123
Copy link
Member

neolit123 commented Nov 30, 2023

So, should we keep using paths /var/lib/kubelet/config.yaml and /var/lib/kubelet/kubeadm-flags.env?

yes, let's close this ticket.
it was already discussed a few times that users who want to change this can use symlinks.
same goes for /etc/kubernetes as a hardcoded path.

EDIT: there is also the --rootfs option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/design Categorizes issue or PR as related to design. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

10 participants