-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
cri/server: use containerd/oci instead of libcontainer/devices (remove libcontainer/devices dependency) #5324
Conversation
Build succeeded.
|
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
Oh; let me make 1 minor change; containerd/oci/spec_opts_unix.go Line 45 in 261c107
containerd/oci/spec_opts_linux.go Line 46 in a4bc817
|
Looks like we had our own copy of the "getDevices" code already, so use that code (which also matches the code that's used to _generate_ the spec, so a better match). Moving the code to a separate file, I also noticed that the _unix and _linux code was _exactly_ the same (baring some `//nolint:` comments), so also removing the duplicated code. With this patch applied, we removed the dependency on the libcontainer/devices package (leaving only libcontainer/user). Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cc67943
to
9bc8d63
Compare
Build succeeded.
|
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
Any dependency reduction is a win!
and it's ✅ 🎉 (which is always nice to see!) |
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
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
Looks like we had our own copy of the "getDevices" code already, so use
that code (which also matches the code that's used to generate the spec,
so a better match).
Moving the code to a separate file, I also noticed that the _unix and _linux
code was exactly the same (baring some
//nolint:
comments), so alsoremoving the duplicated code.
With this patch applied, we removed the dependency on the libcontainer/devices
package (leaving only libcontainer/user).