fix extfs parameter and retcode handling

changelog/51858.fixed.md

@@ -0,0 +1 @@
+Fix extfs.mkfs missing parameter handling for -C, -d, and -e

changelog/54426.fixed.md

@@ -0,0 +1 @@
+Fix extfs.tune has 'reserved' documented twice and is missing the 'reserved_percentage' keyword argument

changelog/65686.fixed.md

@@ -0,0 +1 @@
+Fix extfs.tune doesn't pass retcode to module.run

salt/modules/extfs.py

@@ -23,10 +23,15 @@ def __virtual__():
     return True
 
 
-def mkfs(device, fs_type, **kwargs):
+def mkfs(device, fs_type, full_return=False, **kwargs):
     """
     Create a file system on the specified device
 
+    full_return : False
+        If ``True``, the full ``cmd.run_all`` dictionary will be returned
+        instead of just stdout/stderr text. Useful for setting the result of
+        the ``module.run`` state.
+
     CLI Example:
 
     .. code-block:: bash
@@ -61,6 +66,9 @@ def mkfs(device, fs_type, **kwargs):
     * **fs_type**: set the filesystem type (REQUIRED)
     * **usage_type**: how the filesystem is going to be used
     * **uuid**: set the UUID for the file system
+    * **cluster_size**: specify the size of cluster in bytes for file systems using the bigalloc feature
+    * **root_directory**: copy the contents of the given directory into the root directory of the file system
+    * **errors_behavior**: change the behavior of the kernel code when errors are detected
 
     See the ``mke2fs(8)`` manpage for a more complete description of these
     options.
@@ -90,18 +98,22 @@ def mkfs(device, fs_type, **kwargs):
         "super": "S",
         "usage_type": "T",
         "uuid": "U",
+        "cluster_size": "C",
+        "root_directory": "d",
+        "errors_behavior": "e",
     }
 
     opts = ""
     for key in kwargs:
         if key in kwarg_map:
             opt = kwarg_map[key]
-            if kwargs[key] == "True":
-                opts += "-{} ".format(opt)
+            if str(kwargs[key]).lower() == "true":
+                opts += f"-{opt} "
             else:
-                opts += "-{} {} ".format(opt, kwargs[key])
-    cmd = "mke2fs -F -t {} {}{}".format(fs_type, opts, device)
-    out = __salt__["cmd.run"](cmd, python_shell=False).splitlines()
+                opts += f"-{opt} {kwargs[key]} "
+    cmd = f"mke2fs -F -t {fs_type} {opts}{device}"
+    cmd_ret = __salt__["cmd.run_all"](cmd, python_shell=False)
+    out = "\n".join([cmd_ret["stdout"] or "", cmd_ret["stderr"] or ""]).splitlines()
     ret = []
     for line in out:
         if not line:
@@ -119,13 +131,21 @@ def mkfs(device, fs_type, **kwargs):
         elif line.startswith("Writing superblocks"):
             continue
         ret.append(line)
+    if full_return:
+        cmd_ret["comment"] = ret
+        return cmd_ret
     return ret
 
 
-def tune(device, **kwargs):
+def tune(device, full_return=False, **kwargs):
     """
     Set attributes for the specified device (using tune2fs)
 
+    full_return : False
+        If ``True``, the full ``cmd.run_all`` dictionary will be returned
+        instead of just stdout/stderr text. Useful for setting the result of
+        the ``module.run`` state.
+
     CLI Example:
 
     .. code-block:: bash
@@ -144,7 +164,7 @@ def tune(device, **kwargs):
     * **journal**: set to True to create a journal (default on ext3/4)
     * **journal_opts**: options for the fs journal (comma separated)
     * **label**: label to apply to the file system
-    * **reserved**: percentage of blocks reserved for super-user
+    * **reserved_percentage**: percentage of blocks reserved for super-user
     * **last_dir**: last mounted directory
     * **opts**: mount options (comma separated)
     * **feature**: set or clear a feature (comma separated)
@@ -174,6 +194,7 @@ def tune(device, **kwargs):
         "feature": "O",
         "mmp_check": "p",
         "reserved": "r",
+        "reserved_percentage": "m",
         "quota_opts": "Q",
         "time": "T",
         "user": "u",
@@ -183,12 +204,16 @@ def tune(device, **kwargs):
     for key in kwargs:
         if key in kwarg_map:
             opt = kwarg_map[key]
-            if kwargs[key] == "True":
-                opts += "-{} ".format(opt)
+            if str(kwargs[key]).lower() == "true":
+                opts += f"-{opt} "
             else:
-                opts += "-{} {} ".format(opt, kwargs[key])
-    cmd = "tune2fs {}{}".format(opts, device)
-    out = __salt__["cmd.run"](cmd, python_shell=False).splitlines()
+                opts += f"-{opt} {kwargs[key]} "
+    cmd = f"tune2fs {opts}{device}"
+    cmd_ret = __salt__["cmd.run_all"](cmd, python_shell=False)
+    out = "\n".join([cmd_ret["stdout"] or "", cmd_ret["stderr"] or ""]).splitlines()
+    if full_return:
+        cmd_ret["comment"] = out
+        return cmd_ret
     return out
 
 
@@ -230,7 +255,7 @@ def dump(device, args=None):
 
         salt '*' extfs.dump /dev/sda1
     """
-    cmd = "dumpe2fs {}".format(device)
+    cmd = f"dumpe2fs {device}"
     if args:
         cmd = cmd + " -" + args
     ret = {"attributes": {}, "blocks": {}}
@@ -265,7 +290,7 @@ def dump(device, args=None):
                 line = line.replace("]", "")
                 comps = line.split()
                 blkgrp = comps[1]
-                group = "Group {}".format(blkgrp)
+                group = f"Group {blkgrp}"
                 ret["blocks"][group] = {}
                 ret["blocks"][group]["group"] = blkgrp
                 ret["blocks"][group]["range"] = comps[3]

tests/pytests/unit/modules/test_extfs.py

@@ -21,9 +21,22 @@ def test_mkfs():
     """
     Tests if a file system created on the specified device
     """
-    mock = MagicMock()
-    with patch.dict(extfs.__salt__, {"cmd.run": mock}):
-        assert [] == extfs.mkfs("/dev/sda1", "ext4")
+    mock_ret = {
+        "pid": 14247,
+        "retcode": 0,
+        "stdout": "",
+        "stderr": "",
+    }
+    mock = MagicMock(return_value=mock_ret)
+    with patch.dict(extfs.__salt__, {"cmd.run_all": mock}):
+        assert extfs.mkfs("/dev/sda1", "ext4") == []
+        assert extfs.mkfs("/dev/sda1", "ext4", full_return=True) == {
+            "pid": 14247,
+            "retcode": 0,
+            "stdout": "",
+            "stderr": "",
+            "comment": [],
+        }
 
 
 # 'tune' function tests: 1
@@ -33,11 +46,30 @@ def test_tune():
     """
     Tests if specified group was added
     """
-    mock = MagicMock()
-    with patch.dict(extfs.__salt__, {"cmd.run": mock}), patch(
-        "salt.modules.extfs.tune", MagicMock(return_value="")
-    ):
-        assert "" == extfs.tune("/dev/sda1")
+    mock_ret = {
+        "pid": 14247,
+        "retcode": 1,
+        "stdout": "tune2fs 1.44.5 (15-Dec-2018)",
+        "stderr": "tune2fs: No such file or directory while trying to open /dev/donkey\nCouldn't find valid filesystem superblock.",
+    }
+    mock = MagicMock(return_value=mock_ret)
+    with patch.dict(extfs.__salt__, {"cmd.run_all": mock}):
+        assert extfs.tune("/dev/sda1") == [
+            "tune2fs 1.44.5 (15-Dec-2018)",
+            "tune2fs: No such file or directory while trying to open /dev/donkey",
+            "Couldn't find valid filesystem superblock.",
+        ]
+        assert extfs.tune("/dev/sda1", full_return=True) == {
+            "pid": 14247,
+            "retcode": 1,
+            "stdout": "tune2fs 1.44.5 (15-Dec-2018)",
+            "stderr": "tune2fs: No such file or directory while trying to open /dev/donkey\nCouldn't find valid filesystem superblock.",
+            "comment": [
+                "tune2fs 1.44.5 (15-Dec-2018)",
+                "tune2fs: No such file or directory while trying to open /dev/donkey",
+                "Couldn't find valid filesystem superblock.",
+            ],
+        }
 
 
 # 'dump' function tests: 1