Merge lp:~mpontillo/maas/allow-multiple-macs-on-the-same-node into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Allow multiple physical interfaces to have the same MAC if they are on the same node.

[MAAS Lander (maas-lander)]

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5780. By Mike Pontillo on 2017-03-06

Merge trunk.

5779. By Mike Pontillo on 2017-03-06

Allow multiple physical interfaces to have the same MAC if they are on the same node.

=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2017-02-17 14:23:04 +0000
+++ src/maasserver/models/interface.py	2017-03-06 14:57:51 +0000
@@ -1403,19 +1403,6 @@
         if len(validation_errors) > 0:
             raise ValidationError(validation_errors)
 
-        # MAC address must be unique amongst every PhysicalInterface.
-        other_interfaces = PhysicalInterface.objects.filter(
-            mac_address=self.mac_address)
-        if self.id is not None:
-            other_interfaces = other_interfaces.exclude(id=self.id)
-        other_interfaces = other_interfaces.all()
-        if len(other_interfaces) > 0:
-            raise ValidationError({
-                "mac_address": [
-                    "This MAC address is already in use by %s." % (
-                        other_interfaces[0].get_log_string())]
-            })
-
         # No parents are allow for a physical interface.
         if self.id is not None:
             # Use the precache so less queries are made.
 
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2017-03-02 00:22:23 +0000
+++ src/maasserver/models/node.py	2017-03-06 14:57:51 +0000
@@ -1625,7 +1625,8 @@
         UnknownInterface.objects.filter(mac_address=mac).delete()
         try:
             iface = PhysicalInterface.objects.get(mac_address=mac)
-        except PhysicalInterface.DoesNotExist:
+        except (PhysicalInterface.MultipleObjectsReturned,
+                PhysicalInterface.DoesNotExist):
             return PhysicalInterface.objects.create(
                 node=self, mac_address=mac, name=name)
         if iface.node != self:
 
=== modified file 'src/maasserver/rpc/events.py'
--- src/maasserver/rpc/events.py	2016-10-25 13:57:02 +0000
+++ src/maasserver/rpc/events.py	2017-03-06 14:57:51 +0000
@@ -75,12 +75,11 @@
     except EventType.DoesNotExist:
         raise NoSuchEventType.from_name(type_name)
 
-    try:
-        interface = Interface.objects.get(
-            type=INTERFACE_TYPE.PHYSICAL, mac_address=mac_address)
-    except Interface.DoesNotExist:
+    interface = Interface.objects.filter(
+        type=INTERFACE_TYPE.PHYSICAL, mac_address=mac_address).first()
+    if interface is None:
         # The node doesn't exist, but we don't raise an exception - it's
-        # entirely possible the cluster has started sending events for a node
+        # entirely possible the rack has started sending events for a node
         # that we don't know about yet. This is most likely to happen when a
         # new node is trying to enlist.
         maaslog.debug(
 
=== modified file 'src/metadataserver/builtin_scripts/hooks.py'
--- src/metadataserver/builtin_scripts/hooks.py	2017-03-06 13:32:48 +0000
+++ src/metadataserver/builtin_scripts/hooks.py	2017-03-06 14:57:51 +0000
@@ -7,6 +7,7 @@
     'NODE_INFO_SCRIPTS',
     ]
 
+from collections import defaultdict
 import fnmatch
 import json
 import logging
@@ -91,6 +92,31 @@
     return interface
 
 
+def get_mac_to_ifnames_dict(ip_addr_info):
+    mac_to_ifnames = defaultdict(list)
+    for link in ip_addr_info.values():
+        mac = link.get('mac')
+        if mac is None:
+            # Ignore loopback interfaces.
+            continue
+        else:
+            ifname = link['name']
+            mac_to_ifnames[mac].append(ifname)
+    return mac_to_ifnames
+
+
+def clean_up_moved_nics(node, mac):
+    interfaces = PhysicalInterface.objects.filter(
+        mac_address=mac).exclude(node=node)
+    for interface in interfaces:
+        logger.warning(
+            "Interface with MAC %s moved from node %s to %s. "
+            "(The existing interface will be deleted.)" %
+            (interface.mac_address, interface.node.fqdn,
+             node.fqdn))
+        interface.delete()
+
+
 def update_node_network_information(node, output, exit_status):
     """Updates the network interfaces from the results of `IPADDR_SCRIPT`.
 
@@ -107,50 +133,87 @@
                 node.hostname, exit_status))
         return
     assert isinstance(output, bytes)
-
     # Skip network configuration if set by the user.
     if node.skip_networking:
         return
-
     # Get the MAC addresses of all connected interfaces.
     ip_addr_info = parse_ip_addr(output)
+    # Prepare a set of interfaces found during commissioning, so that any
+    # physical interfaces no longer present on the machine can be removed.
     current_interfaces = set()
-
-    for link in ip_addr_info.values():
-        link_mac = link.get('mac')
-        # Ignore loopback interfaces.
-        if link_mac is None:
-            continue
-        else:
-            ifname = link['name']
-            try:
+    # Pre-process the list to check if multiple physical interfaces share the
+    # same MAC.
+    mac_to_ifnames = get_mac_to_ifnames_dict(ip_addr_info)
+    # Go through the map of { ifname: [mac, ...] } to determine the best course
+    # of action. If interface names correspond 1:1 with MACs (the normal case)
+    # we can avoid losing data (such as child interfaces created by a MAAS
+    # admin by simply renaming interfaces).
+    for mac, ifnames in mac_to_ifnames.items():
+        clean_up_moved_nics(node, mac)
+        existing_iface_count = PhysicalInterface.objects.filter(
+            mac_address=mac, node=node).count()
+        if len(ifnames) == 1:
+            ifname = ifnames[0]
+            if existing_iface_count == 0:
+                # Found a single interface with this MAC, and no interface
+                # was already on the node. Just go ahead and create it.
+                interface = _create_default_physical_interface(
+                    node, ifname, mac)
+                ip_addr_info[ifname]['model'] = interface
+            elif existing_iface_count == 1:
+                # Found an interface that was created by a previous
+                # commissioning, so don't throw away the data. Just change the
+                # name if necessary.
                 interface = PhysicalInterface.objects.get(
-                    mac_address=link_mac)
-                if interface.node is not None and interface.node != node:
-                    logger.warning(
-                        "Interface with MAC %s moved from node %s to %s. "
-                        "(The existing interface will be deleted.)" %
-                        (interface.mac_address, interface.node.fqdn,
-                         node.fqdn))
+                    mac_address=mac, node=node)
+                if interface.name != ifname:
+                    interface.name = ifname
+                    interface.save()
+                ip_addr_info[ifname]['model'] = interface
+            else:
+                # Previously there were multiple physical interfaces with the
+                # same MAC, but now there is only one. Might as well start
+                # fresh.
+                for iface in PhysicalInterface.objects.filter(
+                        mac_address=mac, node=node):
+                    iface.delete()
+                interface = _create_default_physical_interface(
+                    node, ifname, mac)
+                ip_addr_info[ifname]['model'] = interface
+        else:
+            # Multiple interfaces found with the same MAC.
+            # This means we shouldn't try to preserve renamed interfaces.
+            existing = set()
+            for interface in PhysicalInterface.objects.filter(
+                    mac_address=mac, node=node):
+                ifname = interface.name
+                if interface.name not in ifnames:
+                    # Interface has been removed.
                     interface.delete()
+                else:
+                    # Existing interface with the same name as an interface
+                    # we just found during commissioning. Make note of it.
+                    existing.add(interface.name)
+                    ip_addr_info[ifname]['model'] = interface
+            for ifname in ifnames:
+                # Go through the list of interfaces with a shared MAC whose
+                # names don't match any existing interface names and create
+                # them.
+                if ifname not in existing:
                     interface = _create_default_physical_interface(
-                        node, ifname, link_mac)
-                else:
-                    # Interface already exists on this Node, so just update
-                    # the name.
-                    interface.name = ifname
-                    interface.save()
-            except PhysicalInterface.DoesNotExist:
-                interface = _create_default_physical_interface(
-                    node, ifname, link_mac)
-
+                        node, ifname, mac)
+                    ip_addr_info[ifname]['model'] = interface
+    # Record any observed IP addresses on the interfaces we created.
+    for link in ip_addr_info.values():
+        interface = link.get('model')
+        if interface is not None:
             current_interfaces.add(interface)
             ips = link.get('inet', []) + link.get('inet6', [])
             interface.update_ip_addresses(ips)
-
-    for iface in PhysicalInterface.objects.filter(node=node):
-        if iface not in current_interfaces:
-            iface.delete()
+    # Delete any obsolete interfaces on this node.
+    for interface in PhysicalInterface.objects.filter(node=node):
+        if interface not in current_interfaces:
+            interface.delete()
 
 
 def update_node_network_interface_tags(node, output, exit_status):