Merge lp:~jameinel/maas/cpu_mem_tag_constraints into lp:maas/trunk

[John A Meinel]

Add CPU count, Memory and Tags as exposed logic for get_available_node_for_acquisition.

This is my version of gz`s branch: https://code.launchpad.net/~gz/maas/cpu_mem_tag_constraints/+merge/126502

Things were getting big enough that I evolved a helper class, which I think cleans things up and makes it easier to add more constraints in the future.

It also makes it more obvious to directly test that class.

And I changed the tag lookup logic, so it now gives InvalidConstraint rather than 404.

I *think* this helper class will even let us do the same constraints for a search interface, though we'll probably want to name it differently.

[Jeroen T. Vermeulen (jtv)]

As mentioned on IRC, the error message in InvalidConstraint seems misleading. As far as I can see, "Invalid 'X' constraint 'Y'" can mean that Y is not an acceptable value for constraint X. So far, so good. But it can also mean that X is not a known constraint in the first place! I would suggest either splitting these two meanings into separate exception classes, or differentiating the error message so that it will contain the constraint value only if relevant. (Presumably it wouldn't need to be a separate attribute on the exception class).

The AcquisitionConstrainer looks like it wants to be a module, not a class. Its self.nodes looks like it wants to be a parameter and return value to the various filter functions, rather than implicit mutable state. And looking up a computed method name can be a handy shorthand, but if you're duplicating the list of accepted constraints here anyway (you also had it in the constraints map), then you might as well make the lookup in an explicit dict:

def constrain_identical(nodes, key, value):
    """Generic constraint: filter nodes by exact value match."""
    return nodes.filter(key=value)

def constrain_int_gte(nodes, key, value):
    """Integer constraint: greater than or equal to `value`."""
    try:
        int_value = int(value)
    except ValueError as e:
        raise InvalidConstraint(key, value, e)
    return nodes.filter(**{"%s__gte" % key: value})

def constrain_tags(nodes, key, tag_expression):
    """Tags match: restrict to nodes that have all of given tags list."""
    # We support both comma-separated and space-separated tag lists.
    tag_names = tag_expression.replace(",", " ").strip().split()
    for tag_name in tag_names:
        tag = get_one(Tag.objects.filter(name=tag_name))
        if tag is None:
            raise InvalidConstraint('tags', tag_name, "No such tag.")
        nodes = nodes.filter(tags=tag)
    return nodes

# How to filter each individual constraint.
constraint_filters = {
    # This only supports an exact match on arch type. Using an i386 image
    # on amd64 hardware will wait.
    'architecture': constrain_identical,
    'name': constrain_identical,
    'cpu_count': constrain_int_gte,
    'memory': constrain_int_gte,
    'tags': constrain_tags,
}

def constrain_nodes(nodes, constraints):
    for constraint, value in constraints.items():
        filter = constraint_filters.get(constraint)
        if filter is not None:
            nodes = filter(nodes, constraint, value)
    return nodes

I think that's easier to follow, and despite having more documentation it's fewer lines as well.

(By the way, does an equality comparison on a tag value really do an "is in" comparison!? The tests verify it, but )

[Jeroen T. Vermeulen (jtv)]

OH THANK YOU VERY MUCH QUANTAL for submitting my notes before I was done entering them.

[Jeroen T. Vermeulen (jtv)]

I see that the API acceptance tests cover a lot of the same behaviours as the unit tests (and then there is some redundancy between unit tests as well). My guess is that the acceptance tests were written first, then the lower-level helpers were created to satisfy the tests, and those helpers were then unit-tested. I do that a lot as well, but I find it's usually best to strip down the acceptance tests to exercise integration paths only.

[John A Meinel (jameinel)]

A lot of the 'test_api' level tests are there to test the actual HTTP response codes wrt different errors. (So have 1 success level and 1 failure level test for each parameter based on different error codes.)

So I think the test coverage is reasonable as it stands, but we did switch things to using a module.

[Jeroen T. Vermeulen (jtv)]

Thanks for making the changes. I know I'm difficult.

review: Approve

[MAAS Lander (maas-lander)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-quantal/33/console reported an error when processing this lp:~jameinel/maas/cpu_mem_tag_constraints branch.
Not merging it.

[Martin Packman (gz)]

Fixed the test failure back in the branch under my namespace and landed it.

[Jeroen T. Vermeulen (jtv)]

Please, *always* run "make lint" and the full test suite before putting a

[Jeroen T. Vermeulen (jtv)]

branch up for review!

(Thank you Quantal for moving my mouse pointer to the Save Comment button and then clicking it while I was typing)

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 9/29/2012 8:07 AM, Jeroen T. Vermeulen wrote:
> branch up for review!
>
> (Thank you Quantal for moving my mouse pointer to the Save Comment
> button and then clicking it while I was typing)
>

Unfortunately, it looks like upgrading to Quantal broke my 'make lint':

ImportError: ..python2.7/lib-dynload/parser.so: undefined symbol:
_PyNode_Sizeof.

That seems to be from 'compiler' which means my py2.7 install is now
broken?

I have tried to make sure to run it. However, if we are going to be
pedantic about it, shouldn't we just have tarmac run it?

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Cygwin)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlBmfooACgkQJdeBCYSNAAPQrACfTTqPwHVjZ+m3PUUzcK4drAPb
OcsAnRWwmcSWGVs+8UuYZDILRmPCAVMk
=2dua
-----END PGP SIGNATURE-----

[Jeroen T. Vermeulen (jtv)]

That's been known to happen, but "make distclean" seems to fix it.

Having tarmac run it would be a good idea IMO.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-09-28 09:45:18 +0000
+++ src/maasserver/api.py	2012-09-28 11:10:27 +0000
@@ -643,6 +643,17 @@
         return ('nodes_handler', [])
 
 
+# Map the constraint as passed in the request to the name of the constraint in
+# the database. Many of them have the same name
+_constraints_map = {
+    'name': 'hostname',
+    'tags': 'tags',
+    'arch': 'architecture',
+    'cpu_count': 'cpu_count',
+    'mem': 'memory',
+    }
+
+
 def extract_constraints(request_params):
     """Extract a dict of node allocation constraints from http parameters.
 
@@ -651,10 +662,12 @@
     :return: A mapping of applicable constraint names to their values.
     :rtype: :class:`dict`
     """
-    supported_constraints = ('name', 'arch')
-    return {constraint: request_params[constraint]
-        for constraint in supported_constraints
-            if constraint in request_params}
+    constraints = {}
+    for request_name in _constraints_map:
+        if request_name in request_params:
+            db_name = _constraints_map[request_name]
+            constraints[db_name] = request_params[request_name]
+    return constraints
 
 
 @api_operations

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2012-08-21 20:14:12 +0000
+++ src/maasserver/exceptions.py	2012-09-28 11:10:27 +0000
@@ -84,6 +84,20 @@
     api_error = httplib.CONFLICT
 
 
+class InvalidConstraint(MAASAPIBadRequest):
+    """Node allocation constraint given cannot be interpreted."""
+
+    def __init__(self, constraint, value, err=None):
+        super(InvalidConstraint, self).__init__(constraint, value)
+        self.err = err
+
+    def __str__(self):
+        s = "Invalid '%s' constraint '%s'" % self.args
+        if self.err:
+            return "%s: %s" % (s, str(self.err))
+        return s
+
+
 class Redirect(MAASAPIException):
     """Redirect.  The exception message is the target URL."""
     api_error = httplib.FOUND

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-09-26 08:54:29 +0000
+++ src/maasserver/models/node.py	2012-09-28 11:10:27 +0000
@@ -250,21 +250,10 @@
         :type constraints: :class:`dict`
         :return: A matching `Node`, or None if none are available.
         """
-        if constraints is None:
-            constraints = {}
-        available_nodes = (
-            self.get_nodes(for_user, NODE_PERMISSION.VIEW)
-                .filter(status=NODE_STATUS.READY))
-
-        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'])
-
+        from maasserver.models.node_constraint_filter import constrain_nodes
+        available_nodes = self.get_nodes(for_user, NODE_PERMISSION.VIEW)
+        available_nodes = available_nodes.filter(status=NODE_STATUS.READY)
+        available_nodes = constrain_nodes(available_nodes, constraints)
         return get_first(available_nodes)
 
     def stop_nodes(self, ids, by_user):

=== added file 'src/maasserver/models/node_constraint_filter.py'
--- src/maasserver/models/node_constraint_filter.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/models/node_constraint_filter.py	2012-09-28 11:10:27 +0000
@@ -0,0 +1,77 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'constrain_nodes',
+    ]
+
+from maasserver.exceptions import (
+    InvalidConstraint,
+    )
+from maasserver.models import Tag
+from maasserver.utils.orm import get_one
+
+
+def constrain_identical(nodes, key, value):
+    """Match the field 'key' to exactly match 'value'"""
+    return nodes.filter(**{key: value})
+
+
+def constrain_int_greater_or_equal(nodes, key, str_value):
+    """Filter nodes that have value >= supplied value.
+
+    :param str_value: Will be cast to an integer, if this fails, it is treated
+        as an invalid constraint.
+    """
+    try:
+        int_value = int(str_value)
+    except ValueError as e:
+        raise InvalidConstraint(key, str_value, e)
+    return nodes.filter(**{'%s__gte' % (key,): int_value})
+
+
+def constrain_tags(nodes, key, tag_expression):
+    """Tags match: restrict to nodes that have all tags."""
+    # We use ',' separated or space ' ' separated values.
+    tag_names = tag_expression.replace(",", " ").strip().split()
+    for tag_name in tag_names:
+        tag = get_one(Tag.objects.filter(name=tag_name))
+        if tag is None:
+            raise InvalidConstraint('tags', tag_name, 'No such tag')
+        nodes = nodes.filter(tags=tag)
+    return nodes
+
+
+# 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,
+    'hostname': constrain_identical,
+    'cpu_count': constrain_int_greater_or_equal,
+    'memory': constrain_int_greater_or_equal,
+    'tags': constrain_tags,
+}
+
+
+def constrain_nodes(nodes, constraints):
+    """Apply the dict of constraints as filters against nodes.
+
+    :param nodes: A QuerySet against nodes
+    :param constraints: A dict of {'constraint': 'value'}
+    """
+    if not constraints:
+        return nodes
+    for constraint_name, value in constraints.items():
+        filter = constraint_filters.get(constraint_name)
+        if filter is not None:
+            nodes = filter(nodes, constraint_name, value)
+    return nodes

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-09-28 07:31:54 +0000
+++ src/maasserver/tests/test_api.py	2012-09-28 11:10:27 +0000
@@ -829,6 +829,11 @@
         self.logged_in_user.is_superuser = True
         self.logged_in_user.save()
 
+    def assertResponseCode(self, expected_code, response):
+        if response.status_code != expected_code:
+            self.fail("Expected %s response, got %s:\n%s" % (
+                expected_code, response.status_code, response.content))
+
 
 def extract_system_ids(parsed_result):
     """List the system_ids of the nodes in `parsed_result`."""
@@ -1713,6 +1718,81 @@
         })
         self.assertEqual(httplib.CONFLICT, response.status_code)
 
+    def test_POST_acquire_allocates_node_by_cpu(self):
+        # Asking for enough cpu acquires a node with at least that.
+        node = factory.make_node(status=NODE_STATUS.READY, cpu_count=3)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'cpu_count': 2,
+        })
+        self.assertResponseCode(httplib.OK, response)
+        response_json = json.loads(response.content)
+        self.assertEqual(node.system_id, response_json['system_id'])
+
+    def test_POST_acquire_fails_with_invalid_cpu(self):
+        # Asking for an invalid amount of cpu returns a bad request.
+        factory.make_node(status=NODE_STATUS.READY)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'cpu_count': 'plenty',
+        })
+        self.assertResponseCode(httplib.BAD_REQUEST, response)
+
+    def test_POST_acquire_allocates_node_by_mem(self):
+        # Asking for enough memory acquires a node with at least that.
+        node = factory.make_node(status=NODE_STATUS.READY, memory=1024)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'mem': 1024,
+        })
+        self.assertResponseCode(httplib.OK, response)
+        response_json = json.loads(response.content)
+        self.assertEqual(node.system_id, response_json['system_id'])
+
+    def test_POST_acquire_fails_with_invalid_mem(self):
+        # Asking for an invalid amount of cpu returns a bad request.
+        factory.make_node(status=NODE_STATUS.READY)
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'mem': 'bags',
+        })
+        self.assertResponseCode(httplib.BAD_REQUEST, response)
+
+    def test_POST_acquire_allocates_node_by_tags(self):
+        # Asking for particular tags acquires a node with those tags.
+        node = factory.make_node(status=NODE_STATUS.READY)
+        node_tag_names = ["fast", "stable", "cute"]
+        node.tags = [factory.make_tag(t) for t in node_tag_names]
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'tags': 'fast, stable',
+        })
+        self.assertResponseCode(httplib.OK, response)
+        response_json = json.loads(response.content)
+        self.assertEqual(node_tag_names, response_json['tag_names'])
+
+    def test_POST_acquire_fails_without_all_tags(self):
+        # Asking for particular tags does not acquire if no node has all tags.
+        node1 = factory.make_node(status=NODE_STATUS.READY)
+        node1.tags = [factory.make_tag(t) for t in ("fast", "stable", "cute")]
+        node2 = factory.make_node(status=NODE_STATUS.READY)
+        node2.tags = [factory.make_tag("cheap")]
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'tags': 'fast, cheap',
+        })
+        self.assertResponseCode(httplib.CONFLICT, response)
+
+    def test_POST_acquire_fails_with_unknown_tags(self):
+        # Asking for a tag that does not exist gives a specific error.
+        node = factory.make_node(status=NODE_STATUS.READY)
+        node.tags = [factory.make_tag("fast")]
+        response = self.client.post(self.get_uri('nodes/'), {
+            'op': 'acquire',
+            'tags': 'fast, hairy',
+        })
+        self.assertResponseCode(httplib.BAD_REQUEST, response)
+
     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_exceptions.py'
--- src/maasserver/tests/test_exceptions.py	2012-04-27 12:38:18 +0000
+++ src/maasserver/tests/test_exceptions.py	2012-09-28 11:10:27 +0000
@@ -15,6 +15,7 @@
 import httplib
 
 from maasserver.exceptions import (
+    InvalidConstraint,
     MAASAPIBadRequest,
     Redirect,
     )
@@ -38,3 +39,21 @@
         exception = Redirect(target)
         response = exception.make_http_response()
         self.assertEqual(target, extract_redirect(response))
+
+    def test_InvalidConstraint_is_bad_request(self):
+        err = InvalidConstraint("height", "-1", ValueError("Not positive"))
+        response = err.make_http_response()
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+
+    def test_InvalidConstraint_str_without_context(self):
+        err = InvalidConstraint("hue", "shiny")
+        self.assertEqual(str(err), "Invalid 'hue' constraint 'shiny'")
+
+    def test_InvalidConstraint_str_with_context(self):
+        try:
+            int("hundreds")
+        except ValueError as e:
+            context = e
+        err = InvalidConstraint("limbs", "hundreds", context)
+        self.assertEqual(str(err),
+            "Invalid 'limbs' constraint 'hundreds': " + str(context))

=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2012-09-26 08:54:29 +0000
+++ src/maasserver/tests/test_node.py	2012-09-28 11:10:27 +0000
@@ -19,6 +19,7 @@
     PermissionDenied,
     ValidationError,
     )
+from django.http import Http404
 from maasserver.enum import (
     ARCHITECTURE,
     DISTRO_SERIES,
@@ -27,7 +28,10 @@
     NODE_STATUS_CHOICES,
     NODE_STATUS_CHOICES_DICT,
     )
-from maasserver.exceptions import NodeStateViolation
+from maasserver.exceptions import (
+    InvalidConstraint,
+    NodeStateViolation,
+    )
 from maasserver.models import (
     Config,
     MACAddress,
@@ -685,7 +689,7 @@
         self.assertEqual(
             None,
             Node.objects.get_available_node_for_acquisition(
-                user, {'name': node.system_id}))
+                user, {'hostname': node.system_id}))
 
     def test_get_available_node_with_name(self):
         """A single available node can be selected using its hostname"""
@@ -694,33 +698,27 @@
         self.assertEqual(
             nodes[1],
             Node.objects.get_available_node_for_acquisition(
-                user, {'name': nodes[1].hostname}))
-
-    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()}))
+                user, {'hostname': nodes[1].hostname}))
 
     def test_get_available_node_with_arch(self):
-        """An available node can be selected of a given architecture"""
+        """An available node can be selected off 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"})
+                user, {'architecture': "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"""
+    def test_get_available_node_with_tag(self):
+        """An available node can be selected off a given tag"""
+        nodes = [self.make_node() for i in range(2)]
+        tag = factory.make_tag('strong')
         user = factory.make_user()
-        self.assertEqual(
-            None,
-            Node.objects.get_available_node_for_acquisition(
-                user, {'arch': "sparc"}))
+        nodes[1].tags.add(tag)
+        available_node = Node.objects.get_available_node_for_acquisition(
+                user, {'tags': "strong"})
+        self.assertEqual(nodes[1], available_node)
 
     def test_stop_nodes_stops_nodes(self):
         # We don't actually want to fire off power events, but we'll go

=== added file 'src/maasserver/tests/test_node_constraint_filter.py'
--- src/maasserver/tests/test_node_constraint_filter.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_node_constraint_filter.py	2012-09-28 11:10:27 +0000
@@ -0,0 +1,102 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test node filtering on specific constraints."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+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.testing.factory import factory
+from maasserver.testing.testcase import TestCase
+
+
+class TestConstrainNodes(TestCase):
+
+    def assertConstrainedNodes(self, expected_nodes, constraints):
+        nodes = constrain_nodes(Node.objects.all(), constraints)
+        self.assertItemsEqual(expected_nodes, nodes)
+
+    def test_no_constraints(self):
+        node1 = factory.make_node()
+        node2 = factory.make_node()
+        self.assertConstrainedNodes([node1, node2], None)
+        self.assertConstrainedNodes([node1, node2], {})
+
+    def test_hostname(self):
+        node1 = factory.make_node(set_hostname=True)
+        node2 = factory.make_node(set_hostname=True)
+        self.assertConstrainedNodes([node1], {'hostname': node1.hostname})
+        self.assertConstrainedNodes([node2], {'hostname': node2.hostname})
+        self.assertConstrainedNodes([], {'hostname': 'unknown-name'})
+
+    def test_architecture(self):
+        node1 = factory.make_node(architecture=ARCHITECTURE.i386)
+        node2 = factory.make_node(architecture=ARCHITECTURE.armhf)
+        self.assertConstrainedNodes([node1], {'architecture': 'i386'})
+        self.assertConstrainedNodes([node2], {'architecture': 'armhf'})
+        self.assertConstrainedNodes([], {'architecture': 'sparc'})
+
+    def test_cpu_count(self):
+        node1 = factory.make_node(cpu_count=1)
+        node2 = factory.make_node(cpu_count=2)
+        self.assertConstrainedNodes([node1, node2], {'cpu_count': '0'})
+        self.assertConstrainedNodes([node1, node2], {'cpu_count': '1'})
+        self.assertConstrainedNodes([node2], {'cpu_count': '2'})
+        self.assertConstrainedNodes([], {'cpu_count': '4'})
+        self.assertRaises(InvalidConstraint,
+            self.assertConstrainedNodes, [], {'cpu_count': 'notint'})
+
+    def test_memory(self):
+        node1 = factory.make_node(memory=1024)
+        node2 = factory.make_node(memory=4096)
+        self.assertConstrainedNodes([node1, node2], {'memory': '512'})
+        self.assertConstrainedNodes([node1, node2], {'memory': '1024'})
+        self.assertConstrainedNodes([node2], {'memory': '2048'})
+        self.assertConstrainedNodes([node2], {'memory': '4096'})
+        self.assertConstrainedNodes([], {'memory': '8192'})
+        self.assertRaises(InvalidConstraint,
+            self.assertConstrainedNodes, [], {'memory': 'notint'})
+
+    def test_tags(self):
+        tag_big = factory.make_tag(name='big')
+        tag_burly = factory.make_tag(name='burly')
+        node_big = factory.make_node()
+        node_big.tags.add(tag_big)
+        node_burly = factory.make_node()
+        node_burly.tags.add(tag_burly)
+        node_bignburly = factory.make_node()
+        node_bignburly.tags.add(tag_big)
+        node_bignburly.tags.add(tag_burly)
+        self.assertConstrainedNodes([node_big, node_bignburly],
+                                    {'tags': 'big'})
+        self.assertConstrainedNodes([node_burly, node_bignburly],
+                                    {'tags': 'burly'})
+        self.assertConstrainedNodes([node_bignburly],
+                                    {'tags': 'big,burly'})
+        self.assertConstrainedNodes([node_bignburly],
+                                    {'tags': 'big burly'})
+        self.assertRaises(InvalidConstraint,
+            self.assertConstrainedNodes, [], {'tags': 'big unknown'})
+
+    def test_combined_constraints(self):
+        tag_big = factory.make_tag(name='big')
+        node_big = factory.make_node(architecture=ARCHITECTURE.i386)
+        node_big.tags.add(tag_big)
+        node_small = factory.make_node(architecture=ARCHITECTURE.i386)
+        node_big_arm = factory.make_node(architecture=ARCHITECTURE.armhf)
+        node_big_arm.tags.add(tag_big)
+        self.assertConstrainedNodes([node_big, node_big_arm],
+                                    {'tags': 'big'})
+        self.assertConstrainedNodes([node_big],
+                                    {'architecture': 'i386', 'tags': 'big'})