-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
libcontainer/system: move userns utilities, remove GetParentNSeuid
, UIDMapInUserNS
#2850
Conversation
Opening as draft; I'll create a PR in Moby to see if it has the desired result, then move it out of draft |
var ( | ||
// RunningInUserNS detects whether we are currently running in a user namespace. | ||
// Originally copied from github.com/lxc/lxd/shared/util.go | ||
// Deprecated: use github.com/opencontainers/runc/libcontainer/userns.RunningInUserNS |
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.
We haven't been guaranteeing Go compatibility of libcontainer, so I think we can just drop these deprecated functions, but not a strong opinion.
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.
Perhaps keep only "RunninginUserNS" (the "popular" one) and drop the others
f799e71
to
c6f2e2c
Compare
GetParentNSeuid
, UIDMapInUserNS
I removed those functions that we don't use; reordered the commits to first remove these parts before moving the code. Moved this out of draft. I also have a branch to isolate the userns package from the user package, as we only use the |
@kolyshkin ptal 🤗 |
@kolyshkin PTAL (I'll rebase #2868 after this) |
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
close/reopen to kick ci |
looks like ci/centos7 doesn't start; let me try another rebase to force CI to run |
close/reopen to trigger fedora CI |
This function was added in f103de5, but no longer used since 06f789cf26774dd64cb2a9cc0b3c6a6ff832733bc (v1.0.0-rc6) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
`UIDMapInUserNS()` is not used anywhere, only internally. so un-export it. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Moving these utilities to a separate package, so that consumers of this package don't have to pull in the whole "system" package. Looking at uses of these utilities (outside of runc itself); `RunningInUserNS()` is used by [various external consumers][1], so adding a "Deprecated" alias for this. [1]: https://grep.app/search?current=2&q=.RunningInUserNS Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
rebased again to kick centos7 |
All green now 🎉 @kolyshkin @AkihiroSuda ptal |
@kolyshkin ptal 🤗 |
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! I rebased #2868 and moved it out of draft |
libcontainer/system: remove unused
GetParentNSeuid()
This function was added in f103de5 (#1688), but no longer used since 06f789c (#1862) (v1.0.0-rc6)
It is no longer used by runc itself, and only used by a single external project
libcontainer/system: un-export UIDMapInUserNS()
UIDMapInUserNS()
is not used anywhere, only internally. so un-export it.libcontainer/system: move userns utilities to separate package
Moving these utilities to a separate package, so that consumers of this package don't have to pull in the whole "system" package.
Looking at uses of these utilities (outside of runc itself);
RunningInUserNS()
is used by various external consumers,so adding a "Deprecated" alias for this.