/dev/shm should be mounted even in rootless mode. #1777
Conversation
|
@giuseppe PTAL |
|
FIxes #1770 |
|
@mheon @baude @umohnani8 @vrothberg PTAL |
|
LGTM |
libpod/container_internal.go
Outdated
| if err != nil { | ||
| return "", errors.Wrapf(err, "unable to determine if %q is mounted", c.config.ShmDir) | ||
| } | ||
| // TODO: generalize this mount code so it will mount every mount in ctr.config.Mounts |
There was a problem hiding this comment.
Do we or should we have an issue for that? If so could you note it here please?
|
LGTM, but test look like they're sputtering. |
rhatdan
force-pushed
the
shm
branch
10 times, most recently
from
9eff64d
to
3679d04
Compare
|
@giuseppe Any idea why this keeps blowing up? |
I think it was something wrong on the CI. I've tested the change locally and it worked for me. Let's see if it works now after the rebase |
test/e2e/rootless_test.go
Outdated
| Skip("User namespaces not supported.") | ||
| } | ||
| env := os.Environ() | ||
| cmd := podmanTest.PodmanAsUser([]string{"run", "--shm-size=40m", ALPINE, "grep", "shm", "/proc/self/mountinfo"}, 1000, 0, env) |
There was a problem hiding this comment.
this won't be enough to setup the rootless environment. What we are doing now, through runRootlessHelper, since we have no user configured is to setup the environment and use a single mapping inside of the container that runs with --rootfs.
Could you add the new test to runRootlessHelper? Otherwise feel free to skip it for now and I can take a look at it.
rhatdan
force-pushed
the
shm
branch
2 times, most recently
from
fc0fbf9
to
5dd1553
Compare
|
bot, retest this please |
Currently we are mounting /dev/shm from disk, it should be from a tmpfs. User Namespace supports tmpfs mounts for nonroot users, so this section of code should work fine in bother root and rootless mode. Signed-off-by: Daniel J Walsh <[email protected]>
|
@giuseppe @baude @mheon @vrothberg @umohnani8 PTAL I could not get the tests to pass, so lets get this in, and can @giuseppe open a PR to add tests. |
|
LGTM |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
github-actions
bot
added
the
locked - please file new issue/PR
Currently we are mounting /dev/shm from disk, it should be from a tmpfs.
User Namespace supports tmpfs mounts for nonroot users, so this section of
code should work fine in bother root and rootless mode.
Signed-off-by: Daniel J Walsh [email protected]