launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/commission-timeout-api into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/commission-timeout-api/+merge/103794

I think everyone has been involved with the direction of this change at some point or other. Despite where we end up driving this change, the basic API here is sound and tested. It cannot go anywhere else; we want the appservers to always be in control of database changes so that we can rely on transactional integrity in a single request/reply.
-- 
https://code.launchpad.net/~julian-edwards/maas/commission-timeout-api/+merge/103794
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/commission-timeout-api into lp:maas.

=== modified file 'src/maas/settings.py'
--- src/maas/settings.py	2012-04-18 10:51:03 +0000
+++ src/maas/settings.py	2012-04-27 03:09:21 +0000
@@ -276,5 +276,9 @@
 # doing.
 COMMISSIONING_SCRIPT = 'etc/maas/commissioning-user-data'
 
+# The duration, in minutes, after which we consider a commissioning node
+# to have failed and mark it as FAILED_TESTS.
+COMMISSIONING_TIMEOUT=60
+
 # Allow the user to override settings in maas_local_settings.
 import_local_settings()

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-04-24 16:46:33 +0000
+++ src/maasserver/api.py	2012-04-27 03:09:21 +0000
@@ -68,12 +68,17 @@
     ]
 
 from base64 import b64decode
+from datetime import (
+    datetime,
+    timedelta,
+    )
 import httplib
 import json
 import sys
 from textwrap import dedent
 import types
 
+from django.conf import settings
 from django.core.exceptions import (
     PermissionDenied,
     ValidationError,
@@ -488,6 +493,21 @@
         """Accept a node's enlistment: not allowed to anonymous users."""
         raise Unauthorized("You must be logged in to accept nodes.")
 
+    @api_exported("check_commissioning", "POST")
+    def check_commissioning(self, request):
+        """Check all commissioning nodes to see if they are taking too long.
+
+        Anything that has been commissioning for longer than
+        settings.COMMISSIONING_TIMEOUT is moved into the FAILED_TESTS status.
+        """
+        interval = timedelta(minutes=settings.COMMISSIONING_TIMEOUT)
+        cutoff = datetime.now() - interval
+        query = Node.objects.filter(
+            status=NODE_STATUS.COMMISSIONING, updated__lte=cutoff)
+        query.update(status=NODE_STATUS.FAILED_TESTS)
+        # Note that Django doesn't call save() on updated nodes here,
+        # but I don't think anything requires its effects anyway.
+
     @classmethod
     def resource_uri(cls, *args, **kwargs):
         return ('nodes_handler', [])

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-04-26 05:44:05 +0000
+++ src/maasserver/tests/test_api.py	2012-04-27 03:09:21 +0000
@@ -13,6 +13,10 @@
 __all__ = []
 
 from base64 import b64encode
+from datetime import (
+    datetime,
+    timedelta,
+    )
 from collections import namedtuple
 import httplib
 import json
@@ -21,6 +25,7 @@
 import shutil
 
 from django.conf import settings
+from django.db import models
 from django.db.models.signals import post_save
 from django.http import QueryDict
 from fixtures import Fixture
@@ -1799,3 +1804,39 @@
             (response.status_code, response.content))
         self.assertRaises(
             Node.DoesNotExist, Node.objects.get, hostname=hostname)
+
+
+class TestAnonymousCommissioningTimeout(APIv10TestMixin, TestCase):
+    """Testing of commissioning timeout API."""
+
+    def test_check_with_no_action(self):
+        node = factory.make_node(status=NODE_STATUS.READY)
+        response = self.client.post(
+            self.get_uri('nodes/'), {'op': 'check_commissioning'})
+        # Anything that's not commissioning should be ignored.
+        self.assertEqual(NODE_STATUS.READY, node.status)
+
+    def test_check_with_commissioning_but_not_expired_node(self):
+        node = factory.make_node(
+            status=NODE_STATUS.COMMISSIONING)
+        response = self.client.post(
+            self.get_uri('nodes/'), {'op': 'check_commissioning'})
+        self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
+
+    def test_check_with_commissioning_and_expired_node(self):
+        # Remove the custom save() method that updates "updated".
+        def fake_save(self, skip_check=None, *args, **kwargs):
+            models.Model.save(self, *args, **kwargs)
+        self.patch(Node, 'save', fake_save)
+
+        # Have an interval 1 second longer than the timeout.
+        interval = timedelta(seconds=1, minutes=settings.COMMISSIONING_TIMEOUT)
+        updated_at = updated=datetime.now() - interval
+        node = factory.make_node(
+            status=NODE_STATUS.COMMISSIONING, created=datetime.now(),
+            updated=updated_at)
+
+        response = self.client.post(
+            self.get_uri('nodes/'), {'op': 'check_commissioning'})
+        node = reload_object(node)
+        self.assertEqual(NODE_STATUS.FAILED_TESTS, node.status)