net: exclude OVS internal interfaces in get_interfaces (#829)

`get_interfaces` is used to in two ways, broadly: firstly, to determine
the available interfaces when converting cloud network configuration
formats to cloud-init's network configuration formats; and, secondly, to
ensure that any interfaces which are specified in network configuration
are (a) available, and (b) named correctly.  The first of these is
unaffected by this commit, as no clouds support Open vSwitch
configuration in their network configuration formats.

For the second, we check that MAC addresses of physical devices are
unique.  In some OVS configurations, there are OVS-created devices which
have duplicate MAC addresses, either with each other or with physical
devices.  As these interfaces are created by OVS, we can be confident
that (a) they will be available when appropriate, and (b) that OVS will
name them correctly.  As such, this commit excludes any OVS-internal
interfaces from the set of interfaces returned by `get_interfaces`.

LP: #1912844

3 files changed, 131 insertions, 0 deletions

diff --git a/tests/integration_tests/bugs/test_lp1912844.py b/tests/integration_tests/bugs/test_lp1912844.py
new file mode 100644
index 00000000..efafae50
--- /dev/null
+++ b/tests/integration_tests/bugs/test_lp1912844.py
@@ -0,0 +1,103 @@
+"""Integration test for LP: #1912844
+
+cloud-init should ignore OVS-internal interfaces when performing its own
+interface determination: these interfaces are handled fully by OVS, so
+cloud-init should never need to touch them.
+
+This test is a semi-synthetic reproducer for the bug.  It uses a similar
+network configuration, tweaked slightly to DHCP in a way that will succeed even
+on "failed" boots.  The exact bug doesn't reproduce with the NoCloud
+datasource, because it runs at init-local time (whereas the MAAS datasource,
+from the report, runs only at init (network) time): this means that the
+networking code runs before OVS creates its interfaces (which happens after
+init-local but, of course, before networking is up), and so doesn't generate
+the traceback that they cause.  We work around this by calling
+``get_interfaces_by_mac` directly in the test code.
+"""
+import pytest
+
+from tests.integration_tests import random_mac_address
+
+MAC_ADDRESS = random_mac_address()
+
+NETWORK_CONFIG = """\
+bonds:
+    bond0:
+        interfaces:
+            - enp5s0
+        macaddress: {0}
+        mtu: 1500
+bridges:
+        ovs-br:
+            interfaces:
+            - bond0
+            macaddress: {0}
+            mtu: 1500
+            openvswitch: {{}}
+            dhcp4: true
+ethernets:
+    enp5s0:
+      mtu: 1500
+      set-name: enp5s0
+      match:
+          macaddress: {0}
+version: 2
+vlans:
+  ovs-br.100:
+    id: 100
+    link: ovs-br
+    mtu: 1500
+  ovs-br.200:
+    id: 200
+    link: ovs-br
+    mtu: 1500
+""".format(MAC_ADDRESS)
+
+
+SETUP_USER_DATA = """\
+#cloud-config
+packages:
+- openvswitch-switch
+"""
+
+
+@pytest.fixture
+def ovs_enabled_session_cloud(session_cloud):
+    """A session_cloud wrapper, to use an OVS-enabled image for tests.
+
+    This implementation is complicated by wanting to use ``session_cloud``s
+    snapshot cleanup/retention logic, to avoid having to reimplement that here.
+    """
+    old_snapshot_id = session_cloud.snapshot_id
+    with session_cloud.launch(
+        user_data=SETUP_USER_DATA,
+    ) as instance:
+        instance.instance.clean()
+        session_cloud.snapshot_id = instance.snapshot()
+
+    yield session_cloud
+
+    try:
+        session_cloud.delete_snapshot()
+    finally:
+        session_cloud.snapshot_id = old_snapshot_id
+
+
+@pytest.mark.lxd_vm
+def test_get_interfaces_by_mac_doesnt_traceback(ovs_enabled_session_cloud):
+    """Launch our OVS-enabled image and confirm the bug doesn't reproduce."""
+    launch_kwargs = {
+        "config_dict": {
+            "user.network-config": NETWORK_CONFIG,
+            "volatile.eth0.hwaddr": MAC_ADDRESS,
+        },
+    }
+    with ovs_enabled_session_cloud.launch(
+        launch_kwargs=launch_kwargs,
+    ) as client:
+        result = client.execute(
+            "python3 -c"
+            "'from cloudinit.net import get_interfaces_by_mac;"
+            "get_interfaces_by_mac()'"
+        )
+        assert result.ok
diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py
index 6f830cc6..2e2b7847 100644
--- a/tests/unittests/test_datasource/test_configdrive.py
+++ b/tests/unittests/test_datasource/test_configdrive.py
@@ -494,6 +494,10 @@ class TestConfigDriveDataSource(CiTestCase):
         self.assertEqual('config-disk (/dev/anything)', cfg_ds.subplatform)
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestNetJson(CiTestCase):
     def setUp(self):
         super(TestNetJson, self).setUp()
@@ -654,6 +658,10 @@ class TestNetJson(CiTestCase):
             self.assertEqual(out_data, conv_data)
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestConvertNetworkData(CiTestCase):
 
     with_logs = True
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 38d934d4..cb636f41 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -2933,6 +2933,10 @@ iface eth1 inet dhcp
         self.assertEqual(0, mock_settle.call_count)
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestRhelSysConfigRendering(CiTestCase):
 
     with_logs = True
@@ -3620,6 +3624,10 @@ USERCTL=no
                 expected, self._render_and_read(network_config=v2data))
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestOpenSuseSysConfigRendering(CiTestCase):
 
     with_logs = True
@@ -5037,6 +5045,10 @@ class TestNetRenderers(CiTestCase):
             self.assertTrue(result)
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestGetInterfaces(CiTestCase):
     _data = {'bonds': ['bond1'],
              'bridges': ['bridge1'],
@@ -5186,6 +5198,10 @@ class TestInterfaceHasOwnMac(CiTestCase):
         self.assertFalse(interface_has_own_mac("eth0"))
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestGetInterfacesByMac(CiTestCase):
     _data = {'bonds': ['bond1'],
              'bridges': ['bridge1'],
@@ -5342,6 +5358,10 @@ class TestInterfacesSorting(CiTestCase):
             ['enp0s3', 'enp0s8', 'enp0s13', 'enp1s2', 'enp2s0', 'enp2s3'])
 
 
+@mock.patch(
+    "cloudinit.net.is_openvswitch_internal_interface",
+    mock.Mock(return_value=False)
+)
 class TestGetIBHwaddrsByInterface(CiTestCase):
 
     _ib_addr = '80:00:00:28:fe:80:00:00:00:00:00:00:00:11:22:03:00:33:44:56'