Merge lp:~allenap/maas/ntp-service-flip-flop into lp:~maas-committers/maas/trunk

[Gavin Panella]

Record status information for NTP under different names for region and rack.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[MAAS Lander (maas-lander)]

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

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Fetched 94.5 kB in 0s (192 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind avahi-utils 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).
avahi-utils is already the newest version (0.6.32~rc+dfsg-1ubuntu2).
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 v...

=== modified file 'src/maasserver/models/service.py'
--- src/maasserver/models/service.py	2016-09-07 14:23:05 +0000
+++ src/maasserver/models/service.py	2016-10-01 09:05:55 +0000
@@ -26,7 +26,7 @@
 REGION_SERVICES = frozenset({
     "regiond",
     "bind9",
-    "ntp",
+    "ntp_region",
     "proxy",
 })
 
@@ -39,7 +39,7 @@
     "tgt",
     "dhcpd",
     "dhcpd6",
-    "ntp",
+    "ntp_rack",
 })
 
 # Statuses that should be set on each service when node is marked dead. NOTE
@@ -54,7 +54,8 @@
     "tgt": SERVICE_STATUS.UNKNOWN,
     "dhcpd": SERVICE_STATUS.DEAD,
     "dhcpd6": SERVICE_STATUS.DEAD,
-    "ntp": SERVICE_STATUS.DEAD,
+    "ntp_region": SERVICE_STATUS.DEAD,
+    "ntp_rack": SERVICE_STATUS.DEAD,
 }
 
 

=== modified file 'src/maasserver/regiondservices/ntp.py'
--- src/maasserver/regiondservices/ntp.py	2016-09-29 14:04:47 +0000
+++ src/maasserver/regiondservices/ntp.py	2016-10-01 09:05:55 +0000
@@ -108,7 +108,7 @@
         d = deferToThread(
             configure_region, configuration.references, configuration.peers)
         d.addCallback(
-            callOut, service_monitor.restartService, "ntp")
+            callOut, service_monitor.restartService, "ntp_region")
         return d
 
     def _configurationApplied(self, configuration):

=== modified file 'src/maasserver/regiondservices/tests/test_ntp.py'
--- src/maasserver/regiondservices/tests/test_ntp.py	2016-09-29 14:04:47 +0000
+++ src/maasserver/regiondservices/tests/test_ntp.py	2016-10-01 09:05:55 +0000
@@ -95,13 +95,13 @@
         yield service._tryUpdate()
         self.assertThat(configure_region, MockCalledOnceWith(
             configuration.references, configuration.peers))
-        self.assertThat(restartService, MockCalledOnceWith("ntp"))
+        self.assertThat(restartService, MockCalledOnceWith("ntp_region"))
         # If the configuration has not changed then a second call to
         # `_tryUpdate` does not result in another call to `configure_region`.
         yield service._tryUpdate()
         self.assertThat(configure_region, MockCalledOnceWith(
             configuration.references, configuration.peers))
-        self.assertThat(restartService, MockCalledOnceWith("ntp"))
+        self.assertThat(restartService, MockCalledOnceWith("ntp_region"))
 
 
 class TestRegionNetworkTimeProtocolService_Errors(

=== modified file 'src/maasserver/service_monitor.py'
--- src/maasserver/service_monitor.py	2016-08-19 10:22:29 +0000
+++ src/maasserver/service_monitor.py	2016-10-01 09:05:55 +0000
@@ -29,10 +29,10 @@
     service_name = "bind9"
 
 
-class NTPService(AlwaysOnService):
-    """Monitored NTP service."""
+class NTPServiceOnRegion(AlwaysOnService):
+    """Monitored NTP service on a region controller host."""
 
-    name = "ntp"
+    name = "ntp_region"
     service_name = "ntp"
 
 
@@ -53,7 +53,7 @@
                 return (SERVICE_STATE.OFF,
                         "disabled, alternate proxy is configured in settings.")
             elif proxyconfig.is_config_present() is False:
-                return (SERVICE_STATE.OFF, "No configuration file present.")
+                return (SERVICE_STATE.OFF, "no configuration file present.")
             else:
                 return (SERVICE_STATE.ON, None)
 
@@ -64,6 +64,6 @@
 # mirrored in maasserver.model.services.
 service_monitor = ServiceMonitor(
     BIND9Service(),
-    NTPService(),
+    NTPServiceOnRegion(),
     ProxyService(),
 )

=== modified file 'src/maasserver/static/partials/node-details.html'
--- src/maasserver/static/partials/node-details.html	2016-09-22 15:15:31 +0000
+++ src/maasserver/static/partials/node-details.html	2016-10-01 09:05:55 +0000
@@ -401,6 +401,13 @@
                                 <span data-ng-if="services.proxy.status_info"> &mdash; {$ services.proxy.status_info $}</span>
                             </span>
                         </li>
+                        <li class="list__item" data-ng-if="services.ntp_region">
+                            <span>
+                                <span class="icon icon--{$ getServiceClass(services.ntp_region) $}"></span>
+                                &nbsp;ntp
+                                <span data-ng-if="services.ntp_region.status_info"> &mdash; {$ services.ntp_region.status_info $}</span>
+                            </span>
+                        </li>
                     </ul>
                 </div>
             </div>
@@ -454,11 +461,11 @@
                                 <span data-ng-if="services.tgt.status_info"> &mdash; {$ services.tgt.status_info $}</span>
                             </span>
                         </li>
-                        <li class="list__item">
+                        <li class="list__item" data-ng-if="services.ntp_rack">
                             <span>
-                                <span class="icon icon--{$ getServiceClass(services.ntp) $}"></span>
+                                <span class="icon icon--{$ getServiceClass(services.ntp_rack) $}"></span>
                                 &nbsp;ntp
-                                <span data-ng-if="services.ntp.status_info"> &mdash; {$ services.ntp.status_info $}</span>
+                                <span data-ng-if="services.ntp_rack.status_info"> &mdash; {$ services.ntp_rack.status_info $}</span>
                             </span>
                         </li>
                     </ul>

=== modified file 'src/maasserver/tests/test_service_monitor.py'
--- src/maasserver/tests/test_service_monitor.py	2016-08-19 10:22:29 +0000
+++ src/maasserver/tests/test_service_monitor.py	2016-10-01 09:05:55 +0000
@@ -34,7 +34,8 @@
 
     def test__includes_all_services(self):
         self.assertItemsEqual(
-            ["bind9", "ntp", "proxy"], service_monitor._services)
+            ["bind9", "ntp_region", "proxy"],
+            service_monitor._services)
 
 
 class TestProxyService(MAASTransactionServerTestCase):
@@ -70,7 +71,7 @@
         os.environ['MAAS_PROXY_CONFIG_DIR'] = "/tmp/%s" % factory.make_name()
         expected_state = yield maybeDeferred(service.getExpectedState)
         self.assertEqual(
-            (SERVICE_STATE.OFF, "No configuration file present."),
+            (SERVICE_STATE.OFF, "no configuration file present."),
             expected_state)
         del os.environ['MAAS_PROXY_CONFIG_DIR']
 

=== modified file 'src/provisioningserver/rackdservices/ntp.py'
--- src/provisioningserver/rackdservices/ntp.py	2016-09-29 14:04:47 +0000
+++ src/provisioningserver/rackdservices/ntp.py	2016-10-01 09:05:55 +0000
@@ -99,7 +99,7 @@
         """
         if configuration.is_rack and not configuration.is_region:
             d = deferToThread(configure_rack, configuration.references, ())
-            d.addCallback(callOut, service_monitor.restartService, "ntp")
+            d.addCallback(callOut, service_monitor.restartService, "ntp_rack")
             return d
 
     def _configurationApplied(self, configuration):

=== modified file 'src/provisioningserver/rackdservices/tests/test_ntp.py'
--- src/provisioningserver/rackdservices/tests/test_ntp.py	2016-09-29 14:04:47 +0000
+++ src/provisioningserver/rackdservices/tests/test_ntp.py	2016-10-01 09:05:55 +0000
@@ -121,12 +121,12 @@
 
         yield service._tryUpdate()
         self.assertThat(configure_rack, MockCalledOnceWith(servers, ()))
-        self.assertThat(restartService, MockCalledOnceWith("ntp"))
+        self.assertThat(restartService, MockCalledOnceWith("ntp_rack"))
         # If the configuration has not changed then a second call to
         # `_tryUpdate` does not result in another call to `configure`.
         yield service._tryUpdate()
         self.assertThat(configure_rack, MockCalledOnceWith(servers, ()))
-        self.assertThat(restartService, MockCalledOnceWith("ntp"))
+        self.assertThat(restartService, MockCalledOnceWith("ntp_rack"))
 
     @inlineCallbacks
     def test_is_silent_and_does_nothing_when_region_is_not_available(self):

=== modified file 'src/provisioningserver/service_monitor.py'
--- src/provisioningserver/service_monitor.py	2016-08-19 15:59:07 +0000
+++ src/provisioningserver/service_monitor.py	2016-10-01 09:05:55 +0000
@@ -64,10 +64,10 @@
     service_name = "tgt"
 
 
-class NTPService(Service):
-    """Monitored NTP service."""
+class NTPServiceOnRack(Service):
+    """Monitored NTP service on a rack controller host."""
 
-    name = "ntp"
+    name = "ntp_rack"
     service_name = "ntp"
 
     def getExpectedState(self):
@@ -81,8 +81,11 @@
             return d
 
     def _getExpectedStateForControllerType(self, controller_type):
-        if controller_type["is_rack"] and not controller_type["is_region"]:
-            return SERVICE_STATE.ON, None
+        if controller_type["is_rack"]:
+            if controller_type["is_region"]:
+                return SERVICE_STATE.ANY, "managed by the region."
+            else:
+                return SERVICE_STATE.ON, None
         else:
             return SERVICE_STATE.ANY, None
 
@@ -92,6 +95,6 @@
 service_monitor = ServiceMonitor(
     DHCPv4Service(),
     DHCPv6Service(),
-    NTPService(),
+    NTPServiceOnRack(),
     TGTService(),
 )

=== modified file 'src/provisioningserver/tests/test_service_monitor.py'
--- src/provisioningserver/tests/test_service_monitor.py	2016-08-19 15:59:07 +0000
+++ src/provisioningserver/tests/test_service_monitor.py	2016-10-01 09:05:55 +0000
@@ -19,7 +19,7 @@
     DHCPService,
     DHCPv4Service,
     DHCPv6Service,
-    NTPService,
+    NTPServiceOnRack,
     service_monitor,
     TGTService,
 )
@@ -108,15 +108,15 @@
         self.assertEqual((SERVICE_STATE.ON, None), tgt.getExpectedState())
 
 
-class TestNTPService(MAASTestCase):
+class TestNTPServiceOnRack(MAASTestCase):
 
     def test_name_and_service_name(self):
-        ntp = NTPService()
+        ntp = NTPServiceOnRack()
         self.assertEqual("ntp", ntp.service_name)
-        self.assertEqual("ntp", ntp.name)
-
-
-class TestNTPService_Scenarios(MAASTestCase):
+        self.assertEqual("ntp_rack", ntp.name)
+
+
+class TestNTPServiceOnRack_Scenarios(MAASTestCase):
 
     run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
 
@@ -124,37 +124,41 @@
         ("rack", dict(
             is_region=False, is_rack=True,
             expected_state=SERVICE_STATE.ON,
+            expected_info=None,
         )),
         ("region", dict(
             is_region=True, is_rack=False,
             expected_state=SERVICE_STATE.ANY,
+            expected_info=None,
         )),
         ("region+rack", dict(
             is_region=True, is_rack=True,
             expected_state=SERVICE_STATE.ANY,
+            expected_info="managed by the region.",
         )),
         ("machine", dict(
             is_region=False, is_rack=False,
             expected_state=SERVICE_STATE.ANY,
+            expected_info=None,
         )),
     )
 
     def setUp(self):
-        super(TestNTPService_Scenarios, self).setUp()
+        super(TestNTPServiceOnRack_Scenarios, self).setUp()
         return prepareRegionForGetControllerType(
             self, is_region=self.is_region, is_rack=self.is_rack)
 
     @inlineCallbacks
     def test_getExpectedState(self):
-        ntp = NTPService()
+        ntp = NTPServiceOnRack()
         self.assertThat(
             (yield ntp.getExpectedState()),
-            Equals((self.expected_state, None)))
+            Equals((self.expected_state, self.expected_info)))
 
 
 class TestGlobalServiceMonitor(MAASTestCase):
 
     def test__includes_all_services(self):
         self.assertItemsEqual(
-            ["dhcpd", "dhcpd6", "ntp", "tgt"],
+            ["dhcpd", "dhcpd6", "ntp_rack", "tgt"],
             service_monitor._services)