cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py
> index 5c017bf..9a5ed43 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py
> @@ -536,24 +536,43 @@ def convert_ec2_metadata_network_config(network_md, macs_to_nics=None,

I wonder if we should keep both versions of the convert here and specify the version in the signature?

>      @param: fallback_nic: Optionally provide the primary nic interface name.
>         This nic will be guaranteed to minimally have a dhcp4 configuration.
>  
> -    @return A dict of network config version 1 based on the metadata and macs.
> +    @return A dict of network config version 2 based on the metadata and macs.
>      """
> -    netcfg = {'version': 1, 'config': []}
> +    netcfg = {'version': 2, 'ethernets': {}}
>      if not macs_to_nics:
>          macs_to_nics = net.get_interfaces_by_mac()
>      macs_metadata = network_md['interfaces']['macs']
>      for mac, nic_name in macs_to_nics.items():
> +        dev_config = {}
>          nic_metadata = macs_metadata.get(mac)
>          if not nic_metadata:
>              continue  # Not a physical nic represented in metadata
> -        nic_cfg = {'type': 'physical', 'name': nic_name, 'subnets': []}
> -        nic_cfg['mac_address'] = mac
> +        local_ipv4s = nic_metadata.get('local-ipv4s')
>          if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or
> -                nic_metadata.get('local-ipv4s')):
> -            nic_cfg['subnets'].append({'type': 'dhcp4'})
> +                local_ipv4s):
> +            dev_config['dhcp4'] = True
> +            # In version < 2018-09-24 local_ipvs is a str with a single IP
> +            if isinstance(local_ipv4s, list) and len(local_ipv4s) > 1:
> +                if dev_config.get('addresses') is None:
> +                    dev_config['addresses'] = []
> +                subnet_cidr = nic_metadata.get('subnet-ipv4-cidr-block')
> +                if not subnet_cidr or '/' not in subnet_cidr:
> +                    LOG.warning(
> +                        'Could not parse subnet-ipv4-cidr-block %s.'
> +                        ' Network config for Secondary IPs default to /32',
> +                        subnet_cidr)
> +                    prefix = '32'
> +                else:
> +                    _ip, prefix = subnet_cidr.split('/')
> +                for secondary_ip in local_ipv4s[1:]:
> +                    dev_config['addresses'].append(
> +                        '{ip}/{prefix}'.format(ip=secondary_ip, prefix=prefix))
>          if nic_metadata.get('ipv6s'):
> -            nic_cfg['subnets'].append({'type': 'dhcp6'})
> -        netcfg['config'].append(nic_cfg)
> +            dev_config['dhcp6'] = True
> +        dev_config.update({
> +            'match': {'macaddress': mac.lower()},
> +            'set-name': nic_name})
> +        netcfg['ethernets'][nic_name] = dev_config
>      return netcfg
>  
>  
> diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
> index 20d59bf..e031fe9 100644
> --- a/tests/unittests/test_datasource/test_ec2.py
> +++ b/tests/unittests/test_datasource/test_ec2.py
> @@ -277,10 +365,9 @@ class TestEc2(test_helpers.HttprettyTestCase):
>              ds.get_data()
>  
>          mac1 = '06:17:04:d7:26:09'  # Defined in DEFAULT_METADATA
> -        expected = {'version': 1, 'config': [
> -            {'mac_address': '06:17:04:d7:26:09', 'name': 'eth9',
> -             'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}],
> -             'type': 'physical'}]}
> +        expected = {'version': 2, 'ethernets': {'eth9': {
> +            'match': {'macaddress': '06:17:04:d7:26:09'}, 'set-name': 'eth9',
> +            'dhcp4': True, 'dhcp6': True}}}

If we keep both versions of the parser, then we can add a variant to the test calling with with v1 and v2?

>          patch_path = (
>              'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
>          get_interface_mac_path = (
> @@ -309,10 +396,40 @@ class TestEc2(test_helpers.HttprettyTestCase):
>              ds.get_data()
>  
>          mac1 = '06:17:04:d7:26:0A'  # IPv4 only in DEFAULT_METADATA
> -        expected = {'version': 1, 'config': [
> -            {'mac_address': '06:17:04:d7:26:0A', 'name': 'eth9',
> -             'subnets': [{'type': 'dhcp4'}],
> -             'type': 'physical'}]}
> +        expected = {'version': 2, 'ethernets': {'eth9': {
> +            'match': {'macaddress': '06:17:04:d7:26:0a'}, 'set-name': 'eth9',
> +            'dhcp4': True}}}
> +        patch_path = (
> +            'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')

I suggest a class attribute or global for the prefix here:

net_path = 'cloudinit.sources.DataSourceEc2.net'
by_mac = mock.patch(net_path + '.get_interfaces_by_mac'

> +        get_interface_mac_path = (
> +            'cloudinit.sources.DataSourceEc2.net.get_interface_mac')
> +        with mock.patch(patch_path) as m_get_interfaces_by_mac:
> +            with mock.patch(find_fallback_path) as m_find_fallback:
> +                with mock.patch(get_interface_mac_path) as m_get_mac:
> +                    m_get_interfaces_by_mac.return_value = {mac1: 'eth9'}
> +                    m_find_fallback.return_value = 'eth9'
> +                    m_get_mac.return_value = mac1
> +                    self.assertEqual(expected, ds.network_config)
> +
> +    def test_network_config_property_secondary_private_ips(self):
> +        """network_config property configures any secondary ipv4 addresses.
> +
> +        Only one device is configured even when multiple exist in metadata.
> +        """
> +        ds = self._setup_ds(
> +            platform_data=self.valid_platform_data,
> +            sys_cfg={'datasource': {'Ec2': {'strict_id': True}}},
> +            md={'md': SECONDARY_IP_METADATA_2018_09_24})
> +        find_fallback_path = (
> +            'cloudinit.sources.DataSourceEc2.net.find_fallback_nic')
> +        with mock.patch(find_fallback_path) as m_find_fallback:
> +            m_find_fallback.return_value = 'eth9'
> +            ds.get_data()
> +
> +        mac1 = '0a:07:84:3d:6e:38'  # IPv4 with 1 secondary IP
> +        expected = {'version': 2, 'ethernets': {'eth9': {
> +            'match': {'macaddress': mac1}, 'set-name': 'eth9',
> +            'addresses': ['172.31.45.70/20'], 'dhcp4': True}}}
>          patch_path = (
>              'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
>          get_interface_mac_path = (


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master.