Skip to content
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

podman --runroot: remove 50 char length restriction #22277

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Apr 5, 2024

This was added ages ago in commit c65b359, however in the meantime both podman and conmon can support longer socket paths as they use a workaround to open the path via /proc/self/fd, see openUnixSocket() in libpod/oci_conmon_attach_linux.go

Thus this restriction is not needed anymore and we can drop a workaround in the tests.

Fixes #22272

Does this PR introduce a user-facing change?

The --runroot option now can use paths longer than 50 characters.

This was added ages ago in commit c65b359, however in the meantime
both podman and conmon can support longer socket paths as they use a
workaround to open the path via /proc/self/fd, see openUnixSocket() in
libpod/oci_conmon_attach_linux.go

Thus this restriction is not needed anymore and we can drop a workaround
in the tests.

Fixes containers#22272

Signed-off-by: Paul Holzinger <[email protected]>
@openshift-ci openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 5, 2024
@ashley-cui
Copy link
Member

LGTM

Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

openshift-ci bot commented Apr 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member

rhatdan commented Apr 6, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 447d3e2 into containers:main Apr 6, 2024
81 of 94 checks passed
@Luap99 Luap99 deleted the runroot branch April 6, 2024 12:19
@edsantiago
Copy link
Member

Thank you for this! Quick followup for posterity: although this seems to fix podman, slirp4netns still can't handle long paths:

# podman [options] run --network slirp4netns:port_handler=slirp4netns -dt -p 18082:8000 quay.io/libpod/alpine:latest /bin/sh
Error: /usr/bin/slirp4netns failed: "sent tapfd=7 for tap0\n
.....
the specified API socket path is too long (>= 108)\n     <<<<<<<<<<
...
Command failed with exit status 126

Not a big deal, since it's slirp, but it does mean we can't use long tmpdir paths in CI tests.

@Luap99
Copy link
Member Author

Luap99 commented Apr 8, 2024

Thank you for this! Quick followup for posterity: although this seems to fix podman, slirp4netns still can't handle long paths:

# podman [options] run --network slirp4netns:port_handler=slirp4netns -dt -p 18082:8000 quay.io/libpod/alpine:latest /bin/sh
Error: /usr/bin/slirp4netns failed: "sent tapfd=7 for tap0\n
.....
the specified API socket path is too long (>= 108)\n     <<<<<<<<<<
...
Command failed with exit status 126

Not a big deal, since it's slirp, but it does mean we can't use long tmpdir paths in CI tests.

We can report this to slirp4netns, the workaround we use is easy enough to implement there as well. Also it seems to only effect the slirp4netns port_handler (not the default rootlesskit) so it should work fine for most users.

@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jul 8, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jul 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runroot is limited to 50 characters
5 participants