launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/arch_constraint into lp:maas.

Commit message:
Expose 'arch' as a constraint that can be used to select nodes in get_available_node_for_acquisition.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/arch_constraint/+merge/125979

Ignore this branch, it is just one of ~gz's approved branches that needed a trivial merge-conflict fix.
-- 
https://code.launchpad.net/~jameinel/maas/arch_constraint/+merge/125979
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jameinel/maas/arch_constraint into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-09-21 16:36:27 +0000
+++ src/maasserver/api.py	2012-09-24 11:28:20 +0000
@@ -645,11 +645,10 @@
     :return: A mapping of applicable constraint names to their values.
     :rtype: :class:`dict`
     """
-    name = request_params.get('name', None)
-    if name is None:
-        return {}
-    else:
-        return {'name': name}
+    supported_constraints = ('name', 'arch')
+    return {constraint: request_params[constraint]
+        for constraint in supported_constraints
+            if constraint in request_params}
 
 
 @api_operations

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-09-24 09:00:41 +0000
+++ src/maasserver/models/node.py	2012-09-24 11:28:20 +0000
@@ -253,6 +253,11 @@
         if constraints.get('name'):
             available_nodes = available_nodes.filter(
                 hostname=constraints['name'])
+        if constraints.get('arch'):
+            # GZ 2012-09-11: This only supports an exact match on arch type,
+            #                using an i386 image on amd64 hardware will wait.
+            available_nodes = available_nodes.filter(
+                architecture=constraints['arch'])
 
         return get_first(available_nodes)
 

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-09-21 16:36:27 +0000
+++ src/maasserver/tests/test_api.py	2012-09-24 11:28:20 +0000
@@ -1663,17 +1663,6 @@
         self.assertEqual(
             node.hostname, json.loads(response.content)['hostname'])
 
-    def test_POST_acquire_constrains_by_name(self):
-        # Negative test for name constraint.
-        # If a name constraint is given, "acquire" will only consider a
-        # node with that name.
-        factory.make_node(status=NODE_STATUS.READY, owner=None)
-        response = self.client.post(self.get_uri('nodes/'), {
-            'op': 'acquire',
-            'name': factory.getRandomString(),
-        })
-        self.assertEqual(httplib.CONFLICT, response.status_code)
-
     def test_POST_acquire_treats_unknown_name_as_resource_conflict(self):
         # A name constraint naming an unknown node produces a resource
         # conflict: most likely the node existed but has changed or
@@ -1687,6 +1676,27 @@
         })
         self.assertEqual(httplib.CONFLICT, response.status_code)
 
+    def test_POST_acquire_allocates_node_by_arch(self):
+        # Asking for a particular arch acquires a node with that arch.
+        node = factory.make_node(
+            status=NODE_STATUS.READY, architecture=ARCHITECTURE.i386)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'arch': 'i386',
+        })
+        self.assertEqual(httplib.OK, response.status_code)
+        response_json = json.loads(response.content)
+        self.assertEqual(node.architecture, response_json['architecture'])
+
+    def test_POST_acquire_treats_unknown_arch_as_resource_conflict(self):
+        # Asking for an unknown arch returns an HTTP conflict
+        factory.make_node(status=NODE_STATUS.READY)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'arch': 'sparc',
+        })
+        self.assertEqual(httplib.CONFLICT, response.status_code)
+
     def test_POST_acquire_sets_a_token(self):
         # "acquire" should set the Token being used in the request on
         # the Node that is allocated.

=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2012-09-24 09:00:41 +0000
+++ src/maasserver/tests/test_node.py	2012-09-24 11:28:20 +0000
@@ -20,6 +20,7 @@
     ValidationError,
     )
 from maasserver.enum import (
+    ARCHITECTURE,
     DISTRO_SERIES,
     NODE_PERMISSION,
     NODE_STATUS,
@@ -635,7 +636,8 @@
             Node.objects.get_available_node_for_acquisition(
                 user, {'name': node.system_id}))
 
-    def test_get_available_node_constrains_by_name(self):
+    def test_get_available_node_with_name(self):
+        """A single available node can be selected using its hostname"""
         user = factory.make_user()
         nodes = [self.make_node() for counter in range(3)]
         self.assertEqual(
@@ -643,13 +645,32 @@
             Node.objects.get_available_node_for_acquisition(
                 user, {'name': nodes[1].hostname}))
 
-    def test_get_available_node_returns_None_if_name_is_unknown(self):
+    def test_get_available_node_with_unknown_name(self):
+        """None is returned if there is no node with a given name"""
         user = factory.make_user()
         self.assertEqual(
             None,
             Node.objects.get_available_node_for_acquisition(
                 user, {'name': factory.getRandomString()}))
 
+    def test_get_available_node_with_arch(self):
+        """An available node can be selected of a given architecture"""
+        user = factory.make_user()
+        nodes = [self.make_node(architecture=s)
+            for s in (ARCHITECTURE.amd64, ARCHITECTURE.i386)]
+        available_node = Node.objects.get_available_node_for_acquisition(
+                user, {'arch': "i386"})
+        self.assertEqual(ARCHITECTURE.i386, available_node.architecture)
+        self.assertEqual(nodes[1], available_node)
+
+    def test_get_available_node_with_unknown_arch(self):
+        """None is returned if an arch not used by MaaS is given"""
+        user = factory.make_user()
+        self.assertEqual(
+            None,
+            Node.objects.get_available_node_for_acquisition(
+                user, {'arch': "sparc"}))
+
     def test_stop_nodes_stops_nodes(self):
         # We don't actually want to fire off power events, but we'll go
         # through the motions right up to the point where we'd normally