Merge lp:~mpontillo/maas/networking-constraints-django-form-support into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Add a Django form field type (and backing object) for labeled constraint maps. Add a networking field, which makes use of it (and a corresponding validator).

Note that this is still just plumbing. The constraint code itself is still being worked on. This is just a chunk I could carve out to commit separately.

[Blake Rouse (blake-rouse)]

Looks good. Just one question about the python3 comment. Go ahead and make this work with python3, so we don't have to find it later.

review: Approve

[Mike Pontillo (mpontillo)]

This isn't anything that is broken, just something that works better in Python 3. (Re-raising a different exception while keeping the stack trace.)

I looked into this a bit, and didn't see a clean Python 2 equivalent.

[Mike Pontillo (mpontillo)]

The "workaround" way to do this in Python 2 is to use sys.exc_info() to capture sys.exc_info()[2] (the traceback at the time the exception is being handled).

I want to save the traceback for later, and there seem to be some caveats to doing this[1]. So I'll just keep it as-is until we drop support for Python 2.

[1]: https://docs.python.org/2/library/sys.html#sys.exc_info

=== modified file 'src/maasserver/node_constraint_filter_forms.py'
--- src/maasserver/node_constraint_filter_forms.py	2015-08-28 02:01:20 +0000
+++ src/maasserver/node_constraint_filter_forms.py	2015-10-28 08:31:25 +0000
@@ -27,6 +27,7 @@
 from django import forms
 from django.core.exceptions import ValidationError
 from django.db.models import Q
+from django.forms.fields import Field
 from maasserver.fields import mac_validator
 from maasserver.forms import (
     MultipleChoiceField,
@@ -49,6 +50,7 @@
 )
 from netaddr import IPAddress
 from netaddr.core import AddrFormatError
+from provisioningserver.utils.constraints import LabeledConstraintMap
 
 # Matches the storage constraint from Juju. Format is an optional label,
 # followed by an optional colon, then size (which is mandatory) followed by an
@@ -98,6 +100,36 @@
     if ','.join(rendered_groups) != value:
         raise ValidationError('Malformed storage constraint, "%s".' % value)
 
+NETWORKING_CONSTRAINT_KEYS = {
+    'space',
+    'not_space',
+    'mode',
+    'fabric_class',
+    'not_fabric_class',
+    'subnet_cidr',
+    'not_subnet_cidr',
+    'vid',
+    'not_vid',
+    'fabric',
+    'not_fabric',
+    'subnet',
+    'not_subnet',
+}
+
+
+def networking_validator(constraint_map):
+    """Validate the given LabeledConstraintMap object."""
+    # At this point, the basic syntax of a labeled constraint map will have
+    # already been validated by the underlying form field. However, we also
+    # need to validate the specified things we're looking for in the
+    # networking domain.
+    for label in constraint_map:
+        constraints = constraint_map[label]
+        for constraint_name in constraints.iterkeys():
+            if constraint_name not in NETWORKING_CONSTRAINT_KEYS:
+                raise ValidationError(
+                    "Unknown networking constraint: '%s" % constraint_name)
+
 
 def generate_architecture_wildcards(arches):
     """Map 'primary' architecture names to a list of full expansions.
@@ -516,6 +548,20 @@
         raise Subnet.DoesNotExist("No subnet matching '%s'." % spec)
 
 
+class LabeledConstraintMapField(Field):
+
+    def __init__(self, *args, **kwargs):
+        super(LabeledConstraintMapField, self).__init__(*args, **kwargs)
+        self.validators.insert(
+            0, lambda constraint_map: constraint_map.validate(
+                exception_type=ValidationError))
+
+    def to_python(self, value):
+        """Returns a LabeledConstraintMap object."""
+        if value is not None and len(value.strip()) != 0:
+            return LabeledConstraintMap(value)
+
+
 class AcquireNodeForm(RenamableFieldsForm):
     """A form handling the constraints used to acquire a node."""
 
@@ -573,6 +619,9 @@
     storage = forms.CharField(
         validators=[storage_validator], label="Storage", required=False)
 
+    networking = LabeledConstraintMapField(
+        validators=[networking_validator], label="Networking", required=False)
+
     ignore_unknown_constraints = True
 
     @classmethod

=== modified file 'src/maasserver/tests/test_node_constraint_filter_forms.py'
--- src/maasserver/tests/test_node_constraint_filter_forms.py	2015-10-27 20:53:16 +0000
+++ src/maasserver/tests/test_node_constraint_filter_forms.py	2015-10-28 08:31:25 +0000
@@ -40,7 +40,10 @@
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import ignore_unused
-from testtools.matchers import ContainsAll
+from testtools.matchers import (
+    Contains,
+    ContainsAll,
+)
 
 
 class TestUtils(MAASServerTestCase):
@@ -863,6 +866,26 @@
             id=constraint_map[node.id].keys()[1])  # 2nd constraint with name
         self.assertGreaterEqual(disk1.size, 5 * 1000 ** 3)
 
+    def test_networking_constraint_rejected_if_syntax_is_invalid(self):
+        factory.make_Node_with_Interface_on_Subnet()
+        form = AcquireNodeForm({
+            u'networking': u'label:x'})
+        self.assertFalse(form.is_valid(), dict(form.errors))
+        self.assertThat(form.errors, Contains('networking'))
+
+    def test_networking_constraint_rejected_if_key_is_invalid(self):
+        factory.make_Node_with_Interface_on_Subnet()
+        form = AcquireNodeForm({
+            u'networking': u'label:chirp_chirp_thing=silenced'})
+        self.assertFalse(form.is_valid(), dict(form.errors))
+        self.assertThat(form.errors, Contains('networking'))
+
+    def test_networking_constraint_validated(self):
+        factory.make_Node_with_Interface_on_Subnet()
+        form = AcquireNodeForm({
+            u'networking': u'label:fabric=fabric-0'})
+        self.assertTrue(form.is_valid(), dict(form.errors))
+
     def test_combined_constraints(self):
         tag_big = factory.make_Tag(name='big')
         arch = '%s/generic' % factory.make_name('arch')
@@ -962,6 +985,7 @@
             'zone': factory.make_Zone(),
             'not_in_zone': [factory.make_Zone().name],
             'storage': '0(ssd),10(ssd)',
+            'networking': 'label:fabric=fabric-0',
             }
         form = AcquireNodeForm(data=constraints)
         self.assertTrue(form.is_valid(), form.errors)

=== modified file 'src/provisioningserver/utils/constraints.py'
--- src/provisioningserver/utils/constraints.py	2015-10-27 18:35:05 +0000
+++ src/provisioningserver/utils/constraints.py	2015-10-28 08:31:25 +0000
@@ -19,6 +19,41 @@
 import re
 
 
+class LabeledConstraintMap(object):
+    """Class to encapsulate a labeled constraint map, so that it only
+    needs to be parsed once.
+    """
+
+    def __init__(self, value):
+        self.value = value
+        self.map = None
+        self.error = None
+        try:
+            self.map = parse_labeled_constraint_map(value)
+        except ValueError as error:
+            self.error = error
+
+    def __repr__(self):
+        return "%s(%s)" % (self.__class__.__name__, repr(self.value))
+
+    def __unicode__(self):
+        return self.value
+
+    def __iter__(self):
+        if self.map is None:
+            return iter([])
+        return iter(self.map)
+
+    def __getitem__(self, item):
+        return self.map[item]
+
+    def validate(self, exception_type=ValueError):
+        if self.error:
+            # XXX mpontillo 2015-10-28 Need to re-raise this properly once we
+            # get to Python 3.
+            raise exception_type(self.error.message)
+
+
 def parse_labeled_constraint_map(value, exception_type=ValueError):
     """Returns a dictionary of constraints, given the specified constraint
     value. Validates that the following conditions hold true:
@@ -42,6 +77,8 @@
     When multiple keys are contained within a constraint, the values will be
     returned (in the order specified) inside a list.
 
+    When a duplicate label is specified, an exception is thrown.
+
     Single values will also be returned inside a list, for consistency.
 
     :return:dict