From 3967992bfd7061a783613d997bd43c864fb29cfa Mon Sep 17 00:00:00 2001 From: Sergei Zviagintsev Date: Fri, 27 Jan 2017 00:03:55 +0100 Subject: [PATCH 1/3] Fix test for modules.linux_lvm.pvcreate pvdisplay() would be called by pvcreate() twice: firstly to check whether a device is already initialized for use by LVM and then to ensure that the pvcreate executable did its job correctly. The test replaces pvdisplay() with a mock that always returns True and thus pvcreate() would think that a specified device is already initialized and exit. In the other words, instead of testing physical volume initialization the test simulates a case with all the submitted devices already initialized. Fix it by replacing pvdisplay with a mock that returns False on the first call (thus pvcreate thinks that a device is not a PV yet) and True on the second call (after the pvcreate executable is called). --- tests/unit/modules/linux_lvm_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/modules/linux_lvm_test.py b/tests/unit/modules/linux_lvm_test.py index afc2f0deb6..205ad6e9de 100644 --- a/tests/unit/modules/linux_lvm_test.py +++ b/tests/unit/modules/linux_lvm_test.py @@ -152,7 +152,10 @@ class LinuxLVMTestCase(TestCase): self.assertRaises(CommandExecutionError, linux_lvm.pvcreate, 'A') - pvdisplay = MagicMock(return_value=True) + # pvdisplay() would be called by pvcreate() twice: firstly to check + # whether a device is already initialized for use by LVM and then to + # ensure that the pvcreate executable did its job correctly. + pvdisplay = MagicMock(side_effect=[False, True]) with patch('salt.modules.linux_lvm.pvdisplay', pvdisplay): with patch.object(os.path, 'exists', return_value=True): ret = {'stdout': 'saltines', 'stderr': 'cheese', 'retcode': 0, 'pid': '1337'} From 0f84ca24879ecd6a61d43c0c9087708fe25ca33d Mon Sep 17 00:00:00 2001 From: Sergei Zviagintsev Date: Fri, 27 Jan 2017 00:22:29 +0100 Subject: [PATCH 2/3] Add test for modules.linux_lvm.pvcreate on existing LVM PVs If all the devices submitted to pvcreate() are already initialized as LVM physical volumes, pvcreate() should return True and no futher actions should be made. --- tests/unit/modules/linux_lvm_test.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/modules/linux_lvm_test.py b/tests/unit/modules/linux_lvm_test.py index 205ad6e9de..5a17fd464b 100644 --- a/tests/unit/modules/linux_lvm_test.py +++ b/tests/unit/modules/linux_lvm_test.py @@ -163,6 +163,20 @@ class LinuxLVMTestCase(TestCase): with patch.dict(linux_lvm.__salt__, {'cmd.run_all': mock}): self.assertEqual(linux_lvm.pvcreate('A', metadatasize=1000), True) + def test_pvcreate_existing_pvs(self): + ''' + Test a scenario when all the submitted devices are already LVM PVs. + ''' + pvdisplay = MagicMock(return_value=True) + with patch('salt.modules.linux_lvm.pvdisplay', pvdisplay): + with patch.object(os.path, 'exists', return_value=True): + ret = {'stdout': 'saltines', 'stderr': 'cheese', 'retcode': 0, 'pid': '1337'} + cmd_mock = MagicMock(return_value=ret) + with patch.dict(linux_lvm.__salt__, {'cmd.run_all': cmd_mock}): + self.assertEqual(linux_lvm.pvcreate('A', metadatasize=1000), + True) + cmd_mock.assert_not_called() + def test_pvremove(self): ''' Tests for remove a physical device being used as an LVM physical volume From f1e3e86e6a2e9d8e6eb8b037300de6f09ce23371 Mon Sep 17 00:00:00 2001 From: Sergei Zviagintsev Date: Thu, 19 Jan 2017 14:56:05 +0100 Subject: [PATCH 3/3] Fix modules.linux_lvm.pvcreate on existing LVM PVs If all the devices submitted to pvcreate() are already initialized as LVM physical volumes and override is True (which is default), pvcreate() should return True and no futher actions should be made. The 'not cmd[1:]' check which is suited for this scenario is incorrect, as we previously filled the 'cmd' list with two elements and thus the condition would be always False. This would cause pvcreate() to call the pvcreate executable with no arguments if all the submitted devices are already initialized as LVM PVs. Fixes #39070 --- salt/modules/linux_lvm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/linux_lvm.py b/salt/modules/linux_lvm.py index a5004ae42d..09f3f73f20 100644 --- a/salt/modules/linux_lvm.py +++ b/salt/modules/linux_lvm.py @@ -229,7 +229,7 @@ def pvcreate(devices, override=True, **kwargs): elif not override: raise CommandExecutionError('Device "{0}" is already an LVM physical volume.'.format(device)) - if not cmd[1:]: + if not cmd[2:]: # All specified devices are already LVM volumes return True