some little things/food for thought.
and you have some conflicts.
Diff comments:
> diff --git a/cloudinit/sources/DataSourceAzure.py
> b/cloudinit/sources/DataSourceAzure.py
> index 7007d9e..d30435e 100644
> --- a/cloudinit/sources/DataSourceAzure.py
> +++ b/cloudinit/sources/DataSourceAzure.py
> @@ -373,41 +390,86 @@ class DataSourceAzure(sources.DataSource):
> except NonAzureDataSource:
> continue
> except BrokenAzureDataSource as exc:
> - raise exc
> + msg = 'BrokenAzureDataSource: %s' % exc
> + raise sources.InvalidMetaDataException(msg)
> except util.MountFailedError:
> LOG.warning("%s was not mountable", cdev)
> continue
>
> if reprovision or self._should_reprovision(ret):
> ret = self._reprovision()
> - (md, self.userdata_raw, cfg, files) = ret
> + imds_md = get_metadata_from_imds(
> + self.fallback_interface, retries=3)
> + (md, userdata_raw, cfg, files) = ret
> self.seed = cdev
> - self.metadata = util.mergemanydict([md, DEFAULT_METADATA])
> - self.cfg = util.mergemanydict([cfg, BUILTIN_CLOUD_CONFIG])
> + crawled_data.update({
> + 'cfg': cfg,
> + 'files': files,
> + 'metadata': util.mergemanydict(
> + [md, {'imds': imds_md}]),
> + 'userdata_raw': userdata_raw})
> found = cdev
>
> LOG.debug("found datasource in %s", cdev)
> break
>
> if not found:
> - return False
> + raise sources.InvalidMetaDataException('No Azure metadata found')
>
> if found == ddir:
> LOG.debug("using files cached in %s", ddir)
>
> seed = _get_random_seed()
> if seed:
> - self.metadata['random_seed'] = seed
> + crawled_data['metadata']['random_seed'] = seed
> + crawled_data['metadata']['instance-id'] = util.read_dmi_data(
> + 'system-uuid')
> + return crawled_data
> +
> + def is_platform_viable(self):
i like this idea, but I think that this is at best a @classmethod.
Ie, it doesn't use 'self' at all, and doesn't need to.
I think I prefer just having a method in the DataSourceAzure.py
named 'is_platform_viable', and then having this call that.
thoughts?
> + """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:
> + LOG.debug("Non-Azure DMI asset tag '%s' discovered.", asset_tag)
> + return False
> + return True
> +
> + def clear_cached_attrs(self):
> + """Reset any cached class attributes to defaults."""
> + super(DataSourceAzure, self).clear_cached_attrs()
> + self._metadata_imds = sources.UNSET
> +
> + def _get_data(self):
> + """Crawl and process datasource metadata caching metadata as attrs.
> +
> + @return: True on success, False on error, invalid or disabled
> + datasource.
> + """
> + if not self.is_platform_viable():
> + return False
> + try:
> + crawled_data = util.log_time(
> + logfunc=LOG.debug, msg='Crawl of metadata service',
> + func=self.crawl_metadata)
> + except sources.InvalidMetaDataException as e:
> + LOG.warning('Could not crawl Azure metadata: %s', e)
> + return False
> +
> + # Process crawled data and augment with various config defaults
> + self.cfg = util.mergemanydict(
> + [crawled_data['cfg'], BUILTIN_CLOUD_CONFIG])
> + self._metadata_imds = crawled_data['metadata']['imds']
> + self.metadata = util.mergemanydict(
> + [crawled_data['metadata'], DEFAULT_METADATA])
> + self.userdata_raw = crawled_data['userdata_raw']
>
> user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})
> self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])
>
> # walinux agent writes files world readable, but expects
> # the directory to be protected.
> - write_files(ddir, files, dirmode=0o700)
> -
> - self.metadata['instance-id'] = util.read_dmi_data('system-uuid')
> -
> + write_files(
> + self.ds_cfg['data_dir'], crawled_data['files'], dirmode=0o700)
> return True
>
> def device_name_to_device(self, name):
> @@ -543,22 +605,56 @@ class DataSourceAzure(sources.DataSource):
> @property
> def network_config(self):
> """Generate a network config like net.generate_fallback_network()
> with
> - the following execptions.
> + the following exceptions.
>
> 1. Probe the drivers of the net-devices present and inject them in
> the network configuration under params: driver: <driver> value
> 2. Generate a fallback network config that does not include any of
> the blacklisted devices.
> """
> - blacklist = ['mlx4_core']
> if not self._network_config:
> - LOG.debug('Azure: generating fallback configuration')
> - # generate a network config, blacklist picking any mlx4_core devs
> - netconfig = net.generate_fallback_config(
> - blacklist_drivers=blacklist, config_driver=True)
> + if self._metadata_imds != sources.UNSET and self._metadata_imds:
> + netconfig = {'version': 2, 'ethernets': {}}
> + LOG.debug('Azure: generating network configuration from
> IMDS')
> + if self.distro and self.distro.name == 'ubuntu':
> + maybe_remove_ubuntu_network_config_scripts()
this doesnt seem the right place for this.
network_config() doesn't seem like it should have these side effects.
it is supposed to return a network config not potentially delete things.
i dont know wher those actuaons fit, but It doesn't feel right here.
> + network_metadata = self._metadata_imds['network']
> + for idx, intf in enumerate(network_metadata['interface']):
other datasources have this in a non-class method also, which makes it easier
to test
the simple conversion.
self.assertEqual(expected, convert_network_data(some_data))
> + 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)
>
> self._network_config = netconfig
> -
> return self._network_config
>
>
> diff --git a/tests/unittests/test_datasource/test_azure.py
> b/tests/unittests/test_datasource/test_azure.py
> index e82716e..1258d10 100644
> --- a/tests/unittests/test_datasource/test_azure.py
> +++ b/tests/unittests/test_datasource/test_azure.py
> @@ -314,6 +511,20 @@ fdescfs /dev/fd fdescfs rw
> 0 0
> self.assertTrue(ret)
> self.assertEqual(data['agent_invoked'], cfg['agent_command'])
>
> + def test_network_config_set_from_imds(self):
> + """Datasource.network_config returns IMDS network data."""
> + odata = {}
> + data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
> + expected_network_config = {
> + 'ethernets': {
> + 'eth0': {'set-name': 'eth0',
> + 'match': {'macaddress': '00:0d:3a:04:75:98'},
> + 'dhcp4': True}},
> + 'version': 2}
> + dsrc = self._get_ds(data)
> + dsrc.get_data()
> + self.assertEqual(expected_network_config, dsrc.network_config)
this is what i was talking about, if you have a 'convert_network_config'
function in DataSourceAzure.py
then you can just test it with:
self.assertEqual(expected_network_config, convert_network_config("That
Config"))
> +
> def test_user_cfg_set_agent_command(self):
> # set dscfg in via base64 encoded yaml
> cfg = {'agent_command': "my_command"}
--
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
