Merge ~mpontillo/maas:pod-compose--use-given-ifnames into maas:master

[Mike Pontillo]

Use interface labels from contraints when composing machines.

[Newell Jensen (newell-jensen)]

Nicely done. +1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b pod-compose--use-given-ifnames lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3572/consoleText

2f14642... by Mike Pontillo on 2018-08-08

Fix lint.

diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index 4bd535b..d83ea00 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -538,9 +538,11 @@ class ComposeMachineForm(forms.Form):
                 discovered_machine, self.request.user,
                 skip_commissioning=skip_commissioning,
                 creation_type=creation_type,
+                interfaces=self.get_value_for('interfaces'),
                 domain=self.get_value_for('domain'),
                 pool=self.get_value_for('pool'),
-                zone=self.get_value_for('zone'))
+                zone=self.get_value_for('zone'),
+            )
             self.pod.sync_hints(pod_hints)
             return created_machine
 
diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index 52aebe0..b95ace5 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -79,6 +79,7 @@ from provisioningserver.drivers.pod import BlockDeviceType
 from provisioningserver.drivers.power.registry import PowerDriverRegistry
 from provisioningserver.enum import MACVLAN_MODE_CHOICES
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.utils.constraints import LabeledConstraintMap
 from provisioningserver.utils.twisted import asynchronous
 from twisted.internet.defer import inlineCallbacks
 
@@ -668,7 +669,8 @@ class Pod(BMC):
     def create_machine(
             self, discovered_machine, commissioning_user,
             skip_commissioning=False,
-            creation_type=NODE_CREATION_TYPE.PRE_EXISTING, **kwargs):
+            creation_type=NODE_CREATION_TYPE.PRE_EXISTING, interfaces=None,
+            **kwargs):
         """Create's a `Machine` from `discovered_machines` for this pod."""
         if skip_commissioning:
             status = NODE_STATUS.READY
@@ -695,6 +697,9 @@ class Pod(BMC):
         if pool is None:
             pool = self.pool
 
+        if interfaces is not None:
+            assert isinstance(interfaces, LabeledConstraintMap)
+
         # Create the machine.
         machine = Machine(
             hostname=discovered_machine.hostname,
@@ -754,11 +759,30 @@ class Pod(BMC):
         if skip_commissioning:
             machine.set_default_storage_layout()
 
+        # Enumerating the LabeledConstraintMap of interfaces will yield the
+        # name of each interface, in the same order that they will exist
+        # on the hypervisor. (This is a fortunate coincidence, since
+        # dictionaries in Python 3.6+ preserve insertion order.)
+        if interfaces is not None:
+            interface_names = [
+                label[:15]
+                for label in interfaces
+            ]
+        else:
+            interface_names = []
+        if len(discovered_machine.interfaces) > len(interface_names):
+            # The lists should never have different lengths, but use default
+            # names for all interfaces to avoid conflicts, just in case.
+            # (This also happens if no interface labels were supplied.)
+            interface_names = [
+                'eth%d' % i
+                for i in range(len(discovered_machine.interfaces))
+            ]
         # Create the discovered interface and set the default networking
         # configuration.
         for idx, discovered_nic in enumerate(discovered_machine.interfaces):
             interface = self._create_interface(
-                discovered_nic, machine, name='eth%d' % idx)
+                discovered_nic, machine, name=interface_names[idx])
             if discovered_nic.boot:
                 machine.boot_interface = interface
                 machine.save(update_fields=['boot_interface'])
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index 6276e65..66ff22b 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -58,6 +58,7 @@ from provisioningserver.drivers.pod import (
     DiscoveredPodStoragePool,
 )
 from provisioningserver.rpc.cluster import DecomposeMachine
+from provisioningserver.utils.constraints import LabeledConstraintMap
 from testtools.matchers import (
     Equals,
     HasLength,
@@ -968,6 +969,86 @@ class TestPod(MAASServerTestCase):
         machine = pod.create_machine(discovered_machine, factory.make_User())
         self.assertTrue(tag in machine.tags.all())
 
+    def test_create_machine_sets_interface_names_using_constraint_labels(self):
+        discovered_machine = self.make_discovered_machine()
+        self.patch(Machine, "set_default_storage_layout")
+        self.patch(Machine, "set_initial_networking_configuration")
+        self.patch(Machine, "start_commissioning")
+        fabric = factory.make_Fabric()
+        vlan = factory.make_VLAN(
+            fabric=fabric, dhcp_on=True,
+            primary_rack=factory.make_RackController())
+        vlan2 = factory.make_VLAN(
+            fabric=fabric, dhcp_on=False,
+            primary_rack=factory.make_RackController())
+        vlan3 = factory.make_VLAN(
+            fabric=fabric, dhcp_on=False,
+            primary_rack=factory.make_RackController())
+        pod = factory.make_Pod()
+        machine = pod.create_machine(
+            discovered_machine, factory.make_User(),
+            interfaces=LabeledConstraintMap(
+                'maas0:vlan=id:%d;maas1:vlan=id:%d;maas2:vlan=id:%d' % (
+                    vlan.id, vlan2.id, vlan3.id))
+        )
+        # Check that the interface names match the labels provided in the
+        # constraints string.
+        self.assertItemsEqual(
+            ['maas0', 'maas1', 'maas2'],
+            list(machine.interface_set.order_by('id').values_list(
+                'name', flat=True)))
+
+    def test_create_machine_uses_default_ifnames_if_discovered_mismatch(self):
+        """This makes sure that if the discovered machine comes back with
+        a different number of interfaces than the constraint string, the
+        default (ethX) names are used.
+        """
+        discovered_machine = self.make_discovered_machine()
+        self.patch(Machine, "set_default_storage_layout")
+        self.patch(Machine, "set_initial_networking_configuration")
+        self.patch(Machine, "start_commissioning")
+        fabric = factory.make_Fabric()
+        vlan = factory.make_VLAN(
+            fabric=fabric, dhcp_on=True,
+            primary_rack=factory.make_RackController())
+        vlan2 = factory.make_VLAN(
+            fabric=fabric, dhcp_on=False,
+            primary_rack=factory.make_RackController())
+        pod = factory.make_Pod()
+        # The constraint here as two labels, but the discovered machine will
+        # have three interfaces.
+        machine = pod.create_machine(
+            discovered_machine, factory.make_User(),
+            interfaces=LabeledConstraintMap(
+                'maas0:vlan=id:%d;maas1:vlan=id:%d' % (
+                    vlan.id, vlan2.id))
+        )
+        # Check that the interface names use the ethX numbering, since the
+        # provided constraints won't match the number of interfaces that were
+        # returned.
+        self.assertItemsEqual(
+            ['eth0', 'eth1', 'eth2'],
+            list(machine.interface_set.order_by('id').values_list(
+                'name', flat=True)))
+
+    def test_create_machine_uses_default_names_if_no_interfaces(self):
+        discovered_machine = self.make_discovered_machine()
+        self.patch(Machine, "set_default_storage_layout")
+        self.patch(Machine, "set_initial_networking_configuration")
+        self.patch(Machine, "start_commissioning")
+        fabric = factory.make_Fabric()
+        factory.make_VLAN(
+            fabric=fabric, dhcp_on=True,
+            primary_rack=factory.make_RackController())
+        pod = factory.make_Pod()
+        machine = pod.create_machine(discovered_machine, factory.make_User())
+        # Check that the interface names match the labels provided in the
+        # constraints string.
+        self.assertItemsEqual(
+            ['eth0', 'eth1', 'eth2'],
+            list(machine.interface_set.order_by('id').values_list(
+                'name', flat=True)))
+
     def test_sync_pod_creates_new_machines_connected_to_dhcp_vlan(self):
         discovered = self.make_discovered_pod()
         mock_set_default_storage_layout = self.patch(