Merge lp:~mpontillo/maas/fix-bridge-registration-1570104 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Allow racks that have bridges with no parent interfaces to register them with the region. Fixes bug #1570104.

[Blake Rouse (blake-rouse)]

Looks good. This code just keeps getting more complex.... ugh.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/fix-bridge-registration-1570104 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease [247 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe Sources [7,753 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages [7,539 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe Translation-en [4,358 kB]
Fetched 19.9 MB in 3s (5,536 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu2).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
bind9 is already the newest version (1:9.10.3.dfsg.P4-8).
bind9utils is already the newest version (1:9.10.3.dfsg.P4-8).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P4-8).
firefox is already the newest version (45.0.2+build1-0ubuntu1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the n...

[Mike Pontillo (mpontillo)]

Heh, somehow this test is now happily engaging in infinite recursion:

maasserver.tests.test_preseed_network.TestBondNetworkLayout.test__renders_expected_output

Will look into it.

[Mike Pontillo (mpontillo)]

Hah, it was because we called .all() on a manager, then added an interface, then iterated on the .all() (which then included the bond, causing the bond to have itself as a parent).

I guess we should validate that interfaces cannot have themselves as parents. ;-)

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/fix-bridge-registration-1570104 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease [247 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main Sources [866 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main amd64 Packages [1,183 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages [7,538 kB]
Fetched 9,834 kB in 2s (4,810 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu2).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
bind9 is already the newest version (1:9.10.3.dfsg.P4-8).
bind9utils is already the newest version (1:9.10.3.dfsg.P4-8).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P4-8).
firefox is already the newest version (45.0.2+build1-0ubuntu1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest ver...

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~mpontillo/maas/fix-bridge-registration-1570104 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease [247 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main Sources [866 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe Sources [7,753 kB]
Get:7 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/main amd64 Packages [1,183 kB]
Get:8 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages [7,538 kB]
Fetched 17.6 MB in 3s (5,424 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu2).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
bind9 is already the newest version (1:9.10.3.dfsg.P4-8).
bind9utils is already the newest version (1:9.10.3.dfsg.P4-8).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P4-8).
firefox is already the newest version (45.0.2+build1-0ubuntu...

[Mike Pontillo (mpontillo)]

Random failure in:

maasserver.rpc.tests.test_regionservice.TestRegionService.test_start_up_logs_failure_if_all_endpoint_options_fail

Retrying.

=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2016-04-11 16:23:26 +0000
+++ src/maasserver/models/interface.py	2016-04-15 23:41:24 +0000
@@ -1115,23 +1115,29 @@
 
     def get_node(self):
         if self.id is None:
-            return None
+            # This should be correct since the interface was just now created.
+            return self.node
         else:
             parent = self.parents.first()
-            if parent is not None:
+            if parent is not None and parent != self:
                 return parent.get_node()
             else:
-                return None
+                # This could be a bridge interface without a parent.
+                return self.node
 
     def is_enabled(self):
         if self.id is None:
             return True
         else:
-            is_enabled = {
-                parent.is_enabled()
-                for parent in self.parents.all()
-            }
-            return True in is_enabled
+            if self.parents.count() > 0:
+                is_enabled = {
+                    parent.is_enabled()
+                    for parent in self.parents.all()
+                    if parent != self
+                }
+                return True in is_enabled
+            else:
+                return self.enabled
 
     def _validate_acceptable_parent_types(self, parent_types):
         """Raises a ValidationError if the interface has parents which are not

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-04-14 19:54:36 +0000
+++ src/maasserver/models/node.py	2016-04-15 23:41:24 +0000
@@ -3256,9 +3256,11 @@
         parent_nics = Interface.objects.get_interfaces_on_node_by_name(
             self, ifnames)
 
-        # Ignore child interfaces that don't have parents. MAAS won't know what
-        # to do with them since they can't be connected to a fabric.
-        if len(parent_nics) == 0:
+        # Ignore most child interfaces that don't have parents. MAAS won't know
+        # what to do with them since they can't be connected to a fabric.
+        # Bridges are an exception since some MAAS demo/test environments
+        # contain virtual bridges.
+        if len(parent_nics) == 0 and child_type is not BridgeInterface:
             return None
 
         mac_address = config["mac_address"]
@@ -3266,12 +3268,14 @@
             self, name, mac_address, parent_nics)
 
         links = config["links"]
-        self._configure_vlan_from_links(interface, parent_nics, links)
+        found_vlan = self._configure_vlan_from_links(
+            interface, parent_nics, links)
 
         # Update all the IP address on this interface. Fix the VLAN the
         # interface belongs to so its the same as the links and all parents to
         # be on the same VLAN.
-        update_ip_addresses = self._update_links(interface, links)
+        update_ip_addresses = self._update_links(
+            interface, links, use_interface_vlan=found_vlan)
         self._update_parent_vlans(interface, parent_nics, update_ip_addresses)
         return interface
 
@@ -3314,16 +3318,26 @@
                     parent_nic.save()
 
     def _configure_vlan_from_links(self, interface, parent_nics, links):
+        """Attempt to configure the interface VLAN based on the links and
+        connected subnets. Returns True if the VLAN was configured; otherwise,
+        returns False."""
         # Make sure that the VLAN on the interface is correct. When
         # links exists on this interface we place it into the correct
         # VLAN. If it cannot be determined it is placed on the same fabric
         # as its first parent interface.
         connected_to_subnets = self._get_connected_subnets(links)
-        if len(connected_to_subnets) == 0:
+        if len(connected_to_subnets) == 0 and len(parent_nics) > 0:
             # Not connected to any known subnets. We add it to the same
             # VLAN as its first parent.
             interface.vlan = parent_nics[0].vlan
-        interface.save()
+            interface.save()
+            return True
+        elif len(connected_to_subnets) > 0:
+            subnet = next(iter(connected_to_subnets))
+            interface.vlan = subnet.vlan
+            interface.save()
+            return True
+        return False
 
     def _get_connected_subnets(self, links):
         """Return a set of subnets that `links` belongs to."""
@@ -3358,7 +3372,8 @@
                 return ip_address.subnet.vlan
         return None
 
-    def _update_links(self, interface, links, force_vlan=False):
+    def _update_links(
+            self, interface, links, force_vlan=False, use_interface_vlan=True):
         """Update the links on `interface`."""
         interface.ip_addresses.filter(
             alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()
@@ -3366,6 +3381,13 @@
             interface.ip_addresses.exclude(
                 alloc_type=IPADDRESS_TYPE.DISCOVERED))
         updated_ip_addresses = set()
+        if use_interface_vlan:
+            vlan = interface.vlan
+        elif len(links) > 0:
+            fabric = Fabric.objects.create()
+            vlan = fabric.get_default_vlan()
+            interface.vlan = vlan
+            interface.save()
         for link in links:
             if link["mode"] == "dhcp":
                 dhcp_address = self._get_alloc_type_from_ip_addresses(
@@ -3388,7 +3410,7 @@
                     subnet, _ = Subnet.objects.get_or_create(
                         cidr=str(ip_network.cidr), defaults={
                             "name": str(ip_network.cidr),
-                            "vlan": interface.vlan,
+                            "vlan": vlan,
                             "space": Space.objects.get_default_space(),
                         })
 
@@ -3427,7 +3449,7 @@
                 subnet, _ = Subnet.objects.get_or_create(
                     cidr=str(ip_network.cidr), defaults={
                         "name": str(ip_network.cidr),
-                        "vlan": interface.vlan,
+                        "vlan": vlan,
                         "space": Space.objects.get_default_space(),
                     })
 

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-04-14 19:54:36 +0000
+++ src/maasserver/models/tests/test_node.py	2016-04-15 23:41:24 +0000
@@ -6652,6 +6652,66 @@
                 vlan=br0_vlan,
             ))
 
+    def test_registers_bridge_with_no_parents_and_links(self):
+        rack = self.create_empty_rack_controller()
+        interfaces = {
+            'br0': {
+                'enabled': True,
+                'mac_address': '4e:4d:9a:a8:a5:5f',
+                'parents': [],
+                'source': 'ipaddr',
+                'type': 'bridge',
+                'links': [{
+                    'mode': "static",
+                    'address': "192.168.0.1/24"
+                }]
+            },
+            'eth0': {
+                'enabled': True,
+                'mac_address': '52:54:00:77:15:e3',
+                'links': [],
+                'parents': [],
+                'source': 'ipaddr',
+                'type': 'physical'
+            }
+        }
+        rack.update_interfaces(interfaces)
+        eth0 = get_one(
+            PhysicalInterface.objects.filter(node=rack, name='eth0'))
+        br0 = get_one(BridgeInterface.objects.filter(node=rack, name='br0'))
+        self.assertIsNotNone(eth0)
+        self.assertIsNotNone(br0)
+        subnet = get_one(Subnet.objects.filter(cidr='192.168.0.0/24'))
+        self.assertIsNotNone(subnet)
+        self.assertThat(subnet.vlan, Equals(br0.vlan))
+
+    def test_registers_bridge_with_no_parents_and_no_links(self):
+        rack = self.create_empty_rack_controller()
+        interfaces = {
+            'br0': {
+                'enabled': True,
+                'links': [],
+                'mac_address': '4e:4d:9a:a8:a5:5f',
+                'parents': [],
+                'source': 'ipaddr',
+                'type': 'bridge'
+            },
+            'eth0': {
+                'enabled': True,
+                'links': [],
+                'mac_address': '52:54:00:77:15:e3',
+                'parents': [],
+                'source': 'ipaddr',
+                'type': 'physical'
+            },
+        }
+        rack.update_interfaces(interfaces)
+        eth0 = get_one(
+            PhysicalInterface.objects.filter(node=rack, name='eth0'))
+        br0 = get_one(BridgeInterface.objects.filter(node=rack, name='br0'))
+        self.assertIsNotNone(eth0)
+        self.assertIsNotNone(br0)
+
 
 class TestRackController(MAASServerTestCase):
 

=== modified file 'src/maasserver/tests/test_preseed_network.py'
--- src/maasserver/tests/test_preseed_network.py	2016-03-28 13:54:47 +0000
+++ src/maasserver/tests/test_preseed_network.py	2016-04-15 23:41:24 +0000
@@ -244,7 +244,7 @@
     def test__renders_expected_output(self):
         node = factory.make_Node_with_Interface_on_Subnet(
             interface_count=2)
-        interfaces = node.interface_set.all()
+        interfaces = list(node.interface_set.all())
         vlan = node.interface_set.first().vlan
         bond_iface = factory.make_Interface(
             iftype=INTERFACE_TYPE.BOND, node=node, vlan=vlan,
@@ -286,7 +286,7 @@
     def test__renders_expected_output(self):
         node = factory.make_Node_with_Interface_on_Subnet(
             interface_count=2)
-        phys_ifaces = node.interface_set.all()
+        phys_ifaces = list(node.interface_set.all())
         phys_vlan = node.interface_set.first().vlan
         bond_iface = factory.make_Interface(iftype=INTERFACE_TYPE.BOND,
                                             node=node, vlan=phys_vlan,

=== modified file 'src/provisioningserver/utils/network.py'
--- src/provisioningserver/utils/network.py	2016-04-15 18:08:15 +0000
+++ src/provisioningserver/utils/network.py	2016-04-15 23:41:24 +0000
@@ -332,12 +332,18 @@
     @property
     def first(self):
         """Returns the first IP address in this set."""
-        return self.ranges[0].first
+        if len(self.ranges) > 0:
+            return self.ranges[0].first
+        else:
+            return None
 
     @property
     def last(self):
         """Returns the last IP address in this set."""
-        return self.ranges[-1].last
+        if len(self.ranges) > 0:
+            return self.ranges[-1].last
+        else:
+            return None
 
     def ip_has_purpose(self, ip, purpose):
         """Returns True if the specified IP address has the specified purpose