launchpad-reviewers team mailing list archive

[Robie Basak <robie.basak@xxxxxxxxxxxxx>]

Robie Basak has proposed merging lp:~racb/maas/arch-constraint-wildcard into lp:maas.

Commit message:
Add automatic architecture constraint wildcarding and arm alias

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~racb/maas/arch-constraint-wildcard/+merge/128513
-- 
https://code.launchpad.net/~racb/maas/arch-constraint-wildcard/+merge/128513
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~racb/maas/arch-constraint-wildcard into lp:maas.

=== modified file 'src/maasserver/models/node_constraint_filter.py'
--- src/maasserver/models/node_constraint_filter.py	2012-10-04 10:52:31 +0000
+++ src/maasserver/models/node_constraint_filter.py	2012-10-08 15:00:45 +0000
@@ -12,8 +12,10 @@
     'constrain_nodes',
     ]
 
+import itertools
 import math
 
+from maasserver.enum import ARCHITECTURE_CHOICES
 from maasserver.exceptions import (
     InvalidConstraint,
     )
@@ -51,12 +53,66 @@
     return nodes
 
 
+def generate_architecture_wildcards(choices=ARCHITECTURE_CHOICES):
+    """Map 'primary' architecture names to a list of full expansions.
+
+    Return a dictionary keyed by the primary architecture name (the part before
+    the '/'). The value of an entry is a frozenset of full architecture names
+    ('primary_arch/subarch') under the keyed primary architecture.
+
+    """
+
+    sorted_arch_list = sorted(choice[0] for choice in choices)
+
+    def extract_primary_arch(arch):
+        return arch.split('/')[0]
+
+    return {
+        primary_arch: frozenset(subarch_generator)
+        for primary_arch, subarch_generator in itertools.groupby(
+            sorted_arch_list, key=extract_primary_arch
+        )
+    }
+
+
+architecture_wildcards = generate_architecture_wildcards()
+
+
+# juju uses a general "arm" architecture constraint across all of its
+# providers. Since armhf is the cross-distro agreed Linux userspace
+# architecture and ABI, interpret "arm" to mean "armhf" in MAAS.
+#
+# Aliases cannot currently be recursive
+primary_architecture_aliases = {'arm': 'armhf'}
+
+
+def constrain_architecture(nodes, key, value):
+    assert(key == 'architecture')
+
+    # Replace an alias with its value if it is an alias
+    try:
+        aliased_value = primary_architecture_aliases[value]
+    except KeyError:
+        aliased_value = value
+
+    if aliased_value in (choice[0] for choice in ARCHITECTURE_CHOICES):
+        # Full 'arch/subarch' specified directly
+        return nodes.filter(architecture=aliased_value)
+    elif aliased_value in architecture_wildcards:
+        # Try to expand 'arch' to all available 'arch/subarch' matches
+        return nodes.filter(
+            architecture__in=architecture_wildcards[aliased_value])
+    else:
+        raise InvalidConstraint(
+            'architecture', value, 'Architecture not recognised')
+
+
 # this is the mapping of constraint names to how to apply the constraint
 constraint_filters = {
     # Currently architecture only supports exact matching. Eventually, we will
     # probably want more logic to note that eg, amd64 can be used for an i386
     # request
-    'architecture': constrain_identical,
+    'architecture': constrain_architecture,
     'hostname': constrain_identical,
     'cpu_count': constrain_int_greater_or_equal,
     'memory': constrain_int_greater_or_equal,

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-10-08 11:46:58 +0000
+++ src/maasserver/tests/test_api.py	2012-10-08 15:00:45 +0000
@@ -1914,14 +1914,14 @@
         response_json = json.loads(response.content)
         self.assertEqual(node.architecture, response_json['architecture'])
 
-    def test_POST_acquire_treats_unknown_arch_as_resource_conflict(self):
+    def test_POST_acquire_treats_unknown_arch_as_bad_request(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)
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
 
     def test_POST_acquire_allocates_node_by_cpu(self):
         # Asking for enough cpu acquires a node with at least that.

=== modified file 'src/maasserver/tests/test_node_constraint_filter.py'
--- src/maasserver/tests/test_node_constraint_filter.py	2012-10-04 10:52:31 +0000
+++ src/maasserver/tests/test_node_constraint_filter.py	2012-10-08 15:00:45 +0000
@@ -15,7 +15,10 @@
 from maasserver.enum import ARCHITECTURE
 from maasserver.exceptions import InvalidConstraint
 from maasserver.models import Node
-from maasserver.models.node_constraint_filter import constrain_nodes
+from maasserver.models.node_constraint_filter import (
+    constrain_nodes,
+    generate_architecture_wildcards,
+)
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from maasserver.utils import ignore_unused
@@ -27,6 +30,23 @@
         nodes = constrain_nodes(Node.objects.all(), constraints)
         self.assertItemsEqual(expected_nodes, nodes)
 
+    def test_generate_architecture_wildcards(self):
+        single_subarch = factory.getRandomString(), factory.getRandomString()
+        double_subarch_1 = factory.getRandomString(), factory.getRandomString()
+        double_subarch_2 = double_subarch_1[0], factory.getRandomString()
+        choices = (
+            ('/'.join(single_subarch), None),
+            ('/'.join(double_subarch_1), None),
+            ('/'.join(double_subarch_2), None),
+        )
+
+        self.assertEquals({
+            single_subarch[0]: frozenset([choices[0][0]]),
+            double_subarch_1[0]: frozenset([choices[1][0], choices[2][0]]),
+            },
+            generate_architecture_wildcards(choices=choices)
+        )
+
     def test_no_constraints(self):
         node1 = factory.make_node()
         node2 = factory.make_node()
@@ -43,10 +63,18 @@
     def test_architecture(self):
         node1 = factory.make_node(architecture=ARCHITECTURE.i386)
         node2 = factory.make_node(architecture=ARCHITECTURE.armhf_highbank)
+        self.assertConstrainedNodes([node1], {'architecture': 'i386'})
         self.assertConstrainedNodes([node1], {'architecture': 'i386/generic'})
         self.assertConstrainedNodes(
+            [node2], {'architecture': 'arm'})
+        self.assertConstrainedNodes(
+            [node2], {'architecture': 'armhf'})
+        self.assertConstrainedNodes(
             [node2], {'architecture': 'armhf/highbank'})
-        self.assertConstrainedNodes([], {'architecture': 'sparc'})
+        self.assertRaises(InvalidConstraint,
+            self.assertConstrainedNodes, [], {'architecture': 'armhf/generic'})
+        self.assertRaises(InvalidConstraint,
+            self.assertConstrainedNodes, [], {'architecture': 'sparc'})
 
     def test_cpu_count(self):
         node1 = factory.make_node(cpu_count=1)