Merge lp:~gmb/maas/test-poweroff-problem into lp:~maas-committers/maas/trunk

[Graham Binns]

Make the API's release() method allow users to call release() on a node that's marked broken. Also, make sure that Node.release() won't move a BROKEN node back to READY.

[Julian Edwards (julian-edwards)]

On Friday 01 Aug 2014 20:23:23 you wrote:
> === modified file 'Makefile'
> --- Makefile 2014-06-10 14:45:14 +0000
> +++ Makefile 2014-08-01 20:23:08 +0000
> @@ -344,7 +344,7 @@
> # has a bug and always considers apt-source tarballs before the specified
> # branch. So instead, export to a local tarball which is always found.
> # Make sure debhelper and dh-apport packages are installed before using
> this. -PACKAGING := $(CURDIR)/../packaging.trunk
> +PACKAGING := $(CURDIR)/../packaging
> PACKAGING_BRANCH := lp:~maas-maintainers/maas/packaging

You know how I said that you needed to revert that "temporary" change ... ;)

[Graham Binns (gmb)]

On 2 August 2014 00:27, Julian Edwards <email address hidden> wrote:
> You know how I said that you needed to revert that "temporary" change ... ;)

I did revert it… just not until after I'd pushed the branch, that's all ;)

[Julian Edwards (julian-edwards)]

Looks good. I have a couple of things to fix and a small remark but nothing to block it.

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2014-08-04 15:55:13 +0000
+++ src/maasserver/api.py	2014-08-05 09:45:38 +0000
@@ -486,11 +486,16 @@
         """Release a node.  Opposite of `NodesHandler.acquire`."""
         node = Node.objects.get_node_or_404(
             system_id=system_id, user=request.user, perm=NODE_PERMISSION.EDIT)
+        releasable_statuses = [
+            NODE_STATUS.ALLOCATED,
+            NODE_STATUS.RESERVED,
+            NODE_STATUS.BROKEN,
+            ]
         if node.status == NODE_STATUS.READY:
             # Nothing to do.  This may be a redundant retry, and the
             # postcondition is achieved, so call this success.
             pass
-        elif node.status in [NODE_STATUS.ALLOCATED, NODE_STATUS.RESERVED]:
+        elif node.status in releasable_statuses:
             node.release()
         else:
             raise NodeStateViolation(

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-08-04 14:29:05 +0000
+++ src/maasserver/models/node.py	2014-08-05 09:45:38 +0000
@@ -1207,7 +1207,13 @@
         self.delete_static_host_maps(deallocated_ips)
         from maasserver.dns import change_dns_zones
         change_dns_zones([self.nodegroup])
-        self.status = NODE_STATUS.READY
+        # Belt-and-braces: we should never reach a point where the node
+        # is BROKEN and still allocated, since mark_broken() releases
+        # the node anyway, but bug 1351451 seemed to suggest that it was
+        # possible. This can be removed if that proves not to be the
+        # case.
+        if self.status != NODE_STATUS.BROKEN:
+            self.status = NODE_STATUS.READY
         self.owner = None
         self.token = None
         self.agent_name = ''

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-07-31 15:32:52 +0000
+++ src/maasserver/models/tests/test_node.py	2014-08-05 09:45:38 +0000
@@ -620,6 +620,16 @@
             (NODE_STATUS.READY, None, node.agent_name),
             (node.status, node.owner, ''))
 
+    def test_release_does_not_mark_broken_node_ready(self):
+        agent_name = factory.make_name('agent-name')
+        node = factory.make_node(
+            status=NODE_STATUS.BROKEN, owner=factory.make_user(),
+            agent_name=agent_name)
+        node.release()
+        self.assertEqual(
+            (NODE_STATUS.BROKEN, None, node.agent_name),
+            (node.status, node.owner, ''))
+
     def test_release_deletes_static_ip_host_maps(self):
         user = factory.make_user()
         node = factory.make_node_with_mac_attached_to_nodegroupinterface(

=== modified file 'src/maasserver/tests/test_api_node.py'
--- src/maasserver/tests/test_api_node.py	2014-08-04 13:38:09 +0000
+++ src/maasserver/tests/test_api_node.py	2014-08-05 09:45:38 +0000
@@ -381,6 +381,18 @@
             [NODE_STATUS.READY] * len(owned_nodes),
             [node.status for node in reload_objects(Node, owned_nodes)])
 
+    def test_POST_release_releases_broken_node(self):
+        owned_node = factory.make_node(
+            owner=self.logged_in_user, status=NODE_STATUS.BROKEN)
+        response = self.client.post(
+            self.get_node_uri(owned_node), {'op': 'release'})
+        self.assertEqual(
+            httplib.OK, response.status_code, response.content)
+        owned_node = Node.objects.get(id=owned_node.id)
+        self.assertEqual(
+            (NODE_STATUS.BROKEN, None),
+            (owned_node.status, owned_node.owner))
+
     def test_POST_release_turns_on_netboot(self):
         node = factory.make_node(
             status=NODE_STATUS.ALLOCATED, owner=self.logged_in_user)
@@ -448,6 +460,7 @@
 
     def test_POST_release_fails_for_other_node_states(self):
         releasable_statuses = [
+            NODE_STATUS.BROKEN,
             NODE_STATUS.RESERVED,
             NODE_STATUS.ALLOCATED,
             NODE_STATUS.READY,