mount.py no rmdir when state=='absent'

[YvesMP]

SUMMARY

remove rmdir done after umounting when the requested state is 'absent'

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

mount.py

ADDITIONAL INFORMATION

In function main(), remove rmdir in case : if state == 'absent'.

Unmounting a file system should not lead to delete anything that is revealed after unmounting.
Also, it leads to an error if a non empty directory is present under the ex-mountpoint after umount : [Errno 39] Directory not empty . So umount is successfull but the ansible run is failed. Of course, it is solved on second run.

# prepare for test
mkdir /var/my
mkdir /var/my/subdir
echo "some content" > /var/my/subdir/data

tree /var/my/
/var/my/
└── subdir
    └── data

# playbook used to mount
cat mount_mydata.yml
---
- name: mount my data
  hosts: localhost
  gather_facts: no
  become: yes
  tasks:
  - name: mount my data
    ansible.posix.mount:
      path: /var/my
      fstype: xfs
      state: mounted
      src: /dev/testVG/testLV
      # change with your own device

ansible-playbook mount_mydata.yml

PLAY [mount my data] **************************************************************************************************

TASK [mount my data] **************************************************************************************************
changed: [localhost]

PLAY RECAP ************************************************************************************************************
localhost                  : ok=1    changed=1    unreachable=0    failed=0

tree /var/my/
# gives something different
# may give no data or data depending on device content

# playbook to umount
cat unmount_mydata.yml
---
- name: unmount my data
  hosts: localhost
  gather_facts: no
  become: yes
  tasks:
  - name: unmount my data
    ansible.posix.mount:
      path: /var/my
      state: absent
      src: /dev/testVG/testLV
      # change with your own device
      backup: yes

# before code update, first run KO
ansible-playbook unmount_mydata.yml

PLAY [unmount my date] ************************************************************************************************

TASK [unmount my data] ************************************************************************************************
fatal: [localhost]: FAILED! => {"ansible_facts": {"discovered_interpreter_python": "/usr/libexec/platform-python"}, "changed": false, "msg": "Error rmdir /var/my: [Errno 39] Directory not empty: '/var/my'"}

PLAY RECAP ************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0

# and need a second run to be OK
ansible-playbook unmount_mydata.yml

PLAY [unmount my date] ************************************************************************************************

TASK [unmount my data] ************************************************************************************************
ok: [localhost]

PLAY RECAP ************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0


# after code update one run is enough
ansible-playbook unmount_mydata.yml

PLAY [unmount my date] ************************************************************************************************

TASK [unmount my data] ************************************************************************************************
changed: [localhost]

PLAY RECAP ************************************************************************************************************
localhost                  : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

# back to original content
tree /var/my/
/var/my/
└── subdir
    └── data

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/27e112305a9341c3aa8fd778c99f34dd

✔️ ansible-galaxy-importer SUCCESS in 2m 53s
✔️ build-ansible-collection SUCCESS in 5m 42s

[saito-hideki]

Thank you for the PR! :)
I have left a comment as a change request.

[saito-hideki]

This will change the behavior of state: absent.
But I don't think the rmdir logic should be skipped. Because it breaks backward compatibility.
If you want to skip it, how about adding keep_mountpoint or preserve_mountpoint to keep the mountpoint to avoid Directory not empty error instead of removing this section.

If this is set as true, you can skip rmdir section. Also, if the default value is set as false, to keep backward compatibility, you can set it to false by default.

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/1338a51655ea47bc9354d7e151e7e685

✔️ ansible-galaxy-importer SUCCESS in 4m 41s
✔️ build-ansible-collection SUCCESS in 5m 45s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a7f26a4b68f545b69d5d921b5bd7f2ee

✔️ ansible-galaxy-importer SUCCESS in 2m 49s
✔️ build-ansible-collection SUCCESS in 5m 43s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/50430e9521074740bcb624516bd80fcd

✔️ ansible-galaxy-importer SUCCESS in 3m 29s
✔️ build-ansible-collection SUCCESS in 5m 54s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/b36a90c2c4a14e4d93cc57aab727f95e

✔️ ansible-galaxy-importer SUCCESS in 4m 17s
✔️ build-ansible-collection SUCCESS in 5m 47s

[YvesMP]

@saito-hideki
As you suggested, I added the keep_mountpoint option to enable skipping the rmdir logic with state=changed.

[saito-hideki]

Hi @YvesMP
Thanks for the update.
Your changes worked without any issues in my lab. If you address the following three things, we can merge this PR.

• Please create a changelog fragment file according to the following docs:
  • docs: https://docs.ansible.com/ansible/latest/community/development_process.html#creating-changelog-fragments
  • example: https://github.com/ansible-collections/ansible.posix/blob/main/changelogs/fragments/387_callback_output_header.yml
• Could you append some integration tests for keep_mountpoint?
  • https://github.com/ansible-collections/ansible.posix/tree/main/tests/integration/targets/mount/tasks
• Could you append some examples?
  • https://github.com/ansible-collections/ansible.posix/blob/main/plugins/modules/mount.py#L143-L219

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/a1a0cec916ec4b758b0bc3377d086df3

✔️ ansible-galaxy-importer SUCCESS in 2m 48s
✔️ build-ansible-collection SUCCESS in 5m 50s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/0fe1052b906f48ee921b1a6738a90425

✔️ ansible-galaxy-importer SUCCESS in 3m 10s
✔️ build-ansible-collection SUCCESS in 6m 08s

[softwarefactory-project-zuul]

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/e83e051668cf419d8350e42ddb06546a

✔️ ansible-galaxy-importer SUCCESS in 2m 54s
✔️ build-ansible-collection SUCCESS in 5m 44s