BUILD-433: use the right mappings when we don't need to set any #291
Conversation
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/assign @coreydaley he has approval for builder repo now |
adambkaplan
changed the title
cmd/userns.go
Outdated
| isNodeDefaultMapping := func(m []specs.LinuxIDMapping) bool { | ||
| return len(m) == 1 && m[0].ContainerID == 0 && m[0].HostID == 0 && m[0].Size == 0xffffffff | ||
| } |
There was a problem hiding this comment.
Style nit - I prefer we move this to a regular function so that we can document with a unit test.
There was a problem hiding this comment.
Pulled it out as a separate function.
cmd/userns.go
Outdated
| } | ||
| uidMap += "]" | ||
| gidMap += "]" | ||
| klog.Infof("Started in kernel user namespace with UID map %s and GID map %s.", uidMap, gidMap) |
There was a problem hiding this comment.
Is this something that we always want to log, log at debug level (5), or somewhere in between (2)?
There was a problem hiding this comment.
I don't personally have a preference. It's mainly there so that we can test for it, but the test can also adjust the logging level. Do you have a preference?
|
/retest |
|
/retest-required |
|
/retest |
When we create a new user namespace so that we can get CAP_SYS_ADMIN over a mount namespace, if we're already UID 0, map the current set of ID mappings over themselves instead of trying to repeat the mapping that's been applied to the container we're in. If we know we've been started in a namespace that doesn't have node-default mappings, log that we know. Don't default to trying metacopy=on with mount_program=fuse-overlayfs, since it knows nothing about the option and all we do is trigger a repeated warning about it not being recognized. Signed-off-by: Nalin Dahyabhai <[email protected]>
|
/retest |
|
@nalind: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/approve |
openshift-ci
bot
added
the
approved
|
/assign @rolfedh For docs approval - I drafted docs in openshift/openshift-docs#44000 |
|
/cc @jitendar-singh @prietyc123 For QE approval |
|
/cc @RickJWagner For PX approval |
|
/label qe-approved |
openshift-ci
bot
added
the
qe-approved
RickJWagner
left a comment
RickJWagner
left a comment
There was a problem hiding this comment.
Thanks for the small touch-up for readability ('inUserNamespace' to 'inOurUserNamespace'). Posterity appreciates this.
|
/label px-approved |
openshift-ci
bot
added
the
px-approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan, coreydaley, nalind, RickJWagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/label docs-approved This PR doesn't impact builds as is, and is not activated until openshift/openshift-controller-manager#173 merges. Furthermore, there won't be official docs for this feature because we are releasing it as Developer Preview. |
openshift-ci
bot
added
the
docs-approved
8 participants
When we create a new user namespace so that we can get CAP_SYS_ADMIN over a mount namespace, if we're already UID 0, map the current set of ID mappings over themselves instead of trying to repeat the mapping that's been applied to the container we're in.
If we know we've been started in a namespace that doesn't have node-default mappings, log that we know.
Don't default to trying "metacopy=on" with "mount_program=fuse-overlayfs", since it knows nothing about the option and all we do is trigger a repeated warning about it not being recognized.