Merge lp:~blake-rouse/maas/fix-1495775 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Fix the link-subnet to allow mode to be case-insensitive. Fix disabling and enabling an interface where the links are cleaned correctly.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py	2015-07-06 08:52:20 +0000
+++ src/maasserver/fields.py	2015-09-15 14:16:03 +0000
@@ -44,6 +44,7 @@
 from django.db.models.fields.subclassing import Creator
 from django.forms import (
     CharField,
+    ChoiceField,
     ModelChoiceField,
 )
 from django.utils.encoding import force_text
@@ -600,3 +601,12 @@
                         "Invalid IP address: %s; provide a list of "
                         "space-separated IP addresses" % ip)
             return ' '.join(ips)
+
+
+class CaseInsensitiveChoiceField(ChoiceField):
+    """ChoiceField that allows the input to be case insensitive."""
+
+    def to_python(self, value):
+        if value not in self.empty_values:
+            value = value.lower()
+        return super(CaseInsensitiveChoiceField, self).to_python(value)

=== modified file 'src/maasserver/forms_interface_link.py'
--- src/maasserver/forms_interface_link.py	2015-09-03 16:23:08 +0000
+++ src/maasserver/forms_interface_link.py	2015-09-15 14:16:03 +0000
@@ -22,6 +22,7 @@
     INTERFACE_LINK_TYPE_CHOICES,
     IPADDRESS_TYPE,
 )
+from maasserver.fields import CaseInsensitiveChoiceField
 from maasserver.models.interface import Interface
 from maasserver.utils.forms import (
     compose_invalid_choice_text,
@@ -34,7 +35,7 @@
 class InterfaceLinkForm(forms.Form):
     """Interface link form."""
 
-    mode = forms.ChoiceField(
+    mode = CaseInsensitiveChoiceField(
         choices=INTERFACE_LINK_TYPE_CHOICES, required=True,
         error_messages={
             'invalid_choice': compose_invalid_choice_text(

=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2015-09-14 19:31:45 +0000
+++ src/maasserver/models/interface.py	2015-09-15 14:16:03 +0000
@@ -57,6 +57,7 @@
     VerboseRegexValidator,
     MACAddressField,
 )
+from maasserver.utils.signals import connect_to_field_change
 from maasserver.clusterrpc.dhcp import update_host_maps
 from maasserver.models.staticipaddress import StaticIPAddress
 from maasserver.models.cleansave import CleanSave
@@ -235,6 +236,9 @@
     def get_name(self):
         return self.name
 
+    def is_enabled(self):
+        return self.enabled
+
     def get_links(self):
         """Return the definition of links connected to this interface.
 
@@ -1115,9 +1119,6 @@
     def get_type(self):
         return INTERFACE_TYPE.PHYSICAL
 
-    def is_enabled(self):
-        return self.enabled
-
     def clean(self):
         super(PhysicalInterface, self).clean()
         # Node and MAC address is always required for a physical interface.
@@ -1151,6 +1152,39 @@
                     })
 
 
+def interface_enabled_or_disabled(instance, old_values, **kwargs):
+    """When an interface is enabled be sure at minimum a LINK_UP is created.
+    When an interface is disabled make sure that all its links are removed,
+    even for all its children that are now disabled."""
+    if instance.type != INTERFACE_TYPE.PHYSICAL:
+        return
+    if instance.is_enabled():
+        # Make sure it has a LINK_UP link, and for its children.
+        instance.ensure_link_up()
+        for rel in instance.children_relationships.all():
+            rel.child.ensure_link_up()
+    else:
+        # Was disabled. Remove the links.
+        for ip_address in instance.ip_addresses.exclude(
+                alloc_type=IPADDRESS_TYPE.DISCOVERED):
+            instance.unlink_ip_address(ip_address, clearing_config=True)
+        # If any of the children of this interface are now disabled, all of
+        # their links need to be removed as well.
+        for rel in instance.children_relationships.all():
+            if not rel.child.is_enabled():
+                for ip_address in rel.child.ip_addresses.all():
+                    rel.child.unlink_ip_address(
+                        ip_address, clearing_config=True)
+
+
+connect_to_field_change(
+    interface_enabled_or_disabled,
+    Interface, ['enabled'], delete=False)
+connect_to_field_change(
+    interface_enabled_or_disabled,
+    PhysicalInterface, ['enabled'], delete=False)
+
+
 class BondInterface(Interface):
 
     class Meta(Interface.Meta):

=== modified file 'src/maasserver/models/signals/dhcp.py'
--- src/maasserver/models/signals/dhcp.py	2015-09-08 18:41:57 +0000
+++ src/maasserver/models/signals/dhcp.py	2015-09-15 14:16:03 +0000
@@ -27,7 +27,7 @@
     NodeGroupInterface,
     Subnet,
 )
-from maasserver.models.signals.base import connect_to_field_change
+from maasserver.utils.signals import connect_to_field_change
 
 
 def dhcp_post_change_NodeGroupInterface(instance, old_values, **kwargs):

=== modified file 'src/maasserver/models/signals/dns.py'
--- src/maasserver/models/signals/dns.py	2015-09-08 18:41:57 +0000
+++ src/maasserver/models/signals/dns.py	2015-09-15 14:16:03 +0000
@@ -29,7 +29,7 @@
     NodeGroupInterface,
     Subnet,
 )
-from maasserver.models.signals.base import connect_to_field_change
+from maasserver.utils.signals import connect_to_field_change
 
 
 @receiver(post_save, sender=NodeGroup)

=== modified file 'src/maasserver/models/signals/events.py'
--- src/maasserver/models/signals/events.py	2015-06-10 11:24:44 +0000
+++ src/maasserver/models/signals/events.py	2015-09-15 14:16:03 +0000
@@ -21,7 +21,7 @@
     Node,
 )
 from maasserver.models.node import NODE_STATUS
-from maasserver.models.signals.base import connect_to_field_change
+from maasserver.utils.signals import connect_to_field_change
 from provisioningserver.events import (
     EVENT_DETAILS,
     EVENT_TYPES,

=== modified file 'src/maasserver/models/signals/monitors.py'
--- src/maasserver/models/signals/monitors.py	2015-04-01 11:49:45 +0000
+++ src/maasserver/models/signals/monitors.py	2015-09-15 14:16:03 +0000
@@ -17,8 +17,8 @@
 
 
 from maasserver.models import Node
-from maasserver.models.signals.base import connect_to_field_change
 from maasserver.node_status import get_failed_status
+from maasserver.utils.signals import connect_to_field_change
 
 # Useful to disconnect this in testing.
 MONITOR_CANCEL_CONNECT = True

=== modified file 'src/maasserver/models/signals/power.py'
--- src/maasserver/models/signals/power.py	2015-06-10 11:24:44 +0000
+++ src/maasserver/models/signals/power.py	2015-09-15 14:16:03 +0000
@@ -18,13 +18,13 @@
 
 from maasserver.enum import POWER_STATE
 from maasserver.models import Node
-from maasserver.models.signals.base import connect_to_field_change
 from maasserver.node_status import QUERY_TRANSITIONS
 from maasserver.rpc import getClientFor
 from maasserver.utils.orm import (
     post_commit,
     transactional,
 )
+from maasserver.utils.signals import connect_to_field_change
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.power.poweraction import (
     PowerActionFail,

=== modified file 'src/maasserver/models/tests/test_interface.py'
--- src/maasserver/models/tests/test_interface.py	2015-09-14 19:31:45 +0000
+++ src/maasserver/models/tests/test_interface.py	2015-09-15 14:16:03 +0000
@@ -1730,3 +1730,57 @@
                 INTERFACE_LINK_TYPE.STATIC, subnet_v4,
                 ip_address=requested_ip))
         self.assertEquals(sentinel.claimed_ip, claimed_ip)
+
+
+class TestEnableAndDisableInterface(MAASServerTestCase):
+
+    def test__enable_interface_creates_link_up(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, enabled=False)
+        interface.enabled = True
+        interface.save()
+        link_ip = interface.ip_addresses.get(
+            alloc_type=IPADDRESS_TYPE.STICKY, ip=None)
+        self.assertIsNotNone(link_ip)
+
+    def test__enable_interface_creates_link_up_on_children(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, enabled=False)
+        vlan_interface = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, parents=[interface])
+        interface.enabled = True
+        interface.save()
+        link_ip = vlan_interface.ip_addresses.get(
+            alloc_type=IPADDRESS_TYPE.STICKY, ip=None)
+        self.assertIsNotNone(link_ip)
+
+    def test__disable_interface_removes_links(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, enabled=True)
+        interface.ensure_link_up()
+        interface.enabled = False
+        interface.save()
+        self.assertItemsEqual([], interface.ip_addresses.all())
+
+    def test__disable_interface_removes_links_on_children(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, enabled=True)
+        vlan_interface = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, parents=[interface])
+        vlan_interface.ensure_link_up()
+        interface.enabled = False
+        interface.save()
+        self.assertItemsEqual([], vlan_interface.ip_addresses.all())
+
+    def test__disable_interface_doesnt_remove_links_on_enabled_children(self):
+        node = factory.make_Node()
+        nic0 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, enabled=True)
+        nic1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, enabled=True)
+        bond_interface = factory.make_Interface(
+            INTERFACE_TYPE.BOND, parents=[nic0, nic1])
+        bond_interface.ensure_link_up()
+        nic0.enabled = False
+        nic0.save()
+        self.assertEquals(1, bond_interface.ip_addresses.count())

=== modified file 'src/maasserver/tests/test_forms_interface_link.py'
--- src/maasserver/tests/test_forms_interface_link.py	2015-09-08 21:59:19 +0000
+++ src/maasserver/tests/test_forms_interface_link.py	2015-09-15 14:16:03 +0000
@@ -48,7 +48,7 @@
     def test__mode_is_case_insensitive(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         form = InterfaceLinkForm(instance=interface, data={
-            "mode": INTERFACE_LINK_TYPE.DHCP.lower(),
+            "mode": INTERFACE_LINK_TYPE.DHCP.upper(),
         })
         self.assertTrue(form.is_valid(), form.errors)
 

=== renamed file 'src/maasserver/models/signals/base.py' => 'src/maasserver/utils/signals.py'
=== renamed file 'src/maasserver/models/signals/tests/test_base.py' => 'src/maasserver/utils/tests/test_signals.py'
--- src/maasserver/models/signals/tests/test_base.py	2015-06-10 11:24:44 +0000
+++ src/maasserver/utils/tests/test_signals.py	2015-09-15 14:16:03 +0000
@@ -14,10 +14,10 @@
 __metaclass__ = type
 __all__ = []
 
-from maasserver.models.signals.base import connect_to_field_change
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.tests.models import FieldChangeTestModel
+from maasserver.utils.signals import connect_to_field_change
 from maastesting.djangotestcase import TestModelMixin
 from maastesting.matchers import (
     IsCallable,