From 89537615f2eb6b62e55220ebf9f6ff6b243e9f57 Mon Sep 17 00:00:00 2001 From: Claudiu Popa Date: Mon, 16 Feb 2015 13:38:44 +0200 Subject: [PATCH] Move Windows specific bits for NTP plugin to windows section There were some specific parts in common/ntpclient.py, such as setting the trigger mode for the service and starting the service. This is abstracted away now, by using a template method which is provided by the Windows plugin. Change-Id: I830521ab420585e9e92d9712204ccb7a94a84b89 --- cloudbaseinit/plugins/common/factory.py | 2 +- cloudbaseinit/plugins/common/ntpclient.py | 50 +-------- cloudbaseinit/plugins/windows/ntpclient.py | 67 +++++++++++ .../tests/plugins/common/test_ntpclient.py | 79 +------------ .../tests/plugins/windows/test_ntpclient.py | 104 ++++++++++++++++++ 5 files changed, 181 insertions(+), 121 deletions(-) create mode 100644 cloudbaseinit/plugins/windows/ntpclient.py create mode 100644 cloudbaseinit/tests/plugins/windows/test_ntpclient.py diff --git a/cloudbaseinit/plugins/common/factory.py b/cloudbaseinit/plugins/common/factory.py index fbaa532a..4d0840c5 100644 --- a/cloudbaseinit/plugins/common/factory.py +++ b/cloudbaseinit/plugins/common/factory.py @@ -21,7 +21,7 @@ opts = [ 'plugins', default=[ 'cloudbaseinit.plugins.common.mtu.MTUPlugin', - 'cloudbaseinit.plugins.common.ntpclient.NTPClientPlugin', + 'cloudbaseinit.plugins.windows.ntpclient.NTPClientPlugin', 'cloudbaseinit.plugins.common.sethostname.SetHostNamePlugin', 'cloudbaseinit.plugins.common.createuser.CreateUserPlugin', 'cloudbaseinit.plugins.common.networkconfig.NetworkConfigPlugin', diff --git a/cloudbaseinit/plugins/common/ntpclient.py b/cloudbaseinit/plugins/common/ntpclient.py index f750e8d2..1ae80547 100644 --- a/cloudbaseinit/plugins/common/ntpclient.py +++ b/cloudbaseinit/plugins/common/ntpclient.py @@ -13,11 +13,9 @@ # under the License. import socket -import time from oslo.config import cfg -from cloudbaseinit import exception from cloudbaseinit.openstack.common import log as logging from cloudbaseinit.osutils import factory as osutils_factory from cloudbaseinit.plugins.common import base @@ -34,24 +32,15 @@ CONF.register_opts(opts) LOG = logging.getLogger(__name__) -_W32TIME_SERVICE = "w32time" - class NTPClientPlugin(base.BasePlugin): - @staticmethod - def _set_ntp_trigger_mode(osutils): - """Set the trigger mode for w32time service to network availability. + def verify_time_service(self, osutils): + """Verify that the time service is up. - This function changes the triggers for the w32time service, so that - the service will always work when there's networking, but will - stop itself whenever this condition stops being true. - It also changes the current triggers of the service (domain joined - for instance). + Implementing this method is optional, it is + mostly used by the Windows version of this plugin. """ - args = ["sc.exe", "triggerinfo", _W32TIME_SERVICE, - "start/networkon", "stop/networkoff"] - return osutils.execute_system32_process(args) @staticmethod def _unpack_ntp_hosts(ntp_option_data): @@ -59,33 +48,6 @@ class NTPClientPlugin(base.BasePlugin): for index in range(0, len(ntp_option_data), 4)] return list(map(socket.inet_ntoa, chunks)) - def _check_w32time_svc_status(self, osutils): - - svc_start_mode = osutils.get_service_start_mode( - _W32TIME_SERVICE) - - if svc_start_mode != osutils.SERVICE_START_MODE_AUTOMATIC: - osutils.set_service_start_mode( - _W32TIME_SERVICE, - osutils.SERVICE_START_MODE_AUTOMATIC) - - if osutils.check_os_version(6, 0): - self._set_ntp_trigger_mode(osutils) - - svc_status = osutils.get_service_status(_W32TIME_SERVICE) - if svc_status == osutils.SERVICE_STATUS_STOPPED: - osutils.start_service(_W32TIME_SERVICE) - - i = 0 - max_retries = 30 - while svc_status != osutils.SERVICE_STATUS_RUNNING: - if i >= max_retries: - raise exception.CloudbaseInitException( - 'Service %s did not start' % _W32TIME_SERVICE) - time.sleep(1) - svc_status = osutils.get_service_status(_W32TIME_SERVICE) - i += 1 - def execute(self, service, shared_data): if CONF.ntp_use_dhcp_config: osutils = osutils_factory.get_os_utils() @@ -93,7 +55,7 @@ class NTPClientPlugin(base.BasePlugin): ntp_option_data = None - for (mac_address, dhcp_host) in dhcp_hosts: + for (_, dhcp_host) in dhcp_hosts: options_data = dhcp.get_dhcp_options(dhcp_host, [dhcp.OPTION_NTP_SERVERS]) if options_data: @@ -107,7 +69,7 @@ class NTPClientPlugin(base.BasePlugin): ntp_hosts = self._unpack_ntp_hosts(ntp_option_data) - self._check_w32time_svc_status(osutils) + self.verify_time_service(osutils) osutils.set_ntp_client_config(ntp_hosts) LOG.info('NTP client configured. Server(s): %s' % ntp_hosts) diff --git a/cloudbaseinit/plugins/windows/ntpclient.py b/cloudbaseinit/plugins/windows/ntpclient.py new file mode 100644 index 00000000..14f0c01e --- /dev/null +++ b/cloudbaseinit/plugins/windows/ntpclient.py @@ -0,0 +1,67 @@ +# Copyright 2014 Cloudbase Solutions Srl +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import time + +from cloudbaseinit import exception +from cloudbaseinit.plugins.common import ntpclient + + +_W32TIME_SERVICE = "w32time" + + +class NTPClientPlugin(ntpclient.NTPClientPlugin): + + @staticmethod + def _set_ntp_trigger_mode(osutils): + """Set the trigger mode for w32time service to network availability. + + This function changes the triggers for the w32time service, so that + the service will always work when there's networking, but will + stop itself whenever this condition stops being true. + It also changes the current triggers of the service (domain joined + for instance). + """ + args = ["sc.exe", "triggerinfo", _W32TIME_SERVICE, + "start/networkon", "stop/networkoff"] + return osutils.execute_system32_process(args) + + def verify_time_service(self, osutils): + """Verify that the time service is up and try to start it.""" + + svc_start_mode = osutils.get_service_start_mode( + _W32TIME_SERVICE) + + if svc_start_mode != osutils.SERVICE_START_MODE_AUTOMATIC: + osutils.set_service_start_mode( + _W32TIME_SERVICE, + osutils.SERVICE_START_MODE_AUTOMATIC) + + if osutils.check_os_version(6, 0): + self._set_ntp_trigger_mode(osutils) + + svc_status = osutils.get_service_status(_W32TIME_SERVICE) + if svc_status == osutils.SERVICE_STATUS_STOPPED: + osutils.start_service(_W32TIME_SERVICE) + + i = 0 + max_retries = 30 + while svc_status != osutils.SERVICE_STATUS_RUNNING: + if i >= max_retries: + raise exception.CloudbaseInitException( + 'Service %s did not start' % _W32TIME_SERVICE) + time.sleep(1) + svc_status = osutils.get_service_status(_W32TIME_SERVICE) + i += 1 + super(NTPClientPlugin, self).verify_time_service(osutils) diff --git a/cloudbaseinit/tests/plugins/common/test_ntpclient.py b/cloudbaseinit/tests/plugins/common/test_ntpclient.py index ecdf47e5..1ddb1bf4 100644 --- a/cloudbaseinit/tests/plugins/common/test_ntpclient.py +++ b/cloudbaseinit/tests/plugins/common/test_ntpclient.py @@ -19,7 +19,6 @@ try: except ImportError: import mock -from cloudbaseinit import exception from cloudbaseinit.plugins.common import base from cloudbaseinit.plugins.common import ntpclient from cloudbaseinit.tests import testutils @@ -31,87 +30,15 @@ class NTPClientPluginTests(unittest.TestCase): def setUp(self): self._ntpclient = ntpclient.NTPClientPlugin() - def test_set_ntp_trigger_mode(self): - mock_osutils = mock.Mock() - self._ntpclient._set_ntp_trigger_mode(mock_osutils) - mock_osutils.execute_system32_process.assert_called_once_with( - ["sc.exe", "triggerinfo", ntpclient._W32TIME_SERVICE, - "start/networkon", "stop/networkoff"]) - - @mock.patch('time.sleep') - @mock.patch('cloudbaseinit.plugins.common.ntpclient.NTPClientPlugin.' - '_set_ntp_trigger_mode') - def _test_check_w32time_svc_status(self, mock_set_ntp_trigger_mode, - mock_sleep, start_mode, - fail_service_start, - patch_check_os_version=True): - # TODO(rtingirica): use _W32TIME_SERVICE when it will be moved outside - # of method declaration - mock_osutils = mock.MagicMock() - mock_osutils.SERVICE_START_MODE_AUTOMATIC = "Automatic" - mock_osutils.SERVICE_STATUS_RUNNING = "running" - mock_osutils.SERVICE_STATUS_STOPPED = "stopped" - mock_osutils.get_service_start_mode.return_value = start_mode - mock_osutils.check_os_version.return_value = patch_check_os_version - - if fail_service_start: - mock_osutils.get_service_status.return_value = "stopped" - self.assertRaises(exception.CloudbaseInitException, - self._ntpclient._check_w32time_svc_status, - mock_osutils) - - else: - mock_osutils.get_service_status.side_effect = [ - "stopped", mock_osutils.SERVICE_STATUS_RUNNING] - - self._ntpclient._check_w32time_svc_status(osutils=mock_osutils) - - if start_mode != mock_osutils.SERVICE_START_MODE_AUTOMATIC: - mock_osutils.set_service_start_mode.assert_called_once_with( - ntpclient._W32TIME_SERVICE, - mock_osutils.SERVICE_START_MODE_AUTOMATIC) - - mock_sleep.assert_called_once_with(1) - mock_osutils.start_service.assert_called_once_with( - ntpclient._W32TIME_SERVICE) - - mock_osutils.get_service_start_mode.assert_called_once_with( - ntpclient._W32TIME_SERVICE) - mock_osutils.get_service_status.assert_called_with( - ntpclient._W32TIME_SERVICE) - - mock_osutils.check_os_version.assert_called_once_with(6, 0) - if patch_check_os_version: - mock_set_ntp_trigger_mode.assert_called_once_with(mock_osutils) - else: - self.assertFalse(mock_set_ntp_trigger_mode.called) - - def test_check_w32time_svc_status_other_start_mode(self): - self._test_check_w32time_svc_status(start_mode="not automatic", - fail_service_start=False) - - def test_check_w32time_svc_status_start_automatic(self): - self._test_check_w32time_svc_status(start_mode="automatic", - fail_service_start=False) - - def test_check_w32time_svc_status_exception(self): - self._test_check_w32time_svc_status(start_mode="automatic", - fail_service_start=True) - - def test_check_w32time_older_oses(self): - self._test_check_w32time_svc_status(start_mode="automatic", - fail_service_start=False, - patch_check_os_version=False) - @testutils.ConfPatcher('ntp_use_dhcp_config', True) @mock.patch('cloudbaseinit.osutils.factory.get_os_utils') @mock.patch('cloudbaseinit.utils.dhcp.get_dhcp_options') @mock.patch('cloudbaseinit.plugins.common.ntpclient.NTPClientPlugin.' - '_check_w32time_svc_status') + 'verify_time_service') @mock.patch('cloudbaseinit.plugins.common.ntpclient.NTPClientPlugin.' '_unpack_ntp_hosts') def _test_execute(self, mock_unpack_ntp_hosts, - mock_check_w32time_svc_status, + mock_verify_time_service, mock_get_dhcp_options, mock_get_os_utils, original_unpack_hosts, ntp_data, expected_hosts): # Set the side effect to the actual function, in order to @@ -138,7 +65,7 @@ class NTPClientPluginTests(unittest.TestCase): if ntp_data: mock_unpack_ntp_hosts.assert_called_once_with(ntp_data) self.assertEqual((base.PLUGIN_EXECUTION_DONE, False), response) - mock_check_w32time_svc_status.assert_called_once_with(mock_osutils) + mock_verify_time_service.assert_called_once_with(mock_osutils) mock_osutils.set_ntp_client_config.assert_called_once_with( expected_hosts) else: diff --git a/cloudbaseinit/tests/plugins/windows/test_ntpclient.py b/cloudbaseinit/tests/plugins/windows/test_ntpclient.py new file mode 100644 index 00000000..b106f30d --- /dev/null +++ b/cloudbaseinit/tests/plugins/windows/test_ntpclient.py @@ -0,0 +1,104 @@ +# Copyright 2014 Cloudbase Solutions Srl +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest + +try: + import unittest.mock as mock +except ImportError: + import mock +from oslo.config import cfg + +from cloudbaseinit import exception +from cloudbaseinit.plugins.windows import ntpclient + +CONF = cfg.CONF + + +class NTPClientPluginTests(unittest.TestCase): + + def setUp(self): + self._ntpclient = ntpclient.NTPClientPlugin() + + def test_set_ntp_trigger_mode(self): + mock_osutils = mock.Mock() + self._ntpclient._set_ntp_trigger_mode(mock_osutils) + mock_osutils.execute_system32_process.assert_called_once_with( + ["sc.exe", "triggerinfo", ntpclient._W32TIME_SERVICE, + "start/networkon", "stop/networkoff"]) + + @mock.patch('time.sleep') + @mock.patch('cloudbaseinit.plugins.windows.ntpclient.NTPClientPlugin.' + '_set_ntp_trigger_mode') + def _test_check_w32time_svc_status(self, mock_set_ntp_trigger_mode, + mock_sleep, start_mode, + fail_service_start, + patch_check_os_version=True): + # TODO(rtingirica): use _W32TIME_SERVICE when it will be moved outside + # of method declaration + mock_osutils = mock.MagicMock() + mock_osutils.SERVICE_START_MODE_AUTOMATIC = "Automatic" + mock_osutils.SERVICE_STATUS_RUNNING = "running" + mock_osutils.SERVICE_STATUS_STOPPED = "stopped" + mock_osutils.get_service_start_mode.return_value = start_mode + mock_osutils.check_os_version.return_value = patch_check_os_version + + if fail_service_start: + mock_osutils.get_service_status.return_value = "stopped" + self.assertRaises(exception.CloudbaseInitException, + self._ntpclient.verify_time_service, + mock_osutils) + + else: + mock_osutils.get_service_status.side_effect = [ + "stopped", mock_osutils.SERVICE_STATUS_RUNNING] + + self._ntpclient.verify_time_service(osutils=mock_osutils) + + if start_mode != mock_osutils.SERVICE_START_MODE_AUTOMATIC: + mock_osutils.set_service_start_mode.assert_called_once_with( + ntpclient._W32TIME_SERVICE, + mock_osutils.SERVICE_START_MODE_AUTOMATIC) + + mock_sleep.assert_called_once_with(1) + mock_osutils.start_service.assert_called_once_with( + ntpclient._W32TIME_SERVICE) + + mock_osutils.get_service_start_mode.assert_called_once_with( + ntpclient._W32TIME_SERVICE) + mock_osutils.get_service_status.assert_called_with( + ntpclient._W32TIME_SERVICE) + + mock_osutils.check_os_version.assert_called_once_with(6, 0) + if patch_check_os_version: + mock_set_ntp_trigger_mode.assert_called_once_with(mock_osutils) + else: + self.assertFalse(mock_set_ntp_trigger_mode.called) + + def test_check_w32time_svc_status_other_start_mode(self): + self._test_check_w32time_svc_status(start_mode="not automatic", + fail_service_start=False) + + def test_check_w32time_svc_status_start_automatic(self): + self._test_check_w32time_svc_status(start_mode="automatic", + fail_service_start=False) + + def test_check_w32time_svc_status_exception(self): + self._test_check_w32time_svc_status(start_mode="automatic", + fail_service_start=True) + + def test_check_w32time_older_oses(self): + self._test_check_w32time_svc_status(start_mode="automatic", + fail_service_start=False, + patch_check_os_version=False)