Kubeadm: change the kubelet service crash loop behavior

[neolit123]

over time we have seen a number of complains related to the crash loop of the kubelet service in the DEB/RPMs. when the kubelet is installed, the service is enabled but it would fail because it's missing its config.yaml (KubeletConfiguration), unless something like kubeadm creates one for it.

this has caused problems for:

• Windows support
• supporting other service manager like OpenRC

after a discussion of the kubeadm office hours of 10.06.2020 we've agreed that it might be a good idea to change this behavior and keep the service disabled by default. but this would require changes in both kubeadm and the kubelet systemd specs.

the idea we are thinking of is the following:

• [ ] modify kubeadm to always enable the service if its not enabled on kubeadm init/join runtime.
  note that, currently kubeadm just has a preflight check that fails if the service is not enabled and instructs the user how to enable it manually.
• [ ] modify the kubelet systemd service files in the kubernetes/release repository to have the kubelet service disabled by default. this change will require a release note with "action-required" as non-kubeadm users would have to manually enable it (e.g. using: "sysctl enable kubelet").

/kind feature
/priority important-longterm

[neolit123]

the kubeadm change is doable for 1.19, but arguably the k/release change needs wider agreement.

[neolit123]

xref https://github.com/kubernetes/release/pull/1352

[neolit123]

cc @rosti
so i think there is a limitation of our release process as i mentioned here:
https://github.com/kubernetes/release/pull/1352

which means that the PR might be much better than the above proposal.

[BenTheElder]

I thought kubeadm already had code related to doing this https://github.com/kubernetes/kubernetes/blob/875f31e98878fd199a76fd0ba2465d14558788cd/cmd/kubeadm/app/phases/kubelet/kubelet.go#L38

[neolit123]

it only starts and restarts the service but does not manage "enable" status.
it does tell the user how to enable, though.

[rosti]

The necessary modifications here are:

1. Remove the kubelet service running check (from here)
2. Enable the service during init and join. One way is to add the enabling code where the TryStartKubelet calls are. A safer and somewhat backwards compatible alternative is to add new kubelet enable phases to init and join that would just enable permanently the kubelet service after all of the rest of the init/join process has been successfully completed.
3. Disable the kubelet service during kubeadm reset. Again, this can be sitting next to the TryStopKubelet call or be in a separate reset phase.

As said previously. It doesn't hurt for us to implement this. It's not that big of a change on our side. The problem is what to do with the packages where the kubelet service continues to be automatically enabled and crash looping (since it doesn't have a config file).

[neolit123]

your proposed plan for kubeadm seems fine to me.

The problem is what to do with the packages where the kubelet service continues to be automatically enabled and crash looping (since it doesn't have a config file).

so i think ideally we should be making this change only for the latest MINOR.
but in https://github.com/kubernetes/release/pull/1352 we are also discussing the fact that currently such changes are applied to all PATCH release of k8s packages.

currently the kubeadm package has the kubelet package as a dependency (there were discussions to change this too), which supposedly installs the same versions for both package for _most_ users.

there could be users that are bypassing that and installing e.g. kubeadm 1.x and kubelet 1.x-1, due to factor X, and this is a supported skew. for such a skew the old kubelet service may be enabled by default (crashloop) but the new kubeadm could be managing "enable" already.

testing something like systemctl enable kubelet on a service that is already enabled doesn't have an exit status != 0, for windows Set-Service ... -StartupType.. should not return errors. no idea about OpenRC. in any case we may have to call the potential InitSystem#ServiceEnable() without catching its errors, but later fail on InitSystem#ServiceStart().

so overall even if a kubeadm binary encounters an older kubelet service, for the crashloop problem in particular this should be fine, unless i'm missing something.

however for the kubelet flag removal and instance specific CC problem, the kubeadm 1.x and kubelet 1.x-1 skew might bite people that are passing flags to a kublet that no longer supports flags at all.
i can see us removing kubeletExtraArgs from the kubeadm API at some point. but that's a separate topic.

[xlgao-zju]

Tested systemctl enable kubelet.service on a service that is already enabled, and get exit code = 0.

I think @rosti 's plan is fine, and I'd like to help with this.

[xlgao-zju]

/assign

[neolit123]

note, if 1.20 is a "stabilization release" we should not be making this change.
i also think that a KEP is appropriate as it requires changes in both the packages (and in the krel tool to branch?) and kubeadm.

[neolit123]

/sig release cluster-lifecycle

[xlgao-zju]

@neolit123

the part one may not need the KEP

modify kubeadm to always enable the service if its not enabled on kubeadm init/join runtime.

the part two may need the KEP

modify the kubelet systemd service files in the kubernetes/release repository to have the kubelet service disabled by default.

[neolit123]

yet, we can avoid doing part one, if part two never happens, thus documenting the proposed change in one place feels right.

[xlgao-zju]

@neolit123 yes, you are right. so, where should we file the kep?

[neolit123]

the KEP process:
https://github.com/kubernetes/enhancements/tree/master/keps#kubernetes-enhancement-proposals-keps

it should be either here (A):
https://github.com/kubernetes/enhancements/tree/master/keps/sig-cluster-lifecycle/kubeadm
or here (B):
https://github.com/kubernetes/enhancements/tree/master/keps/sig-release

my preference is for A, but let's hold until we decide if we are making this change in 1.20:

note, if 1.20 is a "stabilization release" we should not be making this change.

[BenTheElder]

i can see us removing kubeletExtraArgs from the kubeadm API at some point. but that's a separate topic.

quick note that we cannot do that while the kubelet config is not respected in join.

+1 for https://github.com/kubernetes/kubeadm/issues/2178#issuecomment-642531132
speicfically 2.) kubelet enabling should happen after writing the file(s) / config, so as to avoid any crash loops.

this will also have the benefit of allowing CI tooling to look for panics in the logs without creating loads of noise. (something that would be pointless for us to add today, due to kubeadm)

[BenTheElder]

Let me know if I can help. I would like to eliminate the crashlooping in CI, and I think this will avoid a lot of confused users.

[xlgao-zju]

@BenTheElder Since we will not change the kubernetes/release repository to have the kubelet service disabled by default for now. I think we can stop kubelet, when we do not have the kubelet config. And start the kubelet, until we get the kubelet config?

[neolit123]

@xlgao-zju looks like 1.20 is a regular release so we can proceed with the KEP if you have the time:

my preference is for A, but let's hold until we decide if we are making this change in 1.20:

note, if 1.20 is a "stabilization release" we should not be making this change.

let me know if you have questions.
we can skip some of the details in the KEP, but IMO we need to focus more on the topic how breaking the change can be.

also feedback from the release-eng team will be required.

[xlgao-zju]

@neolit123 hi, I am so sorry that I don't have enough time to file the KEP for now. but I will keep an eye on it.
I think I will be available, when we come to the coding part. and I would like to help.