diff --git a/backport-net-exclude-OVS-internal-interfaces-in-get_interface.patch b/backport-net-exclude-OVS-internal-interfaces-in-get_interface.patch new file mode 100644 index 0000000000000000000000000000000000000000..ac56366d86f6b44c553895a6b45880b7569ecdc4 --- /dev/null +++ b/backport-net-exclude-OVS-internal-interfaces-in-get_interface.patch @@ -0,0 +1,496 @@ +From 121bc04cdf0e6732fe143b7419131dc250c13384 Mon Sep 17 00:00:00 2001 +From: Daniel Watkins +Date: Mon, 8 Mar 2021 12:50:57 -0500 +Subject: [PATCH] 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 +--- + cloudinit/net/__init__.py | 62 +++++++++++ + cloudinit/net/tests/test_init.py | 119 +++++++++++++++++++++ + cloudinit/sources/helpers/tests/test_openstack.py | 5 + + cloudinit/sources/tests/test_oracle.py | 4 + + tests/integration_tests/bugs/test_lp1912844.py | 103 ++++++++++++++++++ + .../unittests/test_datasource/test_configdrive.py | 8 ++ + tests/unittests/test_net.py | 20 ++++ + 7 files changed, 321 insertions(+) + create mode 100644 tests/integration_tests/bugs/test_lp1912844.py + +diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py +index de65e7a..385b7bc 100644 +--- a/cloudinit/net/__init__.py ++++ b/cloudinit/net/__init__.py +@@ -6,6 +6,7 @@ + # This file is part of cloud-init. See LICENSE file for license information. + + import errno ++import functools + import ipaddress + import logging + import os +@@ -19,6 +20,19 @@ from cloudinit.url_helper import UrlError, readurl + LOG = logging.getLogger(__name__) + SYS_CLASS_NET = "/sys/class/net/" + DEFAULT_PRIMARY_INTERFACE = 'eth0' ++OVS_INTERNAL_INTERFACE_LOOKUP_CMD = [ ++ "ovs-vsctl", ++ "--format", ++ "csv", ++ "--no-headings", ++ "--timeout", ++ "10", ++ "--columns", ++ "name", ++ "find", ++ "interface", ++ "type=internal", ++] + + + def natural_sort_key(s, _nsre=re.compile('([0-9]+)')): +@@ -133,6 +147,52 @@ def master_is_openvswitch(devname): + return os.path.exists(ovs_path) + + ++@functools.lru_cache(maxsize=None) ++def openvswitch_is_installed() -> bool: ++ """Return a bool indicating if Open vSwitch is installed in the system.""" ++ ret = bool(subp.which("ovs-vsctl")) ++ if not ret: ++ LOG.debug( ++ "ovs-vsctl not in PATH; not detecting Open vSwitch interfaces" ++ ) ++ return ret ++ ++ ++@functools.lru_cache(maxsize=None) ++def get_ovs_internal_interfaces() -> list: ++ """Return a list of the names of OVS internal interfaces on the system. ++ ++ These will all be strings, and are used to exclude OVS-specific interface ++ from cloud-init's network configuration handling. ++ """ ++ try: ++ out, _err = subp.subp(OVS_INTERNAL_INTERFACE_LOOKUP_CMD) ++ except subp.ProcessExecutionError as exc: ++ if "database connection failed" in exc.stderr: ++ LOG.info( ++ "Open vSwitch is not yet up; no interfaces will be detected as" ++ " OVS-internal" ++ ) ++ return [] ++ raise ++ else: ++ return out.splitlines() ++ ++ ++def is_openvswitch_internal_interface(devname: str) -> bool: ++ """Returns True if this is an OVS internal interface. ++ ++ If OVS is not installed or not yet running, this will return False. ++ """ ++ if not openvswitch_is_installed(): ++ return False ++ ovs_bridges = get_ovs_internal_interfaces() ++ if devname in ovs_bridges: ++ LOG.debug("Detected %s as an OVS interface", devname) ++ return True ++ return False ++ ++ + def is_netfailover(devname, driver=None): + """ netfailover driver uses 3 nics, master, primary and standby. + this returns True if the device is either the primary or standby +@@ -884,6 +944,8 @@ def get_interfaces(blacklist_drivers=None) -> list: + # skip nics that have no mac (00:00....) + if name != 'lo' and mac == zero_mac[:len(mac)]: + continue ++ if is_openvswitch_internal_interface(name): ++ continue + # skip nics that have drivers blacklisted + driver = device_driver(name) + if driver in blacklist_drivers: +diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py +index 0535387..946f8ee 100644 +--- a/cloudinit/net/tests/test_init.py ++++ b/cloudinit/net/tests/test_init.py +@@ -391,6 +391,10 @@ class TestGetDeviceList(CiTestCase): + self.assertCountEqual(['eth0', 'eth1'], net.get_devicelist()) + + ++@mock.patch( ++ "cloudinit.net.is_openvswitch_internal_interface", ++ mock.Mock(return_value=False), ++) + class TestGetInterfaceMAC(CiTestCase): + + def setUp(self): +@@ -1224,6 +1228,121 @@ class TestNetFailOver(CiTestCase): + self.assertFalse(net.is_netfailover(devname, driver)) + + ++class TestOpenvswitchIsInstalled: ++ """Test cloudinit.net.openvswitch_is_installed. ++ ++ Uses the ``clear_lru_cache`` local autouse fixture to allow us to test ++ despite the ``lru_cache`` decorator on the unit under test. ++ """ ++ ++ @pytest.fixture(autouse=True) ++ def clear_lru_cache(self): ++ net.openvswitch_is_installed.cache_clear() ++ ++ @pytest.mark.parametrize( ++ "expected,which_return", [(True, "/some/path"), (False, None)] ++ ) ++ @mock.patch("cloudinit.net.subp.which") ++ def test_mirrors_which_result(self, m_which, expected, which_return): ++ m_which.return_value = which_return ++ assert expected == net.openvswitch_is_installed() ++ ++ @mock.patch("cloudinit.net.subp.which") ++ def test_only_calls_which_once(self, m_which): ++ net.openvswitch_is_installed() ++ net.openvswitch_is_installed() ++ assert 1 == m_which.call_count ++ ++ ++@mock.patch("cloudinit.net.subp.subp", return_value=("", "")) ++class TestGetOVSInternalInterfaces: ++ """Test cloudinit.net.get_ovs_internal_interfaces. ++ ++ Uses the ``clear_lru_cache`` local autouse fixture to allow us to test ++ despite the ``lru_cache`` decorator on the unit under test. ++ """ ++ @pytest.fixture(autouse=True) ++ def clear_lru_cache(self): ++ net.get_ovs_internal_interfaces.cache_clear() ++ ++ def test_command_used(self, m_subp): ++ """Test we use the correct command when we call subp""" ++ net.get_ovs_internal_interfaces() ++ ++ assert [ ++ mock.call(net.OVS_INTERNAL_INTERFACE_LOOKUP_CMD) ++ ] == m_subp.call_args_list ++ ++ def test_subp_contents_split_and_returned(self, m_subp): ++ """Test that the command output is appropriately mangled.""" ++ stdout = "iface1\niface2\niface3\n" ++ m_subp.return_value = (stdout, "") ++ ++ assert [ ++ "iface1", ++ "iface2", ++ "iface3", ++ ] == net.get_ovs_internal_interfaces() ++ ++ def test_database_connection_error_handled_gracefully(self, m_subp): ++ """Test that the error indicating OVS is down is handled gracefully.""" ++ m_subp.side_effect = ProcessExecutionError( ++ stderr="database connection failed" ++ ) ++ ++ assert [] == net.get_ovs_internal_interfaces() ++ ++ def test_other_errors_raised(self, m_subp): ++ """Test that only database connection errors are handled.""" ++ m_subp.side_effect = ProcessExecutionError() ++ ++ with pytest.raises(ProcessExecutionError): ++ net.get_ovs_internal_interfaces() ++ ++ def test_only_runs_once(self, m_subp): ++ """Test that we cache the value.""" ++ net.get_ovs_internal_interfaces() ++ net.get_ovs_internal_interfaces() ++ ++ assert 1 == m_subp.call_count ++ ++ ++@mock.patch("cloudinit.net.get_ovs_internal_interfaces") ++@mock.patch("cloudinit.net.openvswitch_is_installed") ++class TestIsOpenVSwitchInternalInterface: ++ def test_false_if_ovs_not_installed( ++ self, m_openvswitch_is_installed, _m_get_ovs_internal_interfaces ++ ): ++ """Test that OVS' absence returns False.""" ++ m_openvswitch_is_installed.return_value = False ++ ++ assert not net.is_openvswitch_internal_interface("devname") ++ ++ @pytest.mark.parametrize( ++ "detected_interfaces,devname,expected_return", ++ [ ++ ([], "devname", False), ++ (["notdevname"], "devname", False), ++ (["devname"], "devname", True), ++ (["some", "other", "devices", "and", "ours"], "ours", True), ++ ], ++ ) ++ def test_return_value_based_on_detected_interfaces( ++ self, ++ m_openvswitch_is_installed, ++ m_get_ovs_internal_interfaces, ++ detected_interfaces, ++ devname, ++ expected_return, ++ ): ++ """Test that the detected interfaces are used correctly.""" ++ m_openvswitch_is_installed.return_value = True ++ m_get_ovs_internal_interfaces.return_value = detected_interfaces ++ assert expected_return == net.is_openvswitch_internal_interface( ++ devname ++ ) ++ ++ + class TestIsIpAddress: + """Tests for net.is_ip_address. + +diff --git a/cloudinit/sources/helpers/tests/test_openstack.py b/cloudinit/sources/helpers/tests/test_openstack.py +index 2bde1e3..95fb974 100644 +--- a/cloudinit/sources/helpers/tests/test_openstack.py ++++ b/cloudinit/sources/helpers/tests/test_openstack.py +@@ -1,10 +1,15 @@ + # This file is part of cloud-init. See LICENSE file for license information. + # ./cloudinit/sources/helpers/tests/test_openstack.py ++from unittest import mock + + from cloudinit.sources.helpers import openstack + from cloudinit.tests import helpers as test_helpers + + ++@mock.patch( ++ "cloudinit.net.is_openvswitch_internal_interface", ++ mock.Mock(return_value=False) ++) + class TestConvertNetJson(test_helpers.CiTestCase): + + def test_phy_types(self): +diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py +index a7bbdfd..dcf33b9 100644 +--- a/cloudinit/sources/tests/test_oracle.py ++++ b/cloudinit/sources/tests/test_oracle.py +@@ -173,6 +173,10 @@ class TestIsPlatformViable(test_helpers.CiTestCase): + m_read_dmi_data.assert_has_calls([mock.call('chassis-asset-tag')]) + + ++@mock.patch( ++ "cloudinit.net.is_openvswitch_internal_interface", ++ mock.Mock(return_value=False) ++) + class TestNetworkConfigFromOpcImds: + def test_no_secondary_nics_does_not_mutate_input(self, oracle_ds): + oracle_ds._vnics_data = [{}] +diff --git a/tests/integration_tests/bugs/test_lp1912844.py b/tests/integration_tests/bugs/test_lp1912844.py +new file mode 100644 +index 0000000..efafae5 +--- /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 6f830cc..2e2b784 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 38d934d..cb636f4 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' +-- +1.8.3.1 + diff --git a/cloud-init.spec b/cloud-init.spec index 8e626591427255c3863d9e41472306b87edd263d..94f12c2deb8544a2a0f3b44e0b8ce8406ae7aac2 100644 --- a/cloud-init.spec +++ b/cloud-init.spec @@ -1,6 +1,6 @@ Name: cloud-init Version: 20.4 -Release: 1 +Release: 2 Summary: the defacto multi-distribution package that handles early initialization of a cloud instance. License: ASL 2.0 or GPLv3 URL: http://launchpad.net/cloud-init @@ -16,6 +16,7 @@ Patch4: bugfix-sort-requirements.patch Patch5: add-variable-to-forbid-tmp-dir.patch Patch6: cloud-init-20.4-sandbox-ca_certs-tests-to-avoid-failure.patch Patch7: cloud-init-20.4-Revert-ssh_util-handle-non-default-AuthorizedKeysFil.patch +Patch8: backport-net-exclude-OVS-internal-interfaces-in-get_interface.patch Patch9000: Fix-the-error-level-logs-displayed-for-the-cloud-init-local-service.patch @@ -124,6 +125,12 @@ fi %exclude /usr/share/doc/* %changelog +* Thu Jul 29 2021 Hugel - 20.4-2 +- Type:bugfix +- ID:NA +- SUG:NA +- DESC:exclude OVS internal interfaces in get_interfaces + * Sat Jun 26 2021 yangzhuangzhuang - 20.4-1 - Type:update - ID:NA