Merge lp:~andreserl/maas/lp1598149_lp1605252 into lp:~maas-committers/maas/trunk

[Andres Rodriguez]

Ensure that messages when a timeout for deploying, releasing, and commissioning happens is logged (This was supported in older releases but was dropped in 2.0).

Also, drive-by fix the commissioning monitor services. This ensures that the status_expires is not cleared when it shouldn't and allows the monitor to work while commissioning.

Another drive-by improvement, is to ensure that the monitor is only checking MACHINES and not anything else.

[Lee Trager (ltrager)]

Looks good. Did you test that rack and region refresh still works even when they're not on the same machine?

review: Approve

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~andreserl/maas/lp1598149_lp1605252 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
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [328 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [300 kB]
Fetched 818 kB in 0s (1,581 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 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-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).
curl is already the newest version (7.47.0-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-11...

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-07-18 20:06:21 +0000
+++ src/maasserver/models/node.py	2016-07-22 14:42:17 +0000
@@ -123,6 +123,7 @@
     COMMISSIONING_LIKE_STATUSES,
     get_failed_status,
     is_failed_status,
+    NODE_FAILURE_MONITORED_STATUS_TIMEOUTS,
     NODE_TRANSITIONS,
 )
 from maasserver.rpc import (
@@ -1106,7 +1107,9 @@
         """
         # Return a *very* conservative estimate for now.
         # Something that shouldn't conflict with any deployment.
-        return timedelta(minutes=40).total_seconds()
+        return timedelta(
+            minutes=NODE_FAILURE_MONITORED_STATUS_TIMEOUTS[
+                NODE_STATUS.DEPLOYING]).total_seconds()
 
     def get_commissioning_time(self):
         """Return the commissioning time of this node (in seconds).
@@ -1114,14 +1117,18 @@
         This is the maximum time the commissioning is allowed to take.
         """
         # Return a *very* conservative estimate for now.
-        return timedelta(minutes=20).total_seconds()
+        return timedelta(
+            minutes=NODE_FAILURE_MONITORED_STATUS_TIMEOUTS[
+                NODE_STATUS.COMMISSIONING]).total_seconds()
 
     def get_releasing_time(self):
         """Return the releasing time of this node (in seconds).
 
         This is the maximum time that releasing is allowed to take.
         """
-        return timedelta(minutes=5).total_seconds()
+        return timedelta(
+            minutes=NODE_FAILURE_MONITORED_STATUS_TIMEOUTS[
+                NODE_STATUS.RELEASING]).total_seconds()
 
     def _register_request_event(
             self, user, type_name, action='', comment=None):

=== modified file 'src/maasserver/node_status.py'
--- src/maasserver/node_status.py	2015-12-01 18:12:59 +0000
+++ src/maasserver/node_status.py	2016-07-22 14:42:17 +0000
@@ -162,6 +162,22 @@
     NODE_STATUS.DISK_ERASING: NODE_STATUS.FAILED_DISK_ERASING,
 }
 
+# State transitions that are monitored for timeouts for when a node
+# fails:
+# Mapping between in-progress statuses and the corresponding failed
+# statuses.
+NODE_FAILURE_MONITORED_STATUS_TRANSITIONS = {
+    NODE_STATUS.COMMISSIONING: NODE_STATUS.FAILED_COMMISSIONING,
+    NODE_STATUS.DEPLOYING: NODE_STATUS.FAILED_DEPLOYMENT,
+    NODE_STATUS.RELEASING: NODE_STATUS.FAILED_RELEASING,
+}
+
+NODE_FAILURE_MONITORED_STATUS_TIMEOUTS = {
+    NODE_STATUS.COMMISSIONING: 20,
+    NODE_STATUS.DEPLOYING: 40,
+    NODE_STATUS.RELEASING: 5,
+}
+
 # Statuses that correspond to managed steps for which MAAS actively
 # monitors that the status changes after a fixed period of time.
 MONITORED_STATUSES = list(NODE_FAILURE_STATUS_TRANSITIONS.keys())

=== modified file 'src/maasserver/status_monitor.py'
--- src/maasserver/status_monitor.py	2016-02-01 10:28:01 +0000
+++ src/maasserver/status_monitor.py	2016-07-22 14:42:17 +0000
@@ -8,9 +8,16 @@
     'StatusMonitorService',
     ]
 
+from maasserver.enum import (
+    NODE_STATUS_CHOICES_DICT,
+    NODE_TYPE,
+)
 from maasserver.models.node import Node
 from maasserver.models.timestampedmodel import now
-from maasserver.node_status import NODE_FAILURE_STATUS_TRANSITIONS
+from maasserver.node_status import (
+    NODE_FAILURE_MONITORED_STATUS_TIMEOUTS,
+    NODE_FAILURE_MONITORED_STATUS_TRANSITIONS,
+)
 from maasserver.utils.orm import transactional
 from maasserver.utils.threads import deferToDatabase
 from provisioningserver.utils.twisted import synchronous
@@ -21,14 +28,22 @@
     """Mark all nodes in that database as failed where the status did not
     transition in time. `status_expires` is checked on the node to see if the
     current time is newer than the expired time.
+
+    Status monitors are only available for Machines that are Commissioning,
+    Deploying or Releasing.
     """
     current_db_time = now()
     expired_nodes = Node.objects.filter(
-        status__in=NODE_FAILURE_STATUS_TRANSITIONS.keys(),
+        node_type=NODE_TYPE.MACHINE,
+        status__in=NODE_FAILURE_MONITORED_STATUS_TRANSITIONS.keys(),
         status_expires__isnull=False,
         status_expires__lte=current_db_time)
     for node in expired_nodes:
-        node._mark_failed(None, commit=False)
+        comment = "Machine operation '%s' timed out after %s minutes." % (
+            NODE_STATUS_CHOICES_DICT[node.status],
+            NODE_FAILURE_MONITORED_STATUS_TIMEOUTS[node.status],
+            )
+        node._mark_failed(None, commit=False, comment=comment)
         node.status_expires = None
         node.save()
 

=== modified file 'src/maasserver/tests/test_status_monitor.py'
--- src/maasserver/tests/test_status_monitor.py	2016-05-16 09:21:53 +0000
+++ src/maasserver/tests/test_status_monitor.py	2016-07-22 14:42:17 +0000
@@ -14,7 +14,7 @@
 
 from maasserver import status_monitor
 from maasserver.models.signals.testing import SignalsDisabled
-from maasserver.node_status import NODE_FAILURE_STATUS_TRANSITIONS
+from maasserver.node_status import NODE_FAILURE_MONITORED_STATUS_TRANSITIONS
 from maasserver.status_monitor import (
     mark_nodes_failed_after_expiring,
     StatusMonitorService,
@@ -40,7 +40,7 @@
         expired_time = current_time - timedelta(minutes=1)
         nodes = [
             factory.make_Node(status=status, status_expires=expired_time)
-            for status in NODE_FAILURE_STATUS_TRANSITIONS.keys()
+            for status in NODE_FAILURE_MONITORED_STATUS_TRANSITIONS.keys()
         ]
         mark_nodes_failed_after_expiring()
         failed_statuses = [
@@ -48,7 +48,8 @@
             for node in nodes
         ]
         self.assertItemsEqual(
-            NODE_FAILURE_STATUS_TRANSITIONS.values(), failed_statuses)
+            NODE_FAILURE_MONITORED_STATUS_TRANSITIONS.values(),
+            failed_statuses)
 
     def test__skips_those_that_have_not_expired(self):
         self.useFixture(SignalsDisabled("power"))
@@ -57,7 +58,7 @@
         expired_time = current_time + timedelta(minutes=1)
         nodes = [
             factory.make_Node(status=status, status_expires=expired_time)
-            for status in NODE_FAILURE_STATUS_TRANSITIONS.keys()
+            for status in NODE_FAILURE_MONITORED_STATUS_TRANSITIONS.keys()
         ]
         mark_nodes_failed_after_expiring()
         failed_statuses = [
@@ -65,7 +66,7 @@
             for node in nodes
         ]
         self.assertItemsEqual(
-            NODE_FAILURE_STATUS_TRANSITIONS.keys(), failed_statuses)
+            NODE_FAILURE_MONITORED_STATUS_TRANSITIONS.keys(), failed_statuses)
 
 
 class TestStatusMonitorService(MAASServerTestCase):

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2016-06-09 08:10:02 +0000
+++ src/metadataserver/api.py	2016-07-22 14:42:17 +0000
@@ -352,7 +352,6 @@
         # At the end of a top-level event, we change the node status.
         if _is_top_level(activity_name) and event_type == 'finish':
             if node.status == NODE_STATUS.COMMISSIONING:
-                node.status_expires = None
                 if result in ['FAIL', 'FAILURE']:
                     node.status = NODE_STATUS.FAILED_COMMISSIONING
 
@@ -370,6 +369,7 @@
             if node.node_type == NODE_TYPE.MACHINE and node.status in [
                     NODE_STATUS.READY,
                     NODE_STATUS.FAILED_COMMISSIONING]:
+                node.status_expires = None
                 node.owner = None
                 node.error = 'failed: %s' % description
 
@@ -518,8 +518,10 @@
                 if node.power_type != "mscm":
                     store_node_power_parameters(node, request)
 
-            node.status_expires = None
             target_status = self.signaling_statuses.get(status)
+            if target_status in [NODE_STATUS.FAILED_COMMISSIONING,
+               NODE_STATUS.READY]:
+                node.status_expires = None
             # Recalculate tags when commissioning ends.
             if target_status == NODE_STATUS.READY:
                 populate_tags_for_single_node(Tag.objects.all(), node)