Merge ~mpontillo/maas:fix-pod-nonetype-relay-vlan--bug-1797135 into maas:master

[Mike Pontillo]

LP: #1797135 - Fix pod composition check for relay_vlan.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index 1b945e5..77f9f3e 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -353,6 +353,34 @@ class PodForm(MAASModelForm):
             return update_db((discovered_pod, discovered))
 
 
+def get_known_host_interfaces(host: Node) -> list:
+    """Given the specified host node, calculates each KnownHostInterface.
+
+    :return: a list of KnownHostInterface objects for the specified Node.
+    """
+    interfaces = host.interface_set.all()
+    result = []
+    for interface in interfaces:
+        ifname = interface.name
+        if interface.type == INTERFACE_TYPE.BRIDGE:
+            attach_type = InterfaceAttachType.BRIDGE
+        else:
+            attach_type = InterfaceAttachType.MACVLAN
+        dhcp_enabled = False
+        vlan = interface.vlan
+        if vlan is not None:
+            if vlan.dhcp_on:
+                dhcp_enabled = True
+            elif vlan.relay_vlan is not None:
+                if vlan.relay_vlan.dhcp_on:
+                    dhcp_enabled = True
+        result.append(
+            KnownHostInterface(
+                ifname=ifname, attach_type=attach_type,
+                dhcp_enabled=dhcp_enabled))
+    return result
+
+
 class ComposeMachineForm(forms.Form):
 
     def __init__(self, *args, **kwargs):
@@ -543,26 +571,11 @@ class ComposeMachineForm(forms.Form):
 
             # Find the pod's known host interfaces.
             if self.pod.host is not None:
-                result = [
-                    KnownHostInterface(
-                        ifname=interface.name,
-                        attach_type=(
-                            InterfaceAttachType.BRIDGE
-                            if interface.type == INTERFACE_TYPE.BRIDGE
-                            else InterfaceAttachType.MACVLAN),
-                        dhcp_enabled=(
-                            True
-                            if (interface.vlan is not None and
-                                interface.vlan.dhcp_on) or (
-                                    interface.vlan.relay_vlan is not None and
-                                    interface.vlan.relay_vlan.dhcp_on)
-                            else False))
-                    for interface in self.pod.host.interface_set.all()
-                ]
+                interfaces = get_known_host_interfaces(self.pod.host)
             else:
-                result = []
+                interfaces = []
 
-            return (client, result)
+            return client, interfaces
 
         def create_and_sync(result):
             requested_machine, result = result
diff --git a/src/maasserver/forms/tests/test_pods.py b/src/maasserver/forms/tests/test_pods.py
index 0ba8616..e2c8675 100644
--- a/src/maasserver/forms/tests/test_pods.py
+++ b/src/maasserver/forms/tests/test_pods.py
@@ -32,6 +32,7 @@ from maasserver.forms import pods as pods_module
 from maasserver.forms.pods import (
     ComposeMachineForm,
     ComposeMachineForPodsForm,
+    get_known_host_interfaces,
     PodForm,
 )
 from maasserver.models import StaticIPAddress
@@ -72,6 +73,7 @@ from provisioningserver.enum import (
 from testtools import ExpectedException
 from testtools.matchers import (
     Equals,
+    HasLength,
     Is,
     IsInstance,
     MatchesAll,
@@ -1811,3 +1813,108 @@ class TestComposeMachineForPodsForm(MAASServerTestCase):
         data = self.make_data(pods)
         form = ComposeMachineForPodsForm(request=request, data=data, pods=pods)
         self.assertFalse(form.is_valid())
+
+
+class TestGetKnownHostInterfaces(MAASServerTestCase):
+
+    def test__returns_empty_list_if_no_interfaces(self):
+        node = factory.make_Machine_with_Interface_on_Subnet()
+        node.interface_set.all().delete()
+        interfaces = get_known_host_interfaces(node)
+        self.assertThat(interfaces, HasLength(0))
+
+    def test__returns_appropriate_attach_type(self):
+        node = factory.make_Machine_with_Interface_on_Subnet()
+        vlan = factory.make_VLAN(dhcp_on=False)
+        node.interface_set.all().delete()
+        bridge = factory.make_Interface(
+            iftype=INTERFACE_TYPE.BRIDGE, node=node, vlan=vlan)
+        physical = factory.make_Interface(
+            iftype=INTERFACE_TYPE.PHYSICAL, node=node, vlan=vlan)
+        interfaces = get_known_host_interfaces(node)
+        self.assertItemsEqual(
+            interfaces, [
+                KnownHostInterface(
+                    ifname=bridge.name,
+                    attach_type=InterfaceAttachType.BRIDGE,
+                    dhcp_enabled=False
+                ),
+                KnownHostInterface(
+                    ifname=physical.name,
+                    attach_type=InterfaceAttachType.MACVLAN,
+                    dhcp_enabled=False
+                )
+            ]
+        )
+
+    def test__behaves_correctly_when_vlan_is_none(self):
+        node = factory.make_Machine_with_Interface_on_Subnet()
+        node.interface_set.all().delete()
+        bridge = factory.make_Interface(
+            iftype=INTERFACE_TYPE.BRIDGE, node=node, disconnected=True)
+        physical = factory.make_Interface(
+            iftype=INTERFACE_TYPE.PHYSICAL, node=node, disconnected=True)
+        interfaces = get_known_host_interfaces(node)
+        self.assertItemsEqual(
+            interfaces, [
+                KnownHostInterface(
+                    ifname=bridge.name,
+                    attach_type=InterfaceAttachType.BRIDGE,
+                    dhcp_enabled=False
+                ),
+                KnownHostInterface(
+                    ifname=physical.name,
+                    attach_type=InterfaceAttachType.MACVLAN,
+                    dhcp_enabled=False
+                )
+            ]
+        )
+
+    def test__gets_dhcp_status_for_directly_enabled_vlan(self):
+        node = factory.make_Machine_with_Interface_on_Subnet()
+        vlan = factory.make_VLAN(dhcp_on=True)
+        node.interface_set.all().delete()
+        bridge = factory.make_Interface(
+            iftype=INTERFACE_TYPE.BRIDGE, node=node, vlan=vlan)
+        physical = factory.make_Interface(
+            iftype=INTERFACE_TYPE.PHYSICAL, node=node, vlan=vlan)
+        interfaces = get_known_host_interfaces(node)
+        self.assertItemsEqual(
+            interfaces, [
+                KnownHostInterface(
+                    ifname=bridge.name,
+                    attach_type=InterfaceAttachType.BRIDGE,
+                    dhcp_enabled=True
+                ),
+                KnownHostInterface(
+                    ifname=physical.name,
+                    attach_type=InterfaceAttachType.MACVLAN,
+                    dhcp_enabled=True
+                )
+            ]
+        )
+
+    def test__gets_dhcp_status_for_indirectly_enabled_vlan(self):
+        node = factory.make_Machine_with_Interface_on_Subnet()
+        relay_vlan = factory.make_VLAN(dhcp_on=True)
+        vlan = factory.make_VLAN(dhcp_on=False, relay_vlan=relay_vlan)
+        node.interface_set.all().delete()
+        bridge = factory.make_Interface(
+            iftype=INTERFACE_TYPE.BRIDGE, node=node, vlan=vlan)
+        physical = factory.make_Interface(
+            iftype=INTERFACE_TYPE.PHYSICAL, node=node, vlan=vlan)
+        interfaces = get_known_host_interfaces(node)
+        self.assertItemsEqual(
+            interfaces, [
+                KnownHostInterface(
+                    ifname=bridge.name,
+                    attach_type=InterfaceAttachType.BRIDGE,
+                    dhcp_enabled=True
+                ),
+                KnownHostInterface(
+                    ifname=physical.name,
+                    attach_type=InterfaceAttachType.MACVLAN,
+                    dhcp_enabled=True
+                )
+            ]
+        )