From 7ad2d6067400f55dc7b70745216fab20620f35fd Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Wed, 29 Jul 2020 16:11:47 +0200 Subject: [PATCH] Fix __mount_device wrapper (#254) Some recent change in Salt is now doing the right thing, and calling the different states with separated args and kwargs. This change trigger a hidden bug in the __mount_device decorator, that expect those parameter to be in kwargs, as is happening during the test. This patch change the way that the wrapper inside the decorator search for the name and device parameters, first looking into kwargs and later in args if possible. A new test is introduced to exercise both cases. Fix #58012 (cherry picked from commit 2089645e2478751dc795127cfd14d0385c2e0899) --- changelog/58012.fixed | 1 + salt/states/btrfs.py | 6 +++--- tests/unit/states/test_btrfs.py | 27 +++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 changelog/58012.fixed diff --git a/changelog/58012.fixed b/changelog/58012.fixed new file mode 100644 index 0000000000..13a1ef747d --- /dev/null +++ b/changelog/58012.fixed @@ -0,0 +1 @@ +Fix btrfs state decorator, that produces exceptions when creating subvolumes. \ No newline at end of file diff --git a/salt/states/btrfs.py b/salt/states/btrfs.py index af78c8ae00..d0d6095c46 100644 --- a/salt/states/btrfs.py +++ b/salt/states/btrfs.py @@ -103,9 +103,9 @@ def __mount_device(action): ''' @functools.wraps(action) def wrapper(*args, **kwargs): - name = kwargs['name'] - device = kwargs['device'] - use_default = kwargs.get('use_default', False) + name = kwargs.get("name", args[0] if args else None) + device = kwargs.get("device", args[1] if len(args) > 1 else None) + use_default = kwargs.get("use_default", False) ret = { 'name': name, diff --git a/tests/unit/states/test_btrfs.py b/tests/unit/states/test_btrfs.py index c68f6279dc..c722630aef 100644 --- a/tests/unit/states/test_btrfs.py +++ b/tests/unit/states/test_btrfs.py @@ -245,6 +245,33 @@ class BtrfsTestCase(TestCase, LoaderModuleMockMixin): mount.assert_called_once() umount.assert_called_once() + @skipIf(salt.utils.platform.is_windows(), "Skip on Windows") + @patch("salt.states.btrfs._umount") + @patch("salt.states.btrfs._mount") + def test_subvolume_created_exists_decorator(self, mount, umount): + """ + Test creating a subvolume using a non-kwargs call + """ + mount.return_value = "/tmp/xxx" + salt_mock = { + "btrfs.subvolume_exists": MagicMock(return_value=True), + } + opts_mock = { + "test": False, + } + with patch.dict(btrfs.__salt__, salt_mock), patch.dict( + btrfs.__opts__, opts_mock + ): + assert btrfs.subvolume_created("@/var", "/dev/sda1") == { + "name": "@/var", + "result": True, + "changes": {}, + "comment": ["Subvolume @/var already present"], + } + salt_mock["btrfs.subvolume_exists"].assert_called_with("/tmp/xxx/@/var") + mount.assert_called_once() + umount.assert_called_once() + @patch('salt.states.btrfs._umount') @patch('salt.states.btrfs._mount') def test_subvolume_created_exists_test(self, mount, umount): -- 2.27.0