there are some merge issues shown below ("<<<<<").
I have some comments inline. How feasible would it be to add support for azure conversion in the net-convert sub-command? Diff comments: > diff --git a/cloudinit/sources/DataSourceAzure.py > b/cloudinit/sources/DataSourceAzure.py > index 7007d9e..5d5c64e 100644 > --- a/cloudinit/sources/DataSourceAzure.py > +++ b/cloudinit/sources/DataSourceAzure.py > @@ -1025,6 +1079,162 @@ def load_azure_ds_dir(source_dir): > return (md, ud, cfg, {'ovf-env.xml': contents}) > > > +def get_metadata_from_imds(fallback_nic, retries): > + """Query Azure's network metadata service, returning a dictionary. > + > + If network is not up, setup ephemeral dhcp on fallback_nic to talk to the > + IMDS. For more info on IMDS: > + > https://docs.microsoft.com/en-us/azure/virtual-machines/windows/instance-metadata-service > + > + @param fallback_nic: String. The name of the nic which requires active > + networ in order to query IMDS. > + @param retries: The number of retries of the IMDS_URL. > + > + @return: A dict of instance metadata containing compute and network > + info. > + """ > + if net.is_up(fallback_nic): > + return util.log_time( > + logfunc=LOG.debug, > + msg='Crawl of Azure Instance Metadata Service (IMDS)', > + func=_get_metadata_from_imds, args=(retries,)) > + else: > + with EphemeralDHCPv4(fallback_nic): > + return util.log_time( > + logfunc=LOG.debug, > + msg='Crawl of Azure Instance Metadata Service (IMDS)', > + func=_get_metadata_from_imds, args=(retries,)) > + > + > +def _get_metadata_from_imds(retries): > + > + def retry_on_url_error(msg, exception): > + if isinstance(exception, UrlError) and exception.code == 404: > + return True # Continue retries > + return False # Stop retries on all other exceptions, including 404s > + > + url = IMDS_URL + "instance?api-version=2017-12-01" > + headers = {"Metadata": "true"} > + try: > + response = readurl( > + url, timeout=1, headers=headers, retries=retries, > + exception_cb=retry_on_url_error) > + except Exception as e: > + LOG.debug('Ignoring IMDS instance metadata: %s', e) > + return {} > + try: > + return util.load_json(str(response)) > + except json.decoder.JSONDecodeError: > + LOG.warning( > + 'Ignoring non-json IMDS instance metadata: %s', str(response)) > + return {} > + > + > +def maybe_remove_ubuntu_network_config_scripts(paths=None): > + """Remove Azure-specific ubuntu network config for non-primary nics. > + > + @param paths: List of networking scripts or directories to remove when > + present. > + > + In certain supported ubuntu images, static udev rules or netplan yaml > + config is delivered in the base ubuntu image to support dhcp on any > + additional interfaces which get attached by a customer at some point > + after initial boot. Since the Azure datasource can now regenerate > + network configuration as metadata reports these new devices, we no longer > + want the udev rules or netplan's 90-azure-hotplug.yaml to configure > + networking on eth1 or greater as it might collide with cloud-init's > + configuration. > + > + Remove the any existing extended network scripts if the datasource is > + enabled to write network per-boot. > + """ > + if not paths: > + paths = UBUNTU_EXTENDED_NETWORK_SCRIPTS > + logged = False > + for path in paths: > + if os.path.exists(path): > + if not logged: > + LOG.debug( this feels INFO to me. it is moderately important as it could potentially break things and we will only ever do it once. > + 'Removing Ubuntu extended network scripts because' > + ' cloud-init updates Azure network configuration on the' > + ' following event: %s.', > + EventType.BOOT) > + logged = True > + if os.path.isdir(path): > + util.del_dir(path) > + else: > + util.del_file(path) > + > + > +def _is_platform_viable(seed_dir): > + """Check platform environment to report if this datasource may run.""" > + asset_tag = util.read_dmi_data('chassis-asset-tag') > + if asset_tag == AZURE_CHASSIS_ASSET_TAG: > + return True > + LOG.debug("Non-Azure DMI asset tag '%s' discovered.", asset_tag) > + for path in (AGENT_SEED_DIR, seed_dir): > + if os.path.exists(os.path.join(path, 'ovf-env.xml')): > + return True > + if util.which('systemd-detect-virt'): > + (virt_type, _err) = util.subp( > + ['systemd-detect-virt'], rcs=[0, 1], capture=True) > + if virt_type.strip() == 'microsoft': > + return True > + if util.find_devs_with(criteria='LABEL=rd_rdfe_*'): > + return True > + return False > + > + > +def _parse_network_config(imds_metadata): > + """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. > + """ > + 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']): > + nicname = 'eth{idx}'.format(idx=idx) > + dev_config = {} > + for addr4 in intf['ipv4']['ipAddress']: > + privateIpv4 = addr4['privateIpAddress'] > + if privateIpv4: > + if dev_config.get('dhcp4', False): > + # Append static address config for nic > 1 > + netPrefix = intf['ipv4']['subnet'][0].get( > + 'prefix', '24') > + if not dev_config.get('addresses'): > + dev_config['addresses'] = [] > + dev_config['addresses'].append( > + '{ip}/{prefix}'.format( > + ip=privateIpv4, prefix=netPrefix)) > + else: > + dev_config['dhcp4'] = True > + for addr6 in intf['ipv6']['ipAddress']: > + privateIpv6 = addr6['privateIpAddress'] > + if privateIpv6: > + dev_config['dhcp6'] = True > + break > + if dev_config: > + mac = ':'.join(re.findall(r'..', intf['macAddress'])) > + dev_config.update( > + {'match': {'macaddress': mac.lower()}, > + 'set-name': nicname}) > + netconfig['ethernets'][nicname] = dev_config > + 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) > + return netconfig > + > + > class BrokenAzureDataSource(Exception): > pass > > diff --git a/tests/unittests/test_datasource/test_azure.py > b/tests/unittests/test_datasource/test_azure.py > index e82716e..a76cc99 100644 > --- a/tests/unittests/test_datasource/test_azure.py > +++ b/tests/unittests/test_datasource/test_azure.py > @@ -1398,4 +1651,127 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): > self.assertEqual(m_net.call_count, 1) > > > +class TestRemoveUbuntuNetworkConfigScripts(CiTestCase): > + > + with_logs = True > + > + def setUp(self): > + super(TestRemoveUbuntuNetworkConfigScripts, self).setUp() > + self.tmp = self.tmp_dir() > + > + def test_remove_network_scripts_removes_both_files_and_directories(self): > + """Any files or directories in paths are removed when present.""" > + file1 = self.tmp_path('file1', dir=self.tmp) > + subdir = self.tmp_path('sub1', dir=self.tmp) > + subfile = self.tmp_path('leaf1', dir=subdir) > + write_file(file1, 'file1content') > + write_file(subfile, 'leafcontent') > + dsaz.maybe_remove_ubuntu_network_config_scripts(paths=[subdir, > file1]) > + > + for path in (file1, subdir, subfile): > + self.assertFalse(os.path.exists(path), > + 'Found unremoved: %s' % path) > + > + expected_logs = [ > + 'Removing Ubuntu extended network scripts because cloud-init' > + ' updates Azure network configuration on the following event:' > + ' System boot.', > + 'Recursively deleting %s' % subdir, > + 'Attempting to remove %s' % file1] > + for log in expected_logs: > + self.assertIn(log, self.logs.getvalue()) > + > + def > test_remove_network_scripts_only_attemts_removal_if_path_exists(self): > + """Any files or directories absent are skipped without error.""" > + dsaz.maybe_remove_ubuntu_network_config_scripts(paths=[ > + '/not/a/dir/', '/not/a/file']) might as well use the tmpdir to guarantee that those files do not exist. ie, i think if i do: mkdir -p /not/a/dir then i will cause this test to fail. > + self.assertNotIn('/not/a', self.logs.getvalue()) # No delete logs > + > + @mock.patch('cloudinit.sources.DataSourceAzure.os.path.exists') > + def test_remove_network_scripts_default_removes_stock_scripts(self, > + m_exists): > + """Azure's stock ubuntu image scripts and artifacts are removed.""" > + # Report path absent on all to avoid delete operation > + m_exists.return_value = False > + dsaz.maybe_remove_ubuntu_network_config_scripts() > + calls = m_exists.call_args_list > + for path in dsaz.UBUNTU_EXTENDED_NETWORK_SCRIPTS: > + self.assertIn(mock.call(path), calls) > + > + > +class TestWBIsPlatformViable(CiTestCase): > + """White box tests for _is_platform_viable.""" > + with_logs = True > + > + @mock.patch('cloudinit.sources.DataSourceAzure.util.read_dmi_data') > + def test_true_on_non_azure_chassis(self, m_read_dmi_data): > + """Return True if DMI chassis-asset-tag is > AZURE_CHASSIS_ASSET_TAG.""" > + m_read_dmi_data.return_value = dsaz.AZURE_CHASSIS_ASSET_TAG > + self.assertTrue(dsaz._is_platform_viable('doesnotmatter')) > + > + @mock.patch('cloudinit.sources.DataSourceAzure.os.path.exists') > + @mock.patch('cloudinit.sources.DataSourceAzure.util.read_dmi_data') > + def test_true_on_azure_ovf_env_in_seed_dir(self, m_read_dmi_data, > m_exist): > + """Return True if ovf-env.xml exists in known seed dirs.""" > + # Non-matching Azure chassis-asset-tag > + m_read_dmi_data.return_value = dsaz.AZURE_CHASSIS_ASSET_TAG + 'X' > + > + m_exist.side_effect = [False, True] > + self.assertTrue(dsaz._is_platform_viable('/other/seed/dir')) > + self.assertEqual( > + [mock.call('/var/lib/waagent/ovf-env.xml'), > + mock.call('/other/seed/dir/ovf-env.xml')], > + m_exist.call_args_list) > + > + @mock.patch('cloudinit.sources.DataSourceAzure.util.which') > + @mock.patch('cloudinit.sources.DataSourceAzure.util.subp') > + def test_true_on_detect_virt_microsoft(self, m_subp, m_which): > + """Return True if a partition label prefix rd_rdfe is present.""" > + m_which.return_value = '/usr/bin/systemd-detect-virt' > + m_subp.return_value = ('microsoft', '') this probably neesd a '\n' on the end. > + self.assertTrue(wrap_and_call( > + 'cloudinit.sources.DataSourceAzure', > + {'os.path.exists': False, > + # Non-matching Azure chassis-asset-tag > + 'util.read_dmi_data': dsaz.AZURE_CHASSIS_ASSET_TAG + 'X'}, > + dsaz._is_platform_viable, 'doesnotmatter')) > + m_which.assert_called_once_with('systemd-detect-virt') > + m_subp.assert_called_once_with( > + ['systemd-detect-virt'], capture=True, rcs=[0, 1]) > + > + @mock.patch('cloudinit.sources.DataSourceAzure.util.find_devs_with') > + def test_true_on_azure_when_fs_label_prefix_rd_rdfe(self, m_find_devs): > + """Return True if a partition label prefix rd_rdfe is present.""" > + > + m_find_devs.return_value = ['/dev/Imatched/azure'] > + self.assertTrue(wrap_and_call( > + 'cloudinit.sources.DataSourceAzure', > + {'os.path.exists': False, > + # Non-matching Azure chassis-asset-tag > + 'util.read_dmi_data': dsaz.AZURE_CHASSIS_ASSET_TAG + 'X', > + 'util.which': None}, > + dsaz._is_platform_viable, 'doesnotmatter')) > + m_find_devs.assert_called_once_with(criteria='LABEL=rd_rdfe_*') > + > + def test_false_on_no_matching_azure_criteria(self): > + """Report non-azure on unmatched asset tag, ovf-env absent and no > dev. > + > + Return False when the asset tag doesn't match Azure's static > + AZURE_CHASSIS_ASSET_TAG, no ovf-env.xml files exist in known seed > dirs > + and no devices have a label starting with prefix 'rd_rdfe_'. > + """ > + self.assertFalse(wrap_and_call( > + 'cloudinit.sources.DataSourceAzure', > + {'os.path.exists': False, > + # Non-matching Azure chassis-asset-tag > + 'util.read_dmi_data': dsaz.AZURE_CHASSIS_ASSET_TAG + 'X', > + 'util.which': None, > + 'util.find_devs_with': []}, > + dsaz._is_platform_viable, 'doesnotmatter')) > + self.assertIn( > + "DEBUG: Non-Azure DMI asset tag '{0}' discovered.\n".format( > + dsaz.AZURE_CHASSIS_ASSET_TAG + 'X'), > + self.logs.getvalue()) > + > + > # vi: ts=4 expandtab -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/348704 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/azure-network-on-boot into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp