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

Deprecate "containerized kubelet" #74148

Closed
dims opened this issue Feb 15, 2019 · 22 comments · Fixed by #80043
Closed

Deprecate "containerized kubelet" #74148

dims opened this issue Feb 15, 2019 · 22 comments · Fixed by #80043
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@dims
Copy link
Member

dims commented Feb 15, 2019

There was a previous attempt : #43708

We have open issues that is not being worked on:

I think it's time to delete the CI jobs and start the deprecation cycle.

/sig node
/assign @derekwaynecarr

@dchen1107
Copy link
Member

We discussed this at today's SIG Node meeting. SIG Node agreed to deprecate "containerized kubelet" with the following action items:

  • The project was initiated for self-host cluster by SIG cluster lifecycle. We should communicate with them first to understand the potential dependencies.
  • There were some demand from the community for this feature in the past, but we don't know who are using this in their production. We should announce this deprecation at Kubernetes Community weekly meeting and send email to kubernetes-dev@.

@dims
Copy link
Member Author

dims commented Feb 19, 2019

will do thanks @dchen1107

@dims
Copy link
Member Author

dims commented Feb 19, 2019

/sig storage
/sig cluster-lifecycle

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Feb 19, 2019
@dims
Copy link
Member Author

dims commented Feb 19, 2019

cc @timothysc @luxas @roberthbailey @kubernetes/sig-cluster-lifecycle

cc @saad-ali @childsb @kubernetes/sig-storage-misc

@andrewrynhard
Copy link
Contributor

andrewrynhard commented Feb 19, 2019

@dims Does this deprecate hyperkube based kubelet in any way? I have a project that depends on this and I think it is a real use case for a container based kubelet. Running either a hyperkube based kubelet or a dedicated kubelet container is key to our design.

https://github.com/autonomy/talos

@dims
Copy link
Member Author

dims commented Feb 19, 2019

@andrewrynhard it doesn't. the main thing to go would be this flag, if you are not using it, you are good.

[1] bcf6d39

@calvix
Copy link

calvix commented Feb 20, 2019

What is the reason behind this decision? We are heavily using containerized kubelet in production running on CoreOS.

Would that mean that we no longer can run kubelet in a container?

@dims
Copy link
Member Author

dims commented Feb 20, 2019

@calvix

If so, you have about a 3 releases per our deprecation policies to switch over to some other way of running kubelet. RedHat/CoreOS folks who initially championed this approach have given up on it, there are no test suites, no owners. sig-storage is having a hard time trying to keep things working.

So please ping the CoreOS folks to research for an alternative for the future.

Note that kind still runs kubelet in a container, it just does not use the flag or volume mounts as shown above.

@tnozicka
Copy link
Contributor

tnozicka commented Feb 20, 2019

Is it necessary to remove/deprecate the feature or just disable the flaky CI check? I find that feature and running kubelet in container helpful. Briefly looking at the implementation there is at least unit test suit.

@dims
Copy link
Member Author

dims commented Feb 20, 2019

@tnozicka that's the tip of the ice berg, there's a lot more things that can be simplified when we drop this feature, example see #68513

Another example of what we had to go through is here - #63143

There is no appetite to continue taking on this burden.

I'll let @derekwaynecarr @jsafrane @saad-ali and others chime in on the undue cost of keeping this alive.

@timothysc
Copy link
Member

fwiw I don't think we should deprecate the entire use case b/c it's highly valuable for testing. I'm not certain why this matters for the kubelet, b/c it really shouldn't unless it's a design flaw.

@dims
Copy link
Member Author

dims commented Feb 20, 2019

@saad-ali @msau42 @jsafrane @derekwaynecarr can you please chime in with the ongoing cost of supporting this feature? how much effort is to keep it going?

@calvix
Copy link

calvix commented Feb 20, 2019

Do you use the flag as shown here? https://github.com/kubernetes/kubernetes/pull/74176/files#diff-ed3df710e9af7cd30a185896a60897d9L814

Yes

  • Do you have --volume mounts as shown just below the line pointed above

we use this list of volume:

     -v /:/rootfs:ro,rshared \
      -v /sys:/sys:ro \
      -v /dev:/dev:rw \
      -v /var/log:/var/log:rw \
      -v /run/calico/:/run/calico/:rw \
      -v /run/docker/:/run/docker/:rw \
      -v /run/docker.sock:/run/docker.sock:rw \
      -v /usr/lib/os-release:/etc/os-release \
      -v /usr/share/ca-certificates/:/etc/ssl/certs \
      -v /var/lib/calico/:/var/lib/calico \
      -v /var/lib/docker/:/var/lib/docker:rw,rshared \
      -v /var/lib/kubelet/:/var/lib/kubelet:rw,rshared \
      -v /etc/kubernetes/ssl/:/etc/kubernetes/ssl/ \
      -v /etc/kubernetes/config/:/etc/kubernetes/config/ \
      -v /etc/kubernetes/kubeconfig/:/etc/kubernetes/kubeconfig/ \
      -v /etc/kubernetes/manifests/:/etc/kubernetes/manifests/ \
      -v /etc/cni/net.d/:/etc/cni/net.d/ \
      -v /opt/cni/bin/:/opt/cni/bin/ \
      -v /usr/sbin/iscsiadm:/usr/sbin/iscsiadm \
      -v /etc/iscsi/:/etc/iscsi/ \
      -v /dev/disk/by-path/:/dev/disk/by-path/ \
      -v /dev/mapper/:/dev/mapper/ \
      -v /lib/modules:/lib/modules \
      -v /usr/sbin/mkfs.xfs:/usr/sbin/mkfs.xfs \
      -v /usr/lib64/libxfs.so.0:/usr/lib/libxfs.so.0 \
      -v /usr/lib64/libxcmd.so.0:/usr/lib/libxcmd.so.0 \
      -v /usr/lib64/libreadline.so.7:/usr/lib/libreadline.so.7 \

This seems to work pretty nice for us and we don't have any volume issues so far.

@jsafrane
Copy link
Member

jsafrane commented Feb 20, 2019

I'll let @derekwaynecarr @jsafrane @saad-ali and others chime in on the undue cost of keeping this alive.

Maintenance of containerized kubelet is increasingly painful. Kubelet / its storage code must know about all mounts on the host, resolve symlinks on the host, execute various helpers on the host and similar stuff, which is especially hard when it does not actually run on the host but in a weird container. See https://github.com/kubernetes/kubernetes/blob/master/pkg/util/mount/nsenter_mount.go and https://github.com/kubernetes/utils/blob/master/nsenter/nsenter.go for all the hacks we use. They are brittle and tend to break in very unexpected way.

It's also largely undocumented and various ways how to run kubelet in a container are cargo-culted around. See above, -v /usr/lib64/libxfs.so.0:/usr/lib/libxfs.so.0, really? No offense, I know why it's there, but whole containerized kubelet is hack on top of hack on top of hack.

@msau42
Copy link
Member

msau42 commented Feb 20, 2019

Not only is the code complex, but it is also not tested, so we have had to rely on manual testing when making bug fixes, and we don't know about regressions until after the release when users report issues. sig-storage has tried for 6+ months to find an owner that can dedicate resources to support and maintain this environment, but nobody has stepped up.

@saad-ali
Copy link
Member

+1 to what @msau42 asnd @jsafrane said. Either we should deprecate the feature, or find an owners who can commit to fixing issues and adding much needed tests. We tried to find an owner in SIG storage during Q2 2018 but no one stepped up.

@anguslees
Copy link
Member

anguslees commented Feb 21, 2019

Just because I like causing trouble, another alternative is to run kubelet executable from a container (or systemd portable unit, etc), but unify the kubelet's rootfs view with that of the host. Specifically: do the equivalent of #54935, but for kubelet.
Then you would run from docker with -v /:/rootfs:rshared (or equivalent), and early in main() chroot() to /rootfs. This loads the executable from the docker image, but all later filesystem operations are done from a pov equivalent to the host root fs.

I think this keeps the kubelet lifecycle management benefits of running the kubelet binary from a container and allows the storage folks to just do regular filesystem operations, like there was nothing clever going on.

@derekwaynecarr
Copy link
Member

The function needs ownership from both SIG Node and SIG Storage.

There were attempts early in the project to remove containerized kubelet support, but Red Hat offered contributors to maintain this model in both SIGs for ~11 releases of Kubernetes. Kubernetes was much smaller 11 releases ago and our shared project experience was limited. It's one thing for a feature to work initially, and another thing to reason about the impact of alternative deployment models when dealing with CVE responses or just doing normal maintainer review of incoming PRs or feature function. That lived experience is informing our future choices at Red Hat to focus on just deploying kubelet as systemd unit (like many other Kubernetes service providers), but if there are others in the community that want to keep the function alive and maintain it we need owners to volunteer and commit to fix issues and add much needed tests.

If not, it seems the responsible thing for the community is to clearly deprecate it.

@stealthybox
Copy link
Member

Just because I like causing trouble, another alternative is to run kubelet executable from a container (or systemd portable unit, etc), but unify the kubelet's rootfs view with that of the host. Specifically: do the equivalent of #54935, but for kubelet.
Then you would run from docker with -v /:/rootfs:rshared (or equivalent), and early in main() chroot() to /rootfs. This loads the executable from the docker image, but all later filesystem operations are done from a pov equivalent to the host root fs.

I think this keeps the kubelet lifecycle management benefits of running the kubelet binary from a container and allows the storage folks to just do regular filesystem operations, like there was nothing clever going on.

I like this suggestion to simplify the mechanism while retaining the packaging and runtime benefits of the container image.

@andrewrynhard, I'm curious what we're doing in talos if we're not using the --containerized flag and how what we're doing solves the use-case of running kubelet inside a container.

@andrewrynhard
Copy link
Contributor

I kind of like the idea of mounting / to /rootfs as well. Although, one of the benefits we see in mounting only the needed host paths is that it is less of the host filesystem exposed in case of a security vulnerability. On the other hand we currently mount everything we think we need: https://github.com/autonomy/talos/blob/master/internal/app/init/pkg/system/services/kubelet.go#L86-L96
and that could change in any release. The above solution would allow us to never worry about that again really. @stealthybox We don't use the --containerized flag, and I can't say I have seen any of the mentioned issues in our setup. We pass Sonobuoy conformance tests as well. So I can't say I would know exactly what we are doing to solve these problems since I personally have ran into them. I have had rook run just fine as well.

@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 Jun 19, 2019
@dims
Copy link
Member Author

dims commented Jul 11, 2019

/remove-lifecycle stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.