Merge ~mpontillo/maas:compose-machine-form--interpret-interfaces-constraint into maas:master

[Mike Pontillo]

Interpret interfaces constraint when composing machines from a pod.

559c5d8... by Mike Pontillo on 2018-07-27

Format imports.

[Newell Jensen (newell-jensen)]

We should be using MACVLAN_MODE and MACVLAN_MODE_CHOICES from maasserver.enum in several places. See inline comments.

review: Needs Fixing

cfdd414... by Mike Pontillo on 2018-07-27

Address review comments. Add constants for use with interface attach type. Use attach type constants in virsh driver.

bc2a305... by Mike Pontillo on 2018-07-27

Move factory.pick_choice() to parent class. Use new constants in virsh pod driver tests.

[Newell Jensen (newell-jensen)]

Looks good. Thanks for the fixes!

review: Approve

df002f8... by Mike Pontillo on 2018-07-27

Properly format imports.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b compose-machine-form--interpret-interfaces-constraint lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3461/console
COMMIT: ac287a5cba10e8ec323883f10b6c36c93ac761bd

review: Needs Fixing

7a78b35... by Mike Pontillo on 2018-07-30

Move common enums into provisioningserver code.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b compose-machine-form--interpret-interfaces-constraint lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3477/console
COMMIT: f61551b9847df0ae77e750b72ae456ce11d8e74b

review: Needs Fixing

4d7ee79... by Mike Pontillo on 2018-07-30

Fix merge conflict.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b compose-machine-form--interpret-interfaces-constraint lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 4d7ee797ccb3e36258ba203c7d44e01b9e5f9e83

review: Approve

diff --git a/src/maasserver/api/tests/test_pods.py b/src/maasserver/api/tests/test_pods.py
index be26a44..0caf498 100644
--- a/src/maasserver/api/tests/test_pods.py
+++ b/src/maasserver/api/tests/test_pods.py
@@ -9,10 +9,7 @@ import http.client
 import random
 from unittest.mock import MagicMock
 
-from maasserver.enum import (
-    MACVLAN_MODE_CHOICES,
-    NODE_CREATION_TYPE,
-)
+from maasserver.enum import NODE_CREATION_TYPE
 from maasserver.forms import pods
 from maasserver.models.bmc import Pod
 from maasserver.models.node import Machine
@@ -28,6 +25,7 @@ from provisioningserver.drivers.pod import (
     DiscoveredPod,
     DiscoveredPodHints,
 )
+from provisioningserver.enum import MACVLAN_MODE_CHOICES
 from twisted.internet.defer import succeed
 
 
diff --git a/src/maasserver/enum.py b/src/maasserver/enum.py
index 9f066c0..b9e8a4a 100644
--- a/src/maasserver/enum.py
+++ b/src/maasserver/enum.py
@@ -16,8 +16,6 @@ __all__ = [
     'INTERFACE_TYPE_CHOICES',
     'INTERFACE_TYPE_CHOICES_DICT',
     'IPADDRESS_TYPE',
-    'MACVLAN_MODE',
-    'MACVLAN_MODE_CHOICES',
     'NODE_PERMISSION',
     'NODE_STATUS',
     'NODE_STATUS_CHOICES',
@@ -778,19 +776,3 @@ ENDPOINT_CHOICES = (
     (ENDPOINT.API, "API"),
     (ENDPOINT.UI, "WebUI"),
 )
-
-
-class MACVLAN_MODE:
-
-    BRIDGE = "bridge"
-    PASSTHRU = "passthru"
-    PRIVATE = "private"
-    VEPA = "vepa"
-
-
-MACVLAN_MODE_CHOICES = (
-    (MACVLAN_MODE.BRIDGE, "bridge"),
-    (MACVLAN_MODE.PASSTHRU, "passthru"),
-    (MACVLAN_MODE.PRIVATE, "private"),
-    (MACVLAN_MODE.VEPA, "vepa"),
-)
diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index 779a015..764bff8 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -30,7 +30,7 @@ from maasserver.clusterrpc.pods import (
 )
 from maasserver.enum import (
     BMC_TYPE,
-    MACVLAN_MODE_CHOICES,
+    INTERFACE_TYPE,
     NODE_CREATION_TYPE,
 )
 from maasserver.exceptions import PodProblem
@@ -40,6 +40,7 @@ from maasserver.models import (
     BMC,
     BMCRoutableRackControllerRelationship,
     Domain,
+    Interface,
     Machine,
     Node,
     Pod,
@@ -50,6 +51,9 @@ from maasserver.models import (
 )
 from maasserver.node_constraint_filter_forms import (
     get_storage_constraints_from_string,
+    interfaces_validator,
+    LabeledConstraintMapField,
+    nodes_by_interface,
     storage_validator,
 )
 from maasserver.rpc import getClientFromIdentifiers
@@ -63,10 +67,15 @@ import petname
 from provisioningserver.drivers import SETTING_SCOPE
 from provisioningserver.drivers.pod import (
     Capabilities,
+    InterfaceAttachType,
     RequestedMachine,
     RequestedMachineBlockDevice,
     RequestedMachineInterface,
 )
+from provisioningserver.enum import (
+    MACVLAN_MODE,
+    MACVLAN_MODE_CHOICES,
+)
 from provisioningserver.utils.twisted import asynchronous
 from twisted.python.threadable import isInIOThread
 
@@ -398,6 +407,10 @@ class ComposeMachineForm(forms.Form):
         self.fields['storage'] = CharField(
             validators=[storage_validator], required=False)
         self.initial['storage'] = 'root:8(local)'
+        self.fields['interfaces'] = LabeledConstraintMapField(
+            validators=[interfaces_validator], label="Interface constraints",
+            required=False)
+        self.initial['interfaces'] = None
         self.fields['skip_commissioning'] = BooleanField(required=False)
         self.initial['skip_commissioning'] = False
 
@@ -414,16 +427,24 @@ class ComposeMachineForm(forms.Form):
 
     def get_requested_machine(self):
         """Return the `RequestedMachine`."""
-        # XXX blake_r 2017-04-04: Interfaces are hard coded at the
-        # moment. Will be extended later.
         block_devices = []
-        constraints = get_storage_constraints_from_string(
+
+        storage_constraints = get_storage_constraints_from_string(
             self.get_value_for('storage'))
-        for _, size, tags in constraints:
+        for _, size, tags in storage_constraints:
             if tags is None:
                 tags = []
             block_devices.append(
                 RequestedMachineBlockDevice(size=size, tags=tags))
+        interfaces_label_map = self.get_value_for('interfaces')
+        if interfaces_label_map is not None:
+            requested_machine_interfaces = (
+                self._get_requested_machine_interfaces_via_constraints(
+                    interfaces_label_map)
+            )
+        else:
+            requested_machine_interfaces = [RequestedMachineInterface()]
+
         return RequestedMachine(
             hostname=self.get_value_for('hostname'),
             architecture=self.get_value_for('architecture'),
@@ -431,7 +452,49 @@ class ComposeMachineForm(forms.Form):
             memory=self.get_value_for('memory'),
             cpu_speed=self.get_value_for('cpu_speed'),
             block_devices=block_devices,
-            interfaces=[RequestedMachineInterface()])
+            interfaces=requested_machine_interfaces)
+
+    def _get_requested_machine_interfaces_via_constraints(
+            self, interfaces_label_map):
+        requested_machine_interfaces = []
+        if self.pod.host is None:
+            raise ValidationError(
+                "Pod must be on a known host if interfaces are specified.")
+        node_ids, compatible_interfaces = nodes_by_interface(
+            interfaces_label_map)
+        pod_node_id = self.pod.host.id
+        if pod_node_id not in node_ids:
+            raise ValidationError(
+                "This pod does not match the specified networks.")
+        for label in compatible_interfaces.keys():
+            interface_ids = compatible_interfaces[label][pod_node_id]
+            # XXX: We might want to use the "deepest" interface in the
+            # heirarchy, to ensure we get a bridge or bond (if configured)
+            # rather than its parent. max() is a good approximation, since
+            # child interfaces will be created after their parents.
+            requested_machine_interfaces.append(
+                self.get_requested_machine_interface_by_interface_id(
+                    max(interface_ids))
+            )
+        return requested_machine_interfaces
+
+    def get_requested_machine_interface_by_interface_id(self, interface_id):
+        interface = Interface.objects.get(id=interface_id)
+        if interface.type == INTERFACE_TYPE.BRIDGE:
+            rmi = RequestedMachineInterface(
+                attach_name=interface.name,
+                attach_type=InterfaceAttachType.BRIDGE)
+        else:
+            attach_options = self.pod.default_macvlan_mode
+            if attach_options is None:
+                # Default macvlan mode is 'bridge' if not specified, since that
+                # provides the best chance for connectivity.
+                attach_options = MACVLAN_MODE.BRIDGE
+            rmi = RequestedMachineInterface(
+                attach_name=interface.name,
+                attach_type=InterfaceAttachType.MACVLAN,
+                attach_options=attach_options)
+        return rmi
 
     def save(self):
         """Prevent from usage."""
diff --git a/src/maasserver/forms/tests/test_pods.py b/src/maasserver/forms/tests/test_pods.py
index 525ec03..97bd687 100644
--- a/src/maasserver/forms/tests/test_pods.py
+++ b/src/maasserver/forms/tests/test_pods.py
@@ -20,8 +20,8 @@ from django.core.validators import (
 )
 from maasserver.enum import (
     BMC_TYPE,
-    MACVLAN_MODE_CHOICES,
     NODE_CREATION_TYPE,
+    NODE_STATUS,
 )
 from maasserver.exceptions import PodProblem
 from maasserver.forms import pods as pods_module
@@ -53,10 +53,15 @@ from provisioningserver.drivers.pod import (
     DiscoveredPod,
     DiscoveredPodHints,
     DiscoveredPodStoragePool,
+    InterfaceAttachType,
     RequestedMachine,
     RequestedMachineBlockDevice,
     RequestedMachineInterface,
 )
+from provisioningserver.enum import (
+    MACVLAN_MODE,
+    MACVLAN_MODE_CHOICES,
+)
 from testtools import ExpectedException
 from testtools.matchers import (
     Equals,
@@ -910,6 +915,194 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
                             size=Equals(disk_2), tags=Equals([]))),
                     ]))))
 
+    def test__get_machine_with_interfaces_fails_for_no_pod_host(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        pod = make_pod_with_hints()
+        interfaces = "eth0:subnet=%s" % (
+            host.boot_interface.vlan.subnet_set.first().cidr)
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        with ExpectedException(
+                ValidationError,
+                ".*must be on a known host if interfaces are specified.*"):
+            form.get_requested_machine()
+
+    def test__get_machine_with_interfaces_fails_for_no_matching_network(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        # Make a subnet that won't match the host via the constraint.
+        subnet = factory.make_Subnet()
+        pod = make_pod_with_hints()
+        pod.host = host
+        pod.save()
+        interfaces = "eth0:subnet=%s" % (
+            subnet.cidr)
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        with ExpectedException(
+                ValidationError,
+                ".*does not match the specified network.*"):
+            form.get_requested_machine()
+
+    def test__get_machine_with_interfaces_by_subnet(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        space = factory.make_Space('dmz')
+        host.boot_interface.vlan.space = space
+        host.boot_interface.vlan.save()
+        pod = make_pod_with_hints()
+        pod.host = host
+        pod.save()
+        interfaces = "eth0:subnet=%s" % (
+            host.boot_interface.vlan.subnet_set.first().cidr)
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        request_machine = form.get_requested_machine()
+        self.assertThat(request_machine, MatchesAll(
+            IsInstance(RequestedMachine),
+            MatchesStructure(
+                interfaces=MatchesListwise([
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(host.boot_interface.name),
+                            attach_type=Equals(InterfaceAttachType.MACVLAN),
+                            attach_options=Equals(MACVLAN_MODE.BRIDGE)))
+                ]))))
+
+    def test__get_machine_with_interfaces_by_subnet_with_default_mode(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        space = factory.make_Space('dmz')
+        host.boot_interface.vlan.space = space
+        host.boot_interface.vlan.save()
+        pod = make_pod_with_hints()
+        pod.host = host
+        attach_mode = factory.pick_choice(MACVLAN_MODE_CHOICES)
+        pod.default_macvlan_mode = attach_mode
+        pod.save()
+        interfaces = "eth0:subnet=%s" % (
+            host.boot_interface.vlan.subnet_set.first().cidr)
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        request_machine = form.get_requested_machine()
+        self.assertThat(request_machine, MatchesAll(
+            IsInstance(RequestedMachine),
+            MatchesStructure(
+                interfaces=MatchesListwise([
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(host.boot_interface.name),
+                            attach_type=Equals(InterfaceAttachType.MACVLAN),
+                            attach_options=Equals(attach_mode)))
+                ]))))
+
+    def test__get_machine_with_interfaces_by_space(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        space = factory.make_Space('dmz')
+        host.boot_interface.vlan.space = space
+        host.boot_interface.vlan.save()
+        pod = make_pod_with_hints()
+        pod.host = host
+        pod.save()
+        interfaces = "eth0:space=dmz"
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        request_machine = form.get_requested_machine()
+        self.assertThat(request_machine, MatchesAll(
+            IsInstance(RequestedMachine),
+            MatchesStructure(
+                interfaces=MatchesListwise([
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(host.boot_interface.name),
+                            attach_type=Equals(InterfaceAttachType.MACVLAN),
+                            attach_options=Equals(MACVLAN_MODE.BRIDGE)))
+                ]))))
+
+    def test__get_machine_with_interfaces_by_spaces(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet()
+        dmz_space = factory.make_Space('dmz')
+        storage_space = factory.make_Space('storage')
+        # Because PXE booting from the DMZ is /always/ a great idea. ;-)
+        host.boot_interface.vlan.space = dmz_space
+        host.boot_interface.vlan.save()
+        storage_vlan = factory.make_VLAN(space=storage_space)
+        storage_if = factory.make_Interface(node=host, vlan=storage_vlan)
+        pod = make_pod_with_hints()
+        pod.host = host
+        pod.save()
+        interfaces = "eth0:space=dmz;eth1:space=storage"
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        request_machine = form.get_requested_machine()
+        self.assertThat(request_machine, MatchesAll(
+            IsInstance(RequestedMachine),
+            MatchesStructure(
+                interfaces=MatchesListwise([
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(host.boot_interface.name),
+                            attach_type=Equals(InterfaceAttachType.MACVLAN),
+                            attach_options=Equals(MACVLAN_MODE.BRIDGE))),
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(storage_if.name),
+                            attach_type=Equals(InterfaceAttachType.MACVLAN),
+                            attach_options=Equals(MACVLAN_MODE.BRIDGE))),
+                ]))))
+
+    def test__get_machine_with_interfaces_by_space_as_bridge(self):
+        request = MagicMock()
+        host = factory.make_Machine_with_Interface_on_Subnet(
+            status=NODE_STATUS.READY)
+        space = factory.make_Space('dmz')
+        host.boot_interface.vlan.space = space
+        host.boot_interface.vlan.save()
+        # This is just to make sure a bridge will be available for attachment.
+        # We expect bridge mode to be preferred, when available.
+        host.acquire(factory.make_User(), bridge_all=True)
+        pod = make_pod_with_hints()
+        pod.host = host
+        pod.save()
+        interfaces = "eth0:space=dmz"
+        form = ComposeMachineForm(data={
+            'interfaces': interfaces,
+        }, request=request, pod=pod)
+        self.assertTrue(form.is_valid(), form.errors)
+        request_machine = form.get_requested_machine()
+        self.assertThat(request_machine, MatchesAll(
+            IsInstance(RequestedMachine),
+            MatchesStructure(
+                interfaces=MatchesListwise([
+                    MatchesAll(
+                        IsInstance(RequestedMachineInterface),
+                        MatchesStructure(
+                            attach_name=Equals(
+                                "br-" + host.boot_interface.name),
+                            attach_type=Equals(InterfaceAttachType.BRIDGE),
+                            attach_options=Is(None)))
+                ]))))
+
     def test__save_raises_AttributeError(self):
         request = MagicMock()
         pod = make_pod_with_hints()
diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index df7e69e..52aebe0 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -40,7 +40,6 @@ from maasserver.enum import (
     BMC_TYPE_CHOICES,
     INTERFACE_TYPE,
     IPADDRESS_TYPE,
-    MACVLAN_MODE_CHOICES,
     NODE_CREATION_TYPE,
     NODE_STATUS,
 )
@@ -78,6 +77,7 @@ import petname
 from provisioningserver.drivers import SETTING_SCOPE
 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.twisted import asynchronous
 from twisted.internet.defer import inlineCallbacks
diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
index 8ab4018..8a50f8b 100644
--- a/src/maasserver/testing/factory.py
+++ b/src/maasserver/testing/factory.py
@@ -242,23 +242,6 @@ class Factory(maastesting.factory.Factory):
         upload.name = name
         return upload
 
-    def pick_choice(self, choices, but_not=None):
-        """Pick a random item from `choices`.
-
-        :param choices: A sequence of choices in Django form choices format:
-            [
-                ('choice_id_1', "Choice name 1"),
-                ('choice_id_2', "Choice name 2"),
-            ]
-        :param but_not: A list of choices' IDs to exclude.
-        :type but_not: Sequence.
-        :return: The "id" portion of a random choice out of `choices`.
-        """
-        if but_not is None:
-            but_not = ()
-        return random.choice(
-            [choice for choice in choices if choice[0] not in but_not])[0]
-
     def pick_power_type(self, but_not=None):
         """Pick a random power type and return it.
 
diff --git a/src/maastesting/factory.py b/src/maastesting/factory.py
index 7589626..db61d5d 100644
--- a/src/maastesting/factory.py
+++ b/src/maastesting/factory.py
@@ -194,6 +194,23 @@ class Factory:
         assert port_min >= 0 and port_max <= 65535
         return random.randint(port_min, port_max)
 
+    def pick_choice(self, choices, but_not=None):
+        """Pick a random item from `choices`.
+
+        :param choices: A sequence of choices in Django form choices format:
+            [
+                ('choice_id_1', "Choice name 1"),
+                ('choice_id_2', "Choice name 2"),
+            ]
+        :param but_not: A list of choices' IDs to exclude.
+        :type but_not: Sequence.
+        :return: The "id" portion of a random choice out of `choices`.
+        """
+        if but_not is None:
+            but_not = ()
+        return random.choice(
+            [choice for choice in choices if choice[0] not in but_not])[0]
+
     def make_vlan_tag(self, allow_none=False, *, but_not=EMPTY_SET):
         """Create a random VLAN tag.
 
diff --git a/src/provisioningserver/drivers/pod/__init__.py b/src/provisioningserver/drivers/pod/__init__.py
index 602b893..654b46c 100644
--- a/src/provisioningserver/drivers/pod/__init__.py
+++ b/src/provisioningserver/drivers/pod/__init__.py
@@ -172,6 +172,21 @@ class BlockDeviceType:
     ISCSI = 'iscsi'
 
 
+class InterfaceAttachType:
+    """Different interface attachment types."""
+
+    # Interface attached to a network predefined in the hypervisor.
+    # (This is the default if no constraints are specified; MAAS will look for
+    # a 'maas' network, and then fall back to a 'default' network.)
+    NETWORK = 'network'
+
+    # Interface attached to a bridge interface on the hypervisor.
+    BRIDGE = 'bridge'
+
+    # Interface attached via a non-bridge interface using the macvlan driver.
+    MACVLAN = 'macvlan'
+
+
 class AttrHelperMixin:
     """Mixin to add the `fromdict` and `asdict` to the classes."""
 
diff --git a/src/provisioningserver/drivers/pod/tests/test_virsh.py b/src/provisioningserver/drivers/pod/tests/test_virsh.py
index fe4a766..c109c5c 100644
--- a/src/provisioningserver/drivers/pod/tests/test_virsh.py
+++ b/src/provisioningserver/drivers/pod/tests/test_virsh.py
@@ -29,6 +29,7 @@ import pexpect
 from provisioningserver.drivers.pod import (
     Capabilities,
     DiscoveredPodStoragePool,
+    InterfaceAttachType,
     RequestedMachine,
     RequestedMachineBlockDevice,
     RequestedMachineInterface,
@@ -45,6 +46,7 @@ from provisioningserver.drivers.pod.virsh import (
     DOM_TEMPLATE_S390X,
     VirshPodDriver,
 )
+from provisioningserver.enum import MACVLAN_MODE_CHOICES
 from provisioningserver.rpc.exceptions import PodInvalidResources
 from provisioningserver.utils.shell import (
     get_env_with_locale,
@@ -1275,8 +1277,8 @@ class TestVirshSSH(MAASTestCase):
         fake_mac = factory.make_mac_address()
         interface = RequestedMachineInterface(
             attach_name=factory.make_name('name'),
-            attach_type='macvlan', attach_options=random.choice([
-                'private', 'vepa', 'bridge', 'passthru']))
+            attach_type=InterfaceAttachType.MACVLAN,
+            attach_options=factory.pick_choice(MACVLAN_MODE_CHOICES))
         self.patch(virsh, 'generate_mac_address').return_value = fake_mac
         NamedTemporaryFile = self.patch(virsh_module, "NamedTemporaryFile")
         tmpfile = NamedTemporaryFile.return_value
@@ -1309,8 +1311,8 @@ class TestVirshSSH(MAASTestCase):
         fake_mac = factory.make_mac_address()
         interface = RequestedMachineInterface(
             attach_name=factory.make_name('ifname'),
-            attach_type='bridge', attach_options=random.choice([
-                'private', 'vepa', 'bridge', 'passthru']))
+            attach_type=InterfaceAttachType.BRIDGE,
+            attach_options=factory.pick_choice(MACVLAN_MODE_CHOICES))
         self.patch(virsh, 'generate_mac_address').return_value = fake_mac
         NamedTemporaryFile = self.patch(virsh_module, "NamedTemporaryFile")
         tmpfile = NamedTemporaryFile.return_value
diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
index 42914c0..b16e0c4 100644
--- a/src/provisioningserver/drivers/pod/virsh.py
+++ b/src/provisioningserver/drivers/pod/virsh.py
@@ -29,6 +29,7 @@ from provisioningserver.drivers.pod import (
     DiscoveredPod,
     DiscoveredPodHints,
     DiscoveredPodStoragePool,
+    InterfaceAttachType,
     PodDriver,
 )
 from provisioningserver.logger import get_maas_logger
@@ -853,7 +854,7 @@ class VirshSSH(pexpect.spawn):
         """Attach new network interface on `domain` to `network`."""
         mac = generate_mac_address()
         # If attachment type is not specified, default to network.
-        if interface.attach_type in (None, 'network'):
+        if interface.attach_type in (None, InterfaceAttachType.NETWORK):
             # Set the network if we are explicity attaching a network
             # specified by the user.
             if interface.attach_type is not None:
@@ -869,10 +870,10 @@ class VirshSSH(pexpect.spawn):
             'mac_address': mac,
             'attach_name': interface.attach_name,
         }
-        if interface.attach_type == 'macvlan':
+        if interface.attach_type == InterfaceAttachType.MACVLAN:
             device_params['attach_options'] = interface.attach_options
             device_xml = DOM_TEMPLATE_MACVLAN_INTERFACE.format(**device_params)
-        if interface.attach_type == 'bridge':
+        elif interface.attach_type == InterfaceAttachType.BRIDGE:
             device_xml = DOM_TEMPLATE_BRIDGE_INTERFACE.format(**device_params)
 
         # Rewrite the XML in a temporary file to use with 'virsh define'.
diff --git a/src/provisioningserver/enum.py b/src/provisioningserver/enum.py
new file mode 100644
index 0000000..ef8ff9d
--- /dev/null
+++ b/src/provisioningserver/enum.py
@@ -0,0 +1,30 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Enumerations meaningful to the rack contoller (and possibly the region)."""
+
+__all__ = [
+    "MACVLAN_MODE",
+    "MACVLAN_MODE_CHOICES",
+]
+
+# *** IMPORTANT ***
+# Note to all ye who enter here: comments beginning with #: are special
+# to Sphinx. They are extracted and form part of the documentation of
+# the field they directly precede.
+
+
+class MACVLAN_MODE:
+
+    BRIDGE = "bridge"
+    PASSTHRU = "passthru"
+    PRIVATE = "private"
+    VEPA = "vepa"
+
+
+MACVLAN_MODE_CHOICES = (
+    (MACVLAN_MODE.BRIDGE, "bridge"),
+    (MACVLAN_MODE.PASSTHRU, "passthru"),
+    (MACVLAN_MODE.PRIVATE, "private"),
+    (MACVLAN_MODE.VEPA, "vepa"),
+)