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

Avoid operation not supported mounting failure on macos #1146

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Oct 13, 2022

As :Z mounts are usually not available on MacOS, we avoid adding that label.

Fixes: #1145
Related: containers/podman-compose#509

As `:Z` mounts are usually not available on MacOS, we avoid adding
that label.

Fixes: ansible#1145
@ssbarnea ssbarnea requested a review from a team as a code owner October 13, 2022 10:48
@ssbarnea ssbarnea changed the title Fix execution on macos Avoid operation not supported mounting failure on macos Oct 13, 2022
@rhatdan
Copy link

rhatdan commented Oct 13, 2022

Would it be better to try relabeling and then fall back to no labeling on ENOSUP? That way in the future if Mac gains labeling (virtiosfsd has some support) then it will work in both cases.

@ssbarnea
Copy link
Member Author

@rhatdan We start containers quite often, so having a retry approach would have impact on performance and also clutter the logs with retries.

IMHO, I would just make another release once podman starts having support for :Z.

Another interesting question is if we really need :Z even on Linux. I am not convinced that we do but I might miss lots of special use-cases, as runner is used in many ways.

It is true, that I could also look for which is the current container engine and do the trick only with docker as current change modifies both podman/docker.

@nitzmahone
Copy link
Member

nitzmahone commented Oct 13, 2022

This change would also break valid scenarios where relabeling is required, eg where the default target of a podman-remote setup or docker client is a remote host with SELinux enabled (so the actual volume mount sources are on the container host FS, not the client FS). I use this myself on my Apple Silicon Mac to talk to a local Rosetta2-enabled Fedora36 VM that can run x86_64 and aarch64 containers. Getting SELinux configured to work properly with rootless containers and the unlabel-able Rosetta virtiofs mount was "fun", but worthwhile over just shutting it off. So I'd agree with @rhatdan that just turning off relabeling entirely will likely just cause a different set of problems.

And yes, disabling it on Linux will break anyone using rootless containers on a default Fedora or RHEL setup since SELinux is enabled and enforcing by default.

I actually very much dislike the fact that runner is rewriting user-supplied mounts at all, but since it doesn't currently distinguish between its own injected mounts and the user-supplied ones, I guess this shouldn't be a surprise. It's already moving away from the need for injected mounts anyway for the callbacks, so maybe we just need to double down on that and take the performance hit to copy the project in instead of mounting it. Longer-term discussion, I suppose...

@ssbarnea
Copy link
Member Author

AFAIK, there is not mounting support when using real podman-remote (different hw machine), so I am not sure how runner would work with that.

I think that we need to make a (big) difference between a random linux machine (vm or not) and the podman-machine-default on macos, which is basically a VM which has only one purpose: to facility a macos user to use podman, nothing else.

I would never dare to tell a linux user to disable selinux on his on servers or workstations. But on the other hand, I can see how we might need to customize the podman-machine-default to make it work out of the box.

From the extension point of view, we will never be able to use copy instead of mount as we run lots of commands, the penalty would be so big that would render the extension unusable. But the funny bit is that the extension is not affected by the mentioned issues, as we do only mount the code, without all the extra bigs done by runner.

It should be noted that only one of the 5-6 mounts done by runner had the :Z label added, and that was the artifacts one. If we work this out to not need it, we might sort the issue.

In fact I observed that removing the :Z was enough even without having to disable selinux inside the podman-machine.

To summarize, this is what I think we should look into:

  • macos podman itself to change the default security_module for mounts to none, and pre-mount the specified 3 disk locations (which should be enough for most users). Until this happens, we can tell people to follow the manual steps from https://github.com/ansible/vscode-ansible/wiki/macos
  • runner should be changed to avoid using :Z label at least on mac/podman case. Maybe we can use some info returned by podman info to make a more informed decision about use or not of relabeling?

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.

Incompatible with podman on macos due to the use of :Z on volume mount
3 participants