Azure parse_network_config uses fallback cfg when generate IMDS network cfg fails (#549)

Azure datasource's `parse_network_config` throws a fatal uncaught exception when an exception is raised during generation of network config from IMDS metadata. This happens when IMDS metadata is invalid/corrupted (such as when it is missing network or interface metadata). This causes the rest of provisioning to fail.

This changes `parse_network_config` to be a non-fatal implementation. Additionally, when generating network config from IMDS metadata fails, fall back on generating fallback network config (`_generate_network_config_from_fallback_config`).

This also changes fallback network config generation (`_generate_network_config_from_fallback_config`) to blacklist an additional driver: `mlx5_core`.

2 files changed, 164 insertions, 120 deletions

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index e98fd497..773c60d7 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -1387,76 +1387,104 @@ def load_azure_ds_dir(source_dir):
     return (md, ud, cfg, {'ovf-env.xml': contents})
 
 
-def parse_network_config(imds_metadata):
+@azure_ds_telemetry_reporter
+def parse_network_config(imds_metadata) -> dict:
     """Convert imds_metadata dictionary to network v2 configuration.
-
     Parses network configuration from imds metadata if present or generate
     fallback network config excluding mlx4_core devices.
 
     @param: imds_metadata: Dict of content read from IMDS network service.
     @return: Dictionary containing network version 2 standard configuration.
     """
-    with events.ReportEventStack(
-        name="parse_network_config",
-        description="",
-        parent=azure_ds_reporter
-    ) as evt:
-        if imds_metadata != sources.UNSET and imds_metadata:
-            netconfig = {'version': 2, 'ethernets': {}}
-            LOG.debug('Azure: generating network configuration from IMDS')
-            network_metadata = imds_metadata['network']
-            for idx, intf in enumerate(network_metadata['interface']):
-                # First IPv4 and/or IPv6 address will be obtained via DHCP.
-                # Any additional IPs of each type will be set as static
-                # addresses.
-                nicname = 'eth{idx}'.format(idx=idx)
-                dhcp_override = {'route-metric': (idx + 1) * 100}
-                dev_config = {'dhcp4': True, 'dhcp4-overrides': dhcp_override,
-                              'dhcp6': False}
-                for addr_type in ('ipv4', 'ipv6'):
-                    addresses = intf.get(addr_type, {}).get('ipAddress', [])
-                    if addr_type == 'ipv4':
-                        default_prefix = '24'
-                    else:
-                        default_prefix = '128'
-                        if addresses:
-                            dev_config['dhcp6'] = True
-                            # non-primary interfaces should have a higher
-                            # route-metric (cost) so default routes prefer
-                            # primary nic due to lower route-metric value
-                            dev_config['dhcp6-overrides'] = dhcp_override
-                    for addr in addresses[1:]:
-                        # Append static address config for ip > 1
-                        netPrefix = intf[addr_type]['subnet'][0].get(
-                            'prefix', default_prefix)
-                        privateIp = addr['privateIpAddress']
-                        if not dev_config.get('addresses'):
-                            dev_config['addresses'] = []
-                        dev_config['addresses'].append(
-                            '{ip}/{prefix}'.format(
-                                ip=privateIp, prefix=netPrefix))
-                if dev_config:
-                    mac = ':'.join(re.findall(r'..', intf['macAddress']))
-                    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':
-                        dev_config['match']['driver'] = driver
-                    netconfig['ethernets'][nicname] = dev_config
-            evt.description = "network config from imds"
-        else:
-            blacklist = ['mlx4_core']
-            LOG.debug('Azure: generating fallback configuration')
-            # generate a network config, blacklist picking mlx4_core devs
-            netconfig = net.generate_fallback_config(
-                blacklist_drivers=blacklist, config_driver=True)
-            evt.description = "network config from fallback"
-        return netconfig
+    if imds_metadata != sources.UNSET and imds_metadata:
+        try:
+            return _generate_network_config_from_imds_metadata(imds_metadata)
+        except Exception as e:
+            LOG.error(
+                'Failed generating network config '
+                'from IMDS network metadata: %s', str(e))
+    try:
+        return _generate_network_config_from_fallback_config()
+    except Exception as e:
+        LOG.error('Failed generating fallback network config: %s', str(e))
+    return {}
+
+
+@azure_ds_telemetry_reporter
+def _generate_network_config_from_imds_metadata(imds_metadata) -> dict:
+    """Convert imds_metadata dictionary to network v2 configuration.
+    Parses network configuration from imds metadata.
+
+    @param: imds_metadata: Dict of content read from IMDS network service.
+    @return: Dictionary containing network version 2 standard configuration.
+    """
+    netconfig = {'version': 2, 'ethernets': {}}
+    network_metadata = imds_metadata['network']
+    for idx, intf in enumerate(network_metadata['interface']):
+        # First IPv4 and/or IPv6 address will be obtained via DHCP.
+        # Any additional IPs of each type will be set as static
+        # addresses.
+        nicname = 'eth{idx}'.format(idx=idx)
+        dhcp_override = {'route-metric': (idx + 1) * 100}
+        dev_config = {'dhcp4': True, 'dhcp4-overrides': dhcp_override,
+                      'dhcp6': False}
+        for addr_type in ('ipv4', 'ipv6'):
+            addresses = intf.get(addr_type, {}).get('ipAddress', [])
+            if addr_type == 'ipv4':
+                default_prefix = '24'
+            else:
+                default_prefix = '128'
+                if addresses:
+                    dev_config['dhcp6'] = True
+                    # non-primary interfaces should have a higher
+                    # route-metric (cost) so default routes prefer
+                    # primary nic due to lower route-metric value
+                    dev_config['dhcp6-overrides'] = dhcp_override
+            for addr in addresses[1:]:
+                # Append static address config for ip > 1
+                netPrefix = intf[addr_type]['subnet'][0].get(
+                    'prefix', default_prefix)
+                privateIp = addr['privateIpAddress']
+                if not dev_config.get('addresses'):
+                    dev_config['addresses'] = []
+                dev_config['addresses'].append(
+                    '{ip}/{prefix}'.format(
+                        ip=privateIp, prefix=netPrefix))
+        if dev_config:
+            mac = ':'.join(re.findall(r'..', intf['macAddress']))
+            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':
+                dev_config['match']['driver'] = driver
+            netconfig['ethernets'][nicname] = dev_config
+    return netconfig
+
+
+@azure_ds_telemetry_reporter
+def _generate_network_config_from_fallback_config() -> dict:
+    """Generate fallback network config excluding mlx4_core & mlx5_core 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)
 
 
 @azure_ds_telemetry_reporter
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 2dda9925..2b22a879 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -162,8 +162,19 @@ MOCKPATH = 'cloudinit.sources.DataSourceAzure.'
 class TestParseNetworkConfig(CiTestCase):
 
     maxDiff = None
+    fallback_config = {
+        'version': 1,
+        'config': [{
+            'type': 'physical', 'name': 'eth0',
+            'mac_address': '00:11:22:33:44:55',
+            'params': {'driver': 'hv_netsvc'},
+            'subnets': [{'type': 'dhcp'}],
+        }]
+    }
 
-    def test_single_ipv4_nic_configuration(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_single_ipv4_nic_configuration(self, m_driver):
         """parse_network_config emits dhcp on single nic with ipv4"""
         expected = {'ethernets': {
             'eth0': {'dhcp4': True,
@@ -173,7 +184,9 @@ class TestParseNetworkConfig(CiTestCase):
                      'set-name': 'eth0'}}, 'version': 2}
         self.assertEqual(expected, dsaz.parse_network_config(NETWORK_METADATA))
 
-    def test_increases_route_metric_for_non_primary_nics(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_increases_route_metric_for_non_primary_nics(self, m_driver):
         """parse_network_config increases route-metric for each nic"""
         expected = {'ethernets': {
             'eth0': {'dhcp4': True,
@@ -200,7 +213,9 @@ class TestParseNetworkConfig(CiTestCase):
         imds_data['network']['interface'].append(third_intf)
         self.assertEqual(expected, dsaz.parse_network_config(imds_data))
 
-    def test_ipv4_and_ipv6_route_metrics_match_for_nics(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_ipv4_and_ipv6_route_metrics_match_for_nics(self, m_driver):
         """parse_network_config emits matching ipv4 and ipv6 route-metrics."""
         expected = {'ethernets': {
             'eth0': {'addresses': ['10.0.0.5/24', '2001:dead:beef::2/128'],
@@ -242,7 +257,9 @@ class TestParseNetworkConfig(CiTestCase):
         imds_data['network']['interface'].append(third_intf)
         self.assertEqual(expected, dsaz.parse_network_config(imds_data))
 
-    def test_ipv4_secondary_ips_will_be_static_addrs(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_ipv4_secondary_ips_will_be_static_addrs(self, m_driver):
         """parse_network_config emits primary ipv4 as dhcp others are static"""
         expected = {'ethernets': {
             'eth0': {'addresses': ['10.0.0.5/24'],
@@ -262,7 +279,9 @@ class TestParseNetworkConfig(CiTestCase):
         }
         self.assertEqual(expected, dsaz.parse_network_config(imds_data))
 
-    def test_ipv6_secondary_ips_will_be_static_cidrs(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_ipv6_secondary_ips_will_be_static_cidrs(self, m_driver):
         """parse_network_config emits primary ipv6 as dhcp others are static"""
         expected = {'ethernets': {
             'eth0': {'addresses': ['10.0.0.5/24', '2001:dead:beef::2/10'],
@@ -301,6 +320,42 @@ class TestParseNetworkConfig(CiTestCase):
             }}, 'version': 2}
         self.assertEqual(expected, dsaz.parse_network_config(NETWORK_METADATA))
 
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    @mock.patch('cloudinit.net.generate_fallback_config')
+    def test_parse_network_config_uses_fallback_cfg_when_no_network_metadata(
+            self, m_fallback_config, m_driver):
+        """parse_network_config generates fallback network config when the
+        IMDS instance metadata is corrupted/invalid, such as when
+        network metadata is not present.
+        """
+        imds_metadata_missing_network_metadata = copy.deepcopy(
+            NETWORK_METADATA)
+        del imds_metadata_missing_network_metadata['network']
+        m_fallback_config.return_value = self.fallback_config
+        self.assertEqual(
+            self.fallback_config,
+            dsaz.parse_network_config(
+                imds_metadata_missing_network_metadata))
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    @mock.patch('cloudinit.net.generate_fallback_config')
+    def test_parse_network_config_uses_fallback_cfg_when_no_interface_metadata(
+            self, m_fallback_config, m_driver):
+        """parse_network_config generates fallback network config when the
+        IMDS instance metadata is corrupted/invalid, such as when
+        network interface metadata is not present.
+        """
+        imds_metadata_missing_interface_metadata = copy.deepcopy(
+            NETWORK_METADATA)
+        del imds_metadata_missing_interface_metadata['network']['interface']
+        m_fallback_config.return_value = self.fallback_config
+        self.assertEqual(
+            self.fallback_config,
+            dsaz.parse_network_config(
+                imds_metadata_missing_interface_metadata))
+
 
 class TestGetMetadataFromIMDS(HttprettyTestCase):
 
@@ -783,7 +838,9 @@ scbus-1 on xpt0 bus 0
         self.assertTrue(ret)
         self.assertEqual(data['agent_invoked'], cfg['agent_command'])
 
-    def test_network_config_set_from_imds(self):
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
+    def test_network_config_set_from_imds(self, m_driver):
         """Datasource.network_config returns IMDS network data."""
         sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}}
         odata = {}
@@ -801,7 +858,10 @@ scbus-1 on xpt0 bus 0
         dsrc.get_data()
         self.assertEqual(expected_network_config, dsrc.network_config)
 
-    def test_network_config_set_from_imds_route_metric_for_secondary_nic(self):
+    @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):
         """Datasource.network_config adds route-metric to secondary nics."""
         sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}}
         odata = {}
@@ -1157,8 +1217,10 @@ scbus-1 on xpt0 bus 0
         self.assertEqual(
             [mock.call("/dev/cd0")], m_check_fbsd_cdrom.call_args_list)
 
+    @mock.patch('cloudinit.sources.DataSourceAzure.device_driver',
+                return_value=None)
     @mock.patch('cloudinit.net.generate_fallback_config')
-    def test_imds_network_config(self, mock_fallback):
+    def test_imds_network_config(self, mock_fallback, m_driver):
         """Network config is generated from IMDS network data when present."""
         sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}}
         odata = {'HostName': "myhost", 'UserName': "myuser"}
@@ -1245,55 +1307,9 @@ scbus-1 on xpt0 bus 0
 
         netconfig = dsrc.network_config
         self.assertEqual(netconfig, fallback_config)
-        mock_fallback.assert_called_with(blacklist_drivers=['mlx4_core'],
-                                         config_driver=True)
-
-    @mock.patch('cloudinit.net.get_interface_mac')
-    @mock.patch('cloudinit.net.get_devicelist')
-    @mock.patch('cloudinit.net.device_driver')
-    @mock.patch('cloudinit.net.generate_fallback_config')
-    def test_fallback_network_config_blacklist(self, mock_fallback, mock_dd,
-                                               mock_devlist, mock_get_mac):
-        """On absent network metadata, blacklist mlx from fallback config."""
-        odata = {'HostName': "myhost", 'UserName': "myuser"}
-        data = {'ovfcontent': construct_valid_ovf_env(data=odata),
-                'sys_cfg': {}}
-
-        fallback_config = {
-            'version': 1,
-            'config': [{
-                'type': 'physical', 'name': 'eth0',
-                'mac_address': '00:11:22:33:44:55',
-                'params': {'driver': 'hv_netsvc'},
-                'subnets': [{'type': 'dhcp'}],
-            }]
-        }
-        blacklist_config = {
-            'type': 'physical',
-            'name': 'eth1',
-            'mac_address': '00:11:22:33:44:55',
-            'params': {'driver': 'mlx4_core'}
-        }
-        mock_fallback.return_value = fallback_config
-
-        mock_devlist.return_value = ['eth0', 'eth1']
-        mock_dd.side_effect = [
-            'hv_netsvc',  # list composition, skipped
-            'mlx4_core',  # list composition, match
-            'mlx4_core',  # config get driver name
-        ]
-        mock_get_mac.return_value = '00:11:22:33:44:55'
-
-        dsrc = self._get_ds(data)
-        # Represent empty response from network imds
-        self.m_get_metadata_from_imds.return_value = {}
-        ret = dsrc.get_data()
-        self.assertTrue(ret)
-
-        netconfig = dsrc.network_config
-        expected_config = fallback_config
-        expected_config['config'].append(blacklist_config)
-        self.assertEqual(netconfig, expected_config)
+        mock_fallback.assert_called_with(
+            blacklist_drivers=['mlx4_core', 'mlx5_core'],
+            config_driver=True)
 
     @mock.patch(MOCKPATH + 'subp.subp')
     def test_get_hostname_with_no_args(self, m_subp):