Merge pull request #49327 from twangboy/fix_win_service

Fix issues with win_service
This commit is contained in:
Nicole Thomas 2018-09-05 09:24:54 -04:00 committed by GitHub
commit 34e517410f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 129 additions and 34 deletions

View file

@ -7,9 +7,12 @@ Windows Service module.
# Import python libs
from __future__ import absolute_import
import logging
import re
import salt.utils
import time
import logging
# Import salt libs
from salt.exceptions import CommandExecutionError
# Import 3rd party libs
@ -143,6 +146,30 @@ def _status_wait(service_name, end_time, service_states):
return info_results
def _cmd_quote(cmd):
r'''
Helper function to properly format the path to the binary for the service
Must be wrapped in double quotes to account for paths that have spaces. For
example:
``"C:\Program Files\Path\to\bin.exe"``
Args:
cmd (str): Full path to the binary
Returns:
str: Properly quoted path to the binary
'''
# Remove all single and double quotes from the beginning and the end
pattern = re.compile('^(\\"|\').*|.*(\\"|\')$')
while pattern.match(cmd) is not None:
cmd = cmd.strip('"').strip('\'')
# Ensure the path to the binary is wrapped in double quotes to account for
# spaces in the path
cmd = '"{0}"'.format(cmd)
return cmd
def get_enabled():
'''
Return a list of enabled services. Enabled is defined as a service that is
@ -330,7 +357,7 @@ def info(name):
None, None, win32service.SC_MANAGER_CONNECT)
except pywintypes.error as exc:
raise CommandExecutionError(
'Failed to connect to the SCM: {0}'.format(exc[2]))
'Failed to connect to the SCM: {0}'.format(exc.strerror))
try:
handle_svc = win32service.OpenService(
@ -341,7 +368,7 @@ def info(name):
win32service.SERVICE_QUERY_STATUS)
except pywintypes.error as exc:
raise CommandExecutionError(
'Failed To Open {0}: {1}'.format(name, exc[2]))
'Failed To Open {0}: {1}'.format(name, exc.strerror))
try:
config_info = win32service.QueryServiceConfig(handle_svc)
@ -435,7 +462,8 @@ def start(name, timeout=90):
.. versionadded:: 2017.7.9, 2018.3.4
Returns:
bool: ``True`` if successful, otherwise ``False``
bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
if the service is already started
CLI Example:
@ -443,9 +471,6 @@ def start(name, timeout=90):
salt '*' service.start <service name>
'''
if status(name):
return True
# Set the service to manual if disabled
if disabled(name):
modify(name, start_type='Manual')
@ -453,8 +478,10 @@ def start(name, timeout=90):
try:
win32serviceutil.StartService(name)
except pywintypes.error as exc:
raise CommandExecutionError(
'Failed To Start {0}: {1}'.format(name, exc[2]))
if exc.winerror != 1056:
raise CommandExecutionError(
'Failed To Start {0}: {1}'.format(name, exc.strerror))
log.debug('Service "{0}" is running'.format(name))
srv_status = _status_wait(service_name=name,
end_time=time.time() + int(timeout),
@ -477,7 +504,8 @@ def stop(name, timeout=90):
.. versionadded:: 2017.7.9, 2018.3.4
Returns:
bool: ``True`` if successful, otherwise ``False``
bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
if the service is already stopped
CLI Example:
@ -488,9 +516,10 @@ def stop(name, timeout=90):
try:
win32serviceutil.StopService(name)
except pywintypes.error as exc:
if exc[0] != 1062:
if exc.winerror != 1062:
raise CommandExecutionError(
'Failed To Stop {0}: {1}'.format(name, exc[2]))
'Failed To Stop {0}: {1}'.format(name, exc.strerror))
log.debug('Service "{0}" is not running'.format(name))
srv_status = _status_wait(service_name=name,
end_time=time.time() + int(timeout),
@ -747,7 +776,7 @@ def modify(name,
win32service.SERVICE_QUERY_CONFIG)
except pywintypes.error as exc:
raise CommandExecutionError(
'Failed To Open {0}: {1}'.format(name, exc))
'Failed To Open {0}: {1}'.format(name, exc.strerror))
config_info = win32service.QueryServiceConfig(handle_svc)
@ -755,7 +784,8 @@ def modify(name,
# Input Validation
if bin_path is not None:
bin_path = bin_path.strip('"')
# shlex.quote the path to the binary
bin_path = _cmd_quote(bin_path)
if exe_args is not None:
bin_path = '{0} {1}'.format(bin_path, exe_args)
changes['BinaryPath'] = bin_path
@ -1149,8 +1179,8 @@ def create(name,
if name in get_all():
raise CommandExecutionError('Service Already Exists: {0}'.format(name))
# Input validation
bin_path = bin_path.strip('"')
# shlex.quote the path to the binary
bin_path = _cmd_quote(bin_path)
if exe_args is not None:
bin_path = '{0} {1}'.format(bin_path, exe_args)
@ -1342,7 +1372,8 @@ def delete(name, timeout=90):
.. versionadded:: 2017.7.9, 2018.3.4
Returns:
bool: ``True`` if successful, otherwise ``False``
bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
if the service is not present
CLI Example:
@ -1357,8 +1388,12 @@ def delete(name, timeout=90):
handle_svc = win32service.OpenService(
handle_scm, name, win32service.SERVICE_ALL_ACCESS)
except pywintypes.error as exc:
raise CommandExecutionError(
'Failed to open {0}. {1}'.format(name, exc.strerror))
win32service.CloseServiceHandle(handle_scm)
if exc.winerror != 1060:
raise CommandExecutionError(
'Failed to open {0}. {1}'.format(name, exc.strerror))
log.debug('Service "{0}" is not present'.format(name))
return True
try:
win32service.DeleteService(handle_svc)

View file

@ -5,7 +5,7 @@ from __future__ import absolute_import
# Import Salt Testing libs
from tests.support.case import ModuleCase
from tests.support.helpers import destructiveTest
from tests.support.helpers import destructiveTest, flaky
from tests.support.unit import skipIf
# Import Salt libs
@ -58,6 +58,7 @@ class ServiceModuleTest(ModuleCase):
self.run_function('service.disable', [self.service_name])
del self.service_name
@flaky
def test_service_status_running(self):
'''
test service.status execution module

View file

@ -40,6 +40,8 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
self.service_name = 'com.apple.AirPlayXPCHelper'
self.stopped = ''
self.running = '[0-9]'
elif os_family == 'Windows':
self.service_name = 'Spooler'
self.pre_srv_enabled = True if self.service_name in self.run_function('service.get_enabled') else False
self.post_srv_disable = False
@ -47,7 +49,7 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
self.run_function('service.enable', name=self.service_name)
self.post_srv_disable = True
if salt.utils.which(cmd_name) is None:
if os_family != 'Windows' and salt.utils.which(cmd_name) is None:
self.skipTest('{0} is not installed'.format(cmd_name))
def tearDown(self):

View file

@ -23,6 +23,7 @@ import salt.modules.win_service as win_service
try:
WINAPI = True
import win32serviceutil
import pywintypes
except ImportError:
WINAPI = False
@ -119,18 +120,38 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
'''
mock_true = MagicMock(return_value=True)
mock_false = MagicMock(return_value=False)
mock_info = MagicMock(side_effect=[{'Status': 'Stopped'},
{'Status': 'Start Pending'},
{'Status': 'Running'}])
mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
with patch.object(win_service, 'status', mock_true):
with patch.object(win32serviceutil, 'StartService', mock_true), \
patch.object(win_service, 'disabled', mock_false), \
patch.object(win_service, 'info', mock_info):
self.assertTrue(win_service.start('spongebob'))
with patch.object(win_service, 'status', mock_false):
with patch.object(win32serviceutil, 'StartService', mock_true):
with patch.object(win_service, 'info', mock_info):
with patch.object(win_service, 'status', mock_true):
self.assertTrue(win_service.start('spongebob'))
mock_info = MagicMock(side_effect=[{'Status': 'Stopped', 'Status_WaitHint': 0},
{'Status': 'Start Pending', 'Status_WaitHint': 0},
{'Status': 'Running'}])
with patch.object(win32serviceutil, 'StartService', mock_true), \
patch.object(win_service, 'disabled', mock_false), \
patch.object(win_service, 'info', mock_info), \
patch.object(win_service, 'status', mock_true):
self.assertTrue(win_service.start('spongebob'))
@skipIf(not WINAPI, 'pywintypes not available')
def test_start_already_running(self):
'''
Test starting a service that is already running
'''
mock_false = MagicMock(return_value=False)
mock_error = MagicMock(
side_effect=pywintypes.error(1056,
'StartService',
'Service is running'))
mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
with patch.object(win32serviceutil, 'StartService', mock_error), \
patch.object(win_service, 'disabled', mock_false), \
patch.object(win_service, '_status_wait', mock_info):
self.assertTrue(win_service.start('spongebob'))
@skipIf(not WINAPI, 'win32serviceutil not available')
def test_stop(self):
@ -141,8 +162,7 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
mock_false = MagicMock(return_value=False)
mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
patch.object(win32serviceutil, 'StopService', mock_true), \
with patch.object(win32serviceutil, 'StopService', mock_true), \
patch.object(win_service, '_status_wait', mock_info):
self.assertTrue(win_service.stop('spongebob'))
@ -150,12 +170,25 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
{'Status': 'Stop Pending', 'Status_WaitHint': 0},
{'Status': 'Stopped'}])
with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
patch.object(win32serviceutil, 'StopService', mock_true), \
with patch.object(win32serviceutil, 'StopService', mock_true), \
patch.object(win_service, 'info', mock_info), \
patch.object(win_service, 'status', mock_false):
self.assertTrue(win_service.stop('spongebob'))
@skipIf(not WINAPI, 'pywintypes not available')
def test_stop_not_running(self):
'''
Test stopping a service that is already stopped
'''
mock_error = MagicMock(
side_effect=pywintypes.error(1062,
'StopService',
'Service is not running'))
mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
with patch.object(win32serviceutil, 'StopService', mock_error), \
patch.object(win_service, '_status_wait', mock_info):
self.assertTrue(win_service.stop('spongebob'))
def test_restart(self):
'''
Test to restart the named service
@ -264,3 +297,26 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
with patch.object(win_service, 'enabled', mock):
self.assertTrue(win_service.disabled('spongebob'))
self.assertFalse(win_service.disabled('squarepants'))
def test_cmd_quote(self):
'''
Make sure the command gets quoted correctly
'''
# Should always return command wrapped in double quotes
expected = r'"C:\Program Files\salt\test.exe"'
# test no quotes
bin_path = r'C:\Program Files\salt\test.exe'
self.assertEqual(win_service._cmd_quote(bin_path), expected)
# test single quotes
bin_path = r"'C:\Program Files\salt\test.exe'"
self.assertEqual(win_service._cmd_quote(bin_path), expected)
# test double quoted single quotes
bin_path = '"\'C:\\Program Files\\salt\\test.exe\'"'
self.assertEqual(win_service._cmd_quote(bin_path), expected)
# test single quoted, double quoted, single quotes
bin_path = "\'\"\'C:\\Program Files\\salt\\test.exe\'\"\'"
self.assertEqual(win_service._cmd_quote(bin_path), expected)

View file

@ -55,6 +55,7 @@ integration.states.test_pip_state
integration.states.test_pkg
integration.states.test_renderers
integration.states.test_file
integration.states.test_service
integration.states.test_user
integration.utils.testprogram
integration.wheel.test_client