Merge lp:~rbanffy/maas/bug-1337874 into lp:~maas-committers/maas/trunk

[Ricardo Bánffy]

Update node network interface information during commissioning.

Added a new step in the commissioning process that uses `ip link` to gather the MAC addresses of interfaces currently connected to the node and update the node information according to that new information.

[Blake Rouse (blake-rouse)]

Looks close. Just a few comments that should be addressed.

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Little better. Still wonder why my suggestion was not fully implemented.

review: Needs Information

[Blake Rouse (blake-rouse)]

Looks good. This will be so nice being fixed, always been a bug that people wanted fixed.

review: Approve

=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2015-03-25 15:33:23 +0000
+++ src/metadataserver/models/commissioningscript.py	2015-04-01 22:34:36 +0000
@@ -43,6 +43,7 @@
 )
 from lxml import etree
 from maasserver.fields import MAC
+from maasserver.models.macaddress import MACAddress
 from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.tag import Tag
 from metadataserver import DefaultMeta
@@ -118,6 +119,11 @@
     lshw -xml
     """)
 
+# Built-in script to run `ip link`
+IPLINK_SCRIPT = dedent("""\
+    #!/bin/sh
+    ip link
+    """)
 
 # Count the processors which do not declare their number of 'threads'
 # as 1 processor.
@@ -140,6 +146,44 @@
 """
 
 
+def update_node_network_information(node, output, exit_status):
+    """Updates the network interfaces from the results of `IPLINK_SCRIPT`.
+
+    Creates and deletes MACAddresses according to what we currently know about
+    this node's hardware.
+
+    If `exit_status` is non-zero, this function returns without doing
+    anything.
+
+    """
+    assert isinstance(output, bytes)
+    if exit_status != 0:
+        return
+
+    # Get the MAC addresses of all connected interfaces.
+    hw_macaddresses = {
+        MAC(line.split()[1]) for line in output.splitlines()
+        if line.strip().startswith('link/ether')
+    }
+
+    # MAC addresses found in the db but not on the hardware node will be
+    # deleted.
+    for mac_address in node.macaddress_set.all():
+        if mac_address not in hw_macaddresses:
+            mac_address.delete()
+
+    # MAC addresses found in the hardware node but not on the db will be
+    # created or reassigned.
+    for address in hw_macaddresses:
+        if address not in node.macaddress_set.all():
+            try:
+                mac_address = MACAddress.objects.get(mac_address=address)
+                mac_address.node = node
+                mac_address.save()
+            except MACAddress.DoesNotExist:
+                MACAddress(mac_address=address, node=node).save()
+
+
 def update_hardware_details(node, output, exit_status):
     """Process the results of `LSHW_SCRIPT`.
 
@@ -547,6 +591,10 @@
         'content': make_function_call_script(gather_physical_block_devices),
         'hook': update_node_physical_block_devices,
     },
+    '00-maas-07-network-interfaces.out': {
+        'content': IPLINK_SCRIPT.encode('ascii'),
+        'hook': update_node_network_information,
+    },
     '99-maas-01-wait-for-lldpd.out': {
         'content': make_function_call_script(
             lldpd_wait, "/var/run/lldpd.socket", time_delay=60),

=== added file 'src/metadataserver/models/tests/ip_link_results.txt'
--- src/metadataserver/models/tests/ip_link_results.txt	1970-01-01 00:00:00 +0000
+++ src/metadataserver/models/tests/ip_link_results.txt	2015-04-01 22:34:36 +0000
@@ -0,0 +1,18 @@
+1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default
+    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
+2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP mode DEFAULT group default qlen 1000
+    link/ether ec:f4:bb:f9:17:8e brd ff:ff:ff:ff:ff:ff
+3: wlan0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc mq state DOWN mode DORMANT group default qlen 1000
+    link/ether 38:b1:db:cd:f0:ab brd ff:ff:ff:ff:ff:ff
+5: virbr2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default
+    link/ether 52:54:00:03:53:aa brd ff:ff:ff:ff:ff:ff
+6: virbr2-nic: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast master virbr2 state DOWN mode DEFAULT group default qlen 500
+    link/ether 52:54:00:03:53:aa brd ff:ff:ff:ff:ff:ff
+7: virbr1: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default
+    link/ether 52:54:00:67:f5:3a brd ff:ff:ff:ff:ff:ff
+8: virbr1-nic: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast master virbr1 state DOWN mode DEFAULT group default qlen 500
+    link/ether 52:54:00:67:f5:3a brd ff:ff:ff:ff:ff:ff
+9: vnet0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master virbr2 state UNKNOWN mode DEFAULT group default qlen 500
+    link/ether fe:54:00:b4:f1:61 brd ff:ff:ff:ff:ff:ff
+10: vnet1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master virbr2 state UNKNOWN mode DEFAULT group default qlen 500
+    link/ether fe:54:00:02:36:19 brd ff:ff:ff:ff:ff:ff

=== modified file 'src/metadataserver/models/tests/test_noderesults.py'
--- src/metadataserver/models/tests/test_noderesults.py	2015-03-25 15:33:23 +0000
+++ src/metadataserver/models/tests/test_noderesults.py	2015-04-01 22:34:36 +0000
@@ -38,6 +38,7 @@
 
 from fixtures import FakeLogger
 from maasserver.fields import MAC
+from maasserver.models.macaddress import MACAddress
 from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.tag import Tag
 from maasserver.testing.factory import factory
@@ -69,6 +70,7 @@
     set_node_routers,
     set_virtual_tag,
     update_hardware_details,
+    update_node_network_information,
     update_node_physical_block_devices,
 )
 from metadataserver.models.noderesult import NodeResult
@@ -1109,3 +1111,93 @@
         self.assertThat(
             PhysicalBlockDevice.objects.filter(node=node).first().tags,
             Contains('sata'))
+
+
+class TestUpdateNodeNetworkInformation(MAASServerTestCase):
+    """Tests the update_node_network_information function using data from the
+    ip_link_results.txt file to simulate `ip link`'s output.
+
+    The file records 6 different MAC addresses coming from different kinds of
+    interfaces:
+
+    38:b1:db:cd:f0:ab
+    52:54:00:03:53:aa
+    52:54:00:67:f5:3a
+    ec:f4:bb:f9:17:8e
+    fe:54:00:02:36:19
+    fe:54:00:b4:f1:61
+    """
+
+    def test__add_all_interfaces(self):
+        """Test a node that has no previously known interfaces on which we
+        need to add a series of interfaces.
+        """
+        node = factory.make_Node()
+        # Delete all MAC addresses eventually created by factory attached to
+        # this node.
+        MACAddress.objects.filter(node_id=node.id).delete()
+
+        output = open(
+            os.path.dirname(__file__) + '/ip_link_results.txt').read()
+        update_node_network_information(node, output, 0)
+
+        # Makes sure all the test dataset MAC addresses were added to the node.
+        node_macaddresses = [m.mac_address for m in node.macaddress_set.all()]
+        self.assertIn(MAC("38:b1:db:cd:f0:ab"), node_macaddresses)
+        self.assertIn(MAC("52:54:00:03:53:aa"), node_macaddresses)
+        self.assertIn(MAC("52:54:00:67:f5:3a"), node_macaddresses)
+        self.assertIn(MAC("ec:f4:bb:f9:17:8e"), node_macaddresses)
+        self.assertIn(MAC("fe:54:00:02:36:19"), node_macaddresses)
+        self.assertIn(MAC("fe:54:00:b4:f1:61"), node_macaddresses)
+
+    def test__one_mac_missing(self):
+        """Test whether we correcly detach a NIC that no longer appears to be
+        connected to the node.
+        """
+        node = factory.make_Node()
+
+        # Create a MAC address that we know is not in the test dataset.
+        mac_to_be_detached = factory.make_MACAddress(node=node)
+        mac_to_be_detached.mac_address = "01:23:45:67:89:ab"
+        mac_to_be_detached.save()
+
+        output = open(
+            os.path.dirname(__file__) + '/ip_link_results.txt').read()
+        update_node_network_information(node, output, 0)
+        db_macaddresses = [m.mac_address for m in node.macaddress_set.all()]
+
+        # These should have been added to the node.
+        self.assertIn(MAC("38:b1:db:cd:f0:ab"), db_macaddresses)
+        self.assertIn(MAC("52:54:00:03:53:aa"), db_macaddresses)
+        self.assertIn(MAC("52:54:00:67:f5:3a"), db_macaddresses)
+        self.assertIn(MAC("ec:f4:bb:f9:17:8e"), db_macaddresses)
+        self.assertIn(MAC("fe:54:00:02:36:19"), db_macaddresses)
+        self.assertIn(MAC("fe:54:00:b4:f1:61"), db_macaddresses)
+
+        # This one should have been removed because it no longer shows on the
+        # `ip link` output.
+        self.assertNotIn(MAC('01:23:45:67:89:ab'), db_macaddresses)
+
+    def test__reassign_mac(self):
+        """Test whether we can assign a MAC address previously connected to a
+        different node to the current one"""
+        node1 = factory.make_Node()
+
+        # Create a MAC address that we know IS in the test dataset.
+        mac_to_be_reassigned = factory.make_MACAddress(node=node1)
+        mac_to_be_reassigned.mac_address = MAC('38:b1:db:cd:f0:ab')
+        mac_to_be_reassigned.save()
+
+        node2 = factory.make_Node()
+        output = open(
+            os.path.dirname(__file__) + '/ip_link_results.txt').read()
+        update_node_network_information(node2, output, 0)
+
+        node2_db_macaddresses = [m.mac_address
+                                 for m in node2.macaddress_set.all()]
+        self.assertIn(MAC("38:b1:db:cd:f0:ab"), node2_db_macaddresses)
+        self.assertIn(MAC("52:54:00:03:53:aa"), node2_db_macaddresses)
+        self.assertIn(MAC("52:54:00:67:f5:3a"), node2_db_macaddresses)
+        self.assertIn(MAC("ec:f4:bb:f9:17:8e"), node2_db_macaddresses)
+        self.assertIn(MAC("fe:54:00:02:36:19"), node2_db_macaddresses)
+        self.assertIn(MAC("fe:54:00:b4:f1:61"), node2_db_macaddresses)