sources/azure: fix device driver matching for net config (#1914)

The ordering of NICs provided by IMDS may not match the order enumerated
by kernel.  As such, we do not have any guarantee that the nic we're
checking the driver for is the nic we think it is.

Instead of making any assumptions about how the nics are named, check
all interfaces by mac address.  If there is an interface using
"hv_netvsc", match against that.  If there is only one interface driver
that is not blacklisted, use that (in case it is not "hv_netvsc"), but
log a debug event.  If there are multiple hits, don't match against any
of the names and report a warning.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>

2 files changed, 164 insertions, 75 deletions

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index db9234db..c65e17a7 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -22,7 +22,6 @@ from cloudinit import dmi
 from cloudinit import log as logging
 from cloudinit import net, sources, ssh_util, subp, util
 from cloudinit.event import EventScope, EventType
-from cloudinit.net import device_driver
 from cloudinit.net.dhcp import (
     NoDHCPLeaseError,
     NoDHCPLeaseInterfaceError,
@@ -208,6 +207,33 @@ def get_hv_netvsc_macs_normalized() -> List[str]:
     ]
 
 
+@azure_ds_telemetry_reporter
+def determine_device_driver_for_mac(mac: str) -> Optional[str]:
+    """Determine the device driver to match on, if any."""
+    drivers = [
+        i[2]
+        for i in net.get_interfaces(blacklist_drivers=BLACKLIST_DRIVERS)
+        if mac == normalize_mac_address(i[1])
+    ]
+    if "hv_netvsc" in drivers:
+        return "hv_netvsc"
+
+    if len(drivers) == 1:
+        report_diagnostic_event(
+            "Assuming driver for interface with mac=%s drivers=%r"
+            % (mac, drivers),
+            logger_func=LOG.debug,
+        )
+        return drivers[0]
+
+    report_diagnostic_event(
+        "Unable to specify driver for interface with mac=%s drivers=%r"
+        % (mac, drivers),
+        logger_func=LOG.warning,
+    )
+    return None
+
+
 def execute_or_debug(cmd, fail_ret=None) -> str:
     try:
         return subp.subp(cmd).stdout  # pyright: ignore
@@ -2046,11 +2072,8 @@ def generate_network_config_from_instance_network_metadata(
             dev_config.update(
                 {"match": {"macaddress": mac.lower()}, "set-name": nicname}
             )
-            # With netvsc, we can get two interfaces that
-            # share the same MAC, so we need to make sure
-            # our match condition also contains the driver
-            driver = device_driver(nicname)
-            if driver and driver == "hv_netvsc":
+            driver = determine_device_driver_for_mac(mac)
+            if driver:
                 dev_config["match"]["driver"] = driver
             netconfig["ethernets"][nicname] = dev_config
             continue
diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py
index 24fa061c..da2612e8 100644
--- a/tests/unittests/sources/test_azure.py
+++ b/tests/unittests/sources/test_azure.py
@@ -93,16 +93,6 @@ def mock_chassis_asset_tag():
 
 
 @pytest.fixture
-def mock_device_driver():
-    with mock.patch(
-        MOCKPATH + "device_driver",
-        autospec=True,
-        return_value=None,
-    ) as m:
-        yield m
-
-
-@pytest.fixture
 def mock_generate_fallback_config():
     with mock.patch(
         MOCKPATH + "net.generate_fallback_config",
@@ -173,9 +163,19 @@ def mock_net_dhcp_EphemeralIPv4Network():
         yield m
 
 
-@pytest.fixture
+@pytest.fixture(autouse=True)
 def mock_get_interfaces():
-    with mock.patch(MOCKPATH + "net.get_interfaces", return_value=[]) as m:
+    with mock.patch(
+        MOCKPATH + "net.get_interfaces",
+        return_value=[
+            ("dummy0", "9e:65:d6:19:19:01", None, None),
+            ("enP3", "00:11:22:33:44:02", "unknown_accel", "0x3"),
+            ("eth0", "00:11:22:33:44:00", "hv_netvsc", "0x3"),
+            ("eth2", "00:11:22:33:44:01", "unknown", "0x3"),
+            ("eth3", "00:11:22:33:44:02", "unknown_with_unknown_vf", "0x3"),
+            ("lo", "00:00:00:00:00:00", None, None),
+        ],
+    ) as m:
         yield m
 
 
@@ -507,6 +507,116 @@ class TestGenerateNetworkConfig:
                 },
             ),
             (
+                "hv_netvsc driver",
+                {
+                    "interface": [
+                        {
+                            "macAddress": "001122334400",
+                            "ipv6": {"ipAddress": []},
+                            "ipv4": {
+                                "subnet": [
+                                    {"prefix": "24", "address": "10.0.0.0"}
+                                ],
+                                "ipAddress": [
+                                    {
+                                        "privateIpAddress": "10.0.0.4",
+                                        "publicIpAddress": "104.46.124.81",
+                                    }
+                                ],
+                            },
+                        }
+                    ]
+                },
+                {
+                    "ethernets": {
+                        "eth0": {
+                            "dhcp4": True,
+                            "dhcp4-overrides": {"route-metric": 100},
+                            "dhcp6": False,
+                            "match": {
+                                "macaddress": "00:11:22:33:44:00",
+                                "driver": "hv_netvsc",
+                            },
+                            "set-name": "eth0",
+                        }
+                    },
+                    "version": 2,
+                },
+            ),
+            (
+                "unknown",
+                {
+                    "interface": [
+                        {
+                            "macAddress": "001122334401",
+                            "ipv6": {"ipAddress": []},
+                            "ipv4": {
+                                "subnet": [
+                                    {"prefix": "24", "address": "10.0.0.0"}
+                                ],
+                                "ipAddress": [
+                                    {
+                                        "privateIpAddress": "10.0.0.4",
+                                        "publicIpAddress": "104.46.124.81",
+                                    }
+                                ],
+                            },
+                        }
+                    ]
+                },
+                {
+                    "ethernets": {
+                        "eth0": {
+                            "dhcp4": True,
+                            "dhcp4-overrides": {"route-metric": 100},
+                            "dhcp6": False,
+                            "match": {
+                                "macaddress": "00:11:22:33:44:01",
+                                "driver": "unknown",
+                            },
+                            "set-name": "eth0",
+                        }
+                    },
+                    "version": 2,
+                },
+            ),
+            (
+                "unknown with unknown matching VF",
+                {
+                    "interface": [
+                        {
+                            "macAddress": "001122334402",
+                            "ipv6": {"ipAddress": []},
+                            "ipv4": {
+                                "subnet": [
+                                    {"prefix": "24", "address": "10.0.0.0"}
+                                ],
+                                "ipAddress": [
+                                    {
+                                        "privateIpAddress": "10.0.0.4",
+                                        "publicIpAddress": "104.46.124.81",
+                                    }
+                                ],
+                            },
+                        }
+                    ]
+                },
+                {
+                    "ethernets": {
+                        "eth0": {
+                            "dhcp4": True,
+                            "dhcp4-overrides": {"route-metric": 100},
+                            "dhcp6": False,
+                            "match": {
+                                "macaddress": "00:11:22:33:44:02",
+                            },
+                            "set-name": "eth0",
+                        }
+                    },
+                    "version": 2,
+                },
+            ),
+            (
                 "multiple interfaces with increasing route metric",
                 {
                     "interface": [
@@ -648,7 +758,7 @@ class TestGenerateNetworkConfig:
         ],
     )
     def test_parsing_scenarios(
-        self, label, mock_device_driver, metadata, expected
+        self, label, mock_get_interfaces, metadata, expected
     ):
         assert (
             dsaz.generate_network_config_from_instance_network_metadata(
@@ -657,27 +767,6 @@ class TestGenerateNetworkConfig:
             == expected
         )
 
-    def test_match_hv_netvsc(self, mock_device_driver):
-        mock_device_driver.return_value = "hv_netvsc"
-
-        assert dsaz.generate_network_config_from_instance_network_metadata(
-            NETWORK_METADATA["network"]
-        ) == {
-            "ethernets": {
-                "eth0": {
-                    "dhcp4": True,
-                    "dhcp4-overrides": {"route-metric": 100},
-                    "dhcp6": False,
-                    "match": {
-                        "macaddress": "00:0d:3a:04:75:98",
-                        "driver": "hv_netvsc",
-                    },
-                    "set-name": "eth0",
-                }
-            },
-            "version": 2,
-        }
-
 
 class TestNetworkConfig:
     fallback_config = {
@@ -693,7 +782,9 @@ class TestNetworkConfig:
         ],
     }
 
-    def test_single_ipv4_nic_configuration(self, azure_ds, mock_device_driver):
+    def test_single_ipv4_nic_configuration(
+        self, azure_ds, mock_get_interfaces
+    ):
         """Network config emits dhcp on single nic with ipv4"""
         expected = {
             "ethernets": {
@@ -712,7 +803,7 @@ class TestNetworkConfig:
         assert azure_ds.network_config == expected
 
     def test_uses_fallback_cfg_when_apply_network_config_is_false(
-        self, azure_ds, mock_device_driver, mock_generate_fallback_config
+        self, azure_ds, mock_generate_fallback_config
     ):
         azure_ds.ds_cfg["apply_network_config"] = False
         azure_ds._metadata_imds = NETWORK_METADATA
@@ -721,7 +812,7 @@ class TestNetworkConfig:
         assert azure_ds.network_config == self.fallback_config
 
     def test_uses_fallback_cfg_when_imds_metadata_unset(
-        self, azure_ds, mock_device_driver, mock_generate_fallback_config
+        self, azure_ds, mock_generate_fallback_config
     ):
         azure_ds._metadata_imds = UNSET
         mock_generate_fallback_config.return_value = self.fallback_config
@@ -729,7 +820,7 @@ class TestNetworkConfig:
         assert azure_ds.network_config == self.fallback_config
 
     def test_uses_fallback_cfg_when_no_network_metadata(
-        self, azure_ds, mock_device_driver, mock_generate_fallback_config
+        self, azure_ds, mock_generate_fallback_config
     ):
         """Network config generates fallback network config when the
         IMDS instance metadata is corrupted/invalid, such as when
@@ -745,7 +836,7 @@ class TestNetworkConfig:
         assert azure_ds.network_config == self.fallback_config
 
     def test_uses_fallback_cfg_when_no_interface_metadata(
-        self, azure_ds, mock_device_driver, mock_generate_fallback_config
+        self, azure_ds, mock_generate_fallback_config
     ):
         """Network config generates fallback network config when the
         IMDS instance metadata is corrupted/invalid, such as when
@@ -1069,13 +1160,6 @@ scbus-1 on xpt0 bus 0
 
         self.m_get_metadata_from_fabric = mock.MagicMock(return_value=[])
         self.m_report_failure_to_fabric = mock.MagicMock(autospec=True)
-        self.m_get_interfaces = mock.MagicMock(
-            return_value=[
-                ("dummy0", "9e:65:d6:19:19:01", None, None),
-                ("eth0", "00:15:5d:69:63:ba", "hv_netvsc", "0x3"),
-                ("lo", "00:00:00:00:00:00", None, None),
-            ]
-        )
         self.m_list_possible_azure_ds = mock.MagicMock(
             side_effect=_load_possible_azure_ds
         )
@@ -1119,11 +1203,6 @@ scbus-1 on xpt0 bus 0
                     "get_interface_mac",
                     mock.MagicMock(return_value="00:15:5d:69:63:ba"),
                 ),
-                (
-                    dsaz.net,
-                    "get_interfaces",
-                    self.m_get_interfaces,
-                ),
                 (dsaz.subp, "which", lambda x: True),
                 (
                     dsaz.dmi,
@@ -1537,10 +1616,7 @@ scbus-1 on xpt0 bus 0
         self.assertTrue(os.path.isdir(self.waagent_d))
         self.assertEqual(stat.S_IMODE(os.stat(self.waagent_d).st_mode), 0o700)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.device_driver", return_value=None
-    )
-    def test_network_config_set_from_imds(self, m_driver):
+    def test_network_config_set_from_imds(self):
         """Datasource.network_config returns IMDS network data."""
         sys_cfg = {"datasource": {"Azure": {"apply_network_config": True}}}
         data = {
@@ -1563,12 +1639,7 @@ scbus-1 on xpt0 bus 0
         dsrc.get_data()
         self.assertEqual(expected_network_config, dsrc.network_config)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.device_driver", return_value=None
-    )
-    def test_network_config_set_from_imds_route_metric_for_secondary_nic(
-        self, m_driver
-    ):
+    def test_network_config_set_from_imds_route_metric_for_secondary_nic(self):
         """Datasource.network_config adds route-metric to secondary nics."""
         sys_cfg = {"datasource": {"Azure": {"apply_network_config": True}}}
         data = {
@@ -1614,12 +1685,7 @@ scbus-1 on xpt0 bus 0
         dsrc.get_data()
         self.assertEqual(expected_network_config, dsrc.network_config)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.device_driver", return_value=None
-    )
-    def test_network_config_set_from_imds_for_secondary_nic_no_ip(
-        self, m_driver
-    ):
+    def test_network_config_set_from_imds_for_secondary_nic_no_ip(self):
         """If an IP address is empty then there should no config for it."""
         sys_cfg = {"datasource": {"Azure": {"apply_network_config": True}}}
         data = {
@@ -2157,7 +2223,7 @@ scbus-1 on xpt0 bus 0
             [mock.call("/dev/cd0")], m_check_fbsd_cdrom.call_args_list
         )
 
-    @mock.patch(MOCKPATH + "net.get_interfaces", autospec=True)
+    @mock.patch(MOCKPATH + "net.get_interfaces")
     def test_blacklist_through_distro(self, m_net_get_interfaces):
         """Verify Azure DS updates blacklist drivers in the distro's
         networking object."""
@@ -2175,7 +2241,7 @@ scbus-1 on xpt0 bus 0
         )
 
         distro.networking.get_interfaces_by_mac()
-        self.m_get_interfaces.assert_called_with(
+        m_net_get_interfaces.assert_called_with(
             blacklist_drivers=dsaz.BLACKLIST_DRIVERS
         )