Merge lp:~allenap/maas/delete-failing-commissioning-node-bug-994781 into lp:~maas-committers/maas/trunk

[Gavin Panella]

Only inhibit node deletion when allocated.

Jeroen mentioned basically what the problem was this morning during the call.

[Jeroen T. Vermeulen (jtv)]

I wonder if we shouldn't also allow a user to delete nodes allocated _to themselves_. “There. It's running fine now. I want to keep it like this, without further control from MAAS.”

review: Approve

[Gavin Panella (allenap)]

On 9 May 2012 10:22, Jeroen T. Vermeulen <email address hidden> wrote:
> Review: Approve
>
> I wonder if we shouldn't also allow a user to delete nodes allocated
> _to themselves_.  “There.  It's running fine now.  I want to keep it
> like this, without further control from MAAS.”

That's an interesting idea... I've filed bug 997092 about it because I
think it needs at least a little discussion.

Thank you for the review!

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2012-04-27 13:55:54 +0000
+++ src/maasserver/node_action.py	2012-05-08 16:24:22 +0000
@@ -129,7 +129,7 @@
     permission = NODE_PERMISSION.ADMIN
 
     def inhibit(self):
-        if self.node.owner is not None:
+        if self.node.status == NODE_STATUS.ALLOCATED:
             return "You cannot delete this node because it's in use."
         return None
 

=== modified file 'src/maasserver/templates/maasserver/node_view.html'
--- src/maasserver/templates/maasserver/node_view.html	2012-04-25 16:58:45 +0000
+++ src/maasserver/templates/maasserver/node_view.html	2012-05-08 16:24:22 +0000
@@ -17,7 +17,9 @@
     <form id="node_actions" method="post" action=".">
       {% for action in form.action_buttons %}
         <input
-          class="secondary {% if action.inhibition %}disabled{% endif %} {% if not forloop.first %}space-top{% endif %}"
+          class="secondary
+                 {% if action.inhibition %}disabled{% endif %}
+                 {% if not forloop.first %}space-top{% endif %}"
           type="submit"
           name="{{ form.input_name }}"
           value="{{ action.display }}"

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2012-04-27 13:55:54 +0000
+++ src/maasserver/tests/test_node_action.py	2012-05-08 16:24:22 +0000
@@ -162,20 +162,19 @@
         action.fake_inhibition = factory.getRandomString()
         self.assertIsNone(action.inhibition)
 
-    def test_Delete_inhibit_allows_if_node_has_no_owner(self):
-        unowned_node = factory.make_node(status=NODE_STATUS.READY)
-        self.assertIsNone(
-            Delete(unowned_node, factory.make_admin()).inhibit())
-
-    def test_Delete_inhibit_disallows_if_node_has_owner(self):
-        owned_node = factory.make_node(
-            status=NODE_STATUS.ALLOCATED, owner=factory.make_user())
-        action = Delete(owned_node, factory.make_admin())
+    def test_Delete_inhibit_when_node_is_allocated(self):
+        node = factory.make_node(status=NODE_STATUS.ALLOCATED)
+        action = Delete(node, factory.make_admin())
         inhibition = action.inhibit()
-        self.assertIsNotNone(inhibition)
         self.assertEqual(
             "You cannot delete this node because it's in use.", inhibition)
 
+    def test_Delete_does_not_inhibit_otherwise(self):
+        node = factory.make_node(status=NODE_STATUS.FAILED_TESTS)
+        action = Delete(node, factory.make_admin())
+        inhibition = action.inhibit()
+        self.assertIsNone(inhibition)
+
     def test_Delete_redirects_to_node_delete_view(self):
         node = factory.make_node()
         action = Delete(node, factory.make_admin())