launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/enlistment-state into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/enlistment-state/+merge/99865

When nodes are “created” in MAAS (which equates to “enlisted” when seen from outside MAAS), they are immediately made available for deployment.  They are created in the Ready state.

Deployment involves wiping the disks, so we had better make sure that the system's owners approve.  There's no concern when a logged-in MAAS user creates a node, but there is one when a node is enlisted anonymously.  We'd expect that to happen at the node's own request, but we can't be sure.

This change puts nodes that are enlisted anonymously in the Declared state, whence a MAAS user will in the future be able to kick it into the Ready state.  The Declared state already exists, and all descriptions I could find (in the source tree and in design documents) fit this use perfectly.  I'm not entirely sure this is exactly what the state was originally conceived for, so I'll document my considerations.

Things are clear-cut in the current situation: anonymous enlistment creates node in Declared state, approval promotes it to Ready state, and acquisition moves it into Allocated state.  Non-anonymous enlistment implicitly includes approval in the enlistment step: enlistment creates node in Ready state, acquisition moves it into Allocated state.

But in the future we intend to perform commissioning steps as well.  This process will also wipe disks.  The Declared state may have been conceived as a state for a node that's been enlisted but, for whatever reason, hasn't started this commissioning process yet.  Once we incorporate the attached branch, the progression would be: anonymous enlistment creates node in Declared state, approval starts the commissioning process and thus puts the node in Commissioning state; and once that completes, the node becomes Ready.  Non-anonymous enlistment again includes approval: enlistment creates node in Commissioning state, completion promotes it to Ready, and acquisition makes it Allocated.

If it turns out that we don't want commissioning to start right away, or there is some technical reason why we need an intermediate state, we'll have to come up with a new status with a name that fits the situation.  Should we get tired of the cliché words Nascent and Incipient, there's still Inchoate.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/enlistment-state/+merge/99865
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/enlistment-state into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-03-27 10:40:40 +0000
+++ src/maasserver/api.py	2012-03-29 04:19:19 +0000
@@ -353,10 +353,22 @@
         return node
 
 
-def create_node(request):
+def create_node(request, initial_status):
+    """Service an http request to create a node.
+
+    :param request: The http request for this node to be created.
+    :param initial_status: The state the new node should be in.
+    :type initial_status: :class:`NODE_STATUS`
+    :return: A suitable return value for the handler receiving the "new"
+        request that this implements.
+    :rtype: :class:`maasserver.models.Node` or
+        :class:`django.http.HttpResponseBadRequest`.
+    """
     form = NodeWithMACAddressesForm(request.data)
     if form.is_valid():
         node = form.save()
+        node.status = initial_status
+        node.save()
         return node
     else:
         return HttpResponseBadRequest(
@@ -371,8 +383,14 @@
 
     @api_exported('new', 'POST')
     def new(self, request):
-        """Create a new Node."""
-        return create_node(request)
+        """Create a new Node.
+
+        Adding a server to a MAAS puts it on a path that will wipe its disks
+        and re-install its operating system.  In anonymous enlistment,
+        therefore, the node is held in the "Declared" state for approval by a
+        MAAS user.
+        """
+        return create_node(request, NODE_STATUS.DECLARED)
 
     @classmethod
     def resource_uri(cls, *args, **kwargs):
@@ -402,8 +420,12 @@
 
     @api_exported('new', 'POST')
     def new(self, request):
-        """Create a new Node."""
-        return create_node(request)
+        """Create a new Node.
+
+        When a node has been added to MAAS by a logged-in MAAS user, it is
+        ready for allocation to services running on the MAAS.
+        """
+        return create_node(request, NODE_STATUS.READY)
 
     @api_exported('list', 'GET')
     def list(self, request):

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-03-27 10:40:40 +0000
+++ src/maasserver/tests/test_api.py	2012-03-29 04:19:19 +0000
@@ -118,6 +118,25 @@
         self.assertEqual(2, diane.after_commissioning_action)
         self.assertEqual(architecture, diane.architecture)
 
+    def test_POST_new_anonymous_creates_node_in_declared_state(self):
+        # Upon anonymous enlistment, a node goes into the Declared
+        # state.  Deliberate approval is required before we start
+        # reinstalling the system, wiping its disks etc.
+        response = self.client.post(
+            self.get_uri('nodes/'),
+            {
+                'op': 'new',
+                'hostname': factory.getRandomString(),
+                'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+                'after_commissioning_action': '2',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+        system_id = json.loads(response.content)['system_id']
+        self.assertEqual(
+            NODE_STATUS.DECLARED,
+            Node.objects.get(system_id=system_id).status)
+
     def test_POST_new_power_type_defaults_to_asking_config(self):
         architecture = factory.getRandomChoice(ARCHITECTURE_CHOICES)
         response = self.client.post(
@@ -645,6 +664,24 @@
 
         self.assertEqual(httplib.OK, response.status_code)
 
+    def test_POST_new_when_logged_in_creates_node_in_ready_state(self):
+        # When a logged-in 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/'),
+            {
+                'op': 'new',
+                'hostname': factory.getRandomString(),
+                'architecture': factory.getRandomChoice(ARCHITECTURE_CHOICES),
+                'after_commissioning_action': '2',
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+            })
+        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)
+
     def test_GET_list_lists_nodes(self):
         # The api allows for fetching the list of Nodes.
         node1 = factory.make_node()
@@ -1344,7 +1381,7 @@
         error_message = factory.getRandomString()
 
         # Monkey patch api.create_node to have it raise a RuntimeError.
-        def raise_exception(request):
+        def raise_exception(*args, **kwargs):
             raise RuntimeError(error_message)
         self.patch(api, 'create_node', raise_exception)
         response = self.client.post(self.get_uri('nodes/'), {'op': 'new'})