pkg/{runcfanotify, container-hook}: Refactor code to use notify package. #2766
Conversation
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
from
d13ff04
to
7d09622
Compare
| @@ -47,7 +47,6 @@ import ( | |||
|
|
|||
There was a problem hiding this comment.
I don't think we should this. Instead, we should deprecate and delete runc-fanotify in favour of container-hook #1831.
There was a problem hiding this comment.
Indeed, but we can still refactor the code between the env variable code path and the for one.
|
Supersedes: #2764 |
pkg/utils/notify/notify.go
Outdated
| "/usr/local/bin/runc", | ||
| "/usr/local/sbin/runc", | ||
| "/usr/lib/cri-o-runc/sbin/runc", | ||
| "/run/torcx/unpack/docker/bin/runc", // Used in TODO |
There was a problem hiding this comment.
| "/run/torcx/unpack/docker/bin/runc", // Used in TODO | |
| "/run/torcx/unpack/docker/bin/runc", // Used in Flatcar Container Linux |
There was a problem hiding this comment.
Actually, https://www.flatcar.org/blog/2024/04/os-innovation-with-systemd-sysext/ says:
Therefore, we recently removed Torcx and recommend using systemd-sysext for deploying custom Docker/containerd versions.
But we can keep the torcx path for a while for compatibility with older Flatcar versions.
@pothos When using systemd-sysext to install runc, crun or conmon, will it be in /usr/bin/ or do we need to add another path here?
There was a problem hiding this comment.
I also would like to add a link to some documentation pointing to this, do you have one?
There was a problem hiding this comment.
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
from
7d09622
to
a50534e
Compare
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
3 times, most recently
from
b0f9951
to
e8da160
Compare
There was a problem hiding this comment.
3 small things to fix, LGTM after that (if CI is green), from code inspection.
I would like @blanquicet to say whether his concern is addressed. In my opinion, this is fine to merge this PR even if we remove runcfanotify after.
As runc path is a symlink, we also need to use SecureJoin as done in bf76847 ("resolve symlinks in runtime path, add path for k3s"). Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
3 times, most recently
from
ada8e57
to
8691113
Compare
|
The linter was drunken and removed a needed package, it is fixed now. |
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
from
8691113
to
478ee32
Compare
…er package. The runtime-finder package hosts the runtime paths array as well as a function used to try marking a runtime path. Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
eiffel-fl
force-pushed
the
francis/factorize-hook
branch
from
478ee32
to
b045a2e
Compare
|
Thank you for the reviews! |
Hi.
This PR refactors the common code between
runcfanotifyandcontainer-hook.I also unified the runtime paths array, so they now count the same number for both hooks and it is now easier to add a new hook.
Best regards.