launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12611
[Merge] lp:~jameinel/maas/cpu_mem_tag_constraints into lp:maas
John A Meinel has proposed merging lp:~jameinel/maas/cpu_mem_tag_constraints into lp:maas.
Commit message:
Add CPU count, Memory and Tags as exposed logic for get_available_node_for_acquisition.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jameinel/maas/cpu_mem_tag_constraints/+merge/126863
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.
--
https://code.launchpad.net/~jameinel/maas/cpu_mem_tag_constraints/+merge/126863
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jameinel/maas/cpu_mem_tag_constraints into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-09-27 09:24:41 +0000
+++ src/maasserver/api.py 2012-09-28 06:39:21 +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': 'name',
+ 'tags': 'tags',
+ 'arch': 'arch',
+ '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:
+ 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 06:39:21 +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 06:39:21 +0000
@@ -51,7 +51,10 @@
NODE_STATUS_CHOICES,
NODE_STATUS_CHOICES_DICT,
)
-from maasserver.exceptions import NodeStateViolation
+from maasserver.exceptions import (
+ InvalidConstraint,
+ NodeStateViolation,
+ )
from maasserver.fields import (
JSONObjectField,
XMLField,
@@ -61,7 +64,7 @@
from maasserver.models.tag import Tag
from maasserver.models.timestampedmodel import TimestampedModel
from maasserver.utils import get_db_state
-from maasserver.utils.orm import get_first
+from maasserver.utils.orm import get_first, get_one
from piston.models import Token
from provisioningserver.enum import (
POWER_TYPE,
@@ -250,20 +253,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'])
+ available_nodes = self.get_nodes(for_user, NODE_PERMISSION.VIEW)
+ available_nodes = available_nodes.filter(status=NODE_STATUS.READY)
+ constrainer = AcquisitionConstrainer(available_nodes, constraints)
+ available_nodes = constrainer.apply()
return get_first(available_nodes)
@@ -373,6 +366,61 @@
node.save()
+class AcquisitionConstrainer:
+ """Add filters to a query for nodes, based on a constraints dict.
+ """
+
+ _known_constraints = ['name', 'architecture', 'cpu_count', 'memory',
+ 'tags']
+
+ def __init__(self, nodes, constraints):
+ self.nodes = nodes
+ self.constraints = constraints
+
+ def apply(self):
+ if not self.constraints:
+ return self.nodes
+
+ for constraint_name in self._known_constraints:
+ value = self.constraints.get(constraint_name)
+ if value:
+ attr_name = '_' + constraint_name
+ getattr(self, attr_name)(value)
+ return self.nodes
+
+ def _architecture(self, arch):
+ # GZ 2012-09-11: This only supports an exact match on arch type,
+ # using an i386 image on amd64 hardware will wait.
+ self.nodes = self.nodes.filter(architecture=arch)
+
+ def _name(self, name):
+ self.nodes = self.nodes.filter(hostname=name)
+
+ def _int_gte_constraint(self, key, str_value):
+ try:
+ int_value = int(str_value)
+ except ValueError as e:
+ raise InvalidConstraint(key, str_value, e)
+ q = {key + "__gte": int_value}
+ self.nodes = self.nodes.filter(**q)
+
+ def _cpu_count(self, cpu_count):
+ self._int_gte_constraint('cpu_count', cpu_count)
+
+ def _memory(self, mem):
+ self._int_gte_constraint('memory', mem)
+
+ def _tags(self, tag_expression):
+ # 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')
+ self.nodes = self.nodes.filter(tags=tag)
+
+
+
class Node(CleanSave, TimestampedModel):
"""A `Node` represents a physical machine used by the MAAS Server.
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-09-28 05:11:12 +0000
+++ src/maasserver/tests/test_api.py 2012-09-28 06:39:21 +0000
@@ -828,6 +828,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`."""
@@ -1712,6 +1717,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.NOT_FOUND, 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 06:39:21 +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 06:39:21 +0000
@@ -19,6 +19,7 @@
PermissionDenied,
ValidationError,
)
+from django.http import Http404
from maasserver.enum import (
ARCHITECTURE,
DISTRO_SERIES,
@@ -27,14 +28,20 @@
NODE_STATUS_CHOICES,
NODE_STATUS_CHOICES_DICT,
)
-from maasserver.exceptions import NodeStateViolation
+from maasserver.exceptions import (
+ InvalidConstraint,
+ NodeStateViolation,
+ )
from maasserver.models import (
Config,
MACAddress,
Node,
node as node_module,
)
-from maasserver.models.node import NODE_TRANSITIONS
+from maasserver.models.node import (
+ AcquisitionConstrainer,
+ NODE_TRANSITIONS,
+ )
from maasserver.models.user import create_auth_token
from maasserver.testing import reload_object
from maasserver.testing.factory import factory
@@ -722,6 +729,88 @@
Node.objects.get_available_node_for_acquisition(
user, {'arch': "sparc"}))
+ def test_get_available_node_with_cpu_enough(self):
+ user = factory.make_user()
+ node = self.make_node(cpu_count=2)
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'cpu_count': "2"})
+ self.assertEqual(node, available_node)
+
+ def test_get_available_node_with_cpu_not_enough(self):
+ user = factory.make_user()
+ node = self.make_node(cpu_count=1)
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'cpu_count': "2"})
+ self.assertEqual(None, available_node)
+
+ def test_get_available_node_with_cpu_invalid(self):
+ user = factory.make_user()
+ err = self.assertRaises(InvalidConstraint,
+ Node.objects.get_available_node_for_acquisition,
+ user, {'cpu_count': "lots"})
+ self.assertEqual(("cpu_count", "lots"), err.args)
+
+ def test_get_available_node_with_mem_enough(self):
+ user = factory.make_user()
+ node = self.make_node(memory=4096)
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'memory': "2048"})
+ self.assertEqual(node, available_node)
+
+ def test_get_available_node_with_mem_not_enough(self):
+ user = factory.make_user()
+ node = self.make_node(memory=4096)
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'memory': "4097"})
+ self.assertEqual(None, available_node)
+
+ def test_get_available_node_with_mem_invalid(self):
+ user = factory.make_user()
+ err = self.assertRaises(InvalidConstraint,
+ Node.objects.get_available_node_for_acquisition,
+ user, {'memory': "forgetful"})
+ self.assertEqual(("memory", "forgetful"), err.args)
+
+ def test_get_available_node_with_single_matching_tag(self):
+ user = factory.make_user()
+ node = self.make_node()
+ node.tags = [factory.make_tag("happy")]
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'tags': "happy"})
+ self.assertEqual(node, available_node)
+
+ def test_get_available_node_with_multiple_matching_tags(self):
+ user = factory.make_user()
+ node = self.make_node()
+ node.tags = [factory.make_tag("happy"), factory.make_tag("friendly")]
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'tags': "happy, friendly"})
+ self.assertEqual(node, available_node)
+
+ def test_get_available_node_with_single_mismatching_tag(self):
+ user = factory.make_user()
+ node = self.make_node()
+ node.tags = [factory.make_tag("happy")]
+ factory.make_tag("unhappy")
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'tags': "unhappy"})
+ self.assertEqual(None, available_node)
+
+ def test_get_available_node_with_partially_mismatching_tags(self):
+ user = factory.make_user()
+ node = self.make_node()
+ node.tags = [factory.make_tag("happy"), factory.make_tag("friendly")]
+ factory.make_tag("unfriendly")
+ available_node = Node.objects.get_available_node_for_acquisition(
+ user, {'tags': "happy, unfriendly"})
+ self.assertEqual(None, available_node)
+
+ def test_get_available_node_with_single_missing_tag(self):
+ user = factory.make_user()
+ self.assertRaises(Http404,
+ Node.objects.get_available_node_for_acquisition,
+ user, {'tags': "lonely"})
+
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
@@ -897,3 +986,84 @@
node = factory.make_node(netboot=True)
node.set_netboot(False)
self.assertFalse(node.netboot)
+
+
+class TestAcquisitionConstrainer(TestCase):
+
+ def assertConstrainedNodes(self, expected_nodes, constraints):
+ nodes = Node.objects.all()
+ constrainer = AcquisitionConstrainer(nodes, constraints)
+ nodes = constrainer.apply()
+ 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_name(self):
+ node1 = factory.make_node(set_hostname=True)
+ node2 = factory.make_node(set_hostname=True)
+ self.assertConstrainedNodes([node1], {'name': node1.hostname})
+ self.assertConstrainedNodes([node2], {'name': node2.hostname})
+
+ 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'})
+
+ 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'})
Follow ups
