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

Mount fixes #10

Closed
wants to merge 3 commits into from
Closed

Mount fixes #10

wants to merge 3 commits into from

Conversation

patrakov
Copy link
Contributor

SUMMARY

This is a resubmission of ansible/ansible#65869, which became necessary after splitting off the ansible.posix collection. Please apply ansible/ansible#68223 first.

The history is:

Therefore, this pull request reverts the bad fix and implements a hopefully better one, by letting the doomed mount fail and then dealing with the consequences (undoing the fstab line and directory creation).

In ansible/ansible#68102 @Akasurde confirmed that the bad fix needs to be reverted. In ansible/ansible#65869 there was a promise to test the new code, but so far it didn't materialize, and cannot materialize without ansible/ansible#68223.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mount

ADDITIONAL INFORMATION

See more discussion here: ansible/ansible#68155, ansible/ansible#68102, ansible/ansible#65544, ansible/ansible#65869.

Test cases:

# Should succeed assuming that /dev/sdb1 indeed contains xfs
ansible -i hosts myserver --become -m mount -a 'src=/dev/sdb1 path=/mnt/local state=mounted fstype=xfs'

# Should also work, was broken before the change
# "Unable to mount 192.168.0.1:/mnt/bigdisk as it does not exist"
ansible -i hosts myserver --become -m mount -a 'src=192.168.0.1:/mnt/bigdisk path=/mnt/big state=mounted fstype=nfs'

# Should fail without changing /etc/fstab and without leaving /mnt/bad
ansible -i hosts myserver --become -m mount -a 'src=/dev/sdb1 path=/mnt/bad state=mounted fstype=ext4'

…ansible#61752)"

This reverts part of ansible commit 72023d7.

The immediate reason is that it breaks mounts where src is not a path.
Examples of such mounts are network-based filesystems such as nfs, cifs,
glusterfs, ceph, virtual filesystems such as tmpfs or overlayfs, and
also UUID-based mounts. It is too hard to come with an exhaustive list,
especially if we take non-Linux systems into account, so don't even try.

Additionally, it did not really fix the issue (ansible/ansible#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.

Fixes: ansible/ansible#65855
Fixes: ansible/ansible#67588
Fixes: ansible/ansible#67966
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

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

@patrakov Thank you for this PR.
At the moment the integration tests for mount aren't working, so we will hold off merging this PR.
Just leaving a negative review to avoid it being accidentally merged

@Akasurde
Copy link
Member

@patrakov Thanks for the contribution. I merged these changes with additional test via #33. Closing this. Thanks.

@Akasurde Akasurde closed this Jun 10, 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.

3 participants