Overall, this looks good, thanks! I have a few inline comments/questions, but the overall structure/logic seems sound to me.
(Looks like the CI failures are due to linting.) Diff comments: > diff --git a/cloudinit/sources/DataSourceEc2.py > b/cloudinit/sources/DataSourceEc2.py > index 5c017bf..aef5d80 100644 > --- a/cloudinit/sources/DataSourceEc2.py > +++ b/cloudinit/sources/DataSourceEc2.py > @@ -536,24 +536,35 @@ def convert_ec2_metadata_network_config(network_md, > macs_to_nics=None, > @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 > + if isinstance(local_ipv4s, list) and len(local_ipv4s) > 1: What else might local_ipv4s be here? > + subnet_cidr = nic_metadata.get('subnet-ipv4-cidr-block') > + _ip, prefix = subnet_cidr.split('/') subnet_cidr could be None here. > + for secondary_ip in local_ipv4s[1:]: > + if dev_config.get('addresses') is None: Can this move out of the for loop? > + dev_config['addresses'] = [] > + 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': nic_metadata.get('mac').lower()}, This will fail if 'mac' isn't present in nic_metadata. > + '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..c99f58f 100644 > --- a/tests/unittests/test_datasource/test_ec2.py > +++ b/tests/unittests/test_datasource/test_ec2.py > @@ -112,6 +112,94 @@ DEFAULT_METADATA = { > "services": {"domain": "amazonaws.com", "partition": "aws"}, > } > > +# collected from api version 2016-09-02/ with Does this comment reference the wrong version? > +# python3 -c 'import json > +# from cloudinit.ec2_utils import get_instance_metadata as gm > +# print(json.dumps(gm("2018-09-24"), indent=1, sort_keys=True))' > +DEFAULT_METADATA_2018_09_24 = { Is "DEFAULT_METADATA" accurate? (I would expect the default metadata to only include a single private IP, but that may be a mistaken assumption!) > + "ami-id": "ami-0986c2ac728528ac2", > + "ami-launch-index": "0", > + "ami-manifest-path": "(unknown)", > + "block-device-mapping": { > + "ami": "/dev/sda1", > + "root": "/dev/sda1" > + }, > + "events": { > + "maintenance": { > + "history": "[]", > + "scheduled": "[]" > + } > + }, > + "hostname": "ip-172-31-44-13.us-east-2.compute.internal", > + "identity-credentials": { > + "ec2": { > + "info": { > + "AccountId": "329910648901", > + "Code": "Success", > + "LastUpdated": "2019-07-06T14:22:56Z" > + } > + } > + }, > + "instance-action": "none", > + "instance-id": "i-069e01e8cc43732f8", > + "instance-type": "t2.micro", > + "local-hostname": "ip-172-31-44-13.us-east-2.compute.internal", > + "local-ipv4": "172.31.44.13", > + "mac": "0a:07:84:3d:6e:38", > + "metrics": { > + "vhostmd": "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" > + }, > + "network": { > + "interfaces": { > + "macs": { > + "0a:07:84:3d:6e:38": { > + "device-number": "0", > + "interface-id": "eni-0d6335689899ce9cc", > + "ipv4-associations": { > + "18.218.219.181": "172.31.44.13" > + }, > + "local-hostname": ("ip-172-31-44-13.us-east-2." > + "compute.internal"), > + "local-ipv4s": [ > + "172.31.44.13", > + "172.31.45.70" > + ], > + "mac": "0a:07:84:3d:6e:38", > + "owner-id": "329910648901", > + "public-hostname": ("ec2-18-218-219-181.us-east-2." > + "compute.amazonaws.com"), > + "public-ipv4s": "18.218.219.181", > + "security-group-ids": "sg-0c387755222ba8d2e", > + "security-groups": "launch-wizard-4", > + "subnet-id": "subnet-9d7ba0d1", > + "subnet-ipv4-cidr-block": "172.31.32.0/20", > + "vpc-id": "vpc-a07f62c8", > + "vpc-ipv4-cidr-block": "172.31.0.0/16", > + "vpc-ipv4-cidr-blocks": "172.31.0.0/16" > + } > + } > + } > + }, > + "placement": { > + "availability-zone": "us-east-2c" > + }, > + "profile": "default-hvm", > + "public-hostname": ("ec2-18-218-219-181.us-east-2." > + "compute.amazonaws.com"), > + "public-ipv4": "18.218.219.181", > + "public-keys": { > + "yourkeyname,e": [ > + "ssh-rsa AAAAW...DZ yourkeyname" > + ] > + }, > + "reservation-id": "r-09b4917135cdd33be", > + "security-groups": "launch-wizard-4", > + "services": { > + "domain": "amazonaws.com", > + "partition": "aws" > + } > +} > + > > def _register_ssh_keys(rfunc, base_url, keys_data): > """handle ssh key inconsistencies. -- 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. _______________________________________________ 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