From 1f860e5ac7ebb5b809c72d8703a0b7cb3e84ccd0 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 5 Mar 2020 17:38:28 -0700 Subject: ec2: Do not fallback to IMDSv1 on EC2 (#216) The EC2 Data Source needs to handle 3 states of the Instance Metadata Service configured for a given instance: 1. HttpTokens : optional & HttpEndpoint : enabled Either IMDSv2 or IMDSv1 can be used. 2. HttpTokens : required & HttpEndpoint : enabled Calls to IMDS without a valid token (IMDSv1 or IMDSv2 with expired token) will return a 401 error. 3. HttpEndpoint : disabled The IMDS http endpoint will return a 403 error. Previous work to support IMDSv2 in cloud-init handled case 1 and case 2. This commit handles case 3 by bypassing the retry block when IMDS returns HTTP status code >= 400 on official AWS cloud platform. It shaves 2 minutes when rebooting an instance that has its IMDS http token endpoint disabled but creates some inconsistencies. An instance that doesn't set "manual_cache_clean" to "True" will have its /var/lib/cloud/instance symlink removed altogether after it has failed to find a datasource. --- tests/unittests/test_datasource/test_ec2.py | 48 +++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 2a96122f..78e82c7e 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -3,6 +3,7 @@ import copy import httpretty import json +import requests from unittest import mock from cloudinit import helpers @@ -200,6 +201,7 @@ def register_mock_metaserver(base_url, data): class TestEc2(test_helpers.HttprettyTestCase): with_logs = True + maxDiff = None valid_platform_data = { 'uuid': 'ec212f79-87d1-2f1d-588f-d86dc0fd5412', @@ -429,6 +431,52 @@ class TestEc2(test_helpers.HttprettyTestCase): self.assertTrue(ds.get_data()) self.assertFalse(ds.is_classic_instance()) + def test_aws_inaccessible_imds_service_fails_with_retries(self): + """Inaccessibility of http://169.254.169.254 are retried.""" + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, + md=None) + + conn_error = requests.exceptions.ConnectionError( + '[Errno 113] no route to host') + + mock_success = mock.MagicMock(contents=b'fakesuccess') + mock_success.ok.return_value = True + + with mock.patch('cloudinit.url_helper.readurl') as m_readurl: + m_readurl.side_effect = (conn_error, conn_error, mock_success) + with mock.patch('cloudinit.url_helper.time.sleep'): + self.assertTrue(ds.wait_for_metadata_service()) + + # Just one /latest/api/token request + self.assertEqual(3, len(m_readurl.call_args_list)) + for readurl_call in m_readurl.call_args_list: + self.assertIn('latest/api/token', readurl_call[0][0]) + + def test_aws_token_403_fails_without_retries(self): + """Verify that 403s fetching AWS tokens are not retried.""" + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, + md=None) + token_url = self.data_url('latest', data_item='api/token') + httpretty.register_uri(httpretty.PUT, token_url, body={}, status=403) + self.assertFalse(ds.get_data()) + # Just one /latest/api/token request + logs = self.logs.getvalue() + failed_put_log = '"PUT /latest/api/token HTTP/1.1" 403 0' + expected_logs = [ + 'WARNING: Ec2 IMDS endpoint returned a 403 error. HTTP endpoint is' + ' disabled. Aborting.', + "WARNING: IMDS's HTTP endpoint is probably disabled", + failed_put_log + ] + for log in expected_logs: + self.assertIn(log, logs) + self.assertEqual( + 1, len([l for l in logs.splitlines() if failed_put_log in l])) + def test_aws_token_redacted(self): """Verify that aws tokens are redacted when logged.""" ds = self._setup_ds( -- cgit v1.2.3 From 6600c642af3817fe5e0170cb7b4eeac4be3c60eb Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 18 Mar 2020 13:33:37 -0600 Subject: ec2: render network on all NICs and add secondary IPs as static (#114) Add support for rendering secondary static IPv4/IPv6 addresses on any NIC attached to the machine. In order to see secondary IP addresses in Ec2 IMDS network config, cloud-init now reads metadata version 2018-09-24. Metadata services which do not support the Ec2 API version will not get secondary IP addresses configured. In order to discover secondary IP address config, cloud-init now relies on metadata API Parse local-ipv4s, ipv6s, subnet-ipv4-cidr-block and subnet-ipv6-cidr-block metadata keys to determine additional IPs and appropriate subnet prefix to set for a nic. Also add the datasource config option apply_full_imds_netork_config which defaults to true to allow cloud-init to automatically configure secondary IP addresses. Setting this option to false will tell cloud-init to avoid setting up secondary IP addresses. Also in this branch: - Shift Ec2 datasource to emit network config v2 instead of v1. LP: #1866930 --- cloudinit/sources/DataSourceEc2.py | 116 ++++++++-- doc/rtd/topics/datasources/ec2.rst | 19 ++ tests/unittests/test_datasource/test_ec2.py | 329 ++++++++++++++++++++++------ 3 files changed, 379 insertions(+), 85 deletions(-) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 8f0d73bb..4203c3a6 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -62,7 +62,7 @@ class DataSourceEc2(sources.DataSource): # Priority ordered list of additional metadata versions which will be tried # for extended metadata content. IPv6 support comes in 2016-09-02 - extended_metadata_versions = ['2016-09-02'] + extended_metadata_versions = ['2018-09-24', '2016-09-02'] # Setup read_url parameters per get_url_params. url_max_wait = 120 @@ -405,13 +405,16 @@ class DataSourceEc2(sources.DataSource): logfunc=LOG.debug, msg='Re-crawl of metadata service', func=self.get_data) - # Limit network configuration to only the primary/fallback nic iface = self.fallback_interface - macs_to_nics = {net.get_interface_mac(iface): iface} net_md = self.metadata.get('network') if isinstance(net_md, dict): + # SRU_BLOCKER: xenial, bionic and eoan should default + # apply_full_imds_network_config to False to retain original + # behavior on those releases. result = convert_ec2_metadata_network_config( - net_md, macs_to_nics=macs_to_nics, fallback_nic=iface) + net_md, fallback_nic=iface, + full_network_config=util.get_cfg_option_bool( + self.ds_cfg, 'apply_full_imds_network_config', True)) # RELEASE_BLOCKER: xenial should drop the below if statement, # because the issue being addressed doesn't exist pre-netplan. @@ -719,9 +722,10 @@ def _collect_platform_data(): return data -def convert_ec2_metadata_network_config(network_md, macs_to_nics=None, - fallback_nic=None): - """Convert ec2 metadata to network config version 1 data dict. +def convert_ec2_metadata_network_config( + network_md, macs_to_nics=None, fallback_nic=None, + full_network_config=True): + """Convert ec2 metadata to network config version 2 data dict. @param: network_md: 'network' portion of EC2 metadata. generally formed as {"interfaces": {"macs": {}} where @@ -731,28 +735,104 @@ def convert_ec2_metadata_network_config(network_md, macs_to_nics=None, not provided, get_interfaces_by_mac is called to get it from the OS. @param: fallback_nic: Optionally provide the primary nic interface name. This nic will be guaranteed to minimally have a dhcp4 configuration. + @param: full_network_config: Boolean set True to configure all networking + presented by IMDS. This includes rendering secondary IPv4 and IPv6 + addresses on all NICs and rendering network config on secondary NICs. + If False, only the primary nic will be configured and only with dhcp + (IPv4/IPv6). - @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(): + + if not full_network_config: + for mac, nic_name in macs_to_nics.items(): + if nic_name == fallback_nic: + break + dev_config = {'dhcp4': True, + 'dhcp6': False, + 'match': {'macaddress': mac.lower()}, + 'set-name': nic_name} + nic_metadata = macs_metadata.get(mac) + if nic_metadata.get('ipv6s'): # Any IPv6 addresses configured + dev_config['dhcp6'] = True + netcfg['ethernets'][nic_name] = dev_config + return netcfg + # Apply network config for all nics and any secondary IPv4/v6 addresses + nic_idx = 1 + for mac, nic_name in sorted(macs_to_nics.items()): 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 - if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or - nic_metadata.get('local-ipv4s')): - nic_cfg['subnets'].append({'type': 'dhcp4'}) - if nic_metadata.get('ipv6s'): - nic_cfg['subnets'].append({'type': 'dhcp6'}) - netcfg['config'].append(nic_cfg) + dhcp_override = {'route-metric': nic_idx * 100} + nic_idx += 1 + dev_config = {'dhcp4': True, 'dhcp4-overrides': dhcp_override, + 'dhcp6': False, + 'match': {'macaddress': mac.lower()}, + 'set-name': nic_name} + if nic_metadata.get('ipv6s'): # Any IPv6 addresses configured + dev_config['dhcp6'] = True + dev_config['dhcp6-overrides'] = dhcp_override + dev_config['addresses'] = get_secondary_addresses(nic_metadata, mac) + if not dev_config['addresses']: + dev_config.pop('addresses') # Since we found none configured + netcfg['ethernets'][nic_name] = dev_config + # Remove route-metric dhcp overrides if only one nic configured + if len(netcfg['ethernets']) == 1: + for nic_name in netcfg['ethernets'].keys(): + netcfg['ethernets'][nic_name].pop('dhcp4-overrides') + netcfg['ethernets'][nic_name].pop('dhcp6-overrides', None) return netcfg +def get_secondary_addresses(nic_metadata, mac): + """Parse interface-specific nic metadata and return any secondary IPs + + :return: List of secondary IPv4 or IPv6 addresses to configure on the + interface + """ + ipv4s = nic_metadata.get('local-ipv4s') + ipv6s = nic_metadata.get('ipv6s') + addresses = [] + # In version < 2018-09-24 local_ipv4s or ipv6s is a str with one IP + if bool(isinstance(ipv4s, list) and len(ipv4s) > 1): + addresses.extend( + _get_secondary_addresses( + nic_metadata, 'subnet-ipv4-cidr-block', mac, ipv4s, '24')) + if bool(isinstance(ipv6s, list) and len(ipv6s) > 1): + addresses.extend( + _get_secondary_addresses( + nic_metadata, 'subnet-ipv6-cidr-block', mac, ipv6s, '128')) + return sorted(addresses) + + +def _get_secondary_addresses(nic_metadata, cidr_key, mac, ips, default_prefix): + """Return list of IP addresses as CIDRs for secondary IPs + + The CIDR prefix will be default_prefix if cidr_key is absent or not + parseable in nic_metadata. + """ + addresses = [] + cidr = nic_metadata.get(cidr_key) + prefix = default_prefix + if not cidr or len(cidr.split('/')) != 2: + ip_type = 'ipv4' if 'ipv4' in cidr_key else 'ipv6' + LOG.warning( + 'Could not parse %s %s for mac %s. %s network' + ' config prefix defaults to /%s', + cidr_key, cidr, mac, ip_type, prefix) + else: + prefix = cidr.split('/')[1] + # We know we have > 1 ips for in metadata for this IP type + for ip in ips[1:]: + addresses.append( + '{ip}/{prefix}'.format(ip=ip, prefix=prefix)) + return addresses + + # Used to match classes to dependencies datasources = [ (DataSourceEc2Local, (sources.DEP_FILESYSTEM,)), # Run at init-local diff --git a/doc/rtd/topics/datasources/ec2.rst b/doc/rtd/topics/datasources/ec2.rst index a90f3779..1c3a880f 100644 --- a/doc/rtd/topics/datasources/ec2.rst +++ b/doc/rtd/topics/datasources/ec2.rst @@ -42,6 +42,7 @@ Note that there are multiple versions of this data provided, cloud-init by default uses **2009-04-04** but newer versions can be supported with relative ease (newer versions have more data exposed, while maintaining backward compatibility with the previous versions). +Version **2016-09-02** is required for secondary IP address support. To see which versions are supported from your cloud provider use the following URL: @@ -80,6 +81,15 @@ The settings that may be configured are: * **timeout**: the timeout value provided to urlopen for each individual http request. This is used both when selecting a metadata_url and when crawling the metadata service. (default: 50) + * **apply_full_imds_network_config**: Boolean (default: True) to allow + cloud-init to configure any secondary NICs and secondary IPs described by + the metadata service. All network interfaces are configured with DHCP (v4) + to obtain an primary IPv4 address and route. Interfaces which have a + non-empty 'ipv6s' list will also enable DHCPv6 to obtain a primary IPv6 + address and route. The DHCP response (v4 and v6) return an IP that matches + the first element of local-ipv4s and ipv6s lists respectively. All + additional values (secondary addresses) in the static ip lists will be + added to interface. An example configuration with the default values is provided below: @@ -90,6 +100,7 @@ An example configuration with the default values is provided below: metadata_urls: ["http://169.254.169.254:80", "http://instance-data:8773"] max_wait: 120 timeout: 50 + apply_full_imds_network_config: true Notes ----- @@ -102,4 +113,12 @@ Notes The check for the instance type is performed by is_classic_instance() method. + * For EC2 instances with multiple network interfaces (NICs) attached, dhcp4 + will be enabled to obtain the primary private IPv4 address of those NICs. + Wherever dhcp4 or dhcp6 is enabled for a NIC, a dhcp route-metric will be + added with the value of `` * 100`` to ensure dhcp + routes on the primary NIC are preferred to any secondary NICs. + For example: the primary NIC will have a DHCP route-metric of 100, + the next NIC will be 200. + .. vi: textwidth=78 diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 78e82c7e..9126c676 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -113,6 +113,122 @@ DEFAULT_METADATA = { "services": {"domain": "amazonaws.com", "partition": "aws"}, } +# collected from api version 2018-09-24/ with +# 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))' + +NIC1_MD_IPV4_IPV6_MULTI_IP = { + "device-number": "0", + "interface-id": "eni-0d6335689899ce9cc", + "ipv4-associations": { + "18.218.219.181": "172.31.44.13" + }, + "ipv6s": [ + "2600:1f16:292:100:c187:593c:4349:136", + "2600:1f16:292:100:f153:12a3:c37c:11f9", + "2600:1f16:292:100:f152:2222:3333:4444" + ], + "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", + "subnet_ipv6_cidr_blocks": "2600:1f16:292:100::/64", + "vpc-id": "vpc-a07f62c8", + "vpc-ipv4-cidr-block": "172.31.0.0/16", + "vpc-ipv4-cidr-blocks": "172.31.0.0/16", + "vpc_ipv6_cidr_blocks": "2600:1f16:292:100::/56" +} + +NIC2_MD = { + "device_number": "1", + "interface_id": "eni-043cdce36ded5e79f", + "local_hostname": "ip-172-31-47-221.us-east-2.compute.internal", + "local_ipv4s": "172.31.47.221", + "mac": "0a:75:69:92:e2:16", + "owner_id": "329910648901", + "security_group_ids": "sg-0d68fef37d8cc9b77", + "security_groups": "launch-wizard-17", + "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" +} + +SECONDARY_IP_METADATA_2018_09_24 = { + "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": "" + }, + "network": { + "interfaces": { + "macs": { + "0a:07:84:3d:6e:38": NIC1_MD_IPV4_IPV6_MULTI_IP, + } + } + }, + "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" + } +} + +M_PATH_NET = 'cloudinit.sources.DataSourceEc2.net.' + def _register_ssh_keys(rfunc, base_url, keys_data): """handle ssh key inconsistencies. @@ -267,30 +383,23 @@ class TestEc2(test_helpers.HttprettyTestCase): register_mock_metaserver(instance_id_url, None) return ds - def test_network_config_property_returns_version_1_network_data(self): - """network_config property returns network version 1 for metadata. - - Only one device is configured even when multiple exist in metadata. - """ + def test_network_config_property_returns_version_2_network_data(self): + """network_config property returns network version 2 for metadata""" ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, md={'md': DEFAULT_METADATA}) - find_fallback_path = ( - 'cloudinit.sources.DataSourceEc2.net.find_fallback_nic') + find_fallback_path = M_PATH_NET + 'find_fallback_nic' with mock.patch(find_fallback_path) as m_find_fallback: m_find_fallback.return_value = 'eth9' 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'}]} - patch_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac') - get_interface_mac_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interface_mac') + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': '06:17:04:d7:26:09'}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': True}}} + patch_path = M_PATH_NET + 'get_interfaces_by_mac' + get_interface_mac_path = M_PATH_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: @@ -299,30 +408,59 @@ class TestEc2(test_helpers.HttprettyTestCase): m_get_mac.return_value = mac1 self.assertEqual(expected, ds.network_config) - def test_network_config_property_set_dhcp4_on_private_ipv4(self): - """network_config property configures dhcp4 on private ipv4 nics. + def test_network_config_property_set_dhcp4(self): + """network_config property configures dhcp4 on nics with local-ipv4s. - Only one device is configured even when multiple exist in metadata. + Only one device is configured based on get_interfaces_by_mac even when + multiple MACs exist in metadata. """ ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, md={'md': DEFAULT_METADATA}) - find_fallback_path = ( - 'cloudinit.sources.DataSourceEc2.net.find_fallback_nic') + find_fallback_path = M_PATH_NET + 'find_fallback_nic' with mock.patch(find_fallback_path) as m_find_fallback: m_find_fallback.return_value = 'eth9' 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'}]} - patch_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac') - get_interface_mac_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interface_mac') + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': mac1.lower()}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': False}}} + patch_path = M_PATH_NET + 'get_interfaces_by_mac' + get_interface_mac_path = M_PATH_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 based on get_interfaces_by_mac even when + multiple MACs 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 = M_PATH_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' # 1 secondary IPv4 and 2 secondary IPv6 + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': mac1}, 'set-name': 'eth9', + 'addresses': ['172.31.45.70/20', + '2600:1f16:292:100:f152:2222:3333:4444/128', + '2600:1f16:292:100:f153:12a3:c37c:11f9/128'], + 'dhcp4': True, 'dhcp6': True}}} + patch_path = M_PATH_NET + 'get_interfaces_by_mac' + get_interface_mac_path = M_PATH_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: @@ -358,21 +496,18 @@ class TestEc2(test_helpers.HttprettyTestCase): register_mock_metaserver( 'http://169.254.169.254/2009-04-04/meta-data/', DEFAULT_METADATA) mac1 = '06:17:04:d7:26:09' # Defined in DEFAULT_METADATA - get_interface_mac_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interface_mac') + get_interface_mac_path = M_PATH_NET + 'get_interfaces_by_mac' ds.fallback_nic = 'eth9' - with mock.patch(get_interface_mac_path) as m_get_interface_mac: - m_get_interface_mac.return_value = mac1 + with mock.patch(get_interface_mac_path) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.return_value = {mac1: 'eth9'} nc = ds.network_config # Will re-crawl network metadata self.assertIsNotNone(nc) self.assertIn( 'Refreshing stale metadata from prior to upgrade', self.logs.getvalue()) - 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': mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': True}}} self.assertEqual(expected, ds.network_config) def test_ec2_get_instance_id_refreshes_identity_on_upgrade(self): @@ -491,7 +626,7 @@ class TestEc2(test_helpers.HttprettyTestCase): logs_with_redacted = [log for log in all_logs if REDACT_TOK in log] logs_with_token = [log for log in all_logs if 'API-TOKEN' in log] self.assertEqual(1, len(logs_with_redacted_ttl)) - self.assertEqual(79, len(logs_with_redacted)) + self.assertEqual(81, len(logs_with_redacted)) self.assertEqual(0, len(logs_with_token)) @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @@ -612,6 +747,44 @@ class TestEc2(test_helpers.HttprettyTestCase): self.assertIn('Crawl of metadata service took', self.logs.getvalue()) +class TestGetSecondaryAddresses(test_helpers.CiTestCase): + + mac = '06:17:04:d7:26:ff' + with_logs = True + + def test_md_with_no_secondary_addresses(self): + """Empty list is returned when nic metadata contains no secondary ip""" + self.assertEqual([], ec2.get_secondary_addresses(NIC2_MD, self.mac)) + + def test_md_with_secondary_v4_and_v6_addresses(self): + """All secondary addresses are returned from nic metadata""" + self.assertEqual( + ['172.31.45.70/20', '2600:1f16:292:100:f152:2222:3333:4444/128', + '2600:1f16:292:100:f153:12a3:c37c:11f9/128'], + ec2.get_secondary_addresses(NIC1_MD_IPV4_IPV6_MULTI_IP, self.mac)) + + def test_invalid_ipv4_ipv6_cidr_metadata_logged_with_defaults(self): + """Any invalid subnet-ipv(4|6)-cidr-block values use defaults""" + invalid_cidr_md = copy.deepcopy(NIC1_MD_IPV4_IPV6_MULTI_IP) + invalid_cidr_md['subnet-ipv4-cidr-block'] = "something-unexpected" + invalid_cidr_md['subnet-ipv6-cidr-block'] = "not/sure/what/this/is" + self.assertEqual( + ['172.31.45.70/24', '2600:1f16:292:100:f152:2222:3333:4444/128', + '2600:1f16:292:100:f153:12a3:c37c:11f9/128'], + ec2.get_secondary_addresses(invalid_cidr_md, self.mac)) + expected_logs = [ + "WARNING: Could not parse subnet-ipv4-cidr-block" + " something-unexpected for mac 06:17:04:d7:26:ff." + " ipv4 network config prefix defaults to /24", + "WARNING: Could not parse subnet-ipv6-cidr-block" + " not/sure/what/this/is for mac 06:17:04:d7:26:ff." + " ipv6 network config prefix defaults to /128" + ] + logs = self.logs.getvalue() + for log in expected_logs: + self.assertIn(log, logs) + + class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): def setUp(self): @@ -619,16 +792,16 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): self.mac1 = '06:17:04:d7:26:09' self.network_metadata = { 'interfaces': {'macs': { - self.mac1: {'public-ipv4s': '172.31.2.16'}}}} + self.mac1: {'mac': self.mac1, 'public-ipv4s': '172.31.2.16'}}}} def test_convert_ec2_metadata_network_config_skips_absent_macs(self): """Any mac absent from metadata is skipped by network config.""" macs_to_nics = {self.mac1: 'eth9', 'DE:AD:BE:EF:FF:FF': 'vitualnic2'} # DE:AD:BE:EF:FF:FF represented by OS but not in metadata - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': False}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( @@ -642,15 +815,15 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): network_metadata_ipv6['interfaces']['macs'][self.mac1]) nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' nic1_metadata.pop('public-ipv4s') - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', 'subnets': [{'type': 'dhcp6'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': True}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( network_metadata_ipv6, macs_to_nics)) - def test_convert_ec2_metadata_network_config_handles_local_dhcp4(self): + def test_convert_ec2_metadata_network_config_local_only_dhcp4(self): """Config dhcp4 when there are no public addresses in public-ipv4s.""" macs_to_nics = {self.mac1: 'eth9'} network_metadata_ipv6 = copy.deepcopy(self.network_metadata) @@ -658,9 +831,9 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): network_metadata_ipv6['interfaces']['macs'][self.mac1]) nic1_metadata['local-ipv4s'] = '172.3.3.15' nic1_metadata.pop('public-ipv4s') - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': False}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( @@ -675,16 +848,16 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): nic1_metadata['public-ipv4s'] = '' # When no ipv4 or ipv6 content but fallback_nic set, set dhcp4 config. - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': False}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( network_metadata_ipv6, macs_to_nics, fallback_nic='eth9')) def test_convert_ec2_metadata_network_config_handles_local_v4_and_v6(self): - """When dhcp6 is public and dhcp4 is set to local enable both.""" + """When ipv6s and local-ipv4s are non-empty, enable dhcp6 and dhcp4.""" macs_to_nics = {self.mac1: 'eth9'} network_metadata_both = copy.deepcopy(self.network_metadata) nic1_metadata = ( @@ -692,10 +865,35 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' nic1_metadata.pop('public-ipv4s') nic1_metadata['local-ipv4s'] = '10.0.0.42' # Local ipv4 only on vpc - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', - 'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': True}}} + self.assertEqual( + expected, + ec2.convert_ec2_metadata_network_config( + network_metadata_both, macs_to_nics)) + + def test_convert_ec2_metadata_network_config_handles_multiple_nics(self): + """DHCP route-metric increases on secondary NICs for IPv4 and IPv6.""" + mac2 = '06:17:04:d7:26:0a' + macs_to_nics = {self.mac1: 'eth9', mac2: 'eth10'} + network_metadata_both = copy.deepcopy(self.network_metadata) + # Add 2nd nic info + network_metadata_both['interfaces']['macs'][mac2] = NIC2_MD + nic1_metadata = ( + network_metadata_both['interfaces']['macs'][self.mac1]) + nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' + nic1_metadata.pop('public-ipv4s') # No public-ipv4 IPs in cfg + nic1_metadata['local-ipv4s'] = '10.0.0.42' # Local ipv4 only on vpc + expected = {'version': 2, 'ethernets': { + 'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp4-overrides': {'route-metric': 100}, + 'dhcp6': True, 'dhcp6-overrides': {'route-metric': 100}}, + 'eth10': { + 'match': {'macaddress': mac2}, 'set-name': 'eth10', + 'dhcp4': True, 'dhcp4-overrides': {'route-metric': 200}, + 'dhcp6': False}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( @@ -708,10 +906,9 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): nic1_metadata = ( network_metadata_both['interfaces']['macs'][self.mac1]) nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', - 'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}]}]} + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, 'set-name': 'eth9', + 'dhcp4': True, 'dhcp6': True}}} self.assertEqual( expected, ec2.convert_ec2_metadata_network_config( @@ -719,12 +916,10 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): def test_convert_ec2_metadata_gets_macs_from_get_interfaces_by_mac(self): """Convert Ec2 Metadata calls get_interfaces_by_mac by default.""" - expected = {'version': 1, 'config': [ - {'mac_address': self.mac1, 'type': 'physical', - 'name': 'eth9', - 'subnets': [{'type': 'dhcp4'}]}]} - patch_path = ( - 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac') + expected = {'version': 2, 'ethernets': {'eth9': { + 'match': {'macaddress': self.mac1}, + 'set-name': 'eth9', 'dhcp4': True, 'dhcp6': False}}} + patch_path = M_PATH_NET + 'get_interfaces_by_mac' with mock.patch(patch_path) as m_get_interfaces_by_mac: m_get_interfaces_by_mac.return_value = {self.mac1: 'eth9'} self.assertEqual( -- cgit v1.2.3 From 70dbccbbb27f7cc3f2decd692d41403f0d745c62 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 1 May 2020 16:56:56 -0400 Subject: DataSourceEc2: use metadata's NIC ordering to determine route-metrics (#342) We want to set route-metrics such that NICs are configured with the priority that they are given in the network metadata that we receive from the IMDS. (This switches away from using MAC ordering.) This also required the following test changes: * reverse the sort order of the MACs in test data (so that they would trigger the bug being fixed) * fix up the key names in `NIC2_MD` (which were under_scored instead of dash-separated) * use a full interface dict (rather than a minimal one) for `TestConvertEc2MetadataNetworkConfig` LP: #1876312 --- cloudinit/sources/DataSourceEc2.py | 5 ++-- tests/unittests/test_datasource/test_ec2.py | 41 ++++++++++++++++------------- 2 files changed, 26 insertions(+), 20 deletions(-) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 4203c3a6..355b4e2f 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -762,13 +762,14 @@ def convert_ec2_metadata_network_config( netcfg['ethernets'][nic_name] = dev_config return netcfg # Apply network config for all nics and any secondary IPv4/v6 addresses - nic_idx = 1 for mac, nic_name in sorted(macs_to_nics.items()): nic_metadata = macs_metadata.get(mac) if not nic_metadata: continue # Not a physical nic represented in metadata + # device-number is zero-indexed, we want it 1-indexed for the + # multiplication on the following line + nic_idx = int(nic_metadata['device-number']) + 1 dhcp_override = {'route-metric': nic_idx * 100} - nic_idx += 1 dev_config = {'dhcp4': True, 'dhcp4-overrides': dhcp_override, 'dhcp6': False, 'match': {'macaddress': mac.lower()}, diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 9126c676..d413d1cd 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -38,6 +38,8 @@ DYNAMIC_METADATA = { # python3 -c 'import json # from cloudinit.ec2_utils import get_instance_metadata as gm # print(json.dumps(gm("2016-09-02"), indent=1, sort_keys=True))' +# Note that the MAC addresses have been modified to sort in the opposite order +# to the device-number attribute, to test LP: #1876312 DEFAULT_METADATA = { "ami-id": "ami-8b92b4ee", "ami-launch-index": "0", @@ -77,7 +79,7 @@ DEFAULT_METADATA = { "vpc-ipv4-cidr-blocks": "172.31.0.0/16", "vpc-ipv6-cidr-blocks": "2600:1f16:aeb:b200::/56" }, - "06:17:04:d7:26:0A": { + "06:17:04:d7:26:08": { "device-number": "1", # Only IPv4 local config "interface-id": "eni-e44ef49f", "ipv4-associations": {"": "172.3.3.16"}, @@ -85,7 +87,7 @@ DEFAULT_METADATA = { "local-hostname": ("ip-172-3-3-16.us-east-2." "compute.internal"), "local-ipv4s": "172.3.3.16", - "mac": "06:17:04:d7:26:0A", + "mac": "06:17:04:d7:26:08", "owner-id": "950047163771", "public-hostname": ("ec2-172-3-3-16.us-east-2." "compute.amazonaws.com"), @@ -152,19 +154,19 @@ NIC1_MD_IPV4_IPV6_MULTI_IP = { } NIC2_MD = { - "device_number": "1", - "interface_id": "eni-043cdce36ded5e79f", - "local_hostname": "ip-172-31-47-221.us-east-2.compute.internal", - "local_ipv4s": "172.31.47.221", + "device-number": "1", + "interface-id": "eni-043cdce36ded5e79f", + "local-hostname": "ip-172-31-47-221.us-east-2.compute.internal", + "local-ipv4s": "172.31.47.221", "mac": "0a:75:69:92:e2:16", - "owner_id": "329910648901", - "security_group_ids": "sg-0d68fef37d8cc9b77", - "security_groups": "launch-wizard-17", - "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" + "owner-id": "329910648901", + "security-group-ids": "sg-0d68fef37d8cc9b77", + "security-groups": "launch-wizard-17", + "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" } SECONDARY_IP_METADATA_2018_09_24 = { @@ -423,7 +425,7 @@ class TestEc2(test_helpers.HttprettyTestCase): m_find_fallback.return_value = 'eth9' ds.get_data() - mac1 = '06:17:04:d7:26:0A' # IPv4 only in DEFAULT_METADATA + mac1 = '06:17:04:d7:26:08' # IPv4 only in DEFAULT_METADATA expected = {'version': 2, 'ethernets': {'eth9': { 'match': {'macaddress': mac1.lower()}, 'set-name': 'eth9', 'dhcp4': True, 'dhcp6': False}}} @@ -790,9 +792,12 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): def setUp(self): super(TestConvertEc2MetadataNetworkConfig, self).setUp() self.mac1 = '06:17:04:d7:26:09' + interface_dict = copy.deepcopy( + DEFAULT_METADATA['network']['interfaces']['macs'][self.mac1]) + # These tests are written assuming the base interface doesn't have IPv6 + interface_dict.pop('ipv6s') self.network_metadata = { - 'interfaces': {'macs': { - self.mac1: {'mac': self.mac1, 'public-ipv4s': '172.31.2.16'}}}} + 'interfaces': {'macs': {self.mac1: interface_dict}}} def test_convert_ec2_metadata_network_config_skips_absent_macs(self): """Any mac absent from metadata is skipped by network config.""" @@ -875,7 +880,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): def test_convert_ec2_metadata_network_config_handles_multiple_nics(self): """DHCP route-metric increases on secondary NICs for IPv4 and IPv6.""" - mac2 = '06:17:04:d7:26:0a' + mac2 = '06:17:04:d7:26:08' macs_to_nics = {self.mac1: 'eth9', mac2: 'eth10'} network_metadata_both = copy.deepcopy(self.network_metadata) # Add 2nd nic info -- cgit v1.2.3 From 4ab3303ec4bb49f029b7821d6dba53a6b02b6dc1 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Mon, 1 Jun 2020 14:20:39 -0700 Subject: test: fix all flake8 E741 errors (#401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This removes the use of variables named ‘l’, ‘O’, or ‘I’. Generally these are used in list comprehension to read the line of lines. --- cloudinit/distros/__init__.py | 2 +- cloudinit/net/__init__.py | 8 ++++---- cloudinit/sources/DataSourceOpenNebula.py | 6 +++--- cloudinit/sources/helpers/openstack.py | 17 +++++++++++------ cloudinit/ssh_util.py | 4 +++- tests/cloud_tests/platforms/instances.py | 4 ++-- tests/cloud_tests/testcases/base.py | 4 ++-- tests/unittests/test_datasource/test_ec2.py | 4 +++- tests/unittests/test_datasource/test_ovf.py | 2 +- tests/unittests/test_ds_identify.py | 6 ++++-- .../test_handler/test_handler_apt_source_v3.py | 4 +++- tests/unittests/test_sshutil.py | 2 +- tox.ini | 3 +-- 13 files changed, 39 insertions(+), 27 deletions(-) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index e99529df..35a10590 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -582,7 +582,7 @@ class Distro(metaclass=abc.ABCMeta): # passwd must use short '-l' due to SLES11 lacking long form '--lock' lock_tools = (['passwd', '-l', name], ['usermod', '--lock', name]) try: - cmd = next(l for l in lock_tools if util.which(l[0])) + cmd = next(tool for tool in lock_tools if util.which(tool[0])) except StopIteration: raise RuntimeError(( "Unable to lock user account '%s'. No tools available. " diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index cb8c1601..8af24fa9 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -824,13 +824,13 @@ def get_interfaces_by_mac_on_freebsd(): # flatten each interface block in a single line def flatten(out): curr_block = '' - for l in out.split('\n'): - if l.startswith('\t'): - curr_block += l + for line in out.split('\n'): + if line.startswith('\t'): + curr_block += line else: if curr_block: yield curr_block - curr_block = l + curr_block = line yield curr_block # looks for interface and mac in a list of flatten block diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 02c9a7b8..a08ab404 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -417,9 +417,9 @@ def read_context_disk_dir(source_dir, asuser=None): if ssh_key_var: lines = context.get(ssh_key_var).splitlines() - results['metadata']['public-keys'] = [l for l in lines - if len(l) and not - l.startswith("#")] + results['metadata']['public-keys'] = [ + line for line in lines if len(line) and not line.startswith("#") + ] # custom hostname -- try hostname or leave cloud-init # itself create hostname from IP address later diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index e91398ea..a4373f24 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -411,8 +411,11 @@ class ConfigDriveReader(BaseReader): keydata = meta_js.get('public-keys', keydata) if keydata: lines = keydata.splitlines() - md['public-keys'] = [l for l in lines - if len(l) and not l.startswith("#")] + md['public-keys'] = [ + line + for line in lines + if len(line) and not line.startswith("#") + ] # config-drive-v1 has no way for openstack to provide the instance-id # so we copy that into metadata from the user input @@ -674,11 +677,13 @@ def convert_net_json(network_json=None, known_macs=None): raise ValueError("Unable to find a system nic for %s" % d) d['name'] = known_macs[mac] - for cfg, key, fmt, target in link_updates: - if isinstance(target, (list, tuple)): - cfg[key] = [fmt % link_id_info[l]['name'] for l in target] + for cfg, key, fmt, targets in link_updates: + if isinstance(targets, (list, tuple)): + cfg[key] = [ + fmt % link_id_info[target]['name'] for target in targets + ] else: - cfg[key] = fmt % link_id_info[target]['name'] + cfg[key] = fmt % link_id_info[targets]['name'] # Infiniband interfaces may be referenced in network_data.json by a 6 byte # Ethernet MAC-style address, and we use that address to look up the diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py index c3a9b5b7..918c4aec 100644 --- a/cloudinit/ssh_util.py +++ b/cloudinit/ssh_util.py @@ -344,7 +344,9 @@ def update_ssh_config(updates, fname=DEF_SSHD_CFG): changed = update_ssh_config_lines(lines=lines, updates=updates) if changed: util.write_file( - fname, "\n".join([str(l) for l in lines]) + "\n", copy_mode=True) + fname, "\n".join( + [str(line) for line in lines] + ) + "\n", copy_mode=True) return len(changed) != 0 diff --git a/tests/cloud_tests/platforms/instances.py b/tests/cloud_tests/platforms/instances.py index 529e79cd..efc35c7f 100644 --- a/tests/cloud_tests/platforms/instances.py +++ b/tests/cloud_tests/platforms/instances.py @@ -132,8 +132,8 @@ class Instance(TargetBase): """ def clean_test(test): """Clean formatting for system ready test testcase.""" - return ' '.join(l for l in test.strip().splitlines() - if not l.lstrip().startswith('#')) + return ' '.join(line for line in test.strip().splitlines() + if not line.lstrip().startswith('#')) boot_timeout = self.config['boot_timeout'] tests = [self.config['system_ready_script']] diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 7b67f54e..68d59111 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -141,8 +141,8 @@ class CloudTestCase(unittest.TestCase): def test_no_warnings_in_log(self): """Unexpected warnings should not be found in the log.""" warnings = [ - l for l in self.get_data_file('cloud-init.log').splitlines() - if 'WARN' in l] + line for line in self.get_data_file('cloud-init.log').splitlines() + if 'WARN' in line] joined_warnings = '\n'.join(warnings) for expected_warning in self.expected_warnings: self.assertIn( diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index d413d1cd..ad1ea595 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -612,7 +612,9 @@ class TestEc2(test_helpers.HttprettyTestCase): for log in expected_logs: self.assertIn(log, logs) self.assertEqual( - 1, len([l for l in logs.splitlines() if failed_put_log in l])) + 1, + len([line for line in logs.splitlines() if failed_put_log in line]) + ) def test_aws_token_redacted(self): """Verify that aws tokens are redacted when logged.""" diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py index a19c35c8..486a2345 100644 --- a/tests/unittests/test_datasource/test_ovf.py +++ b/tests/unittests/test_datasource/test_ovf.py @@ -48,7 +48,7 @@ def fill_properties(props, template=OVF_ENV_CONTENT): for key, val in props.items(): lines.append(prop_tmpl.format(key=key, val=val)) indent = " " - properties = ''.join([indent + l + "\n" for l in lines]) + properties = ''.join([indent + line + "\n" for line in lines]) return template.format(properties=properties) diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index f0e96b44..c2318570 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -611,8 +611,10 @@ class TestDsIdentify(DsIdentifyBase): ret = self._check_via_dict( cust, RC_FOUND, func=".", args=[os.path.join(rootd, mpp)], rootd=rootd) - line = [l for l in ret.stdout.splitlines() if l.startswith(pre)][0] - toks = line.replace(pre, "").split(":") + match = [ + line for line in ret.stdout.splitlines() if line.startswith(pre) + ][0] + toks = match.replace(pre, "").split(":") expected = ["/sbin", "/bin", "/usr/sbin", "/usr/bin", "/mycust/path"] self.assertEqual(expected, [p for p in expected if p in toks], "path did not have expected tokens") diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py index 4762dbef..aefe26c4 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v3.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py @@ -1033,7 +1033,9 @@ class TestDebconfSelections(TestCase): # assumes called with *args value. selections = m_set_sel.call_args_list[0][0][0].decode() - missing = [l for l in lines if l not in selections.splitlines()] + missing = [ + line for line in lines if line not in selections.splitlines() + ] self.assertEqual([], missing) @mock.patch("cloudinit.config.cc_apt_configure.dpkg_reconfigure") diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py index 0be41924..b4767f0c 100644 --- a/tests/unittests/test_sshutil.py +++ b/tests/unittests/test_sshutil.py @@ -299,7 +299,7 @@ class TestUpdateSshConfigLines(test_helpers.CiTestCase): lines = ssh_util.parse_ssh_config_lines(list(self.exlines)) result = ssh_util.update_ssh_config_lines(lines, updates) self.assertEqual([], result) - self.assertEqual(self.exlines, [str(l) for l in lines]) + self.assertEqual(self.exlines, [str(line) for line in lines]) def test_keycase_not_modified(self): """Original case of key should not be changed on update. diff --git a/tox.ini b/tox.ini index 611c3111..57b20568 100644 --- a/tox.ini +++ b/tox.ini @@ -49,10 +49,9 @@ deps = -r{toxinidir}/test-requirements.txt # E226: missing whitespace around arithmetic operator # E241: multiple spaces after ‘,’ # E402: module level import not at top of file -# E741: do not use variables named ‘l’, ‘O’, or ‘I’ # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,E241,E402,E741,W503,W504 +ignore=E121,E123,E126,E226,E241,E402,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools [testenv:doc] -- cgit v1.2.3 From d53921ea3396c8301c65cad3abf04b4542d4b7a0 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 2 Jun 2020 09:06:07 -0700 Subject: test: fix all flake8 E121 and E123 errors (#404) This fixes issues with closing brackets not matching the opening bracket's line and continuation line under-idented for hanging indent. --- cloudinit/net/network_state.py | 2 +- cloudinit/reporting/handlers.py | 12 +++---- cloudinit/sources/helpers/azure.py | 6 ++-- tests/unittests/test_datasource/test_aliyun.py | 2 +- tests/unittests/test_datasource/test_azure.py | 8 ++--- tests/unittests/test_datasource/test_ec2.py | 15 +++++---- tests/unittests/test_datasource/test_scaleway.py | 39 +++++++++++++--------- tests/unittests/test_distros/test_netconfig.py | 4 +-- tests/unittests/test_ds_identify.py | 4 +-- .../unittests/test_handler/test_handler_puppet.py | 23 ++++++++----- tests/unittests/test_net.py | 18 +++++----- tox.ini | 4 +-- 12 files changed, 75 insertions(+), 62 deletions(-) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 35c279f9..f28973dc 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -215,7 +215,7 @@ class NetworkState(object): return ( route.get('prefix') == 0 and route.get('network') in default_nets - ) + ) class NetworkStateInterpreter(metaclass=CommandHandlerMeta): diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py index 946df7e0..00e8d2e5 100755 --- a/cloudinit/reporting/handlers.py +++ b/cloudinit/reporting/handlers.py @@ -219,7 +219,7 @@ class HyperVKvpReportingHandler(ReportingHandler): v = ( record_data[ self.HV_KVP_EXCHANGE_MAX_KEY_SIZE:self.HV_KVP_RECORD_SIZE - ].decode('utf-8').strip('\x00')) + ].decode('utf-8').strip('\x00')) return {'key': k, 'value': v} @@ -265,11 +265,11 @@ class HyperVKvpReportingHandler(ReportingHandler): """ key = self._event_key(event) meta_data = { - "name": event.name, - "type": event.event_type, - "ts": (datetime.utcfromtimestamp(event.timestamp) - .isoformat() + 'Z'), - } + "name": event.name, + "type": event.event_type, + "ts": (datetime.utcfromtimestamp(event.timestamp) + .isoformat() + 'Z'), + } if hasattr(event, self.RESULT_KEY): meta_data[self.RESULT_KEY] = event.result meta_data[self.MSG_KEY] = event.description diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index fc760581..82b6730c 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -64,13 +64,15 @@ def is_byte_swapped(previous_id, current_id): return ''.join(dd) parts = current_id.split('-') - swapped_id = '-'.join([ + swapped_id = '-'.join( + [ swap_bytestring(parts[0]), swap_bytestring(parts[1]), swap_bytestring(parts[2]), parts[3], parts[4] - ]) + ] + ) return previous_id == swapped_id diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py index 1e66fcdb..b626229e 100644 --- a/tests/unittests/test_datasource/test_aliyun.py +++ b/tests/unittests/test_datasource/test_aliyun.py @@ -143,7 +143,7 @@ class TestAliYunDatasource(test_helpers.HttprettyTestCase): self.assertEqual('aliyun', self.ds.cloud_name) self.assertEqual('ec2', self.ds.platform) self.assertEqual( - 'metadata (http://100.100.100.200)', self.ds.subplatform) + 'metadata (http://100.100.100.200)', self.ds.subplatform) @mock.patch("cloudinit.sources.DataSourceAliYun._is_aliyun") def test_returns_false_when_not_on_aliyun(self, m_is_aliyun): diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 2f8561cb..d8d812e3 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -114,14 +114,14 @@ NETWORK_METADATA = { "ipv4": { "subnet": [ { - "prefix": "24", - "address": "10.0.0.0" + "prefix": "24", + "address": "10.0.0.0" } ], "ipAddress": [ { - "privateIpAddress": "10.0.0.4", - "publicIpAddress": "104.46.124.81" + "privateIpAddress": "10.0.0.4", + "publicIpAddress": "104.46.124.81" } ] } diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index ad1ea595..80e69c03 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -576,7 +576,8 @@ class TestEc2(test_helpers.HttprettyTestCase): md=None) conn_error = requests.exceptions.ConnectionError( - '[Errno 113] no route to host') + '[Errno 113] no route to host' + ) mock_success = mock.MagicMock(contents=b'fakesuccess') mock_success.ok.return_value = True @@ -777,12 +778,12 @@ class TestGetSecondaryAddresses(test_helpers.CiTestCase): '2600:1f16:292:100:f153:12a3:c37c:11f9/128'], ec2.get_secondary_addresses(invalid_cidr_md, self.mac)) expected_logs = [ - "WARNING: Could not parse subnet-ipv4-cidr-block" - " something-unexpected for mac 06:17:04:d7:26:ff." - " ipv4 network config prefix defaults to /24", - "WARNING: Could not parse subnet-ipv6-cidr-block" - " not/sure/what/this/is for mac 06:17:04:d7:26:ff." - " ipv6 network config prefix defaults to /128" + "WARNING: Could not parse subnet-ipv4-cidr-block" + " something-unexpected for mac 06:17:04:d7:26:ff." + " ipv4 network config prefix defaults to /24", + "WARNING: Could not parse subnet-ipv6-cidr-block" + " not/sure/what/this/is for mac 06:17:04:d7:26:ff." + " ipv6 network config prefix defaults to /128" ] logs = self.logs.getvalue() for log in expected_logs: diff --git a/tests/unittests/test_datasource/test_scaleway.py b/tests/unittests/test_datasource/test_scaleway.py index 1b4dd0ad..15441454 100644 --- a/tests/unittests/test_datasource/test_scaleway.py +++ b/tests/unittests/test_datasource/test_scaleway.py @@ -371,25 +371,32 @@ class TestDataSourceScaleway(HttprettyTestCase): m_get_cmdline.return_value = 'scaleway' fallback_nic.return_value = 'ens2' self.datasource.metadata['ipv6'] = { - 'address': '2000:abc:4444:9876::42:999', - 'gateway': '2000:abc:4444:9876::42:000', - 'netmask': '127', - } + 'address': '2000:abc:4444:9876::42:999', + 'gateway': '2000:abc:4444:9876::42:000', + 'netmask': '127', + } netcfg = self.datasource.network_config - resp = {'version': 1, - 'config': [{ - 'type': 'physical', - 'name': 'ens2', - 'subnets': [{'type': 'dhcp4'}, - {'type': 'static', - 'address': '2000:abc:4444:9876::42:999', - 'gateway': '2000:abc:4444:9876::42:000', - 'netmask': '127', } - ] - - }] + resp = { + 'version': 1, + 'config': [ + { + 'type': 'physical', + 'name': 'ens2', + 'subnets': [ + { + 'type': 'dhcp4' + }, + { + 'type': 'static', + 'address': '2000:abc:4444:9876::42:999', + 'gateway': '2000:abc:4444:9876::42:000', + 'netmask': '127', + } + ] } + ] + } self.assertEqual(netcfg, resp) @mock.patch('cloudinit.sources.DataSourceScaleway.net.find_fallback_nic') diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index ccf66161..910207ca 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -532,7 +532,7 @@ class TestNetCfgDistroRedhat(TestNetCfgDistroBase): NETWORKING_IPV6=yes IPV6_AUTOCONF=no """), - } + } # rh_distro.apply_network_config(V1_NET_CFG_IPV6, False) self._apply_and_verify(self.distro.apply_network_config, V1_NET_CFG_IPV6, @@ -656,7 +656,7 @@ class TestNetCfgDistroArch(TestNetCfgDistroBase): IP=dhcp Interface=eth1 """), - } + } # ub_distro.apply_network_config(V1_NET_CFG, False) self._apply_and_verify(self.distro.apply_network_config, diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index c2318570..65a96090 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -1040,11 +1040,11 @@ VALID_CFG = { 'Ec2-E24Cloud': { 'ds': 'Ec2', 'files': {P_SYS_VENDOR: 'e24cloud\n'}, - }, + }, 'Ec2-E24Cloud-negative': { 'ds': 'Ec2', 'files': {P_SYS_VENDOR: 'e24cloudyday\n'}, - } + } } # vi: ts=4 expandtab diff --git a/tests/unittests/test_handler/test_handler_puppet.py b/tests/unittests/test_handler/test_handler_puppet.py index 04aa7d03..2506d18a 100644 --- a/tests/unittests/test_handler/test_handler_puppet.py +++ b/tests/unittests/test_handler/test_handler_puppet.py @@ -149,15 +149,20 @@ class TestPuppetHandle(CiTestCase): mycloud.distro = mock.MagicMock() cfg = { 'puppet': { - 'csr_attributes': { - 'custom_attributes': { - '1.2.840.113549.1.9.7': '342thbjkt82094y0ut' - 'hhor289jnqthpc2290'}, - 'extension_requests': { - 'pp_uuid': 'ED803750-E3C7-44F5-BB08-41A04433FE2E', - 'pp_image_name': 'my_ami_image', - 'pp_preshared_key': '342thbjkt82094y0uthhor289jnqthpc2290'} - }}} + 'csr_attributes': { + 'custom_attributes': { + '1.2.840.113549.1.9.7': + '342thbjkt82094y0uthhor289jnqthpc2290' + }, + 'extension_requests': { + 'pp_uuid': 'ED803750-E3C7-44F5-BB08-41A04433FE2E', + 'pp_image_name': 'my_ami_image', + 'pp_preshared_key': + '342thbjkt82094y0uthhor289jnqthpc2290' + } + } + } + } csr_attributes = 'cloudinit.config.cc_puppet.' \ 'PUPPET_CSR_ATTRIBUTES_PATH' with mock.patch(csr_attributes, self.csr_attributes_path): diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 84d3a5f0..23626395 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3332,7 +3332,7 @@ USERCTL=no USERCTL=no VLAN=yes """) - } + } self._compare_files_to_expected( expected, self._render_and_read(network_config=v2data)) @@ -3406,7 +3406,7 @@ USERCTL=no TYPE=Ethernet USERCTL=no """), - } + } for dhcp_ver in ('dhcp4', 'dhcp6'): v2data = copy.deepcopy(v2base) if dhcp_ver == 'dhcp6': @@ -4765,13 +4765,13 @@ class TestNetRenderers(CiTestCase): def test_sysconfig_available_uses_variant_mapping(self, m_distro, m_avail): m_avail.return_value = True distro_values = [ - ('opensuse', '', ''), - ('opensuse-leap', '', ''), - ('opensuse-tumbleweed', '', ''), - ('sles', '', ''), - ('centos', '', ''), - ('fedora', '', ''), - ('redhat', '', ''), + ('opensuse', '', ''), + ('opensuse-leap', '', ''), + ('opensuse-tumbleweed', '', ''), + ('sles', '', ''), + ('centos', '', ''), + ('fedora', '', ''), + ('redhat', '', ''), ] for (distro_name, distro_version, flavor) in distro_values: m_distro.return_value = (distro_name, distro_version, flavor) diff --git a/tox.ini b/tox.ini index 5ee67368..e04c7791 100644 --- a/tox.ini +++ b/tox.ini @@ -43,13 +43,11 @@ basepython = python2.7 deps = -r{toxinidir}/test-requirements.txt [flake8] -# E121: continuation line under-indented for hanging indent -# E123: closing bracket does not match indentation of opening bracket’s line # E126: continuation line over-indented for hanging indent # E226: missing whitespace around arithmetic operator # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,W503,W504 +ignore=E126,E226,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools per-file-ignores = cloudinit/cmd/main.py:E402 -- cgit v1.2.3 From d083a0315faf67c00acdecdc3f95c700edf6ba06 Mon Sep 17 00:00:00 2001 From: Moustafa Moustafa Date: Fri, 19 Jun 2020 06:38:05 -0700 Subject: printing the error stream of the dhclient process before killing it (#369) This introduces a way to log the dhclient error stream, and uses it for the Azure datasource (where we have a specific requirement for this data to be logged). --- cloudinit/net/dhcp.py | 33 +++++++++++++------- cloudinit/net/tests/test_dhcp.py | 38 +++++++++++++++++++++++ cloudinit/sources/DataSourceAzure.py | 6 ++-- cloudinit/sources/helpers/azure.py | 8 ++++- tests/unittests/test_datasource/test_ec2.py | 2 +- tests/unittests/test_datasource/test_openstack.py | 2 +- 6 files changed, 73 insertions(+), 16 deletions(-) (limited to 'tests/unittests/test_datasource/test_ec2.py') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index d03baeab..9e004688 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -40,10 +40,11 @@ class NoDHCPLeaseError(Exception): class EphemeralDHCPv4(object): - def __init__(self, iface=None, connectivity_url=None): + def __init__(self, iface=None, connectivity_url=None, dhcp_log_func=None): self.iface = iface self._ephipv4 = None self.lease = None + self.dhcp_log_func = dhcp_log_func self.connectivity_url = connectivity_url def __enter__(self): @@ -81,7 +82,8 @@ class EphemeralDHCPv4(object): if self.lease: return self.lease try: - leases = maybe_perform_dhcp_discovery(self.iface) + leases = maybe_perform_dhcp_discovery( + self.iface, self.dhcp_log_func) except InvalidDHCPLeaseFileError: raise NoDHCPLeaseError() if not leases: @@ -131,13 +133,15 @@ class EphemeralDHCPv4(object): result[internal_mapping] = self.lease.get(different_names) -def maybe_perform_dhcp_discovery(nic=None): +def maybe_perform_dhcp_discovery(nic=None, dhcp_log_func=None): """Perform dhcp discovery if nic valid and dhclient command exists. If the nic is invalid or undiscoverable or dhclient command is not found, skip dhcp_discovery and return an empty dict. @param nic: Name of the network interface we want to run dhclient on. + @param dhcp_log_func: A callable accepting the dhclient output and error + streams. @return: A list of dicts representing dhcp options for each lease obtained from the dhclient discovery if run, otherwise an empty list is returned. @@ -159,7 +163,7 @@ def maybe_perform_dhcp_discovery(nic=None): prefix='cloud-init-dhcp-', needs_exe=True) as tdir: # Use /var/tmp because /run/cloud-init/tmp is mounted noexec - return dhcp_discovery(dhclient_path, nic, tdir) + return dhcp_discovery(dhclient_path, nic, tdir, dhcp_log_func) def parse_dhcp_lease_file(lease_file): @@ -193,13 +197,15 @@ def parse_dhcp_lease_file(lease_file): return dhcp_leases -def dhcp_discovery(dhclient_cmd_path, interface, cleandir): +def dhcp_discovery(dhclient_cmd_path, interface, cleandir, dhcp_log_func=None): """Run dhclient on the interface without scripts or filesystem artifacts. @param dhclient_cmd_path: Full path to the dhclient used. @param interface: Name of the network inteface on which to dhclient. @param cleandir: The directory from which to run dhclient as well as store dhcp leases. + @param dhcp_log_func: A callable accepting the dhclient output and error + streams. @return: A list of dicts of representing the dhcp leases parsed from the dhcp.leases file or empty list. @@ -223,7 +229,7 @@ def dhcp_discovery(dhclient_cmd_path, interface, cleandir): subp.subp(['ip', 'link', 'set', 'dev', interface, 'up'], capture=True) cmd = [sandbox_dhclient_cmd, '-1', '-v', '-lf', lease_file, '-pf', pid_file, interface, '-sf', '/bin/true'] - subp.subp(cmd, capture=True) + out, err = subp.subp(cmd, capture=True) # Wait for pid file and lease file to appear, and for the process # named by the pid file to daemonize (have pid 1 as its parent). If we @@ -240,6 +246,7 @@ def dhcp_discovery(dhclient_cmd_path, interface, cleandir): return [] ppid = 'unknown' + daemonized = False for _ in range(0, 1000): pid_content = util.load_file(pid_file).strip() try: @@ -251,13 +258,17 @@ def dhcp_discovery(dhclient_cmd_path, interface, cleandir): if ppid == 1: LOG.debug('killing dhclient with pid=%s', pid) os.kill(pid, signal.SIGKILL) - return parse_dhcp_lease_file(lease_file) + daemonized = True + break time.sleep(0.01) - LOG.error( - 'dhclient(pid=%s, parentpid=%s) failed to daemonize after %s seconds', - pid_content, ppid, 0.01 * 1000 - ) + if not daemonized: + LOG.error( + 'dhclient(pid=%s, parentpid=%s) failed to daemonize after %s ' + 'seconds', pid_content, ppid, 0.01 * 1000 + ) + if dhcp_log_func is not None: + dhcp_log_func(out, err) return parse_dhcp_lease_file(lease_file) diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index d4881592..7987f9d1 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -309,6 +309,7 @@ class TestDHCPDiscoveryClean(CiTestCase): Lease processing still occurs and no proc kill is attempted. """ + m_subp.return_value = ('', '') tmpdir = self.tmp_dir() dhclient_script = os.path.join(tmpdir, 'dhclient.orig') script_content = '#!/bin/bash\necho fake-dhclient' @@ -344,6 +345,7 @@ class TestDHCPDiscoveryClean(CiTestCase): m_kill, m_getppid): """dhcp_discovery waits for the presence of pidfile and dhcp.leases.""" + m_subp.return_value = ('', '') tmpdir = self.tmp_dir() dhclient_script = os.path.join(tmpdir, 'dhclient.orig') script_content = '#!/bin/bash\necho fake-dhclient' @@ -370,6 +372,7 @@ class TestDHCPDiscoveryClean(CiTestCase): It also returns the parsed dhcp.leases file generated in the sandbox. """ + m_subp.return_value = ('', '') tmpdir = self.tmp_dir() dhclient_script = os.path.join(tmpdir, 'dhclient.orig') script_content = '#!/bin/bash\necho fake-dhclient' @@ -406,6 +409,41 @@ class TestDHCPDiscoveryClean(CiTestCase): 'eth9', '-sf', '/bin/true'], capture=True)]) m_kill.assert_has_calls([mock.call(my_pid, signal.SIGKILL)]) + @mock.patch('cloudinit.net.dhcp.util.get_proc_ppid') + @mock.patch('cloudinit.net.dhcp.os.kill') + @mock.patch('cloudinit.net.dhcp.subp.subp') + def test_dhcp_output_error_stream(self, m_subp, m_kill, m_getppid): + """"dhcp_log_func is called with the output and error streams of + dhclinet when the callable is passed.""" + dhclient_err = 'FAKE DHCLIENT ERROR' + dhclient_out = 'FAKE DHCLIENT OUT' + m_subp.return_value = (dhclient_out, dhclient_err) + tmpdir = self.tmp_dir() + dhclient_script = os.path.join(tmpdir, 'dhclient.orig') + script_content = '#!/bin/bash\necho fake-dhclient' + write_file(dhclient_script, script_content, mode=0o755) + lease_content = dedent(""" + lease { + interface "eth9"; + fixed-address 192.168.2.74; + option subnet-mask 255.255.255.0; + option routers 192.168.2.1; + } + """) + lease_file = os.path.join(tmpdir, 'dhcp.leases') + write_file(lease_file, lease_content) + pid_file = os.path.join(tmpdir, 'dhclient.pid') + my_pid = 1 + write_file(pid_file, "%d\n" % my_pid) + m_getppid.return_value = 1 # Indicate that dhclient has daemonized + + def dhcp_log_func(out, err): + self.assertEqual(out, dhclient_out) + self.assertEqual(err, dhclient_err) + + dhcp_discovery( + dhclient_script, 'eth9', tmpdir, dhcp_log_func=dhcp_log_func) + class TestSystemdParseLeases(CiTestCase): diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 6d569057..41431a7a 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -35,7 +35,8 @@ from cloudinit.sources.helpers.azure import ( get_system_info, report_diagnostic_event, EphemeralDHCPv4WithReporting, - is_byte_swapped) + is_byte_swapped, + dhcp_log_cb) LOG = logging.getLogger(__name__) @@ -636,7 +637,8 @@ class DataSourceAzure(sources.DataSource): name="obtain-dhcp-lease", description="obtain dhcp lease", parent=azure_ds_reporter): - self._ephemeral_dhcp_ctx = EphemeralDHCPv4() + self._ephemeral_dhcp_ctx = EphemeralDHCPv4( + dhcp_log_func=dhcp_log_cb) lease = self._ephemeral_dhcp_ctx.obtain_lease() if vnet_switched: diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index 7bace8ca..7afa7ed8 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -626,10 +626,16 @@ def get_metadata_from_fabric(fallback_lease_file=None, dhcp_opts=None, shim.clean_up() +def dhcp_log_cb(out, err): + report_diagnostic_event("dhclient output stream: %s" % out) + report_diagnostic_event("dhclient error stream: %s" % err) + + class EphemeralDHCPv4WithReporting(object): def __init__(self, reporter, nic=None): self.reporter = reporter - self.ephemeralDHCPv4 = EphemeralDHCPv4(iface=nic) + self.ephemeralDHCPv4 = EphemeralDHCPv4( + iface=nic, dhcp_log_func=dhcp_log_cb) def __enter__(self): with events.ReportEventStack( diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 80e69c03..a93f2195 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -744,7 +744,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ret = ds.get_data() self.assertTrue(ret) - m_dhcp.assert_called_once_with('eth9') + m_dhcp.assert_called_once_with('eth9', None) m_net.assert_called_once_with( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', prefix_or_mask='255.255.255.0', router='192.168.2.1', diff --git a/tests/unittests/test_datasource/test_openstack.py b/tests/unittests/test_datasource/test_openstack.py index b534457c..3cfba74d 100644 --- a/tests/unittests/test_datasource/test_openstack.py +++ b/tests/unittests/test_datasource/test_openstack.py @@ -279,7 +279,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): self.assertEqual(2, len(ds_os_local.files)) self.assertEqual(VENDOR_DATA, ds_os_local.vendordata_pure) self.assertIsNone(ds_os_local.vendordata_raw) - m_dhcp.assert_called_with('eth9') + m_dhcp.assert_called_with('eth9', None) def test_bad_datasource_meta(self): os_files = copy.deepcopy(OS_FILES) -- cgit v1.2.3