fix(interfaces): a bridge over a bond or vlan keeps the device's settings

A new bridge always declared its member as type='ethernet'. On a host
with the netcf backend that rewrote the member's configuration: a bond
lost its BONDING_OPTS and a vlan its VLAN=yes. The member is now taken
from the host's own definition of the device (a bond keeps its mode and
devices, a vlan its tag), without the start mode, MTU and addresses that
only a top-level interface has; a device the host does not define stays
ethernet.

interfaces/test_forms.py

@@ -2,6 +2,8 @@
 
 from unittest.mock import MagicMock, patch
 
+import libvirt
+
 from computes.models import Compute
 from django.contrib.auth import get_user_model
 from django.test import SimpleTestCase, TestCase
@@ -64,6 +66,7 @@ class Ipv6XmlTestCase(SimpleTestCase):
         conn = wvmInterfaces.__new__(wvmInterfaces)
         conn.define_iface = MagicMock()
         conn.get_iface = MagicMock()
+        conn.get_iface.return_value.XMLDesc.return_value = "<interface type='ethernet' name='eth1'/>"
         conn.create_iface("br1", "bridge", "onboot", "eth1", "none", "", "", "static", "2001:db8::10/64", gw, "on", 0)
         return etree.fromstring(conn.define_iface.call_args[0][0])
 
@@ -147,6 +150,7 @@ class Ipv4XmlTestCase(SimpleTestCase):
         conn = wvmInterfaces.__new__(wvmInterfaces)
         conn.define_iface = MagicMock()
         conn.get_iface = MagicMock()
+        conn.get_iface.return_value.XMLDesc.return_value = "<interface type='ethernet' name='eth1'/>"
         conn.create_iface("br1", "bridge", "onboot", "eth1", "static", addr, gw, "none", "", "", "on", 0)
         return etree.fromstring(conn.define_iface.call_args[0][0])
 
@@ -179,6 +183,78 @@ class HostDeviceTestCase(SimpleTestCase):
         self.assertIn("name", f.errors)
 
 
+def no_interface(name):
+    err = libvirt.libvirtError("Interface not found: no interface with matching name '%s'" % name)
+    err.err = (libvirt.VIR_ERR_NO_INTERFACE, libvirt.VIR_FROM_INTERFACE, str(err), 2, name, None, None, 0, 0)
+    return err
+
+
+class BridgeMemberXmlTestCase(SimpleTestCase):
+    """A bridge member is declared as the host defines it: a bond keeps its
+    mode and devices, a vlan its tag and device."""
+
+    def member(self, host_xml):
+        conn = wvmInterfaces.__new__(wvmInterfaces)
+        conn.define_iface = MagicMock()
+        created = MagicMock()
+
+        def get_iface(name):
+            if name == "br1":
+                return created
+            if host_xml is None:
+                raise no_interface(name)
+            iface = MagicMock()
+            iface.XMLDesc.return_value = host_xml
+            return iface
+
+        conn.get_iface = MagicMock(side_effect=get_iface)
+        conn.create_iface("br1", "bridge", "onboot", "bond0", "none", "", "", "none", "", "", "on", 0)
+        self.assertTrue(created.create.called)
+        members = etree.fromstring(conn.define_iface.call_args[0][0]).findall("bridge/interface")
+        self.assertEqual(len(members), 1)
+        return members[0]
+
+    def test_a_bond_keeps_its_mode_and_devices(self):
+        member = self.member(
+            "<interface type='bond' name='bond0'><start mode='onboot'/><mtu size='9000'/>"
+            "<protocol family='ipv4'><dhcp/></protocol>"
+            "<bond mode='active-backup'><miimon freq='100'/>"
+            "<interface type='ethernet' name='eth0'><mac address='52:54:00:00:00:01'/></interface>"
+            "<interface type='ethernet' name='eth1'/></bond></interface>"
+        )
+        self.assertEqual((member.get("type"), member.get("name")), ("bond", "bond0"))
+        self.assertEqual(member.find("bond").get("mode"), "active-backup")
+        self.assertEqual([i.get("name") for i in member.findall("bond/interface")], ["eth0", "eth1"])
+        self.assertEqual(member.find("bond/miimon").get("freq"), "100")
+        # what only a top-level interface has: the bridge holds the addresses
+        for tag in ("start", "mtu", "protocol"):
+            self.assertIsNone(member.find(tag), tag)
+
+    def test_a_vlan_keeps_its_tag_and_device(self):
+        member = self.member(
+            "<interface type='vlan' name='bond0'><start mode='onboot'/>"
+            "<vlan tag='42'><interface name='eth0'/></vlan></interface>"
+        )
+        self.assertEqual(member.get("type"), "vlan")
+        self.assertEqual(member.find("vlan").get("tag"), "42")
+        self.assertEqual(member.find("vlan/interface").get("name"), "eth0")
+        self.assertIsNone(member.find("start"))
+
+    def test_a_device_the_host_does_not_define_is_ethernet(self):
+        member = self.member(None)
+        self.assertEqual(member.attrib, {"type": "ethernet", "name": "bond0"})
+
+    def test_other_lookup_errors_are_not_hidden(self):
+        conn = wvmInterfaces.__new__(wvmInterfaces)
+        conn.define_iface = MagicMock()
+        err = libvirt.libvirtError("cannot recv data")
+        err.err = (libvirt.VIR_ERR_SYSTEM_ERROR, libvirt.VIR_FROM_RPC, str(err), 2, None, None, None, 0, 0)
+        conn.get_iface = MagicMock(side_effect=err)
+        with self.assertRaises(libvirt.libvirtError):
+            conn.create_iface("br1", "bridge", "onboot", "bond0", "none", "", "", "none", "", "", "on", 0)
+        self.assertFalse(conn.define_iface.called)
+
+
 class BridgeSettingsTestCase(SimpleTestCase):
     def test_a_bridge_needs_stp_and_a_delay(self):
         for field in ("stp", "delay"):

vrtManager/interface.py

@@ -1,6 +1,6 @@
 from xml.etree import ElementTree
 
-from libvirt import VIR_INTERFACE_XML_INACTIVE
+from libvirt import VIR_ERR_NO_INTERFACE, VIR_INTERFACE_XML_INACTIVE, libvirtError
 
 from vrtManager import util
 from vrtManager.connection import wvmConnect
@@ -10,6 +10,21 @@ class wvmInterfaces(wvmConnect):
     def define_iface(self, xml, flag=0):
         self.wvm.interfaceDefineXML(xml, flag)
 
+    def bridge_member_xml(self, netdev):
+        """netdev as a bridge member: as the host defines it (a bond keeps its
+        mode and devices, a vlan its tag), without what only a top-level
+        interface has. A device the host does not define is ethernet."""
+        try:
+            member = ElementTree.fromstring(self.get_iface(netdev).XMLDesc(VIR_INTERFACE_XML_INACTIVE))
+        except libvirtError as err:
+            if err.get_error_code() != VIR_ERR_NO_INTERFACE:
+                raise
+            return f"<interface type='ethernet' name='{netdev}'/>"
+        for tag in ("start", "mtu", "protocol"):
+            for child in member.findall(tag):
+                member.remove(child)
+        return ElementTree.tostring(member, encoding="unicode")
+
     def create_iface(
         self,
         name,
@@ -51,7 +66,7 @@ class wvmInterfaces(wvmConnect):
             xml += """</protocol>"""
         if itype == "bridge":
             xml += f"""<bridge stp='{stp}' delay='{delay}'>
-                        <interface name='{netdev}' type='ethernet'/>
+                        {self.bridge_member_xml(netdev)}
                       </bridge>"""
         xml += """</interface>"""
         self.define_iface(xml)