Merge lp:~mpontillo/maas/commissioning-fixes-trunk into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Merge fix for bug 1524924 from MAAS 1.9 at revsion 4530. Fixes node interfaces being configured in an incorrect subnet after commissioning.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2015-12-09 02:02:23 +0000
+++ src/maasserver/models/interface.py	2015-12-11 03:55:02 +0000
@@ -455,8 +455,20 @@
             # to create or update the Subnet)
             try:
                 subnet = Subnet.objects.get(cidr=cidr)
+                # Update VLAN based on the VLAN this Subnet belongs to.
+                if subnet.vlan != self.vlan:
+                    vlan = subnet.vlan
+                    maaslog.info("%s: Observed connected to %s via %s." % (
+                        self.get_log_string(), vlan.fabric.get_name(), cidr))
+                    self.vlan = vlan
+                    self.save()
             except Subnet.DoesNotExist:
-                # XXX mpontillo 2015-11-01 configuration != state
+                # XXX mpontillo 2015-11-01 Configuration != state. That is,
+                # this means we have just a subnet on an unknown Fabric/VLAN,
+                # and this fact should be recorded somewhere, so that the user
+                # gets a chance to configure it. Note, however, that if this
+                # is already a managed cluster interface, a Fabric/VLAN will
+                # already have been created.
                 subnet = Subnet.objects.create_from_cidr(cidr)
                 maaslog.info(
                     "Creating subnet %s connected to interface %s "
 
=== modified file 'src/maasserver/models/tests/test_interface.py'
--- src/maasserver/models/tests/test_interface.py	2015-12-09 02:02:23 +0000
+++ src/maasserver/models/tests/test_interface.py	2015-12-11 03:55:02 +0000
@@ -1038,6 +1038,38 @@
                 alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet_list[i],
                 ip=str(IPNetwork(cidr_list[i]).ip)))
 
+    def test__links_interface_to_vlan_on_existing_subnet_with_logging(self):
+        ng = factory.make_NodeGroup()
+        fabric2 = factory.make_Fabric()
+        fabric3 = factory.make_Fabric()
+        ngi1 = factory.make_NodeGroupInterface(ng)
+        ngi2 = factory.make_NodeGroupInterface(ng, fabric=fabric2)
+        ngi3 = factory.make_NodeGroupInterface(ng, fabric=fabric3)
+        interface1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        interface2 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        interface3 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        vlan1 = ngi1.subnet.vlan
+        vlan2 = ngi2.subnet.vlan
+        vlan3 = ngi3.subnet.vlan
+        maaslog = self.patch_autospec(interface_module, "maaslog")
+        interface1.update_ip_addresses([ngi1.subnet.cidr])
+        interface2.update_ip_addresses([ngi2.subnet.cidr])
+        interface3.update_ip_addresses([ngi3.subnet.cidr])
+        self.assertThat(interface1.vlan, Equals(vlan1))
+        self.assertThat(interface2.vlan, Equals(vlan2))
+        self.assertThat(interface3.vlan, Equals(vlan3))
+        self.assertThat(maaslog.info, MockCallsMatch(
+            call(("%s: Observed connected to %s via %s." % (
+                interface1.get_log_string(), interface1.vlan.fabric.get_name(),
+                ngi1.subnet.cidr))),
+            call(("%s: Observed connected to %s via %s." % (
+                interface2.get_log_string(), interface2.vlan.fabric.get_name(),
+                ngi2.subnet.cidr))),
+            call(("%s: Observed connected to %s via %s." % (
+                interface3.get_log_string(), interface3.vlan.fabric.get_name(),
+                ngi3.subnet.cidr))),
+        ))
+
     def test__deletes_old_discovered_ip_addresses_on_interface(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         # Create existing DISCOVERED IP address on the interface. These should
 
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2015-12-09 01:24:24 +0000
+++ src/maasserver/testing/factory.py	2015-12-11 03:55:02 +0000
@@ -482,7 +482,8 @@
                                 ip_range_low=None, ip_range_high=None,
                                 interface=None, management=None,
                                 static_ip_range_low=None,
-                                static_ip_range_high=None, **kwargs):
+                                static_ip_range_high=None, fabric=None,
+                                **kwargs):
         interface_settings = self.get_interface_fields(
             name=name, ip=ip, router_ip=router_ip,
             network=network, subnet=subnet, subnet_mask=subnet_mask,
@@ -498,12 +499,17 @@
             cidr = create_cidr(
                 interface_settings['ip'], interface_settings['subnet_mask'])
 
+            defaults = {
+                'name': cidr,
+                'cidr': cidr,
+                'space': Space.objects.get_default_space(),
+            }
+
+            if fabric is not None:
+                defaults['vlan'] = fabric.get_default_vlan()
+
             subnet, _ = Subnet.objects.get_or_create(
-                cidr=cidr, defaults={
-                    'name': cidr,
-                    'cidr': cidr,
-                    'space': Space.objects.get_default_space(),
-                })
+                cidr=cidr, defaults=defaults)
         elif interface_settings['subnet']:
             subnet = interface_settings.pop('subnet')
             interface_settings['subnet_mask'] = subnet.get_ipnetwork().netmask
 
=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2015-12-01 18:12:59 +0000
+++ src/metadataserver/models/commissioningscript.py	2015-12-11 03:55:02 +0000
@@ -130,8 +130,7 @@
 """
 
 
-def _create_default_physical_interface(
-        node, ifname, mac, fabric=None, vlan=None):
+def _create_default_physical_interface(node, ifname, mac):
     """Assigns the specified interface to the specified Node.
 
     Creates or updates a PhysicalInterface that corresponds to the given MAC.
@@ -140,18 +139,15 @@
     :param ifname: the interface name (for example, 'eth0')
     :param mac: the Interface to update and associate
     """
-    if fabric is None:
-        fabric = Fabric.objects.get_default_fabric()
-
-    if vlan is None:
-        vlan = fabric.get_default_vlan()
-
+    # We don't yet have enough information to put this newly-created Interface
+    # into the proper Fabric/VLAN. (We'll do this on a "best effort" basis
+    # later, if we are able to determine that the interface is on a particular
+    # subnet due to a DHCP reply during commissioning.)
+    fabric = Fabric.objects.get_default_fabric()
+    vlan = fabric.get_default_vlan()
     interface = PhysicalInterface.objects.create(
         mac_address=mac, name=ifname, node=node, vlan=vlan)
 
-    # XXX:fabric - for now we use the default fabric, but we need to be smarter
-    # about determining which fabric the node is in. (perhaps the cluster
-    # interface the node is linked to has an associated fabric, for example)
     return interface
 
 
@@ -185,7 +181,6 @@
         else:
             ifname = link['name']
             try:
-                # XXX:fabric - this MAC might exist on a different Fabric
                 interface = PhysicalInterface.objects.get(
                     mac_address=link_mac)
                 if interface.node is not None and interface.node != node:
@@ -195,7 +190,6 @@
                         (interface.mac_address, interface.node.fqdn,
                          node.fqdn))
                     interface.delete()
-                    # XXX: mpontillo 2015-08-27 how do we get Fabric/VLAN here?
                     interface = _create_default_physical_interface(
                         node, ifname, link_mac)
                 else:
@@ -204,7 +198,6 @@
                     interface.name = ifname
                     interface.save()
             except PhysicalInterface.DoesNotExist:
-                # XXX: mpontillo 2015-08-27 how do we get Fabric/VLAN here?
                 interface = _create_default_physical_interface(
                     node, ifname, link_mac)
 
@@ -681,10 +674,6 @@
         'content': LIST_MODALIASES_SCRIPT.encode('ascii'),
         'hook': null_hook,
     },
-    '00-maas-05-network-interfaces.out': {
-        'content': IPADDR_SCRIPT.encode('ascii'),
-        'hook': update_node_network_information,
-    },
     '00-maas-06-dhcp-unconfigured-ifaces': {
         'content': make_function_call_script(dhcp_explore),
         'hook': null_hook,
@@ -702,6 +691,10 @@
         'content': make_function_call_script(lldpd_capture),
         'hook': set_node_routers,
     },
+    '99-maas-03-network-interfaces.out': {
+        'content': IPADDR_SCRIPT.encode('ascii'),
+        'hook': update_node_network_information,
+    },
 }