-
Notifications
You must be signed in to change notification settings - Fork 61
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
pre-install: Create containerd config it it doesn't exist #273
pre-install: Create containerd config it it doesn't exist #273
Conversation
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are already aware of this, but when I building the image and testing out the code it created the /etc/containerd/config.toml
, but it had no content
4382f2f
to
47e3d28
Compare
We're relying here on containerd being on /usr/local/bin/, which we do have access to, in order to create such config. We're creating the config as the first thing on the script, as we don't want to get into the situation where a configuration was added / removed and we end up with that in the system due to us using a newer version of containerd than the one present in the system. We may face more issues in the future, such as: * containerd is only in /usr/bin/ * We can start exposing this as a host path mount, but let's not do it unless it becomes necessary * when CRI-O is supported, this file should only be created in case containerd is used However, for now, let's go with this approach and avoid users hitting a possible error due to the lack of contained configuration. Fixes: confidential-containers#271 Signed-off-by: Fabiano Fidêncio <[email protected]>
47e3d28
to
150d381
Compare
That was happening due to the lack of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working for me now. On my system that wasn't using a containerd config I
- Rebuild the req payload and pushed it to quay, then updated the peer-pod cc runtime kustomize to use that as a new image
- I ssh'd into my worker node and confirmed that the /etc/containerd was empty:
ls -al /etc/containerd/
total 8
drwxr-xr-x 2 root root 4096 Oct 27 15:58 .
drwxr-xr-x 97 root root 4096 Oct 27 15:59 ..
- I deployed the CoCo manager and my peer pods CC runtime an waited for the pre-install-daemon to be running
- I re-checked the directory in my worker and the new file existed and was non-empty now:
ls -al /etc/containerd/
total 20
drwxr-xr-x 3 root root 4096 Oct 27 16:13 .
drwxr-xr-x 97 root root 4096 Oct 27 15:59 ..
-rw-r--r-- 1 root root 7911 Oct 27 16:13 config.toml
drwxr-xr-x 2 root root 4096 Oct 27 16:13 config.toml.d
root@sh-sm-z-cluster-2023-10-26-node-1:~# cat /etc/containerd/config.toml
disabled_plugins = []
imports = ["/etc/containerd/config.toml.d/nydus-snapshotter.toml"]
oom_score = 0
plugin_dir = ""
required_plugins = []
root = "/var/lib/containerd"
state = "/run/containerd"
...
Thanks a lot for the fix!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test |
We're relying here on containerd being on /usr/local/bin/, which we do have access to, in order to create such config.
We're creating the config as the first thing on the script, as we don't want to get into the situation where a configuration was added / removed and we end up with that in the system due to us using a newer version of containerd than the one present in the system.
We may face more issues in the future, such as:
However, for now, let's go with this approach and avoid users hitting a possible error due to the lack of contained configuration.
Fixes: #271