From 752b6f854687d3a60d611d52debe94cb0e184926 Mon Sep 17 00:00:00 2001 From: cstarke Date: Thu, 20 Sep 2018 11:33:53 +0200 Subject: [PATCH 1/5] Seperate prlctl and prlsrvctl checks into each requiring function --- salt/modules/parallels.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/salt/modules/parallels.py b/salt/modules/parallels.py index 03590475b222..b8c5d24c8b13 100644 --- a/salt/modules/parallels.py +++ b/salt/modules/parallels.py @@ -43,17 +43,6 @@ GUID_REGEX = re.compile(r'{?([0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12})}?', re.I) -def __virtual__(): - ''' - Load this module if prlctl is available - ''' - if not salt.utils.path.which('prlctl'): - return (False, 'prlctl utility not available') - if not salt.utils.path.which('prlsrvctl'): - return (False, 'prlsrvctl utility not available') - return __virtualname__ - - def _normalize_args(args): ''' Return args as a list of strings @@ -111,6 +100,9 @@ def prlsrvctl(sub_cmd, args=None, runas=None): salt '*' parallels.prlsrvctl usb list runas=macdev salt -- '*' parallels.prlsrvctl set '--mem-limit auto' runas=macdev ''' + if not salt.utils.path.which('prlsrvctl'): + raise CommandExecutionError('prlsrvctl utility not available') + # Construct command cmd = ['prlsrvctl', sub_cmd] if args: @@ -141,6 +133,9 @@ def prlctl(sub_cmd, args=None, runas=None): salt '*' parallels.prlctl exec 'macvm uname' runas=macdev salt -- '*' parallels.prlctl capture 'macvm --file macvm.display.png' runas=macdev ''' + if not salt.utils.path.which('prlctl'): + raise CommandExecutionError('prlctl utility not available') + # Construct command cmd = ['prlctl', sub_cmd] if args: From 9034c4e7cfcc3650fb34888965e73161fcc3bdc9 Mon Sep 17 00:00:00 2001 From: cstarke Date: Thu, 20 Sep 2018 12:45:28 +0200 Subject: [PATCH 2/5] Add import for CommandExecutionError --- salt/modules/parallels.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/parallels.py b/salt/modules/parallels.py index b8c5d24c8b13..6e87b24dc615 100644 --- a/salt/modules/parallels.py +++ b/salt/modules/parallels.py @@ -29,7 +29,7 @@ import salt.utils.locales import salt.utils.path import salt.utils.yaml -from salt.exceptions import SaltInvocationError +from salt.exceptions import SaltInvocationError, CommandExecutionError # Import 3rd party libs from salt.ext import six From 3c96dd2b5e2f3629970b55487be85e9251f3f30e Mon Sep 17 00:00:00 2001 From: cstarke Date: Thu, 20 Sep 2018 16:46:22 +0200 Subject: [PATCH 3/5] Fix tests for parallels module --- tests/unit/modules/test_parallels.py | 51 +++++++++++++--------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/tests/unit/modules/test_parallels.py b/tests/unit/modules/test_parallels.py index 302a7e893015..717c1f496f48 100644 --- a/tests/unit/modules/test_parallels.py +++ b/tests/unit/modules/test_parallels.py @@ -25,27 +25,6 @@ class ParallelsTestCase(TestCase, LoaderModuleMockMixin): def setup_loader_modules(self): return {parallels: {}} - def test___virtual__(self): - ''' - Test parallels.__virtual__ - ''' - mock_true = MagicMock(return_value=True) - mock_false = MagicMock(return_value=False) - - # Validate false return - with patch('salt.utils.path.which', mock_false): - ret = parallels.__virtual__() - self.assertTrue(isinstance(ret, tuple)) - self.assertEqual(len(ret), 2) - self.assertFalse(ret[0]) - self.assertTrue(isinstance(ret[1], six.string_types)) - - # Validate true return - with patch('salt.utils.path.which', mock_true): - ret = parallels.__virtual__() - self.assertTrue(ret) - self.assertEqual(ret, 'parallels') - def test__normalize_args(self): ''' Test parallels._normalize_args @@ -97,21 +76,30 @@ def test_prlsrvctl(self): # Validate 'prlsrvctl info' info_cmd = ['prlsrvctl', 'info'] info_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': info_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': info_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlsrvctl('info', runas=runas) info_fcn.assert_called_once_with(info_cmd, runas=runas) # Validate 'prlsrvctl usb list' usb_cmd = ['prlsrvctl', 'usb', 'list'] usb_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': usb_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': usb_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlsrvctl('usb', 'list', runas=runas) usb_fcn.assert_called_once_with(usb_cmd, runas=runas) # Validate 'prlsrvctl set "--mem-limit auto"' set_cmd = ['prlsrvctl', 'set', '--mem-limit', 'auto'] set_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': set_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': set_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlsrvctl('set', '--mem-limit auto', runas=runas) set_fcn.assert_called_once_with(set_cmd, runas=runas) @@ -124,21 +112,30 @@ def test_prlctl(self): # Validate 'prlctl user list' user_cmd = ['prlctl', 'user', 'list'] user_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': user_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': user_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlctl('user', 'list', runas=runas) user_fcn.assert_called_once_with(user_cmd, runas=runas) # Validate 'prlctl exec "macvm uname"' exec_cmd = ['prlctl', 'exec', 'macvm', 'uname'] exec_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': exec_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': exec_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlctl('exec', 'macvm uname', runas=runas) exec_fcn.assert_called_once_with(exec_cmd, runas=runas) # Validate 'prlctl capture "macvm --file macvm.display.png"' cap_cmd = ['prlctl', 'capture', 'macvm', '--file', 'macvm.display.png'] cap_fcn = MagicMock() - with patch.dict(parallels.__salt__, {'cmd.run': cap_fcn}): + with patch.dict( + parallels.__salt__, + {'cmd.run': cap_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} + ): parallels.prlctl('capture', 'macvm --file macvm.display.png', runas=runas) cap_fcn.assert_called_once_with(cap_cmd, runas=runas) From 4f8476b2ad9bf5909cce2f817b8a1b35ed9f05fb Mon Sep 17 00:00:00 2001 From: cstarke Date: Fri, 21 Sep 2018 11:39:43 +0200 Subject: [PATCH 4/5] Fix tests; add test for CommandExecutionError --- tests/unit/modules/test_parallels.py | 116 +++++++++++++-------------- 1 file changed, 57 insertions(+), 59 deletions(-) diff --git a/tests/unit/modules/test_parallels.py b/tests/unit/modules/test_parallels.py index 717c1f496f48..953092316ab8 100644 --- a/tests/unit/modules/test_parallels.py +++ b/tests/unit/modules/test_parallels.py @@ -6,7 +6,7 @@ # Import Salt Libs import salt.modules.parallels as parallels -from salt.exceptions import SaltInvocationError +from salt.exceptions import SaltInvocationError, CommandExecutionError # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin @@ -73,35 +73,34 @@ def test_prlsrvctl(self): ''' runas = 'macdev' - # Validate 'prlsrvctl info' - info_cmd = ['prlsrvctl', 'info'] - info_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': info_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlsrvctl('info', runas=runas) - info_fcn.assert_called_once_with(info_cmd, runas=runas) - - # Validate 'prlsrvctl usb list' - usb_cmd = ['prlsrvctl', 'usb', 'list'] - usb_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': usb_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlsrvctl('usb', 'list', runas=runas) - usb_fcn.assert_called_once_with(usb_cmd, runas=runas) - - # Validate 'prlsrvctl set "--mem-limit auto"' - set_cmd = ['prlsrvctl', 'set', '--mem-limit', 'auto'] - set_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': set_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlsrvctl('set', '--mem-limit auto', runas=runas) - set_fcn.assert_called_once_with(set_cmd, runas=runas) + # Test missing prlsrvctl binary + with patch('salt.utils.path.which', MagicMock(return_value=False)): + with self.assertRaises(CommandExecutionError): + parallels.prlsrvctl('info', runas=runas) + + # Simulate the existence of prlsrvctl + with patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/prlsrvctl")): + + # Validate 'prlsrvctl info' + info_cmd = ['prlsrvctl', 'info'] + info_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': info_fcn}): + parallels.prlsrvctl('info', runas=runas) + info_fcn.assert_called_once_with(info_cmd, runas=runas) + + # Validate 'prlsrvctl usb list' + usb_cmd = ['prlsrvctl', 'usb', 'list'] + usb_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': usb_fcn}): + parallels.prlsrvctl('usb', 'list', runas=runas) + usb_fcn.assert_called_once_with(usb_cmd, runas=runas) + + # Validate 'prlsrvctl set "--mem-limit auto"' + set_cmd = ['prlsrvctl', 'set', '--mem-limit', 'auto'] + set_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': set_fcn}): + parallels.prlsrvctl('set', '--mem-limit auto', runas=runas) + set_fcn.assert_called_once_with(set_cmd, runas=runas) def test_prlctl(self): ''' @@ -109,35 +108,34 @@ def test_prlctl(self): ''' runas = 'macdev' - # Validate 'prlctl user list' - user_cmd = ['prlctl', 'user', 'list'] - user_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': user_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlctl('user', 'list', runas=runas) - user_fcn.assert_called_once_with(user_cmd, runas=runas) - - # Validate 'prlctl exec "macvm uname"' - exec_cmd = ['prlctl', 'exec', 'macvm', 'uname'] - exec_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': exec_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlctl('exec', 'macvm uname', runas=runas) - exec_fcn.assert_called_once_with(exec_cmd, runas=runas) - - # Validate 'prlctl capture "macvm --file macvm.display.png"' - cap_cmd = ['prlctl', 'capture', 'macvm', '--file', 'macvm.display.png'] - cap_fcn = MagicMock() - with patch.dict( - parallels.__salt__, - {'cmd.run': cap_fcn, 'salt.utils.path.which': MagicMock(return_value=True)} - ): - parallels.prlctl('capture', 'macvm --file macvm.display.png', runas=runas) - cap_fcn.assert_called_once_with(cap_cmd, runas=runas) + # Test missing prlctl binary + with patch('salt.utils.path.which', MagicMock(return_value=False)): + with self.assertRaises(CommandExecutionError): + parallels.prlctl('info', runas=runas) + + # Simulate the existence of prlctl + with patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/prlctl")): + + # Validate 'prlctl user list' + user_cmd = ['prlctl', 'user', 'list'] + user_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': user_fcn}): + parallels.prlctl('user', 'list', runas=runas) + user_fcn.assert_called_once_with(user_cmd, runas=runas) + + # Validate 'prlctl exec "macvm uname"' + exec_cmd = ['prlctl', 'exec', 'macvm', 'uname'] + exec_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': exec_fcn}): + parallels.prlctl('exec', 'macvm uname', runas=runas) + exec_fcn.assert_called_once_with(exec_cmd, runas=runas) + + # Validate 'prlctl capture "macvm --file macvm.display.png"' + cap_cmd = ['prlctl', 'capture', 'macvm', '--file', 'macvm.display.png'] + cap_fcn = MagicMock() + with patch.dict(parallels.__salt__, {'cmd.run': cap_fcn}): + parallels.prlctl('capture', 'macvm --file macvm.display.png', runas=runas) + cap_fcn.assert_called_once_with(cap_cmd, runas=runas) def test_list_vms(self): ''' From b0be6aa4226d2c011625c8b25abd35eb4a700383 Mon Sep 17 00:00:00 2001 From: cstarke Date: Fri, 21 Sep 2018 11:41:02 +0200 Subject: [PATCH 5/5] Add documentation for prlctl and prlsrvctl binary requirement --- salt/modules/parallels.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/salt/modules/parallels.py b/salt/modules/parallels.py index 6e87b24dc615..ce2589032884 100644 --- a/salt/modules/parallels.py +++ b/salt/modules/parallels.py @@ -6,6 +6,9 @@ see the `Parallels Desktop Reference Guide `_. +This module requires the prlctl binary to be installed to run most functions. +To run parallels.prlsrvctl, the prlsrvctl binary is required. + What has not been implemented yet can be accessed through ``parallels.prlctl`` and ``parallels.prlsrvctl`` (note the preceding double dash ``--`` as necessary):