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

We need to use global GraphDriverOptions for rootless environments #709

Closed
wants to merge 1 commit into from

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Sep 3, 2020

Signed-off-by: Daniel J Walsh [email protected]

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Member Author

rhatdan commented Sep 3, 2020

Currently in rootless podman we are not using the global envionment. We need to pass at least ignore_chown_errors down for rootless podman.

We need to make sure that non VFS options are ignored if overlay is default.

This would fix containers/podman#7513 but I think it is risky, we want this in the long run, but for the podman-2.0.* release we should just handle the ignore_chown_errors.

@rhatdan
Copy link
Member Author

rhatdan commented Sep 3, 2020

@mheon @nalind @giuseppe PTAL

utils.go Outdated
@@ -210,7 +209,7 @@ func getRootlessStorageOpts(rootlessUID int) (StoreOptions, error) {
opts.RootlessStoragePath = opts.GraphRoot
if path, err := exec.LookPath("fuse-overlayfs"); err == nil {
opts.GraphDriverName = "overlay"
opts.GraphDriverOptions = []string{fmt.Sprintf("overlay.mount_program=%s", path)}
opts.GraphDriverOptions = append(opts.GraphDriverOptions, fmt.Sprintf("overlay.mount_program=%s", path))
Copy link
Member

Choose a reason for hiding this comment

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

Does this break when the system opts.GraphDriverName is not "overlay"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure. I think this could cause a few issues, which is why I stated that we need to be careful, even if this is the correct thing to do. I guess if we do the else option, we should just clear these fields.

Copy link
Member

Choose a reason for hiding this comment

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

Could we copy these options only when opts.GraphDriver is also set to overlay?

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

@rhatdan
Copy link
Member Author

rhatdan commented Sep 4, 2020

@giuseppe PTAL

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

@rhatdan
Copy link
Member Author

rhatdan commented Sep 9, 2020

I am closing, since I now believe we want to move some of these flags to containtainers/common.

@rhatdan rhatdan closed this Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants