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.

Sign up for GitHub

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

Jump to bottom

rootless: default to fuse-overlayfs when available #1726

Merged
merged 1 commit into from
Nov 9, 2018
Merged

rootless: default to fuse-overlayfs when available #1726

merged 1 commit into from
Nov 9, 2018

Conversation

giuseppe
Copy link
Member

If fuse-overlayfs is present, rootless containers default to use it.
This can still be overriden either via the command line with
--storage-driver or in the ~/.config/containers/storage.conf
configuration file.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@giuseppe
Copy link
Member Author

/cc @dustymabe

@openshift-ci-robot
Copy link
Collaborator

@giuseppe: GitHub didn't allow me to request PR reviews from the following users: dustymabe.

Note that only containers members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @dustymabe

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.

@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2018

/approve
@dustymabe FYI

@openshift-ci-robot
Copy link
Collaborator

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2018
@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2018 •
edited
Loading

Does podman info report correctly?

@giuseppe
Copy link
Member Author

@rhatdan should we change the default selinux policy to allow fuse from containers?

@TomSweeneyRedHat
Copy link
Member

Is/will this be documented somewhere that the user can find?

@giuseppe
Copy link
Member Author

Does podman info report correctly?

yes, it is part of podman info

@giuseppe
Copy link
Member Author

giuseppe commented Oct 30, 2018 •
edited
Loading

Is/will this be documented somewhere that the user can find?

this is the default value used when there is no configuration. It gets closer to "root containers" where we default to overlay.

EDIT: I've updated the documentation to say the default is not changed when fuse-overlayfs is present

@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2018

Yes we will need to work on SELinux policy to allow container_t to use fusefs_t by default.

rhatdan
rhatdan reviewed Oct 30, 2018
View reviewed changes
Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

👍

@rhatdan
Copy link
Member

rhatdan commented Oct 30, 2018

We can fix this in a different PR, but this is broken,

podman --help | grep storage-driver
   --storage-driver value, -s value  select which storage driver is used to manage storage of images and containers (default is overlay)

mheon
mheon reviewed Oct 30, 2018
View reviewed changes
docs/podman.1.md Outdated
@@ -56,7 +56,7 @@ Path to the OCI compatible binary used to run containers

**--storage-driver, -s**=**value**

Storage driver. The default storage driver for UID 0 is configured in /etc/containers/storage.conf (`$HOME/.config/containers/storage.conf` in rootless mode), and is *vfs* for other users. The `STORAGE_DRIVER` environment variable overrides the default. The --storage-driver specified driver overrides all.
Storage driver. The default storage driver for UID 0 is configured in /etc/containers/storage.conf (`$HOME/.config/containers/storage.conf` in rootless mode), and is *vfs* for other users when *fuse-overlayfs* is not available. The `STORAGE_DRIVER` environment variable overrides the default. The --storage-driver specified driver overrides all.
Copy link
Member

Choose a reason for hiding this comment

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

Suggestions: "for other users" -> "for non-root users"

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in a new version

TomSweeneyRedHat
TomSweeneyRedHat reviewed Oct 30, 2018
View reviewed changes
docs/podman.1.md
@@ -56,7 +56,7 @@ Path to the OCI compatible binary used to run containers

**--storage-driver, -s**=**value**

Storage driver. The default storage driver for UID 0 is configured in /etc/containers/storage.conf (`$HOME/.config/containers/storage.conf` in rootless mode), and is *vfs* for other users. The `STORAGE_DRIVER` environment variable overrides the default. The --storage-driver specified driver overrides all.
Storage driver. The default storage driver for UID 0 is configured in /etc/containers/storage.conf (`$HOME/.config/containers/storage.conf` in rootless mode), and is *vfs* for non-root users when *fuse-overlayfs* is not available. The `STORAGE_DRIVER` environment variable overrides the default. The --storage-driver specified driver overrides all.
Copy link
Member

Choose a reason for hiding this comment

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

👍 Thanks @giuseppe

@TomSweeneyRedHat
Copy link
Member

LGTM assuming happy tests.

@umohnani8
Copy link
Member

LGTM

@baude
Copy link
Member

baude commented Nov 1, 2018

bot, retest this please

1 similar comment
@giuseppe
Copy link
Member Author

giuseppe commented Nov 6, 2018

bot, retest this please

@rhatdan
Copy link
Member

rhatdan commented Nov 6, 2018

I think we need a default nonroot-storage.conf file in /usr/share/containers/, this file should get copied to users homedir first time podman is run as non root. Not sure if there is a way to see if the user ever modified the file in his homedir.
Perhaps if we made a symlink to the systems nonroot-storage.conf on first use, then if the user modifies it, it would be permanent in the homedir.

@mheon
Copy link
Member

mheon commented Nov 7, 2018

@giuseppe Please rebase to pick up CI fixes

@giuseppe
rootless: default to fuse-overlayfs when available
c7926aa 
If fuse-overlayfs is present, rootless containers default to use it.
This can still be overriden either via the command line with
--storage-driver or in the ~/.config/containers/storage.conf
configuration file.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member Author

giuseppe commented Nov 9, 2018

tests are passing

@rhatdan
Copy link
Member

rhatdan commented Nov 9, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 9, 2018
@openshift-merge-robot openshift-merge-robot merged commit e8e16fc into containers:master Nov 9, 2018
@dustymabe dustymabe mentioned this pull request Nov 12, 2018
Closed
@eero-t eero-t mentioned this pull request Apr 14, 2023
Closed
@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 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rhatdan rhatdan rhatdan left review comments

@mheon mheon mheon left review comments

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@umohnani8 umohnani8 Awaiting requested review from umohnani8

Assignees

@rhatdan rhatdan

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@giuseppe @openshift-ci-robot @rhatdan @TomSweeneyRedHat @umohnani8 @baude @mheon @openshift-merge-robot