module filesystem: partially fix idempotency issue #1457 (resizefs) (#1478) (#1720)

* Use 'xfs_info' to query fs size, that doesn't always require the device be
  mounted. Although still query mountpoint first for backward compatibility.
* Do not fail whith fstype=xfs and resizefs=yes if filesystem already fills
  its underlying device.
* Include xfs in the tasks that test idempotency of resizefs option
* Add changelogs/fragments/1478-filesystem-fix-1457-resizefs-idempotency.yml

(cherry picked from commit aa95d8a5b7)

Co-authored-by: quidame <quidame@poivron.org>

changelogs/fragments/1478-filesystem-fix-1457-resizefs-idempotency.yml

@@ -0,0 +1,5 @@
+---
+bugfixes:
+  - filesystem - do not fail when ``resizefs=yes`` and ``fstype=xfs`` if there is nothing to do, even if
+    the filesystem is not mounted. This only covers systems supporting access to unmounted XFS filesystems.
+    Others will still fail (https://github.com/ansible-collections/community.general/issues/1457, https://github.com/ansible-collections/community.general/pull/1478).

plugins/modules/system/filesystem.py

@@ -240,26 +240,35 @@ class XFS(Filesystem):
     GROW = 'xfs_growfs'
 
     def get_fs_size(self, dev):
-        cmd = self.module.get_bin_path('xfs_growfs', required=True)
+        cmd = self.module.get_bin_path('xfs_info', required=True)
+
         mountpoint = dev.get_mountpoint()
+        if mountpoint:
+            rc, out, err = self.module.run_command([cmd, str(mountpoint)], environ_update=self.LANG_ENV)
+        else:
+            # Recent GNU/Linux distros support access to unmounted XFS filesystems
+            rc, out, err = self.module.run_command([cmd, str(dev)], environ_update=self.LANG_ENV)
+        if rc != 0:
+            self.module.fail_json(msg="Error while attempting to query size of XFS filesystem: %s" % err)
 
-        if not mountpoint:
+        for line in out.splitlines():
-            # xfs filesystem needs to be mounted
-            self.module.fail_json(msg="%s needs to be mounted for xfs operations" % dev)
-
-        _, size, _ = self.module.run_command([cmd, '-n', str(mountpoint)], check_rc=True, environ_update=self.LANG_ENV)
-        for line in size.splitlines():
             col = line.split('=')
             if col[0].strip() == 'data':
                 if col[1].strip() != 'bsize':
-                    self.module.fail_json(msg='Unexpected output format from xfs_growfs (could not locate "bsize")')
+                    self.module.fail_json(msg='Unexpected output format from xfs_info (could not locate "bsize")')
                 if col[2].split()[1] != 'blocks':
-                    self.module.fail_json(msg='Unexpected output format from xfs_growfs (could not locate "blocks")')
+                    self.module.fail_json(msg='Unexpected output format from xfs_info (could not locate "blocks")')
                 block_size = int(col[2].split()[0])
                 block_count = int(col[3].split(',')[0])
                 return block_size * block_count
 
     def grow_cmd(self, dev):
+        # Check first if growing is needed, and then if it is doable or not.
+        devsize_in_bytes = dev.size()
+        fssize_in_bytes = self.get_fs_size(dev)
+        if not fssize_in_bytes < devsize_in_bytes:
+            self.module.exit_json(changed=False, msg="%s filesystem is using the whole device %s" % (self.fstype, dev))
+
         mountpoint = dev.get_mountpoint()
         if not mountpoint:
             # xfs filesystem needs to be mounted

tests/integration/targets/filesystem/defaults/main.yml

@@ -1,3 +1,4 @@
+---
 tested_filesystems:
   # key: fstype
   #   fssize: size (Mo)

tests/integration/targets/filesystem/meta/main.yml

@@ -1,3 +1,4 @@
+---
 dependencies:
   - setup_pkg_mgr
   - setup_remote_tmp_dir

tests/integration/targets/filesystem/tasks/create_device.yml

@@ -1,3 +1,4 @@
+---
 - name: 'Create a "disk" file'
   command: 'dd if=/dev/zero of={{ image_file }} bs=1M count={{ fssize }}'
 

tests/integration/targets/filesystem/tasks/create_fs.yml

@@ -43,40 +43,45 @@
       - 'fs3_result is success'
       - 'uuid.stdout != uuid3.stdout'
 
-- name: increase fake device
-  shell: 'dd if=/dev/zero bs=1M count=1 >> {{ image_file }}'
-
-- when: fstype == 'lvm'
-  block:
-    - name: Resize loop device for LVM
-      command: losetup -c {{ dev }}
 
 - when: 'grow|bool and (fstype != "vfat" or resize_vfat)'
   block:
-  - name: Expand filesystem
+    - name: increase fake device
-    filesystem:
+      shell: 'dd if=/dev/zero bs=1M count=1 >> {{ image_file }}'
-      dev: '{{ dev }}'
-      fstype: '{{ fstype }}'
-      resizefs: yes
-    register: fs4_result
 
-  - command: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+    - name: Resize loop device for LVM
-    register: uuid4
+      command: losetup -c {{ dev }}
+      when: fstype == 'lvm'
 
-  - assert:
+    - name: Expand filesystem
-      that:
+      filesystem:
-        - 'fs4_result is changed'
+        dev: '{{ dev }}'
-        - 'fs4_result is success'
+        fstype: '{{ fstype }}'
-        - 'uuid3.stdout == uuid4.stdout' # unchanged
+        resizefs: yes
+      register: fs4_result
 
-  - name: Try to expand filesystem again
+    - command: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
-    filesystem:
+      register: uuid4
-      dev: '{{ dev }}'
-      fstype: '{{ fstype }}'
-      resizefs: yes
-    register: fs5_result
 
-  - assert:
+    - assert:
-      that:
+        that:
-        - 'not (fs5_result is changed)'
+          - 'fs4_result is changed'
-        - 'fs5_result is successful'
+          - 'fs4_result is success'
+          - 'uuid3.stdout == uuid4.stdout' # unchanged
+
+- when:
+    - (grow | bool and (fstype != "vfat" or resize_vfat)) or
+      (fstype == "xfs" and ansible_system == "Linux" and
+      ansible_distribution not in ["CentOS", "Ubuntu"])
+  block:
+    - name: Check that resizefs does nothing if device size is not changed
+      filesystem:
+        dev: '{{ dev }}'
+        fstype: '{{ fstype }}'
+        resizefs: yes
+      register: fs5_result
+
+    - assert:
+        that:
+          - 'fs5_result is not changed'
+          - 'fs5_result is succeeded'

tests/integration/targets/filesystem/tasks/main.yml

@@ -1,3 +1,4 @@
+---
 ####################################################################
 # WARNING: These are designed specifically for Ansible tests       #
 # and should not be used as examples of how to write Ansible roles #

tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml

@@ -1,3 +1,4 @@
+---
 - name: 'Recreate "disk" file'
   command: 'dd if=/dev/zero of={{ image_file }} bs=1M count={{ fssize }}'
 

tests/integration/targets/filesystem/tasks/setup.yml

@@ -1,3 +1,4 @@
+---
 - name: install filesystem tools
   package:
     name: '{{ item }}'