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

Use storage that better supports rootless overlayfs #13375

Merged
merged 1 commit into from
Mar 2, 2022
Merged

Use storage that better supports rootless overlayfs #13375

merged 1 commit into from
Mar 2, 2022

Conversation

kousu
Copy link
Contributor

@kousu kousu commented Feb 28, 2022

overlayfs -- the kernel's version, not fuse-overlayfs -- recently learned
(as of linux 5.16.0, I believe) how to support rootless users. Previously,
rootless users had to use these storage.conf(5) settings:

  • storage.driver=vfs (aka STORAGE_DRIVER=vfs), or
  • storage.driver=overlay (aka STORAGE_DRIVER=overlay),
    storage.options.overlay.mount_program=/usr/bin/fuse-overlayfs
    (aka STORAGE_OPTS=overlay.mount_program=/usr/bin/fuse-overlayfs) -- and this is the current default

Now a third backend is available, setting only:

  • storage.driver=overlay (aka STORAGE_DRIVER=overlay)

With this configuration, #13123 reported EXDEV errors during the normal operation of their container. Tracing it out, the problem turned out to be that their container was being mounted without 'userxattr'; I don't fully understand why, but mount(8) mentions this is needed for rootless users:

userxattr

Use the "user.overlay." xattr namespace instead of "trusted.overlay.".
This is useful for unprivileged mounting of overlayfs.

containers/storage#1156 found and fixed the source of the issue, and this just pulls in that via

go get github.com/containers/storage@ebc90ab
go mod vendor
make vendor

Closes #13123

@kousu
Copy link
Contributor Author

kousu commented Feb 28, 2022

Hi! I might be jumping the gun here. I wanted to be able to test my fix from containers/storage#1156, but I haven't even posted my full investigation on #13123 nor actually tested this yet, and containers/storage hasn't had a new release yet.

The branch will be useful on my end just for testing, but no worries if you need to close this and instead wait for the next release of containers/storage.

@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

This is fine.
LGTM
/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2022
@rhatdan
Copy link
Member

rhatdan commented Feb 28, 2022

@containers/podman-maintainers PTAL

@kousu
Copy link
Contributor Author

kousu commented Feb 28, 2022

Thank you for the pleasant experience @rhatdan.

To test this, you need to activate the overlayfs driver without using fuse-overlayfs:

mkdir -p ~/.config/containers
cat <<EOF >~/.config/containers/storage.conf
[storage]
driver = "overlay"
EOF
podman system reset  # BEWARE: will erase all your containers; consider using a blank test user
podman system info -f '{{.Store.GraphDriverName}} {{.Store.GraphOptions}}' # should report "overlay map[]"

then you need to find a container with many layers. You can use docker.io/kkharlamov/bugreport-enomem:

podman run -it docker.io/kkharlamov/bugreport-enomem apt install libreadline-dev

In the broken case, you'll see:

dpkg: error processing archive /var/cache/apt/archives/libreadline-dev_7.0-3_amd64.deb (--unpack):
 unable to install new version of './usr/include/readline': Invalid cross-device link

The working case, with this patch in, should succeed and exit with 0.

That container is very very large. I'm still working on making a reduced test case.

@TomSweeneyRedHat
Copy link
Member

LGTM
but I think a rebase is needed.

@baude
Copy link
Member

baude commented Mar 1, 2022

rebase and it will get merged

overlayfs -- the kernel's version, not fuse-overlayfs -- recently learned
(as of linux 5.16.0, I believe) how to support rootless users. Previously,
rootless users had to use these storage.conf(5) settings:

* storage.driver=vfs          (aka STORAGE_DRIVER=vfs), or
* storage.driver=overlay      (aka STORAGE_DRIVER=overlay),
  storage.options.overlay.mount_program=/usr/bin/fuse-overlayfs
                              (aka STORAGE_OPTS=/usr/bin/fuse-overlayfs)

Now that a third backend is available, setting only:

* storage.driver=overlay      (aka STORAGE_DRIVER=overlay)

#13123 reported EXDEV errors
during the normal operation of their container. Tracing it out, the
problem turned out to be that their container was being mounted without
'userxattr'; I don't fully understand why, but mount(8) mentions this is
needed for rootless users:

> userxattr
>
>   Use the "user.overlay." xattr namespace instead of "trusted.overlay.".
>   This is useful for unprivileged mounting of overlayfs.

containers/storage#1156 found and fixed the issue
in podman, and this just pulls in that via

    go get github.com/containers/storage@ebc90ab
    go mod vendor
    make vendor

Closes #13123

Signed-off-by: Nick Guenther <nick.guenther@polymtl.ca>
@kousu
Copy link
Contributor Author

kousu commented Mar 1, 2022

rebase and it will get merged

You've got it :)

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

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

@jwhonce
Copy link
Member

jwhonce commented Mar 1, 2022

/hold pending tests passing

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2022
@kousu
Copy link
Contributor Author

kousu commented Mar 1, 2022

Is there anything I can do about the test failure? It looks like it's this bug #12624?

@edsantiago
Copy link
Member

@kousu I've restarted the offending test; chances are good that it will pass on re-run.

@edsantiago edsantiago removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 2, 2022
@edsantiago
Copy link
Member

Tests are green, I've released the hold. Thank you for your work and your patience @kousu!

@openshift-merge-robot openshift-merge-robot merged commit 7877b02 into containers:main Mar 2, 2022
@kousu kousu deleted the repair-13123 branch March 2, 2022 00:17
@kousu
Copy link
Contributor Author

kousu commented Mar 2, 2022

And thanks for podman! I'm using it to help develop some ansible scripts to help. Our targets need systemd, and so it was either rent a VPS or otherwise run a new VM everytime I test, or use podman 🎉.

And thanks also for the extremely quick turnaround. Everyone I've shown has commented that <24hrs is extremely fast for open source work to get done.

@rhatdan
Copy link
Member

rhatdan commented Mar 2, 2022

@kousu This just showed up today. https://github.com/linux-system-roles/podman
Might be of some interest to you.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

overlay-mode containerization breaks apt: Invalid cross-device link
8 participants