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

@@ -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',

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 verify_time_service(self, osutils):
-    def _set_ntp_trigger_mode(osutils):
+        """Verify that the time service is up.
-        """Set the trigger mode for w32time service to network availability.
 
-        This function changes the triggers for the w32time service, so that
+        Implementing this method is optional, it is
-        the service will always work when there's networking, but will
+        mostly used by the Windows version of this plugin.
-        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)
 
     @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)

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)

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:

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)