Partially revert "mount: Check if src exists before mounted (#61752)"

[patrakov]

This reverts part of commit 72023d7.

The immediate reason is that it breaks mounts where src is not a path.
Examples of such mounts are nfs, cifs, glusterfs, ceph, tmpfs,
overlayfs, and it is too hard to come with an exhaustive list,
especially if we take non-Linux systems into account.

Additionally, it did not really fix the issue (#59183) that it intended
to fix, because the mount could fail but leave a non-working fstab entry
for reasons other than non-existing src path.

SUMMARY

This was originally submitted as #65869, with some extra commits that attempt to re-fix #59183. However, that pull request was not properly reviewed in 2+ months, presumably because the additional commits are too complex. Therefore, I am resubmitting just the minimal fix that is hopefully easier to review.

Fixes: #65855
Fixes: #67588
Fixes: #67966

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

mount

ADDITIONAL INFORMATION

See #65544 and #65869.

[ansibot]

cc @jtyr
click here for bot help

[bcoca]

I would prefer the fix would add an aditional check not startswith('UUID=', 'LABEL=') and then keep the path.exists check for those cases

[patrakov]

I am absolutely opposed to any additional checks. We have already tried that while discussing #65544 and failed. Martin initially missed CIFS. I missed the UUID and LABEL cases. You, in your own previous comment, missed PARTUUID. Then OpenBSD has special syntax (which looks like 55128c3700af5491.a) too, which is described in section "DUIDs and /etc/fstab" in the Absolute OpenBSD book. I am sure we miss many other cases.

Finally, missing src is not the only reason why a mount could fail. E.g., there could have been a wrong filesystem, or the kernel has not been compiled with support for the correct filesystem, or there are incorrect options, or ... Therefore, checking src does not, by itself, fix the bug (#59183) that it was meant to fix. Even if we do check src, we would need to write code that deals with any unexplained mount failures. And once we have this code (see #65869), checking src becomes unnecessary.

In summary, we should not go into the business of predicting whether the mount command will succeed. Just let it fail when it needs to fail and then deal with the consequences.

[patrakov]

Also, why is it P3? It is a fix for a critical regression (cannot mount remote filesystems at all) in the mount module, but the regression is only in unreleased versions. So it should block release, so P2. One of the rationales for this merge request is exactly to revert the regression as soon as possible, because it is far worse than the bug that the commit 72023d7 tried to fix.

[patrakov]

I would prefer the fix would add an aditional check not startswith('UUID=', 'LABEL=') and then keep the path.exists check for those cases

Done (as a new pull request, see #68155), but I would very much prefer this pull request (and ideally #65869) to be merged instead of that. That's also why a new pull request instead of amending this one.

[Akasurde]

First of all, sorry for introducing the wrong fix. I agree with @patrakov, we should not predict anything about the mount and just remove this check once.

[patrakov]

@Akasurde could you please review the proper fix (i.e. the attempt to remove created directories and restore the fstab if the mount fails) in #65869 ?

[Akasurde]

@Akasurde could you please review the proper fix (i.e. the attempt to remove created directories and restore the fstab if the mount fails) in #65869 ?

Code LGTM, I will test and comment on #65869. I would see this PR getting merged first.

[Akasurde]

superseded by ansible-collections/ansible.posix#33

[Akasurde]

@patrakov Thanks for the contribution.