Merge lp:~rvb/maas/check-commissioning-bug-1084292-1.2 into lp:maas/1.2

[Raphaël Badin]

Backport revision 1392.

Backport revision 1392.

[Raphaël Badin (rvb)]

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-12-04 04:05:17 +0000
+++ src/maasserver/api.py	2012-12-04 10:32:25 +0000
@@ -707,21 +707,6 @@
         """Accept a node's enlistment: not allowed to anonymous users."""
         raise Unauthorized("You must be logged in to accept nodes.")
 
-    @operation(idempotent=False)
-    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', [])
@@ -840,6 +825,23 @@
         return filter(None, nodes)
 
     @operation(idempotent=False)
+    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)
+        results = list(query)
+        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.
+        return results
+
+    @operation(idempotent=False)
     def release(self, request):
         """Release multiple nodes.
 

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-12-04 04:26:52 +0000
+++ src/maasserver/tests/test_api.py	2012-12-04 10:32:25 +0000
@@ -3346,24 +3346,28 @@
             (response.status_code, response.content))
 
 
-class TestAnonymousCommissioningTimeout(APIv10TestMixin, TestCase):
+class TestCommissioningTimeout(APIv10TestMixin, LoggedInTestCase):
     """Testing of commissioning timeout API."""
 
     def test_check_with_no_action(self):
         node = factory.make_node(status=NODE_STATUS.READY)
-        self.client.post(
+        response = self.client.post(
             self.get_uri('nodes/'), {'op': 'check_commissioning'})
         # Anything that's not commissioning should be ignored.
         node = reload_object(node)
-        self.assertEqual(NODE_STATUS.READY, node.status)
+        self.assertEqual(
+            (httplib.OK, NODE_STATUS.READY),
+            (response.status_code, node.status))
 
     def test_check_with_commissioning_but_not_expired_node(self):
         node = factory.make_node(
             status=NODE_STATUS.COMMISSIONING)
-        self.client.post(
+        response = self.client.post(
             self.get_uri('nodes/'), {'op': 'check_commissioning'})
         node = reload_object(node)
-        self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
+        self.assertEqual(
+            (httplib.OK, NODE_STATUS.COMMISSIONING),
+            (response.status_code, node.status))
 
     def test_check_with_commissioning_and_expired_node(self):
         # Have an interval 1 second longer than the timeout.
@@ -3373,10 +3377,19 @@
             status=NODE_STATUS.COMMISSIONING, created=datetime.now(),
             updated=updated_at)
 
-        self.client.post(
+        response = self.client.post(
             self.get_uri('nodes/'), {'op': 'check_commissioning'})
-        node = reload_object(node)
-        self.assertEqual(NODE_STATUS.FAILED_TESTS, node.status)
+        self.assertEqual(
+            (
+                httplib.OK,
+                NODE_STATUS.FAILED_TESTS,
+                [node.system_id]
+            ),
+            (
+                response.status_code,
+                reload_object(node).status,
+                [node['system_id'] for node in json.loads(response.content)],
+            ))
 
 
 class TestPXEConfigAPI(AnonAPITestCase):