launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/maas/arch_constraint into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

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

Experiment with adding another constraint to MaaS using the existing architecture field stored on Node. This probably isn't much use, especially as it doesn't know you can fulfil a 'i386' request by sticking an i386 image on an amd64 box.
-- 
https://code.launchpad.net/~gz/maas/arch_constraint/+merge/123789
Your team MAAS Maintainers is requested to review the proposed merge of lp:~gz/maas/arch_constraint into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-09-10 14:42:52 +0000
+++ src/maasserver/api.py	2012-09-11 16:57:27 +0000
@@ -616,11 +616,12 @@
     :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}
+    constraints = {}
+    for k in ('name', 'arch'):
+        v = request_params.get(k, None)
+        if v is not None:
+            constraints[k] = v
+    return constraints
 
 
 @api_operations

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-08-24 10:45:47 +0000
+++ src/maasserver/models/node.py	2012-09-11 16:57:27 +0000
@@ -249,6 +249,11 @@
         if constraints.get('name'):
             available_nodes = available_nodes.filter(
                 hostname=constraints['name'])
+        if constraints.get('arch'):
+            # GZ 2012-09-11: Need more logic here when picking an image works,
+            #                as can satisfy i386 with amd64 and so on.
+            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-10 14:27:00 +0000
+++ src/maasserver/tests/test_api.py	2012-09-11 16:57:27 +0000
@@ -1587,17 +1587,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
@@ -1611,6 +1600,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-08-24 10:28:29 +0000
+++ src/maasserver/tests/test_node.py	2012-09-11 16:57:27 +0000
@@ -20,6 +20,7 @@
     ValidationError,
     )
 from maasserver.enum import (
+    ARCHITECTURE,
     NODE_PERMISSION,
     NODE_STATUS,
     NODE_STATUS_CHOICES,
@@ -616,7 +617,7 @@
             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_constraints_by_name(self):
         user = factory.make_user()
         nodes = [self.make_node() for counter in range(3)]
         self.assertEqual(
@@ -631,6 +632,22 @@
             Node.objects.get_available_node_for_acquisition(
                 user, {'name': factory.getRandomString()}))
 
+    def test_get_available_node_constraints_by_arch(self):
+        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_returns_None_if_arch_is_unknown(self):
+        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