Fix/improve pkg/storage.InitFSMounts

[kolyshkin]

Fix

Found the bug by reading the source for InitFSMounts(), confirmed with:

$ ./bin/podman run -v /tmp:/tmp alpine true; echo $?
0

$ ./bin/podman run -v /tmp:/tmp:ro alpine true; echo $?
0

$ ./bin/podman run -v /tmp:/w0w:ro alpine true; echo $?
> Error: container_linux.go:349: starting container process caused "process_linux.go:449: container init caused \"rootfs_linux.go:58: mounting \\\"/tmp\\\" to rootfs \\\"/home/kir/.local/share/containers/storage/overlay/7636ef3650fc91ee4996ccc026532bb3cff7182c0430db662fffb933e0bcadc9/merged\\\" at \\\"/home/kir/.local/share/containers/storage/overlay/7636ef3650fc91ee4996ccc026532bb3cff7182c0430db662fffb933e0bcadc9/merged/w0w\\\" caused \\\"operation not permitted\\\"\"": OCI runtime permission denied error
> 126

The last command is not working because in-container mount point
(i.e. mount destination, not the mount source) is used to search
for a parent mount in /proc/self/mountinfo.

And yet the following

$ ./bin/podman run -v /tmp:/run/test:ro alpine true; echo $?
0

still works fine! Here's why:

$ mount | grep -E '/run |/tmp '
tmpfs on /run type tmpfs (rw,nosuid,nodev,seclabel,mode=755)
tmpfs on /tmp type tmpfs (rw,nosuid,nodev,seclabel)

Fixes: #2432 (commit 0f5ae3c)
Related-to: #2312

Improve

Instead of getting mount options from /proc/self/mountinfo, which is
very costly to read/parse (and can even be unreliable), let's use
statfs(2) to figure out the flags we need.

[openshift-ci-robot]

Hi @kolyshkin. Thanks for your PR.

I'm waiting for a containers member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[kolyshkin]

@giuseppe PTAL

[giuseppe]

/approve

[rhatdan]

LGTM, but you have compiler errors.

[mheon]

/approve
/ok-to-test

I love the simplification of the mount parsing - this is much, much cleaner.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, kolyshkin, mheon

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:

• OWNERS [giuseppe,mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mheon]

Looks like it needs to be gated to only compile on Linux, though, it's blowing up on Darwin.

[kolyshkin]

• changed InitFSMounts to change options in place rather than creating a new slice
• fixed non-Linux compilation (moved getting default options to pkg/util)

[kolyshkin]

Looks like it needs to be gated to only compile on Linux, though, it's blowing up on Darwin.

done

@mheon May I ask why do we care for darwin?

[kolyshkin]

Oh, any suggestions about how to add a regression test to CI for this are welcome. I tried it before (see e336840) but a CI has passed :-\

[rhatdan]

@kolyshkin containers/image which vendors in container/storage is used for skopeo on Macs. So we want to make sure the tool chain builds for Darwin.

[mheon]

The Darwin stuff in Libpod is a result of the current remote darwin client situation. Hopefully, once Podmanv2 is done, we will no longer need to have Podman build on OS X.

[kolyshkin]

I fixed the issues with my patch, current CI failures are (to my best knowledge) unrelated.

[mheon]

Look like known flakes. Code LGTM

[rhatdan]

/lgtm
/hold
hold cancel when tests pass

[kolyshkin]

@rhatdan CI is all green now

[mheon]

/hold cancel