-
Notifications
You must be signed in to change notification settings - Fork 23
Containerd Support: Add features cmd #106
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
Conversation
Signed-off-by: galal-hussein <hussein.galal.ahmed.11@gmail.com>
Hi @galal-hussein, thanks for the contribution, will take a look ASAP.
Can you do Also, does each pod get a different user-ns mapping? |
Sure, here are the results of two different pods running the sysbox-runc runtime:
The user namespace is intact, and each pod gets a different mapping. |
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.
Thanks @galal-hussein for the contribution!
Mostly looks good, just a few comments.
return nil, err | ||
} | ||
if len(unknownCaps) > 0 { | ||
logrus.Warn("ignoring unknown or unavailable capabilities: ", mapKeys(unknownCaps)) |
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.
Any particular reason for not using slices.Sorted(maps.Keys(unknownCaps))
(and therefore avoid the mapKeys()
function)?
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.
no not really, slices.Sorted seems more efficient, will fix
libcontainer/specconv/spec_linux.go
Outdated
var exists bool | ||
if config.RootPropagation, exists = mountPropagationMapping[spec.Linux.RootfsPropagation]; !exists { | ||
return nil, fmt.Errorf("rootfsPropagation=%v is not supported", spec.Linux.RootfsPropagation) | ||
|
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.
Remove empty line please.
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.
sure will do
types/features/features.go
Outdated
AnnotationRuncVersion = "org.opencontainers.runc.version" | ||
|
||
// AnnotationRuncCommit corresponds to the output of `git describe --dirty --long --always` in the runc repo. | ||
// Third party implementations such as crun and runsc SHOULD NOT use this annotation, as their repo is different from the runc repo. |
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.
Seems like we should not use this annotation per the comment?
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.
ah the features annotation were copied from runc implementation of runc features command, I will remove this particular annotation tho per their instruction.
Signed-off-by: galal-hussein <hussein.galal.ahmed.11@gmail.com>
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, thanks!
Thanks @galal-hussein again for the contribution! I've merged the PR and will test it with containerd user-namespaces. Thanks! |
@ctalledo Thank you very much! |
It seems that I had to build the
🎉 |
Hello, can you share the exact steps to run sysbox with containerd > 2.0 in k3s cluster?
|
The PR fixes nestybox/sysbox#958 and adds support to containerd > 2.0, after investigation I found out that containerd was failing to run
sysbox-runc features
and failing to register the runtime as a runtime that supports user namesapces:After enabling the debug log in containerd, the issue was more clear:
The PR adds the features command to sysbox-runc hence enabling contaienrd to work with sysbox, this was tested on k3s setup with no modification to containerd except for adding runtime to the config:
/var/lib/rancher/k3s/agent/etc/containerd/config.toml.tmpl
And running pod with the runtime class for sysbox-runc and
hostUsers: false
:I was able to run k3s within that pod successfully.