rootless netns: eval symlink for XDG_RUNTIME_DIR #14668
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 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
bot
added
the
approved
test/system/500-networking.bats
Outdated
|
|
||
| # NOTE: the --root/--runroot are required to force a new boltdb and not reuse old chached paths | ||
| XDG_RUNTIME_DIR="$NEW_XDG_RUNTIME_DIR" run_podman --root $PODMAN_TMPDIR/root --runroot $PODMAN_TMPDIR/runroot unshare --rootless-netns ip a | ||
| is "$output" ".*tap0.*" "slirp4netns interface exists in netns" |
There was a problem hiding this comment.
I just ran this against podman @ fe8e536 (main) and it passes. This suggests that you are not testing what you think you're testing.
There was a problem hiding this comment.
Thanks for checking. I think I have to actual run a container with netavark to trigger this problem.
There was a problem hiding this comment.
Should fail with main now and pass on this PR if netavark is used.
|
LGTM |
libpod/networking_linux.go
Outdated
| @@ -134,6 +134,12 @@ func (r *RootlessNetNS) Do(toRun func() error) error { | |||
| if err != nil { | |||
| return errors.Wrap(err, "could not get runtime directory") | |||
| } | |||
| // eval symlinks since they may not exists in the new path: https://github.com/containers/podman/issues/14606 | |||
| xdgRuntimeDir, err = filepath.EvalSymlinks(xdgRuntimeDir) | |||
There was a problem hiding this comment.
would it work to have the check once only in SetXdgDirs() instead of two different places?
There was a problem hiding this comment.
That sounds good but we have to be careful not to break any existing systems.
Luap99
force-pushed
the
netns-evalsymlink
branch
from
6e37c47 to
adda5fc
Compare
When we bind mount the old XDG_RUNTIME_DIR to the new fake /run it will cause issues when the XDG_RUNTIME_DIR is a symlink since they do not exists in the new path hierarchy. To fix this we can just follow the symlink before we try to use the path. This fix is kinda ugly, our XDG_RUNTIME_DIR code is all over the place. We should work on consolidating this sooner than later. Fixes containers#14606 Signed-off-by: Paul Holzinger <[email protected]>
Luap99
force-pushed
the
netns-evalsymlink
branch
from
adda5fc to
f5c3395
Compare
I'm seeing the same failure on my f36 laptop. |
I clearly do not understand the issue correctly, something very strange is happening here. It works for me the first time but when I rerun the it it fails. |
|
Ping (I am going through older PRs). |
|
Fixing it properly is more complicated, we have to start sharing the XDG code across the repos, c/common uses a different function than podman thus breaking the test here. |
openshift-merge-robot
added
the
needs-rebase
|
@Luap99: PR needs rebase. Instructions 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. |
|
A friendly reminder that this PR had no activity for 30 days. |
|
replaced by #15918 |
github-actions
bot
added
the
locked - please file new issue/PR
When we bind mount the old XDG_RUNTIME_DIR to the new fake /run it will
cause issues when the XDG_RUNTIME_DIR is a symlink since they do not
exists in the new path hierarchy. To fix this we can just follow the
symlink before we try to use the path.
Fixes #14606
Does this PR introduce a user-facing change?