Merge lp:~allenap/maas/ntp-service-in-rack into lp:~maas-committers/maas/trunk

[Gavin Panella]

Monitor the NTP service in the rack when it's not also a region controller.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~allenap/maas/ntp-service-in-rack into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [95.7 kB]
Get:3 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Fetched 190 kB in 0s (439 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1).
python-netifaces is already the newest version (0.10.4-...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~allenap/maas/ntp-service-in-rack into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Hit:4 http://security.ubuntu.com/ubuntu xenial-security InRelease
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common isc-dhcp-server libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-attr python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1).
python-netifaces is already the newest version (0.10.4-0.1build2).
python-psycopg2 is already the newest ve...

=== modified file 'src/maastesting/factory.py'
--- src/maastesting/factory.py	2016-08-09 15:56:46 +0000
+++ src/maastesting/factory.py	2016-08-19 13:45:36 +0000
@@ -10,6 +10,7 @@
     ]
 
 import datetime
+from enum import Enum
 from functools import partial
 import http.client
 import io
@@ -168,15 +169,22 @@
     def pick_enum(self, enum, *, but_not=EMPTY_SET):
         """Pick a random item from an enumeration class.
 
-        :param enum: An enumeration class such as `NODE_STATUS`.
+        :param enum: An enumeration class such as `NODE_STATUS`. Can also be
+            an `enum.Enum` subclass.
         :return: The value of one of its items.
         :param but_not: A list of choices' IDs to exclude.
         :type but_not: Sequence.
         """
-        return random.choice([
-            value for key, value in vars(enum).items()
-            if not key.startswith('_') and value not in but_not
-        ])
+        if issubclass(enum, Enum):
+            return random.choice([
+                value for value in enum
+                if value not in but_not
+            ])
+        else:
+            return random.choice([
+                value for key, value in vars(enum).items()
+                if not key.startswith('_') and value not in but_not
+            ])
 
     def pick_port(self, port_min=1024, port_max=65535):
         assert port_min >= 0 and port_max <= 65535

=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py	2016-08-09 16:45:55 +0000
+++ src/provisioningserver/plugin.py	2016-08-19 13:45:36 +0000
@@ -150,6 +150,12 @@
         service_monitor.setName("service_monitor")
         return service_monitor
 
+    def _makeNetworkTimeProtocolService(self, rpc_service):
+        from provisioningserver.rackdservices import ntp
+        ntp_service = ntp.RackNetworkTimeProtocolService(rpc_service, reactor)
+        ntp_service.setName("ntp")
+        return ntp_service
+
     def _makeServices(self, tftp_root, tftp_port):
         # Several services need to make use of the RPC service.
         rpc_service = self._makeRPCService()
@@ -160,12 +166,11 @@
         yield self._makeLeaseSocketService(rpc_service)
         yield self._makeNodePowerMonitorService()
         yield self._makeServiceMonitorService(rpc_service)
-        yield self._makeImageDownloadService(
-            rpc_service, tftp_root)
+        yield self._makeImageDownloadService(rpc_service, tftp_root)
+        yield self._makeNetworkTimeProtocolService(rpc_service)
         # The following are network-accessible services.
         yield self._makeImageService(tftp_root)
-        yield self._makeTFTPService(
-            tftp_root, tftp_port, rpc_service)
+        yield self._makeTFTPService(tftp_root, tftp_port, rpc_service)
 
     def _configureCrochet(self):
         # Prevent other libraries from starting the reactor via crochet.

=== added file 'src/provisioningserver/rackdservices/testing.py'
--- src/provisioningserver/rackdservices/testing.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/rackdservices/testing.py	2016-08-19 13:45:36 +0000
@@ -0,0 +1,36 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Testing resources for `provisioningserver.rackdservices`."""
+
+__all__ = [
+    "prepareRegionForGetControllerType",
+]
+
+from maastesting.factory import factory
+from maastesting.twisted import always_succeed_with
+from provisioningserver import services
+from provisioningserver.rpc import region
+from provisioningserver.rpc.testing import MockLiveClusterToRegionRPCFixture
+
+
+def prepareRegionForGetControllerType(test, is_region=False, is_rack=True):
+    """Set up a mock region controller that responds to `GetControllerType`.
+
+    In addition it sets the MAAS ID to a new random value. It arranges for
+    tear-down at the end of the test.
+
+    :return: The running RPC service, and the protocol instance.
+    """
+    fixture = test.useFixture(MockLiveClusterToRegionRPCFixture())
+    protocol, connecting = fixture.makeEventLoop(region.GetControllerType)
+    protocol.RegisterRackController.side_effect = always_succeed_with(
+        {"system_id": factory.make_name("maas-id")})
+    protocol.GetControllerType.side_effect = always_succeed_with({
+        "is_region": is_region, "is_rack": is_rack})
+
+    def connected(teardown):
+        test.addCleanup(teardown)
+        return services.getServiceNamed("rpc"), protocol
+
+    return connecting.addCallback(connected)

=== modified file 'src/provisioningserver/rackdservices/tests/test_ntp.py'
--- src/provisioningserver/rackdservices/tests/test_ntp.py	2016-08-17 14:48:54 +0000
+++ src/provisioningserver/rackdservices/tests/test_ntp.py	2016-08-19 13:45:36 +0000
@@ -19,18 +19,16 @@
     MAASTestCase,
     MAASTwistedRunTest,
 )
-from maastesting.twisted import (
-    always_succeed_with,
-    TwistedLoggerFixture,
-)
-from provisioningserver import services
+from maastesting.twisted import TwistedLoggerFixture
 from provisioningserver.rackdservices import ntp
+from provisioningserver.rackdservices.testing import (
+    prepareRegionForGetControllerType,
+)
 from provisioningserver.rpc import (
     common,
     exceptions,
     region,
 )
-from provisioningserver.rpc.testing import MockLiveClusterToRegionRPCFixture
 from provisioningserver.rpc.testing.doubles import FakeConnectionToRegion
 from testtools.matchers import (
     Equals,
@@ -80,28 +78,6 @@
             return common.Client(conn)
 
 
-def prepareRegionForGetControllerType(test, is_region=False, is_rack=True):
-    """Set up a mock region controller that responds to `GetControllerType`.
-
-    In addition it sets the MAAS ID to a new random value. It arranges for
-    tear-down at the end of the test.
-
-    :return: The running RPC service, and the protocol instance.
-    """
-    fixture = test.useFixture(MockLiveClusterToRegionRPCFixture())
-    protocol, connecting = fixture.makeEventLoop(region.GetControllerType)
-    protocol.RegisterRackController.side_effect = always_succeed_with(
-        {"system_id": factory.make_name("maas-id")})
-    protocol.GetControllerType.side_effect = always_succeed_with({
-        "is_region": is_region, "is_rack": is_rack})
-
-    def connected(teardown):
-        test.addCleanup(teardown)
-        return services.getServiceNamed("rpc"), protocol
-
-    return connecting.addCallback(connected)
-
-
 class TestRackNetworkTimeProtocolService(MAASTestCase):
     """Tests for `RackNetworkTimeProtocolService`."""
 

=== modified file 'src/provisioningserver/rackdservices/tests/test_service_monitor_service.py'
--- src/provisioningserver/rackdservices/tests/test_service_monitor_service.py	2016-07-30 01:17:54 +0000
+++ src/provisioningserver/rackdservices/tests/test_service_monitor_service.py	2016-08-19 13:45:36 +0000
@@ -30,6 +30,7 @@
 from provisioningserver.rpc.testing import MockLiveClusterToRegionRPCFixture
 from provisioningserver.service_monitor import service_monitor
 from provisioningserver.utils.service_monitor import (
+    AlwaysOnService,
     SERVICE_STATE,
     ServiceState,
 )
@@ -111,7 +112,14 @@
         protocol, connecting = self.patch_rpc_methods()
         self.addCleanup((yield connecting))
 
-        service = self.pick_service()
+        class ExampleService(AlwaysOnService):
+            name = service_name = factory.make_name("service")
+
+        service = ExampleService()
+        # Inveigle this new service into the service monitor.
+        self.addCleanup(service_monitor._services.pop, service.name)
+        service_monitor._services[service.name] = service
+
         state = ServiceState(SERVICE_STATE.ON, "running")
         mock_ensureServices = self.patch(service_monitor, "ensureServices")
         mock_ensureServices.return_value = succeed({

=== modified file 'src/provisioningserver/service_monitor.py'
--- src/provisioningserver/service_monitor.py	2016-08-18 15:21:07 +0000
+++ src/provisioningserver/service_monitor.py	2016-08-19 13:45:36 +0000
@@ -6,6 +6,9 @@
 __all__ = [
     ]
 
+from provisioningserver.rpc import getRegionClient
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from provisioningserver.rpc.region import GetControllerType
 from provisioningserver.utils.service_monitor import (
     AlwaysOnService,
     Service,
@@ -61,10 +64,34 @@
     service_name = "tgt"
 
 
+class NTPService(Service):
+    """Monitored NTP service."""
+
+    name = "ntp"
+    service_name = "ntp"
+
+    def getExpectedState(self):
+        try:
+            client = getRegionClient()
+        except NoConnectionsAvailable:
+            return SERVICE_STATE.ANY, None
+        else:
+            d = client(GetControllerType, system_id=client.localIdent)
+            d.addCallback(self._getExpectedStateForControllerType)
+            return d
+
+    def _getExpectedStateForControllerType(self, controller_type):
+        if controller_type["is_rack"] and not controller_type["is_region"]:
+            return SERVICE_STATE.ON, None
+        else:
+            return SERVICE_STATE.ANY, None
+
+
 # Global service monitor for rackd. NOTE that changes to this need to be
 # mirrored in maasserver.model.services.
 service_monitor = ServiceMonitor(
     DHCPv4Service(),
     DHCPv6Service(),
+    NTPService(),
     TGTService(),
 )

=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py	2016-07-30 01:17:54 +0000
+++ src/provisioningserver/tests/test_plugin.py	2016-08-19 13:45:36 +0000
@@ -99,7 +99,7 @@
         self.assertIsInstance(service, MultiService)
         expected_services = [
             "dhcp_probe", "networks_monitor", "image_download",
-            "lease_socket_service", "node_monitor", "rpc", "tftp",
+            "lease_socket_service", "node_monitor", "ntp", "rpc", "tftp",
             "image_service", "service_monitor",
             ]
         self.assertItemsEqual(expected_services, service.namedServices)

=== modified file 'src/provisioningserver/tests/test_service_monitor.py'
--- src/provisioningserver/tests/test_service_monitor.py	2016-08-18 15:21:07 +0000
+++ src/provisioningserver/tests/test_service_monitor.py	2016-08-19 13:45:36 +0000
@@ -8,15 +8,24 @@
 from unittest.mock import sentinel
 
 from maastesting.factory import factory
-from maastesting.testcase import MAASTestCase
+from maastesting.testcase import (
+    MAASTestCase,
+    MAASTwistedRunTest,
+)
+from provisioningserver.rackdservices.testing import (
+    prepareRegionForGetControllerType,
+)
 from provisioningserver.service_monitor import (
     DHCPService,
     DHCPv4Service,
     DHCPv6Service,
+    NTPService,
     service_monitor,
     TGTService,
 )
 from provisioningserver.utils.service_monitor import SERVICE_STATE
+from testtools.matchers import Equals
+from twisted.internet.defer import inlineCallbacks
 
 
 class TestDHCPService(MAASTestCase):
@@ -99,8 +108,53 @@
         self.assertEqual((SERVICE_STATE.ON, None), tgt.getExpectedState())
 
 
+class TestNTPService(MAASTestCase):
+
+    def test_name_and_service_name(self):
+        ntp = NTPService()
+        self.assertEqual("ntp", ntp.service_name)
+        self.assertEqual("ntp", ntp.name)
+
+
+class TestNTPService_Scenarios(MAASTestCase):
+
+    run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
+
+    scenarios = (
+        ("rack", dict(
+            is_region=False, is_rack=True,
+            expected_state=SERVICE_STATE.ON,
+        )),
+        ("region", dict(
+            is_region=True, is_rack=False,
+            expected_state=SERVICE_STATE.ANY,
+        )),
+        ("region+rack", dict(
+            is_region=True, is_rack=True,
+            expected_state=SERVICE_STATE.ANY,
+        )),
+        ("machine", dict(
+            is_region=False, is_rack=False,
+            expected_state=SERVICE_STATE.ANY,
+        )),
+    )
+
+    def setUp(self):
+        super(TestNTPService_Scenarios, self).setUp()
+        return prepareRegionForGetControllerType(
+            self, is_region=self.is_region, is_rack=self.is_rack)
+
+    @inlineCallbacks
+    def test_getExpectedState(self):
+        ntp = NTPService()
+        self.assertThat(
+            (yield ntp.getExpectedState()),
+            Equals((self.expected_state, None)))
+
+
 class TestGlobalServiceMonitor(MAASTestCase):
 
     def test__includes_all_services(self):
         self.assertItemsEqual(
-            ["dhcpd", "dhcpd6", "tgt"], service_monitor._services.keys())
+            ["dhcpd", "dhcpd6", "ntp", "tgt"],
+            service_monitor._services)

=== modified file 'src/provisioningserver/utils/service_monitor.py'
--- src/provisioningserver/utils/service_monitor.py	2016-08-18 15:33:43 +0000
+++ src/provisioningserver/utils/service_monitor.py	2016-08-19 13:45:36 +0000
@@ -107,7 +107,7 @@
         """
         def deriveStatusInfo(expected_state_and_info, service):
             expected_state, status_info = expected_state_and_info
-            _check_service_state_observed(expected_state)
+            _check_service_state_expected(expected_state)
             if status_info is None:
                 status_info = ""
             if self.active_state == SERVICE_STATE.UNKNOWN: