From 84f434cc76b04bc8f91582ca6aa11e471b7e3988 Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Mon, 11 Dec 2023 12:15:40 -0500 Subject: [PATCH] fix extfs parameter and retcode handling --- changelog/51858.fixed.md | 1 + changelog/54426.fixed.md | 1 + changelog/65686.fixed.md | 1 + salt/modules/extfs.py | 55 +++++++++++++++++------- tests/pytests/unit/modules/test_extfs.py | 48 +++++++++++++++++---- 5 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 changelog/51858.fixed.md create mode 100644 changelog/54426.fixed.md create mode 100644 changelog/65686.fixed.md diff --git a/changelog/51858.fixed.md b/changelog/51858.fixed.md new file mode 100644 index 000000000000..72778ff25998 --- /dev/null +++ b/changelog/51858.fixed.md @@ -0,0 +1 @@ +Fix extfs.mkfs missing parameter handling for -C, -d, and -e diff --git a/changelog/54426.fixed.md b/changelog/54426.fixed.md new file mode 100644 index 000000000000..172458ae2589 --- /dev/null +++ b/changelog/54426.fixed.md @@ -0,0 +1 @@ +Fix extfs.tune has 'reserved' documented twice and is missing the 'reserved_percentage' keyword argument diff --git a/changelog/65686.fixed.md b/changelog/65686.fixed.md new file mode 100644 index 000000000000..11dad52ae683 --- /dev/null +++ b/changelog/65686.fixed.md @@ -0,0 +1 @@ +Fix extfs.tune doesn't pass retcode to module.run diff --git a/salt/modules/extfs.py b/salt/modules/extfs.py index 63d2cfeb1e14..2737cbe2b729 100644 --- a/salt/modules/extfs.py +++ b/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] diff --git a/tests/pytests/unit/modules/test_extfs.py b/tests/pytests/unit/modules/test_extfs.py index c4ab8d3bc6b9..4099f21b2ff8 100644 --- a/tests/pytests/unit/modules/test_extfs.py +++ b/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