net: add the ability to blacklist network interfaces based on driver during enumeration of physical network devices (#591)

4 files changed, 86 insertions, 40 deletions

diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py
index 10ed249d..e407fa29 100644
--- a/cloudinit/distros/networking.py
+++ b/cloudinit/distros/networking.py
@@ -22,6 +22,9 @@ class Networking(metaclass=abc.ABCMeta):
     Hierarchy" in HACKING.rst for full details.
     """
 
+    def __init__(self):
+        self.blacklist_drivers = None
+
     def _get_current_rename_info(self) -> dict:
         return net._get_current_rename_info()
 
@@ -68,7 +71,8 @@ class Networking(metaclass=abc.ABCMeta):
         return net.get_interfaces()
 
     def get_interfaces_by_mac(self) -> dict:
-        return net.get_interfaces_by_mac()
+        return net.get_interfaces_by_mac(
+            blacklist_drivers=self.blacklist_drivers)
 
     def get_master(self, devname: DeviceName):
         return net.get_master(devname)
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index e233149a..75e79ca8 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -746,18 +746,22 @@ def get_ib_interface_hwaddr(ifname, ethernet_format):
         return mac
 
 
-def get_interfaces_by_mac():
+def get_interfaces_by_mac(blacklist_drivers=None) -> dict:
     if util.is_FreeBSD():
-        return get_interfaces_by_mac_on_freebsd()
+        return get_interfaces_by_mac_on_freebsd(
+            blacklist_drivers=blacklist_drivers)
     elif util.is_NetBSD():
-        return get_interfaces_by_mac_on_netbsd()
+        return get_interfaces_by_mac_on_netbsd(
+            blacklist_drivers=blacklist_drivers)
     elif util.is_OpenBSD():
-        return get_interfaces_by_mac_on_openbsd()
+        return get_interfaces_by_mac_on_openbsd(
+            blacklist_drivers=blacklist_drivers)
     else:
-        return get_interfaces_by_mac_on_linux()
+        return get_interfaces_by_mac_on_linux(
+            blacklist_drivers=blacklist_drivers)
 
 
-def get_interfaces_by_mac_on_freebsd():
+def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict():
     (out, _) = subp.subp(['ifconfig', '-a', 'ether'])
 
     # flatten each interface block in a single line
@@ -784,7 +788,7 @@ def get_interfaces_by_mac_on_freebsd():
     return results
 
 
-def get_interfaces_by_mac_on_netbsd():
+def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict():
     ret = {}
     re_field_match = (
         r"(?P<ifname>\w+).*address:\s"
@@ -800,7 +804,7 @@ def get_interfaces_by_mac_on_netbsd():
     return ret
 
 
-def get_interfaces_by_mac_on_openbsd():
+def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict():
     ret = {}
     re_field_match = (
         r"(?P<ifname>\w+).*lladdr\s"
@@ -815,12 +819,13 @@ def get_interfaces_by_mac_on_openbsd():
     return ret
 
 
-def get_interfaces_by_mac_on_linux():
+def get_interfaces_by_mac_on_linux(blacklist_drivers=None) -> dict:
     """Build a dictionary of tuples {mac: name}.
 
     Bridges and any devices that have a 'stolen' mac are excluded."""
     ret = {}
-    for name, mac, _driver, _devid in get_interfaces():
+    for name, mac, _driver, _devid in get_interfaces(
+            blacklist_drivers=blacklist_drivers):
         if mac in ret:
             raise RuntimeError(
                 "duplicate mac found! both '%s' and '%s' have mac '%s'" %
@@ -838,11 +843,13 @@ def get_interfaces_by_mac_on_linux():
     return ret
 
 
-def get_interfaces():
+def get_interfaces(blacklist_drivers=None) -> list:
     """Return list of interface tuples (name, mac, driver, device_id)
 
     Bridges and any devices that have a 'stolen' mac are excluded."""
     ret = []
+    if blacklist_drivers is None:
+        blacklist_drivers = []
     devs = get_devicelist()
     # 16 somewhat arbitrarily chosen.  Normally a mac is 6 '00:' tokens.
     zero_mac = ':'.join(('00',) * 16)
@@ -866,7 +873,11 @@ def get_interfaces():
         # skip nics that have no mac (00:00....)
         if name != 'lo' and mac == zero_mac[:len(mac)]:
             continue
-        ret.append((name, mac, device_driver(name), device_devid(name)))
+        # skip nics that have drivers blacklisted
+        driver = device_driver(name)
+        if driver in blacklist_drivers:
+            continue
+        ret.append((name, mac, driver, device_devid(name)))
     return ret
 
 
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 773c60d7..fc32f8b1 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -83,6 +83,25 @@ UBUNTU_EXTENDED_NETWORK_SCRIPTS = [
     '/run/network/interfaces.ephemeral.d',
 ]
 
+# This list is used to blacklist devices that will be considered
+# for renaming or fallback interfaces.
+#
+# On Azure network devices using these drivers are automatically
+# configured by the platform and should not be configured by
+# cloud-init's network configuration.
+#
+# Note:
+# Azure Dv4 and Ev4 series VMs always have mlx5 hardware.
+# https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series
+# https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series
+# Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs)
+# can have either mlx4 or mlx5 hardware, with the older series VMs
+# having a higher chance of coming with mlx4 hardware.
+# https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series
+# https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series
+# https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series
+BLACKLIST_DRIVERS = ['mlx4_core', 'mlx5_core']
+
 
 def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid):
     # extract the 'X' from dev.storvsc.X. if deviceid matches
@@ -529,6 +548,8 @@ class DataSourceAzure(sources.DataSource):
         except Exception as e:
             LOG.warning("Failed to get system information: %s", e)
 
+        self.distro.networking.blacklist_drivers = BLACKLIST_DRIVERS
+
         try:
             crawled_data = util.log_time(
                 logfunc=LOG.debug, msg='Crawl of metadata service',
@@ -1468,23 +1489,12 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict:
 
 @azure_ds_telemetry_reporter
 def _generate_network_config_from_fallback_config() -> dict:
-    """Generate fallback network config excluding mlx4_core & mlx5_core devices.
+    """Generate fallback network config excluding blacklisted devices.
 
     @return: Dictionary containing network version 2 standard configuration.
     """
-    # Azure Dv4 and Ev4 series VMs always have mlx5 hardware.
-    # https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series
-    # https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series
-    # Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs)
-    # can have either mlx4 or mlx5 hardware, with the older series VMs
-    # having a higher chance of coming with mlx4 hardware.
-    # https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series
-    # https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series
-    # https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series
-    blacklist = ['mlx4_core', 'mlx5_core']
-    # generate a network config, blacklist picking mlx4_core and mlx5_core devs
     return net.generate_fallback_config(
-        blacklist_drivers=blacklist, config_driver=True)
+        blacklist_drivers=BLACKLIST_DRIVERS, config_driver=True)
 
 
 @azure_ds_telemetry_reporter
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 2b22a879..3b9456f7 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -526,7 +526,7 @@ scbus-1 on xpt0 bus 0
         ])
         return dsaz
 
-    def _get_ds(self, data, agent_command=None, distro=None,
+    def _get_ds(self, data, agent_command=None, distro='ubuntu',
                 apply_network=None):
 
         def dsdevs():
@@ -576,7 +576,7 @@ scbus-1 on xpt0 bus 0
                 side_effect=_wait_for_files)),
         ])
 
-        if distro is not None:
+        if isinstance(distro, str):
             distro_cls = distros.fetch(distro)
             distro = distro_cls(distro, data.get('sys_cfg', {}), self.paths)
         dsrc = dsaz.DataSourceAzure(
@@ -638,7 +638,7 @@ scbus-1 on xpt0 bus 0
         # Return a non-matching asset tag value
         m_is_platform_viable.return_value = False
         dsrc = dsaz.DataSourceAzure(
-            {}, distro=None, paths=self.paths)
+            {}, distro=mock.Mock(), paths=self.paths)
         self.assertFalse(dsrc.get_data())
         m_is_platform_viable.assert_called_with(dsrc.seed_dir)
 
@@ -1311,6 +1311,28 @@ scbus-1 on xpt0 bus 0
             blacklist_drivers=['mlx4_core', 'mlx5_core'],
             config_driver=True)
 
+    @mock.patch(MOCKPATH + 'net.get_interfaces')
+    @mock.patch(MOCKPATH + 'util.is_FreeBSD')
+    def test_blacklist_through_distro(
+            self, m_is_freebsd, m_net_get_interfaces):
+        """Verify Azure DS updates blacklist drivers in the distro's
+           networking object."""
+        odata = {'HostName': "myhost", 'UserName': "myuser"}
+        data = {'ovfcontent': construct_valid_ovf_env(data=odata),
+                'sys_cfg': {}}
+
+        distro_cls = distros.fetch('ubuntu')
+        distro = distro_cls('ubuntu', {}, self.paths)
+        dsrc = self._get_ds(data, distro=distro)
+        dsrc.get_data()
+        self.assertEqual(distro.networking.blacklist_drivers,
+                         dsaz.BLACKLIST_DRIVERS)
+
+        m_is_freebsd.return_value = False
+        distro.networking.get_interfaces_by_mac()
+        m_net_get_interfaces.assert_called_with(
+            blacklist_drivers=dsaz.BLACKLIST_DRIVERS)
+
     @mock.patch(MOCKPATH + 'subp.subp')
     def test_get_hostname_with_no_args(self, m_subp):
         dsaz.get_hostname()
@@ -1421,8 +1443,7 @@ class TestAzureBounce(CiTestCase):
         if ovfcontent is not None:
             populate_dir(os.path.join(self.paths.seed_dir, "azure"),
                          {'ovf-env.xml': ovfcontent})
-        dsrc = dsaz.DataSourceAzure(
-            {}, distro=None, paths=self.paths)
+        dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         if agent_command is not None:
             dsrc.ds_cfg['agent_command'] = agent_command
         return dsrc
@@ -1906,7 +1927,7 @@ class TestClearCachedData(CiTestCase):
         tmp = self.tmp_dir()
         paths = helpers.Paths(
             {'cloud_dir': tmp, 'run_dir': tmp})
-        dsrc = dsaz.DataSourceAzure({}, distro=None, paths=paths)
+        dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=paths)
         clean_values = [dsrc.metadata, dsrc.userdata, dsrc._metadata_imds]
         dsrc.metadata = 'md'
         dsrc.userdata = 'ud'
@@ -1970,7 +1991,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         """The _should_reprovision method should return true with config
            flag present."""
         isfile.return_value = False
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         self.assertTrue(dsa._should_reprovision(
             (None, None, {'PreprovisionedVm': True}, None)))
 
@@ -1978,7 +1999,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         """The _should_reprovision method should return True if the sentinal
            exists."""
         isfile.return_value = True
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         self.assertTrue(dsa._should_reprovision(
             (None, None, {'preprovisionedvm': False}, None)))
 
@@ -1986,7 +2007,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         """The _should_reprovision method should return False
            if config and sentinal are not present."""
         isfile.return_value = False
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         self.assertFalse(dsa._should_reprovision((None, None, {}, None)))
 
     @mock.patch(MOCKPATH + 'DataSourceAzure._poll_imds')
@@ -1997,7 +2018,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase):
         username = "myuser"
         odata = {'HostName': hostname, 'UserName': username}
         _poll_imds.return_value = construct_valid_ovf_env(data=odata)
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         dsa._reprovision()
         _poll_imds.assert_called_with()
 
@@ -2051,7 +2072,7 @@ class TestPreprovisioningPollIMDS(CiTestCase):
 
         fake_resp.side_effect = fake_timeout_once
 
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file):
             dsa._poll_imds()
         self.assertEqual(report_ready_func.call_count, 1)
@@ -2071,7 +2092,7 @@ class TestPreprovisioningPollIMDS(CiTestCase):
             'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
             'unknown-245': '624c3620'}]
         m_media_switch.return_value = None
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file):
             dsa._poll_imds()
         self.assertEqual(report_ready_func.call_count, 0)
@@ -2109,7 +2130,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         full_url = url.format(host)
         fake_resp.return_value = mock.MagicMock(status_code=200, text="ovf",
                                                 content="ovf")
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         self.assertTrue(len(dsa._poll_imds()) > 0)
         self.assertEqual(fake_resp.call_args_list,
                          [mock.call(allow_redirects=True,
@@ -2146,7 +2167,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         content = construct_valid_ovf_env(data=odata)
         fake_resp.return_value = mock.MagicMock(status_code=200, text=content,
                                                 content=content)
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths)
         md, _ud, cfg, _d = dsa._reprovision()
         self.assertEqual(md['local-hostname'], hostname)
         self.assertEqual(cfg['system_info']['default_user']['name'], username)