Disable k3s embedded containerd process, start k3s-containerd binary as external process #3478
Conversation
Signed-off-by: Andrew Durbin <[email protected]>
…still use the k3s supplied binary to allow us to change the containerd root. Add missing findutils package for longhorn. Raise io priority for embedded etcd. Add Pramodhs containerd.go change to skip starting containerd-user in pillar. Signed-off-by: Andrew Durbin <[email protected]>
andrewd-zededa
requested review from
deitch,
rouming and
zedi-pramodh
as code owners
|
I find it surprising that k3s doesn't let you select a different containerd or control the path, as that is the kind of thing they would, but maybe they never got to it. The two questions I have are:
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3478 +/- ##
==========================================
- Coverage 20.32% 20.30% -0.02%
==========================================
Files 198 198
Lines 45230 45246 +16
==========================================
- Hits 9192 9187 -5
- Misses 35355 35374 +19
- Partials 683 685 +2
☔ View full report in Codecov by Sentry. |
|
To answer your questions.
2)I think Andrew meant that we will be using pillar user containerd not the main containerd. So basically we will be using /run/containerd-user/containerd.sock that is the socket access point for deployed images. |
Thanks for the explanation; this definitely helps. To the best of my knowledge, Kubernetes actually does not have a download mechanism. It depends entirely on the container runtime (in this case, containerd). Either way, pointing k3s at a different containerd doesn't change its download flow. If you ask it to run a pod, and the image is not there, it will use the containerd API (not precisely, but close enough) to download the image. If containerd has network access, it will do so. This raises an interesting question, which @eriknordmark is likely to know as well. Right now, I do not think our containerd is network-blocked. We do not use it to download - instead we use the volumemgr+downloader+verifier and load it into containerd, and then use containerd to run it. But is containerd actually blocked from downloading? I don't think it is. Maybe we configure it to have no network access, but I am somewhat sure we do not have that. So pointing at a different containerd does not change the flow. If k3s is asked for a container, it will ask containerd to get that image unless available. Mind, if I think we have some experiments here to determine:
We currently have 2 containerd running: system (runs pillar, etc.) and user (runs user workloads, i.e. ECOs). I think you are saying, k3s will add another, which would make it 3. Change it so that it remains only 2, and k3s points at the user containerd managed by pillar? If that is so, 100% in favour. I still am surprised you cannot just point k3s at an existing containerd, but you did the research, so I must put my surprise aside and accept it. Summary:
|
|
@deitch You brought up good points let me summarize it for you.
|
That might be a pretty big change. Have we tested the shipped version with everything else that we do that depends on containerd? Version and functionality mismatches? CVEs opened or closed? It might be fine, but we definitely need to consciously decide, "we are ok switching the specific version we are using".
You can get containerd to pull directly, whether using
Yes, makes sense. That has been our philosophy all along.
That probably should do it. I think we still should test if containerd has the ability to bypass our downloading. It is pretty simple:
|
I am not sure I understood your question correctly. This is a new eve flavor and we will not be upgrading any existing kvm based installs. This is a new product altogether and we will be starting with containerd that comes with k3s.
Yes containerd can pull images if we explicitly ask it to, but as a product we are not doing that and will not do that. No user is logging into the device to do that manually. So basically nothing changes from what we have now. |
I wasn't worried about upgrades. We still are releasing a new eve, which has lots of components that depend on that containerd. It likely will be just fine, but we should not change the version or location of a central component in eve without validating first that we are ok. To put it in other terms: let's say that the current version is 1.7.0, and k3s comes with 1.7.6. We should first check that we are ok with 1.7.6 in place, and then switch. Conversely, if we are running 1.7.6, and k3s is running 1.6.5, then we very well might have compatibility issues.
Which is good. We should, however, disable containerd from doing that entirely, as long as that is our philosophy, and not rely on, "we don't do that". This is not an issue for your deliverable here; you just happen to be lucky enough to have brought it to the fore by having another component that communicates with containerd. |
I am sorry but this is what I am not getting. We do not run any different user containerd version in eve. We just run the version that comes with k3s. So the compatibility between k3s and user containerd is always maintained. |
|
Now I am confused. We already install a containerd in eve. Didn't we write above that we will replace that one with the one that comes with k3s? |
We won't replace the one that that comes with eve. We just do not start the one that comes with eve and start the one that comes with k3s. |
|
That is replacing it. It doesn't matter if we have it on disk or not, it matters if we use it. Again, it could be fine, but now we are tying our eve containerd version to the k3s one. That is a change - completely independent of k3s usage - that requires testing. BTW, what happens when we need to upgrade containerd to some version for a feature, if k3s isn't there yet? |
This change is only in kubevirt eve and will not effect kvm based eve. So we will be always using containerd that comes with k3s on kubevirt eve only. This does not effect our existing kvm eve and the upgrades of it. Are you concerned about eve containerd client code working with containerd from k3s ? Yes that will be tested, so far we did not see any issue in our testing. |
|
Yes, I am concerned about that. I also am concerned about having variants on eve that change fundamental compatibility components. |
I will be happy to get on call and learn about your concerns. From my point of view this commit or upcoming commits are no-op on existing eve ie does not change anything. Only changes are in new kubevirt eve. |
|
To address the question of containerd versions here are each so it does appear the k3s one is at least one minor version behind. k3s version: 8c857dec-7cb8-45cb-8e20-a6c7569446c4:/# /var/lib/rancher/k3s/data/current/bin/containerd -v existing pillar version: (ns: pillar) 8c857dec-7cb8-45cb-8e20-a6c7569446c4:/# /usr/bin/containerd -v |
|
We're currently on k3s version v1.26.3+k3s1. If we upgrade to the latest k3s v1.28.2+k3s1 then we'll get a containerd upgrade to v1.7.6-k3s1. |
|
We are opening a Pandora's box (which, ironically, probably wasn't a box, as they didn't really have "boxes" back then 😄 ), in having different versions of containerd on different versions of eve which are almost the same but not quite. That is my concern. We should have eve, with its tested and supported version of its components, and things that plug into it and are tested with it.
Sure, let's coordinate offline. |
|
Re-opening from @zedi-pramodh 's request. Were there any requested changes to this PR from the offline conversation @deitch and @zedi-pramodh ? |
|
Not that I know of @andrewd-zededa. @deitch reviewed the design document and in general agreed with this approach. But I will leave it to him to review and approve. |
pkg/kube/cluster-init.sh
Outdated
| @@ -83,6 +120,14 @@ setup_prereqs | |||
|
|
|||
| date >> $INSTALL_LOG | |||
| HOSTNAME=$(/bin/hostname) | |||
| logmsg "Starting wait for hostname, currently: $HOSTNAME" | |||
| while [[ $HOSTNAME = linuxkit* ]]; | |||
There was a problem hiding this comment.
What are you trying to check?
It is the case that EVE comes up with a linuxkit* name early in boot, but then the hostname is changed to be the device UUID string. So I wouldn't depend on it being linuxkit* if you are trying to check whether this is running EVE-OS.
There was a problem hiding this comment.
If you want to check whether or not the device has been onboarded then we have a way to do that using pubsub and you can potentially use the pubsub checkpoint file in /persist/status/zedclient/OnboardingStatus/global.json to check this.
There was a problem hiding this comment.
I could have added a good comment in the script there. This was added to avoid a hang further down in startup where k3s node ready state was polled looking for a node in the ready state with the linuxkit* name (which doesn't exist anymore at this point in runtime). The k3s check ready path has been refactored since the hostname poll change was put in place. With fresh eyes now I think a smaller change could be in check_node_ready() and just read a fresh copy of the system hostname.
There was a problem hiding this comment.
I see several merge commits and those need to be removed since can not be pulled to master.
FWIW I use git fetch upstream master; git rebase -i upstream/master
to avoid ever getting any merge commits.
Also there are a number of yetus issue to address in the script(s).
Signed-off-by: Andrew Durbin <[email protected]>
|
Converted to draft since with the merge commits this is clearly not ready for review. |
|
Sorry I'm just closing this to reopen with a cleaner pr, the rebase isn't happening cleanly. |
The embedded k3s containerd instance does not allow changing the containerd root path away from a subdirectory of the k3s-data-dir as they specify a root on command line args overriding whatever is in the config.toml file. So we will disable the embedded k3s containerd process and instead use the k3s supplied containerd binary to start a containerd instance next to k3s (still in the kube container) which will allow us to allow us to change the containerd root. This containerd instance will re-use the pillar /run/containerd-user/containerd.sock to allow ease of reusing wait-ready code on the pillar side.
Add missing findutils package for longhorn.
Raise io priority for embedded etcd.
Add Pramodhs containerd.go change to skip starting containerd-user in pillar.