diff --git a/salt/modules/win_servermanager.py b/salt/modules/win_servermanager.py index f909d687ea..b201b34bf9 100644 --- a/salt/modules/win_servermanager.py +++ b/salt/modules/win_servermanager.py @@ -4,6 +4,7 @@ Manage Windows features via the ServerManager powershell module ''' from __future__ import absolute_import import logging +import json # Import python libs try: @@ -24,13 +25,13 @@ def __virtual__(): Load only on windows with servermanager module ''' if not salt.utils.is_windows(): - return (False, 'Failed to load win_servermanager module:\n' - 'Only available on Windows systems.') + return False, 'Failed to load win_servermanager module:\n' \ + 'Only available on Windows systems.' if not _check_server_manager(): - return (False, 'Failed to load win_servermanager module:\n' - 'ServerManager module not available.\n' - 'May need to install Remote Server Administration Tools.') + return False, 'Failed to load win_servermanager module:\n' \ + 'ServerManager module not available.\n' \ + 'May need to install Remote Server Administration Tools.' return __virtualname__ @@ -46,31 +47,19 @@ def _check_server_manager(): python_shell=True) -def _pshell(func): +def _pshell_json(cmd, cwd=None): ''' - Execute a powershell command and return the STDOUT + Execute the desired powershell command and ensure that it returns data + in json format and load that into python ''' - return __salt__['cmd.run']('{0}'.format(func), - shell='powershell', - python_shell=True) - - -def _parse_powershell_list(lst): - ''' - Parse command output when piped to format-list - Need to look at splitting with ':' so you can get the full value - Need to check for error codes and return false if it's trying to parse - ''' - ret = {} - for line in lst.splitlines(): - if line: - splt = line.split() - # Ensure it's not a malformed line, e.g.: - # FeatureResult : {foo, bar, - # baz} - if len(splt) > 2: - ret[splt[0]] = splt[2] - ret['message'] = lst + if 'convertto-json' not in cmd.lower(): + cmd = ' '.join([cmd, '| ConvertTo-Json']) + log.debug('PowerShell: {0}'.format(cmd)) + ret = __salt__['cmd.shell'](cmd, shell='powershell', cwd=cwd) + try: + ret = json.loads(ret, strict=False) + except ValueError: + log.debug('Json not returned') return ret @@ -87,8 +76,9 @@ def list_available(): salt '*' win_servermanager.list_available ''' - return _pshell('Get-WindowsFeature -erroraction silentlycontinue ' - '-warningaction silentlycontinue') + cmd = 'Get-WindowsFeature -erroraction silentlycontinue ' \ + '-warningaction silentlycontinue' + return __salt__['cmd.shell'](cmd, shell='powershell') def list_installed(): @@ -105,22 +95,16 @@ def list_installed(): salt '*' win_servermanager.list_installed ''' + cmd = 'Get-WindowsFeature -erroraction silentlycontinue ' \ + '-warningaction silentlycontinue | ' \ + 'Select DisplayName,Name,Installed' + features = _pshell_json(cmd) + ret = {} - names = _pshell('Get-WindowsFeature -erroraction silentlycontinue ' - '-warningaction silentlycontinue | Select DisplayName,Name') - for line in names.splitlines()[2:]: - splt = line.split() - name = splt.pop(-1) - display_name = ' '.join(splt) - ret[name] = display_name - state = _pshell('Get-WindowsFeature -erroraction silentlycontinue ' - '-warningaction silentlycontinue | Select Installed,Name') - for line in state.splitlines()[2:]: - splt = line.split() - if splt[0] == 'False' and splt[1] in ret: - del ret[splt[1]] - if '----' in splt[0]: - del ret[splt[1]] + for entry in features: + if entry['Installed']: + ret[entry['Name']] = entry['DisplayName'] + return ret @@ -153,11 +137,18 @@ def install(feature, recurse=False): sub = '' if recurse: sub = '-IncludeAllSubFeature' - out = _pshell('Add-WindowsFeature -Name {0} {1} ' - '-erroraction silentlycontinue ' - '-warningaction silentlycontinue ' - '| format-list'.format(_cmd_quote(feature), sub)) - return _parse_powershell_list(out) + + cmd = 'Add-WindowsFeature -Name {0} {1} ' \ + '-ErrorAction SilentlyContinue ' \ + '-WarningAction SilentlyContinue'.format(_cmd_quote(feature), sub) + out = _pshell_json(cmd) + + ret = {'ExitCode': out['ExitCode'], + 'DisplayName': out['FeatureResult'][0]['DisplayName'], + 'RestartNeeded': out['FeatureResult'][0]['RestartNeeded'], + 'Success': out['Success']} + + return ret def remove(feature): @@ -182,8 +173,14 @@ def remove(feature): salt -t 600 '*' win_servermanager.remove Telnet-Client ''' - out = _pshell('Remove-WindowsFeature -Name {0} ' - '-erroraction silentlycontinue ' - '-warningaction silentlycontinue ' - '| format-list'.format(_cmd_quote(feature))) - return _parse_powershell_list(out) + cmd = 'Remove-WindowsFeature -Name {0} ' \ + '-ErrorAction SilentlyContinue ' \ + '-WarningAction SilentlyContinue'.format(_cmd_quote(feature)) + out = _pshell_json(cmd) + + ret = {'ExitCode': out['ExitCode'], + 'DisplayName': out['FeatureResult'][0]['DisplayName'], + 'RestartNeeded': out['FeatureResult'][0]['RestartNeeded'], + 'Success': out['Success']} + + return ret diff --git a/salt/states/win_servermanager.py b/salt/states/win_servermanager.py index 0e5cd08efa..1e66354253 100644 --- a/salt/states/win_servermanager.py +++ b/salt/states/win_servermanager.py @@ -65,14 +65,14 @@ def installed(name, recurse=False, force=False): ret['changes'] = {'feature': __salt__['win_servermanager.install'](name, recurse)} if 'Success' in ret['changes']['feature']: - ret['result'] = ret['changes']['feature']['Success'] == 'True' + ret['result'] = ret['changes']['feature']['Success'] if not ret['result']: ret['comment'] = 'Failed to install {0}: {1}'.format(name, ret['changes']['feature']['ExitCode']) else: ret['comment'] = 'Installed {0}'.format(name) else: ret['result'] = False - ret['comment'] = 'Failed to install {0}.\nError Message:\n{1}'.format(name, ret['changes']['feature']['message']) + ret['comment'] = 'Failed to install {0}.\nError Message:\n{1}'.format(name, ret['changes']['feature']) ret['changes'] = {} return ret @@ -119,7 +119,7 @@ def removed(name): # Remove the features ret['changes'] = {'feature': __salt__['win_servermanager.remove'](name)} - ret['result'] = ret['changes']['feature']['Success'] == 'True' + ret['result'] = ret['changes']['feature']['Success'] if not ret['result']: ret['comment'] = 'Failed to uninstall the feature {0}'.format(ret['changes']['feature']['ExitCode']) diff --git a/tests/unit/modules/win_servermanager_test.py b/tests/unit/modules/win_servermanager_test.py index 465befcbb1..1cfb22122c 100644 --- a/tests/unit/modules/win_servermanager_test.py +++ b/tests/unit/modules/win_servermanager_test.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- ''' - :codeauthor: :email:`Jayesh Kariya ` +:codeauthor: :email:`Jayesh Kariya ` ''' # Import Python Libs @@ -8,13 +8,7 @@ from __future__ import absolute_import # Import Salt Testing Libs from salttesting import TestCase, skipIf -from salttesting.mock import ( - MagicMock, - patch, - NO_MOCK, - NO_MOCK_REASON -) - +from salttesting.mock import MagicMock, patch, NO_MOCK, NO_MOCK_REASON from salttesting.helpers import ensure_in_syspath ensure_in_syspath('../../') @@ -31,52 +25,61 @@ class WinServermanagerTestCase(TestCase): ''' Test cases for salt.modules.win_servermanager ''' - @staticmethod - def _m_run(): - ''' - Mock return value for cmd.run - ''' - return MagicMock(return_value='') - - # 'list_available' function tests: 1 - def test_list_available(self): ''' - Test if it list available features to install. + Test win_servermanager.list_available ''' - with patch.dict(win_servermanager.__salt__, {'cmd.run': self._m_run()}): + mock = MagicMock(return_value='') + with patch.dict(win_servermanager.__salt__, {'cmd.shell': mock}): self.assertEqual(win_servermanager.list_available(), '') - # 'list_installed' function tests: 1 - def test_list_installed(self): ''' - Test if it list installed features. + Test win_servermanager.list_installed ''' - with patch.dict(win_servermanager.__salt__, {'cmd.run': self._m_run()}): - self.assertDictEqual(win_servermanager.list_installed(), {}) - - # 'install' function tests: 1 + mock = MagicMock(return_value=[{'Installed': True, + 'Name': 'Spongebob', + 'DisplayName': 'Square Pants'}, + {'Installed': False, + 'Name': 'Patrick', + 'DisplayName': 'Plankton'}]) + with patch.object(win_servermanager, '_pshell_json', mock): + expected = {'Spongebob': 'Square Pants'} + self.assertDictEqual(win_servermanager.list_installed(), expected) def test_install(self): ''' - Test if it install a feature. + Test win_servermanager.install ''' - with patch.dict(win_servermanager.__salt__, {'cmd.run': self._m_run()}): - self.assertDictEqual(win_servermanager.install('Telnet-Client'), { - 'message': '' - }) - - # 'remove' function tests: 1 + mock = MagicMock(return_value={'ExitCode': 0, + 'Success': True, + 'FeatureResult': + [{'DisplayName': 'Spongebob', + 'RestartNeeded': False}]}) + with patch.object(win_servermanager, '_pshell_json', mock): + expected = {'ExitCode': 0, + 'DisplayName': 'Spongebob', + 'RestartNeeded': False, + 'Success': True} + self.assertDictEqual( + win_servermanager.install('Telnet-Client'), expected) def test_remove(self): ''' - Test if it remove an installed feature. + Test win_servermanager.remove ''' - with patch.dict(win_servermanager.__salt__, {'cmd.run': self._m_run()}): - self.assertDictEqual(win_servermanager.remove('Telnet-Client'), { - 'message': '' - }) + mock = MagicMock(return_value={'ExitCode': 0, + 'Success': True, + 'FeatureResult': + [{'DisplayName': 'Spongebob', + 'RestartNeeded': False}]}) + with patch.object(win_servermanager, '_pshell_json', mock): + expected = {'ExitCode': 0, + 'DisplayName': 'Spongebob', + 'RestartNeeded': False, + 'Success': True} + self.assertDictEqual( + win_servermanager.remove('Telnet-Client'), expected) if __name__ == '__main__': diff --git a/tests/unit/states/win_servermanager_test.py b/tests/unit/states/win_servermanager_test.py index 4c783bc640..68e7f071f6 100644 --- a/tests/unit/states/win_servermanager_test.py +++ b/tests/unit/states/win_servermanager_test.py @@ -40,7 +40,7 @@ class WinServermanagerTestCase(TestCase): 'result': True, 'comment': ''} mock = MagicMock(side_effect=['salt', 'stack', 'stack']) - mock1 = MagicMock(return_value={'Success': 'True'}) + mock1 = MagicMock(return_value={'Success': True}) with patch.dict(win_servermanager.__salt__, {"win_servermanager.list_installed": mock, "win_servermanager.install": mock1}): @@ -55,7 +55,7 @@ class WinServermanagerTestCase(TestCase): self.assertDictEqual(win_servermanager.installed('salt'), ret) with patch.dict(win_servermanager.__opts__, {"test": False}): - ret.update({'changes': {'feature': {'Success': 'True'}}, + ret.update({'changes': {'feature': {'Success': True}}, 'result': True, 'comment': 'Installed salt'}) self.assertDictEqual(win_servermanager.installed('salt'), ret) @@ -69,7 +69,7 @@ class WinServermanagerTestCase(TestCase): 'result': True, 'comment': ''} mock = MagicMock(side_effect=['stack', 'salt', 'salt']) - mock1 = MagicMock(return_value={'Success': 'True'}) + mock1 = MagicMock(return_value={'Success': True}) with patch.dict(win_servermanager.__salt__, {"win_servermanager.list_installed": mock, "win_servermanager.remove": mock1}): @@ -83,7 +83,7 @@ class WinServermanagerTestCase(TestCase): self.assertDictEqual(win_servermanager.removed('salt'), ret) with patch.dict(win_servermanager.__opts__, {"test": False}): - ret.update({'changes': {'feature': {'Success': 'True'}}, + ret.update({'changes': {'feature': {'Success': True}}, 'result': True}) self.assertDictEqual(win_servermanager.removed('salt'), ret)