launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #979452 in MAAS: "Automatically commision nodes"
  https://bugs.launchpad.net/maas/+bug/979452

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

Fix bug 979452.  Changes:

 * auto commissioning with auto-enlistment
 * manual enlistment starts commissioning too
 * The "enlist" button on the node page has gone and there's now just the single action "accept & commission" instead.

pre-imp with Francis.
-- 
https://code.launchpad.net/~julian-edwards/maas/auto-commission/+merge/101682
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/maas/auto-commission into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-04-06 15:57:11 +0000
+++ src/maasserver/api.py	2012-04-12 05:05:22 +0000
@@ -491,7 +491,7 @@
         """
         node = create_node(request)
         if request.user.is_superuser:
-            node.accept_enlistment()
+            node.accept_enlistment(request.user)
         return node
 
     @api_exported('accept', 'POST')
@@ -529,7 +529,8 @@
                 "You don't have the required permission to accept the "
                 "following node(s): %s." % (
                     ', '.join(system_ids - ids)))
-        return filter(None, [node.accept_enlistment() for node in nodes])
+        return filter(
+            None, [node.accept_enlistment(request.user) for node in nodes])
 
     @api_exported('list', 'GET')
     def list(self, request):

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-04-12 04:21:18 +0000
+++ src/maasserver/forms.py	2012-04-12 05:05:22 +0000
@@ -272,13 +272,7 @@
 NODE_ACTIONS = {
     NODE_STATUS.DECLARED: [
         {
-            'display': "Accept Enlisted node",
-            'permission': NODE_PERMISSION.ADMIN,
-            'execute': lambda node, user: Node.accept_enlistment(node),
-            'message': "Node accepted into the pool."
-        },
-        {
-            'display': "Commission node",
+            'display': "Accept & commission",
             'permission': NODE_PERMISSION.ADMIN,
             'execute': lambda node, user: Node.start_commissioning(node, user),
             'message': "Node commissioning started."

=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py	2012-04-12 03:13:25 +0000
+++ src/maasserver/models.py	2012-04-12 05:05:22 +0000
@@ -583,7 +583,7 @@
         if mac:
             mac.delete()
 
-    def accept_enlistment(self):
+    def accept_enlistment(self, user):
         """Accept this node's (anonymous) enlistment.
 
         This call makes sense only on a node in Declared state, i.e. one that
@@ -595,10 +595,6 @@
         :return: This node if it has made the transition from Declared, or
             None if it was already in an accepted state.
         """
-        # Accepting a node puts it into Ready state.  This will change
-        # once we implement commissioning.
-        target_state = NODE_STATUS.READY
-
         accepted_states = [NODE_STATUS.READY, NODE_STATUS.COMMISSIONING]
         if self.status in accepted_states:
             return None
@@ -607,8 +603,7 @@
                 "Cannot accept node enlistment: node %s is in state %s."
                 % (self.system_id, NODE_STATUS_CHOICES_DICT[self.status]))
 
-        self.status = target_state
-        self.save()
+        self.start_commissioning(user)
         return self
 
     def start_commissioning(self, user):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-04-12 04:21:18 +0000
+++ src/maasserver/tests/test_api.py	2012-04-12 05:05:22 +0000
@@ -386,9 +386,8 @@
     # This is an actual test case that uses the EnlistmentAPITest mixin
     # and adds enlistement tests specific to admin users.
 
-    def test_POST_admin_creates_node_in_ready_state(self):
+    def test_POST_admin_creates_node_in_commissioning_state(self):
         # When an admin user enlists a node, it goes into the Ready state.
-        # This will change once we start doing proper commissioning.
         response = self.client.post(
             self.get_uri('nodes/'),
             {
@@ -401,7 +400,8 @@
         self.assertEqual(httplib.OK, response.status_code)
         system_id = json.loads(response.content)['system_id']
         self.assertEqual(
-            NODE_STATUS.READY, Node.objects.get(system_id=system_id).status)
+            NODE_STATUS.COMMISSIONING,
+            Node.objects.get(system_id=system_id).status)
 
     def test_POST_returns_limited_fields(self):
         response = self.client.post(
@@ -1121,7 +1121,7 @@
         # This will change when we add provisioning.  Until then,
         # acceptance gets a node straight to Ready state.
         self.become_admin()
-        target_state = NODE_STATUS.READY
+        target_state = NODE_STATUS.COMMISSIONING
 
         node = factory.make_node(status=NODE_STATUS.DECLARED)
         response = self.client.post(
@@ -1199,7 +1199,7 @@
         # This will change when we add provisioning.  Until then,
         # acceptance gets a node straight to Ready state.
         self.become_admin()
-        target_state = NODE_STATUS.READY
+        target_state = NODE_STATUS.COMMISSIONING
 
         nodes = [
             factory.make_node(status=NODE_STATUS.DECLARED)

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2012-04-12 04:30:48 +0000
+++ src/maasserver/tests/test_forms.py	2012-04-12 05:05:22 +0000
@@ -269,7 +269,7 @@
         form = get_action_form(admin)(node)
         actions = form.available_action_methods(node, admin)
         self.assertEqual(
-            ["Accept Enlisted node", "Commission node"],
+            ["Accept & commission"],
             [action['display'] for action in actions])
         # All permissions should be ADMIN.
         self.assertEqual(
@@ -304,7 +304,7 @@
         form = get_action_form(admin)(node)
 
         self.assertItemsEqual(
-            ["Accept Enlisted node", "Commission node"],
+            ["Accept & commission"],
             form.action_dict)
 
     def test_get_action_form_for_user(self):
@@ -320,10 +320,10 @@
         admin = factory.make_admin()
         node = factory.make_node(status=NODE_STATUS.DECLARED)
         form = get_action_form(admin)(
-            node, {NodeActionForm.input_name: "Accept Enlisted node"})
+            node, {NodeActionForm.input_name: "Accept & commission"})
         form.save()
 
-        self.assertEqual(NODE_STATUS.READY, node.status)
+        self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
 
     def test_get_action_form_for_user_save(self):
         user = factory.make_user()

=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py	2012-04-12 03:17:17 +0000
+++ src/maasserver/tests/test_models.py	2012-04-12 05:05:22 +0000
@@ -207,13 +207,10 @@
     def test_accept_enlistment_gets_node_out_of_declared_state(self):
         # If called on a node in Declared state, accept_enlistment()
         # changes the node's status, and returns the node.
-
-        # This will change when we add commissioning.  Until then,
-        # acceptance gets a node straight to Ready state.
-        target_state = NODE_STATUS.READY
+        target_state = NODE_STATUS.COMMISSIONING
 
         node = factory.make_node(status=NODE_STATUS.DECLARED)
-        return_value = node.accept_enlistment()
+        return_value = node.accept_enlistment(factory.make_user())
         self.assertEqual((node, target_state), (return_value, node.status))
 
     def test_accept_enlistment_does_nothing_if_already_accepted(self):
@@ -229,7 +226,7 @@
             for status in accepted_states}
 
         return_values = {
-            status: node.accept_enlistment()
+            status: node.accept_enlistment(factory.make_user())
             for status, node in nodes.items()}
 
         self.assertEqual(
@@ -257,7 +254,7 @@
         exceptions = {status: False for status in unacceptable_states}
         for status, node in nodes.items():
             try:
-                node.accept_enlistment()
+                node.accept_enlistment(factory.make_user())
             except NodeStateViolation:
                 exceptions[status] = True
 

=== modified file 'src/maasserver/tests/test_views.py'
--- src/maasserver/tests/test_views.py	2012-04-12 04:21:18 +0000
+++ src/maasserver/tests/test_views.py	2012-04-12 05:05:22 +0000
@@ -661,33 +661,6 @@
         self.assertEqual(httplib.FOUND, response.status_code)
         self.assertAttributes(node, params)
 
-    def test_view_node_admin_has_button_to_accept_enlistement(self):
-        self.become_admin()
-        node = factory.make_node(status=NODE_STATUS.DECLARED)
-        node_link = reverse('node-view', args=[node.system_id])
-        response = self.client.get(node_link)
-        doc = fromstring(response.content)
-        inputs = [
-            input for input in doc.cssselect('form#node_actions input')
-            if input.name == NodeActionForm.input_name]
-
-        self.assertIn(
-            "Accept Enlisted node", [input.value for input in inputs])
-
-    def test_view_node_POST_admin_can_enlist_node(self):
-        self.become_admin()
-        node = factory.make_node(status=NODE_STATUS.DECLARED)
-        node_link = reverse('node-view', args=[node.system_id])
-        response = self.client.post(
-            node_link,
-            data={
-                NodeActionForm.input_name: "Accept Enlisted node",
-            })
-
-        self.assertEqual(httplib.FOUND, response.status_code)
-        self.assertEqual(
-            NODE_STATUS.READY, reload_object(node).status)
-
     def test_view_node_has_button_to_accept_enlistement_for_user(self):
         # A simple user can't see the button to enlist a declared node.
         node = factory.make_node(status=NODE_STATUS.DECLARED)
@@ -722,7 +695,7 @@
         response = self.client.post(
             node_link,
             data={
-                NodeActionForm.input_name: "Commission node",
+                NodeActionForm.input_name: "Accept & commission",
             })
         self.assertEqual(httplib.FOUND, response.status_code)
         self.assertEqual(
@@ -738,20 +711,11 @@
         response = self.client.get(node_link)
         return response
 
-    def test_enlist_action_displays_message(self):
-        self.become_admin()
-        node = factory.make_node(status=NODE_STATUS.DECLARED)
-        response = self.perform_action_and_get_node_page(
-            node, "Accept Enlisted node")
-        self.assertEqual(
-            ["Node accepted into the pool."],
-            [message.message for message in response.context['messages']])
-
     def test_start_commisionning_displays_message(self):
         self.become_admin()
         node = factory.make_node(status=NODE_STATUS.DECLARED)
         response = self.perform_action_and_get_node_page(
-            node, "Commission node")
+            node, "Accept & commission")
         self.assertEqual(
             ["Node commissioning started."],
             [message.message for message in response.context['messages']])