Merge lp:~gmb/maas/start-and-allocate-bug-1365591 into lp:~maas-committers/maas/trunk

[Graham Binns]

The "Start node" button in the UI now:
 - Also acquires the node for the user if it hasn't already been acquired.
 - Displays "Acquire and start node" if the node isn't allocated to the user, and "Start node" if it is.

Per comments on bug 1365591, I considered adding "(with default options)" to the button label, but until bug 1379187 is fixed, this is kind of meaningless and confusing.

[Gavin Panella (allenap)]

review: Approve

[Raphaël Badin (rvb)]

I think there is a small problem with this branch: you've implemented this in the UI code and thus if you try to 'start' a non-allocated from the API, it will fail. I think this difference is bad because you'll get two different behavior depending on where you perform the same action (UI vs. API).

Maybe you can salvage something from this: https://code.launchpad.net/~rvb/maas/bug-1365591-transparent-allocation/+merge/237778 (complete WIP that I started last week but didn't finish yet)

[Raphaël Badin (rvb)]

> I think this difference is bad because you'll get two different behavior depending on where you perform the same action (UI vs. API).

On second thought, this is wrong: this branch is adding a shortcut that we're free to decide not to add to the API.

review: Approve

[Julian Edwards (julian-edwards)]

On Monday 13 Oct 2014 10:44:27 you wrote:
> + @property
> + def display(self):
> + if self.node.owner == self.user:
> + label = "Start node"
> + else:
> + label = "Acquire and start node"
> + return label

Can we ever get in the situation where self.node.owner is set but is != to
self.user?

[Graham Binns (gmb)]

On 14 October 2014 00:12, Julian Edwards <email address hidden> wrote:
>
> Can we ever get in the situation where self.node.owner is set but is != to
> self.user?

Not one where this button will still be displayed, AFAICT, but I'll
double-check.

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2014-10-08 22:56:31 +0000
+++ src/maasserver/node_action.py	2014-10-13 10:43:54 +0000
@@ -304,12 +304,19 @@
 class StartNode(NodeAction):
     """Start a node."""
     name = "start"
-    display = "Start node"
     display_bulk = "Start selected nodes"
     actionable_statuses = (
-        NODE_STATUS.ALLOCATED, NODE_STATUS.DEPLOYED)
+        NODE_STATUS.READY, NODE_STATUS.ALLOCATED, NODE_STATUS.DEPLOYED)
     permission = NODE_PERMISSION.EDIT
 
+    @property
+    def display(self):
+        if self.node.owner == self.user:
+            label = "Start node"
+        else:
+            label = "Acquire and start node"
+        return label
+
     def inhibit(self):
         """The user must have an SSH key, so that they access the node."""
         if not SSHKey.objects.get_keys_for_user(self.user).exists():
@@ -323,6 +330,9 @@
 
     def execute(self, allow_redirect=True):
         """See `NodeAction.execute`."""
+        if self.node.owner is None:
+            self.node.acquire(self.user, token=None)
+
         try:
             Node.objects.start_nodes([self.node.system_id], self.user)
         except StaticIPAddressExhaustion:

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2014-10-08 09:43:51 +0000
+++ src/maasserver/tests/test_node_action.py	2014-10-13 10:43:54 +0000
@@ -338,6 +338,31 @@
             user.has_perm(NODE_PERMISSION.EDIT, node))
         self.assertFalse(StartNode(node, user).is_permitted())
 
+    def test_StartNode_allocates_node_if_node_not_already_allocated(self):
+        self.patch(Node.objects, "start_nodes")
+        user = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.READY)
+        action = StartNode(node, user)
+        action.execute()
+
+        self.assertEqual(user, node.owner)
+        self.assertEqual(NODE_STATUS.ALLOCATED, node.status)
+
+    def test_StartNode_label_shows_allocate_if_unallocated(self):
+        self.patch(Node.objects, "start_nodes")
+        user = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.READY)
+        action = StartNode(node, user)
+        self.assertEqual("Acquire and start node", action.display)
+
+    def test_StartNode_label_hids_allocate_if_allocated(self):
+        self.patch(Node.objects, "start_nodes")
+        user = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.READY)
+        node.acquire(user)
+        action = StartNode(node, user)
+        self.assertEqual("Start node", action.display)
+
 
 class TestStopNodeNodeAction(MAASServerTestCase):