From 638f09e82260ea3256c387cd55408e0411f36c23 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sun, 7 Oct 2018 19:31:50 +0000 Subject: tests: fix apt_configure_primary to be more flexible Commit d3e803ad316e6796e5d83e7e8f8f4f7224b92df9 added deb-src comments to the cloud-init apt templates. This doubled the number of matching entries seen in /etc/apt/sources.list in apt_configure_primary integration test. This test was really asserting that GaTech urls were present in /etc//apt/sources.list instead of archive.ubuntu.com. Fix the test to be a bit more flexible in case cloud-init changes its bas apt template again. --- .../cloud_tests/testcases/modules/apt_configure_primary.py | 14 +++++++++----- .../testcases/modules/apt_configure_primary.yaml | 7 ------- 2 files changed, 9 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/tests/cloud_tests/testcases/modules/apt_configure_primary.py b/tests/cloud_tests/testcases/modules/apt_configure_primary.py index c1c4bbc0..4950a2ef 100644 --- a/tests/cloud_tests/testcases/modules/apt_configure_primary.py +++ b/tests/cloud_tests/testcases/modules/apt_configure_primary.py @@ -9,12 +9,16 @@ class TestAptconfigurePrimary(base.CloudTestCase): def test_ubuntu_sources(self): """Test no default Ubuntu entries exist.""" - out = self.get_data_file('ubuntu.sources.list') - self.assertEqual(0, int(out)) + out = self.get_data_file('sources.list') + ubuntu_source_count = len( + [line for line in out.split('\n') if 'archive.ubuntu.com' in line]) + self.assertEqual(0, ubuntu_source_count) def test_gatech_sources(self): - """Test GaTech entires exist.""" - out = self.get_data_file('gatech.sources.list') - self.assertEqual(20, int(out)) + """Test GaTech entries exist.""" + out = self.get_data_file('sources.list') + gatech_source_count = len( + [line for line in out.split('\n') if 'gtlib.gatech.edu' in line]) + self.assertGreater(gatech_source_count, 0) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/apt_configure_primary.yaml b/tests/cloud_tests/testcases/modules/apt_configure_primary.yaml index 41bcf2fd..cc067d4f 100644 --- a/tests/cloud_tests/testcases/modules/apt_configure_primary.yaml +++ b/tests/cloud_tests/testcases/modules/apt_configure_primary.yaml @@ -12,13 +12,6 @@ cloud_config: | - default uri: "http://www.gtlib.gatech.edu/pub/ubuntu-releases/" collect_scripts: - ubuntu.sources.list: | - #!/bin/bash - grep -v '^#' /etc/apt/sources.list | sed '/^\s*$/d' | grep -c archive.ubuntu.com - gatech.sources.list: | - #!/bin/bash - grep -v '^#' /etc/apt/sources.list | sed '/^\s*$/d' | grep -c gtlib.gatech.edu - sources.list: | #!/bin/bash cat /etc/apt/sources.list -- cgit v1.2.3 From 00e36d3ded0b0f81f352de993036fc9f89e14a7a Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 9 Oct 2018 17:31:33 +0000 Subject: net: ignore nics that have "zero" mac address. Previously we explicitly excluded mac address '00:00:00:00:00:00'. But then some nics (tunl0 and sit0) ended up having a mac address like '00:00:00:00'. The change here just ignores all 00[:00[:00...]]. LP: #1796917 --- cloudinit/net/__init__.py | 6 ++++-- tests/unittests/test_net.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index f83d3681..ad98a595 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -612,7 +612,8 @@ def get_interfaces(): Bridges and any devices that have a 'stolen' mac are excluded.""" ret = [] devs = get_devicelist() - empty_mac = '00:00:00:00:00:00' + # 16 somewhat arbitrarily chosen. Normally a mac is 6 '00:' tokens. + zero_mac = ':'.join(('00',) * 16) for name in devs: if not interface_has_own_mac(name): continue @@ -624,7 +625,8 @@ def get_interfaces(): # some devices may not have a mac (tun0) if not mac: continue - if mac == empty_mac and name != 'lo': + # skip nics that have no mac (00:00....) + if name != 'lo' and mac == zero_mac[:len(mac)]: continue ret.append((name, mac, device_driver(name), device_devid(name))) return ret diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 5d9c7d92..8e383739 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3339,9 +3339,23 @@ class TestGetInterfacesByMac(CiTestCase): addnics = ('greptap1', 'lo', 'greptap2') self.data['macs'].update(dict((k, empty_mac) for k in addnics)) self.data['devices'].update(set(addnics)) + self.data['own_macs'].extend(list(addnics)) ret = net.get_interfaces_by_mac() self.assertEqual('lo', ret[empty_mac]) + def test_skip_all_zeros(self): + """Any mac of 00:... should be skipped.""" + self._mock_setup() + emac1, emac2, emac4, emac6 = ( + '00', '00:00', '00:00:00:00', '00:00:00:00:00:00') + addnics = {'empty1': emac1, 'emac2a': emac2, 'emac2b': emac2, + 'emac4': emac4, 'emac6': emac6} + self.data['macs'].update(addnics) + self.data['devices'].update(set(addnics)) + self.data['own_macs'].extend(addnics.keys()) + ret = net.get_interfaces_by_mac() + self.assertEqual('lo', ret['00:00:00:00:00:00']) + def test_ib(self): ib_addr = '80:00:00:28:fe:80:00:00:00:00:00:00:00:11:22:03:00:33:44:56' ib_addr_eth_format = '00:11:22:33:44:56' -- cgit v1.2.3 From f0bc02d7e221c9aa5982b267739481420c761ead Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 9 Oct 2018 21:46:35 +0000 Subject: instance-data: Add standard keys platform and subplatform. Refactor ec2. Add the following instance-data.json standardized keys: * v1._beta_keys: List any v1 keys in beta development, e.g. ['subplatform']. * v1.public_ssh_keys: List of any cloud-provided ssh keys for the instance. * v1.platform: String representing the cloud platform api supporting the datasource. For example: 'ec2' for aws, aliyun and brightbox cloud names. * v1.subplatform: String with more details about the source of the metadata consumed. For example, metadata uri, config drive device path or seed directory. To support the new platform and subplatform standardized instance-data, DataSource and its subclasses grew platform and subplatform attributes. The platform attribute defaults to the lowercase string datasource name at self.dsname. This method is overridden in NoCloud, Ec2 and ConfigDrive datasources. The subplatform attribute calls a _get_subplatform method which will return a string containing a simple slug for subplatform type such as metadata, seed-dir or config-drive followed by a detailed uri, device or directory path where the datasource consumed its configuration. As part of this work, DatasourceEC2 methods _get_data and _crawl_metadata have been refactored for a few reasons: - crawl_metadata is now a read-only operation, persisting no attributes on the datasource instance and returns a dictionary of consumed metadata. - crawl_metadata now closely represents the raw stucture of the ec2 metadata consumed, so that end-users can leverage public ec2 metadata documentation where possible. - crawl_metadata adds a '_metadata_api_version' key to the crawled ds.metadata to advertise what version of EC2's api was consumed by cloud-init. - _get_data now does all the processing of crawl_metadata and saves datasource instance attributes userdata_raw, metadata etc. Additional drive-bys: * unit test rework for test_altcloud and test_azure to simplify mocks and make use of existing util and test_helpers functions. --- cloudinit/sources/DataSourceAliYun.py | 20 +-- cloudinit/sources/DataSourceAltCloud.py | 33 ++-- cloudinit/sources/DataSourceAzure.py | 8 + cloudinit/sources/DataSourceBigstep.py | 4 + cloudinit/sources/DataSourceCloudSigma.py | 6 +- cloudinit/sources/DataSourceConfigDrive.py | 12 ++ cloudinit/sources/DataSourceEc2.py | 115 ++++++------- cloudinit/sources/DataSourceIBMCloud.py | 4 + cloudinit/sources/DataSourceMAAS.py | 4 + cloudinit/sources/DataSourceNoCloud.py | 21 +++ cloudinit/sources/DataSourceNone.py | 4 + cloudinit/sources/DataSourceOVF.py | 6 + cloudinit/sources/DataSourceOpenNebula.py | 8 + cloudinit/sources/DataSourceOracle.py | 4 + cloudinit/sources/DataSourceSmartOS.py | 3 + cloudinit/sources/__init__.py | 98 ++++++++--- cloudinit/sources/tests/test_init.py | 12 +- cloudinit/sources/tests/test_oracle.py | 8 + cloudinit/tests/test_util.py | 16 ++ cloudinit/util.py | 5 + doc/rtd/topics/instancedata.rst | 183 +++++++++++++++------ tests/cloud_tests/testcases/base.py | 13 +- tests/unittests/test_datasource/test_aliyun.py | 4 + tests/unittests/test_datasource/test_altcloud.py | 118 +++++++------ tests/unittests/test_datasource/test_azure.py | 110 +++++++------ tests/unittests/test_datasource/test_cloudsigma.py | 6 + .../unittests/test_datasource/test_configdrive.py | 3 + tests/unittests/test_datasource/test_ec2.py | 20 ++- tests/unittests/test_datasource/test_ibmcloud.py | 40 ++++- tests/unittests/test_datasource/test_nocloud.py | 45 +++-- tests/unittests/test_datasource/test_opennebula.py | 4 + tests/unittests/test_datasource/test_ovf.py | 52 +++++- tests/unittests/test_datasource/test_smartos.py | 7 + 33 files changed, 722 insertions(+), 274 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py index 858e0827..45cc9f00 100644 --- a/cloudinit/sources/DataSourceAliYun.py +++ b/cloudinit/sources/DataSourceAliYun.py @@ -1,7 +1,5 @@ # This file is part of cloud-init. See LICENSE file for license information. -import os - from cloudinit import sources from cloudinit.sources import DataSourceEc2 as EC2 from cloudinit import util @@ -18,25 +16,17 @@ class DataSourceAliYun(EC2.DataSourceEc2): min_metadata_version = '2016-01-01' extended_metadata_versions = [] - def __init__(self, sys_cfg, distro, paths): - super(DataSourceAliYun, self).__init__(sys_cfg, distro, paths) - self.seed_dir = os.path.join(paths.seed_dir, "AliYun") - def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): return self.metadata.get('hostname', 'localhost.localdomain') def get_public_ssh_keys(self): return parse_public_keys(self.metadata.get('public-keys', {})) - @property - def cloud_platform(self): - if self._cloud_platform is None: - if _is_aliyun(): - self._cloud_platform = EC2.Platforms.ALIYUN - else: - self._cloud_platform = EC2.Platforms.NO_EC2_METADATA - - return self._cloud_platform + def _get_cloud_name(self): + if _is_aliyun(): + return EC2.CloudNames.ALIYUN + else: + return EC2.CloudNames.NO_EC2_METADATA def _is_aliyun(): diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py index 8cd312d0..5270fda8 100644 --- a/cloudinit/sources/DataSourceAltCloud.py +++ b/cloudinit/sources/DataSourceAltCloud.py @@ -89,7 +89,9 @@ class DataSourceAltCloud(sources.DataSource): ''' Description: Get the type for the cloud back end this instance is running on - by examining the string returned by reading the dmi data. + by examining the string returned by reading either: + CLOUD_INFO_FILE or + the dmi data. Input: None @@ -99,7 +101,14 @@ class DataSourceAltCloud(sources.DataSource): 'RHEV', 'VSPHERE' or 'UNKNOWN' ''' - + if os.path.exists(CLOUD_INFO_FILE): + try: + cloud_type = util.load_file(CLOUD_INFO_FILE).strip().upper() + except IOError: + util.logexc(LOG, 'Unable to access cloud info file at %s.', + CLOUD_INFO_FILE) + return 'UNKNOWN' + return cloud_type system_name = util.read_dmi_data("system-product-name") if not system_name: return 'UNKNOWN' @@ -134,15 +143,7 @@ class DataSourceAltCloud(sources.DataSource): LOG.debug('Invoked get_data()') - if os.path.exists(CLOUD_INFO_FILE): - try: - cloud_type = util.load_file(CLOUD_INFO_FILE).strip().upper() - except IOError: - util.logexc(LOG, 'Unable to access cloud info file at %s.', - CLOUD_INFO_FILE) - return False - else: - cloud_type = self.get_cloud_type() + cloud_type = self.get_cloud_type() LOG.debug('cloud_type: %s', str(cloud_type)) @@ -161,6 +162,15 @@ class DataSourceAltCloud(sources.DataSource): util.logexc(LOG, 'Failed accessing user data.') return False + def _get_subplatform(self): + """Return the subplatform metadata details.""" + cloud_type = self.get_cloud_type() + if not hasattr(self, 'source'): + self.source = sources.METADATA_UNKNOWN + if cloud_type == 'RHEV': + self.source = '/dev/fd0' + return '%s (%s)' % (cloud_type.lower(), self.source) + def user_data_rhevm(self): ''' RHEVM specific userdata read @@ -232,6 +242,7 @@ class DataSourceAltCloud(sources.DataSource): try: return_str = util.mount_cb(cdrom_dev, read_user_data_callback) if return_str: + self.source = cdrom_dev break except OSError as err: if err.errno != errno.ENOENT: diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 783445e1..39391d01 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -351,6 +351,14 @@ class DataSourceAzure(sources.DataSource): metadata['public-keys'] = key_value or pubkeys_from_crt_files(fp_files) return metadata + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + if self.seed.startswith('/dev'): + subplatform_type = 'config-disk' + else: + subplatform_type = 'seed-dir' + return '%s (%s)' % (subplatform_type, self.seed) + def crawl_metadata(self): """Walk all instance metadata sources returning a dict on success. diff --git a/cloudinit/sources/DataSourceBigstep.py b/cloudinit/sources/DataSourceBigstep.py index 699a85b5..52fff20a 100644 --- a/cloudinit/sources/DataSourceBigstep.py +++ b/cloudinit/sources/DataSourceBigstep.py @@ -36,6 +36,10 @@ class DataSourceBigstep(sources.DataSource): self.userdata_raw = decoded["userdata_raw"] return True + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return 'metadata (%s)' % get_url_from_file() + def get_url_from_file(): try: diff --git a/cloudinit/sources/DataSourceCloudSigma.py b/cloudinit/sources/DataSourceCloudSigma.py index c816f349..2955d3f0 100644 --- a/cloudinit/sources/DataSourceCloudSigma.py +++ b/cloudinit/sources/DataSourceCloudSigma.py @@ -7,7 +7,7 @@ from base64 import b64decode import re -from cloudinit.cs_utils import Cepko +from cloudinit.cs_utils import Cepko, SERIAL_PORT from cloudinit import log as logging from cloudinit import sources @@ -84,6 +84,10 @@ class DataSourceCloudSigma(sources.DataSource): return True + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return 'cepko (%s)' % SERIAL_PORT + def get_hostname(self, fqdn=False, resolve_ip=False, metadata_only=False): """ Cleans up and uses the server's name if the latter is set. Otherwise diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index 664dc4b7..564e3eb3 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -160,6 +160,18 @@ class DataSourceConfigDrive(openstack.SourceMixin, sources.DataSource): LOG.debug("no network configuration available") return self._network_config + @property + def platform(self): + return 'openstack' + + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + if self.seed_dir in self.source: + subplatform_type = 'seed-dir' + elif self.source.startswith('/dev'): + subplatform_type = 'config-disk' + return '%s (%s)' % (subplatform_type, self.source) + def read_config_drive(source_dir): reader = openstack.ConfigDriveReader(source_dir) diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 968ab3f7..9ccf2cdc 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -28,18 +28,16 @@ STRICT_ID_PATH = ("datasource", "Ec2", "strict_id") STRICT_ID_DEFAULT = "warn" -class Platforms(object): - # TODO Rename and move to cloudinit.cloud.CloudNames - ALIYUN = "AliYun" - AWS = "AWS" - BRIGHTBOX = "Brightbox" - SEEDED = "Seeded" +class CloudNames(object): + ALIYUN = "aliyun" + AWS = "aws" + BRIGHTBOX = "brightbox" # UNKNOWN indicates no positive id. If strict_id is 'warn' or 'false', # then an attempt at the Ec2 Metadata service will be made. - UNKNOWN = "Unknown" + UNKNOWN = "unknown" # NO_EC2_METADATA indicates this platform does not have a Ec2 metadata # service available. No attempt at the Ec2 Metadata service will be made. - NO_EC2_METADATA = "No-EC2-Metadata" + NO_EC2_METADATA = "no-ec2-metadata" class DataSourceEc2(sources.DataSource): @@ -61,8 +59,6 @@ class DataSourceEc2(sources.DataSource): url_max_wait = 120 url_timeout = 50 - _cloud_platform = None - _network_config = sources.UNSET # Used to cache calculated network cfg v1 # Whether we want to get network configuration from the metadata service. @@ -71,30 +67,21 @@ class DataSourceEc2(sources.DataSource): def __init__(self, sys_cfg, distro, paths): super(DataSourceEc2, self).__init__(sys_cfg, distro, paths) self.metadata_address = None - self.seed_dir = os.path.join(paths.seed_dir, "ec2") def _get_cloud_name(self): """Return the cloud name as identified during _get_data.""" - return self.cloud_platform + return identify_platform() def _get_data(self): - seed_ret = {} - if util.read_optional_seed(seed_ret, base=(self.seed_dir + "/")): - self.userdata_raw = seed_ret['user-data'] - self.metadata = seed_ret['meta-data'] - LOG.debug("Using seeded ec2 data from %s", self.seed_dir) - self._cloud_platform = Platforms.SEEDED - return True - strict_mode, _sleep = read_strict_mode( util.get_cfg_by_path(self.sys_cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), ("warn", None)) - LOG.debug("strict_mode: %s, cloud_platform=%s", - strict_mode, self.cloud_platform) - if strict_mode == "true" and self.cloud_platform == Platforms.UNKNOWN: + LOG.debug("strict_mode: %s, cloud_name=%s cloud_platform=%s", + strict_mode, self.cloud_name, self.platform) + if strict_mode == "true" and self.cloud_name == CloudNames.UNKNOWN: return False - elif self.cloud_platform == Platforms.NO_EC2_METADATA: + elif self.cloud_name == CloudNames.NO_EC2_METADATA: return False if self.perform_dhcp_setup: # Setup networking in init-local stage. @@ -103,13 +90,22 @@ class DataSourceEc2(sources.DataSource): return False try: with EphemeralDHCPv4(self.fallback_interface): - return util.log_time( + self._crawled_metadata = util.log_time( logfunc=LOG.debug, msg='Crawl of metadata service', - func=self._crawl_metadata) + func=self.crawl_metadata) except NoDHCPLeaseError: return False else: - return self._crawl_metadata() + self._crawled_metadata = util.log_time( + logfunc=LOG.debug, msg='Crawl of metadata service', + func=self.crawl_metadata) + if not self._crawled_metadata: + return False + self.metadata = self._crawled_metadata.get('meta-data', None) + self.userdata_raw = self._crawled_metadata.get('user-data', None) + self.identity = self._crawled_metadata.get( + 'dynamic', {}).get('instance-identity', {}).get('document', {}) + return True @property def launch_index(self): @@ -117,6 +113,15 @@ class DataSourceEc2(sources.DataSource): return None return self.metadata.get('ami-launch-index') + @property + def platform(self): + # Handle upgrade path of pickled ds + if not hasattr(self, '_platform_type'): + self._platform_type = DataSourceEc2.dsname.lower() + if not self._platform_type: + self._platform_type = DataSourceEc2.dsname.lower() + return self._platform_type + def get_metadata_api_version(self): """Get the best supported api version from the metadata service. @@ -144,7 +149,7 @@ class DataSourceEc2(sources.DataSource): return self.min_metadata_version def get_instance_id(self): - if self.cloud_platform == Platforms.AWS: + if self.cloud_name == CloudNames.AWS: # Prefer the ID from the instance identity document, but fall back if not getattr(self, 'identity', None): # If re-using cached datasource, it's get_data run didn't @@ -254,7 +259,7 @@ class DataSourceEc2(sources.DataSource): @property def availability_zone(self): try: - if self.cloud_platform == Platforms.AWS: + if self.cloud_name == CloudNames.AWS: return self.identity.get( 'availabilityZone', self.metadata['placement']['availability-zone']) @@ -265,7 +270,7 @@ class DataSourceEc2(sources.DataSource): @property def region(self): - if self.cloud_platform == Platforms.AWS: + if self.cloud_name == CloudNames.AWS: region = self.identity.get('region') # Fallback to trimming the availability zone if region is missing if self.availability_zone and not region: @@ -277,16 +282,10 @@ class DataSourceEc2(sources.DataSource): return az[:-1] return None - @property - def cloud_platform(self): # TODO rename cloud_name - if self._cloud_platform is None: - self._cloud_platform = identify_platform() - return self._cloud_platform - def activate(self, cfg, is_new_instance): if not is_new_instance: return - if self.cloud_platform == Platforms.UNKNOWN: + if self.cloud_name == CloudNames.UNKNOWN: warn_if_necessary( util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), cfg) @@ -306,13 +305,13 @@ class DataSourceEc2(sources.DataSource): result = None no_network_metadata_on_aws = bool( 'network' not in self.metadata and - self.cloud_platform == Platforms.AWS) + self.cloud_name == CloudNames.AWS) if no_network_metadata_on_aws: LOG.debug("Metadata 'network' not present:" " Refreshing stale metadata from prior to upgrade.") util.log_time( logfunc=LOG.debug, msg='Re-crawl of metadata service', - func=self._crawl_metadata) + func=self.get_data) # Limit network configuration to only the primary/fallback nic iface = self.fallback_interface @@ -340,28 +339,32 @@ class DataSourceEc2(sources.DataSource): return super(DataSourceEc2, self).fallback_interface return self._fallback_interface - def _crawl_metadata(self): + def crawl_metadata(self): """Crawl metadata service when available. - @returns: True on success, False otherwise. + @returns: Dictionary of crawled metadata content containing the keys: + meta-data, user-data and dynamic. """ if not self.wait_for_metadata_service(): - return False + return {} api_version = self.get_metadata_api_version() + crawled_metadata = {} try: - self.userdata_raw = ec2.get_instance_userdata( + crawled_metadata['user-data'] = ec2.get_instance_userdata( api_version, self.metadata_address) - self.metadata = ec2.get_instance_metadata( + crawled_metadata['meta-data'] = ec2.get_instance_metadata( api_version, self.metadata_address) - if self.cloud_platform == Platforms.AWS: - self.identity = ec2.get_instance_identity( - api_version, self.metadata_address).get('document', {}) + if self.cloud_name == CloudNames.AWS: + identity = ec2.get_instance_identity( + api_version, self.metadata_address) + crawled_metadata['dynamic'] = {'instance-identity': identity} except Exception: util.logexc( LOG, "Failed reading from metadata address %s", self.metadata_address) - return False - return True + return {} + crawled_metadata['_metadata_api_version'] = api_version + return crawled_metadata class DataSourceEc2Local(DataSourceEc2): @@ -375,10 +378,10 @@ class DataSourceEc2Local(DataSourceEc2): perform_dhcp_setup = True # Use dhcp before querying metadata def get_data(self): - supported_platforms = (Platforms.AWS,) - if self.cloud_platform not in supported_platforms: + supported_platforms = (CloudNames.AWS,) + if self.cloud_name not in supported_platforms: LOG.debug("Local Ec2 mode only supported on %s, not %s", - supported_platforms, self.cloud_platform) + supported_platforms, self.cloud_name) return False return super(DataSourceEc2Local, self).get_data() @@ -439,20 +442,20 @@ def identify_aws(data): if (data['uuid'].startswith('ec2') and (data['uuid_source'] == 'hypervisor' or data['uuid'] == data['serial'])): - return Platforms.AWS + return CloudNames.AWS return None def identify_brightbox(data): if data['serial'].endswith('brightbox.com'): - return Platforms.BRIGHTBOX + return CloudNames.BRIGHTBOX def identify_platform(): - # identify the platform and return an entry in Platforms. + # identify the platform and return an entry in CloudNames. data = _collect_platform_data() - checks = (identify_aws, identify_brightbox, lambda x: Platforms.UNKNOWN) + checks = (identify_aws, identify_brightbox, lambda x: CloudNames.UNKNOWN) for checker in checks: try: result = checker(data) diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index a5358148..21e6ae6b 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -157,6 +157,10 @@ class DataSourceIBMCloud(sources.DataSource): return True + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return '%s (%s)' % (self.platform, self.source) + def check_instance_id(self, sys_cfg): """quickly (local check only) if self.instance_id is still valid diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py index bcb38544..61aa6d7e 100644 --- a/cloudinit/sources/DataSourceMAAS.py +++ b/cloudinit/sources/DataSourceMAAS.py @@ -109,6 +109,10 @@ class DataSourceMAAS(sources.DataSource): LOG.warning("Invalid content in vendor-data: %s", e) self.vendordata_raw = None + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return 'seed-dir (%s)' % self.base_url + def wait_for_metadata_service(self, url): mcfg = self.ds_cfg max_wait = 120 diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py index 2daea59d..9010f06c 100644 --- a/cloudinit/sources/DataSourceNoCloud.py +++ b/cloudinit/sources/DataSourceNoCloud.py @@ -186,6 +186,27 @@ class DataSourceNoCloud(sources.DataSource): self._network_eni = mydata['meta-data'].get('network-interfaces') return True + @property + def platform_type(self): + # Handle upgrade path of pickled ds + if not hasattr(self, '_platform_type'): + self._platform_type = None + if not self._platform_type: + self._platform_type = 'lxd' if util.is_lxd() else 'nocloud' + return self._platform_type + + def _get_cloud_name(self): + """Return unknown when 'cloud-name' key is absent from metadata.""" + return sources.METADATA_UNKNOWN + + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + if self.seed.startswith('/dev'): + subplatform_type = 'config-disk' + else: + subplatform_type = 'seed-dir' + return '%s (%s)' % (subplatform_type, self.seed) + def check_instance_id(self, sys_cfg): # quickly (local check only) if self.instance_id is still valid # we check kernel command line or files. diff --git a/cloudinit/sources/DataSourceNone.py b/cloudinit/sources/DataSourceNone.py index e63a7e39..e6250801 100644 --- a/cloudinit/sources/DataSourceNone.py +++ b/cloudinit/sources/DataSourceNone.py @@ -28,6 +28,10 @@ class DataSourceNone(sources.DataSource): self.metadata = self.ds_cfg['metadata'] return True + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return 'config' + def get_instance_id(self): return 'iid-datasource-none' diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index 178ccb0f..045291e7 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -275,6 +275,12 @@ class DataSourceOVF(sources.DataSource): self.cfg = cfg return True + def _get_subplatform(self): + system_type = util.read_dmi_data("system-product-name").lower() + if system_type == 'vmware': + return 'vmware (%s)' % self.seed + return 'ovf (%s)' % self.seed + def get_public_ssh_keys(self): if 'public-keys' not in self.metadata: return [] diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 77ccd128..e62e9729 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -95,6 +95,14 @@ class DataSourceOpenNebula(sources.DataSource): self.userdata_raw = results.get('userdata') return True + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + if self.seed_dir in self.seed: + subplatform_type = 'seed-dir' + else: + subplatform_type = 'config-disk' + return '%s (%s)' % (subplatform_type, self.seed) + @property def network_config(self): if self.network is not None: diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index fab39af3..70b9c58a 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -91,6 +91,10 @@ class DataSourceOracle(sources.DataSource): def crawl_metadata(self): return read_metadata() + def _get_subplatform(self): + """Return the subplatform metadata source details.""" + return 'metadata (%s)' % METADATA_ENDPOINT + def check_instance_id(self, sys_cfg): """quickly check (local only) if self.instance_id is still valid diff --git a/cloudinit/sources/DataSourceSmartOS.py b/cloudinit/sources/DataSourceSmartOS.py index 593ac91a..32b57cdd 100644 --- a/cloudinit/sources/DataSourceSmartOS.py +++ b/cloudinit/sources/DataSourceSmartOS.py @@ -303,6 +303,9 @@ class DataSourceSmartOS(sources.DataSource): self._set_provisioned() return True + def _get_subplatform(self): + return 'serial (%s)' % SERIAL_DEVICE + def device_name_to_device(self, name): return self.ds_cfg['disk_aliases'].get(name) diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 5ac98826..9b90680f 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -54,6 +54,7 @@ REDACT_SENSITIVE_VALUE = 'redacted for non-root user' METADATA_CLOUD_NAME_KEY = 'cloud-name' UNSET = "_unset" +METADATA_UNKNOWN = 'unknown' LOG = logging.getLogger(__name__) @@ -133,6 +134,14 @@ class DataSource(object): # Cached cloud_name as determined by _get_cloud_name _cloud_name = None + # Cached cloud platform api type: e.g. ec2, openstack, kvm, lxd, azure etc. + _platform_type = None + + # More details about the cloud platform: + # - metadata (http://169.254.169.254/) + # - seed-dir () + _subplatform = None + # Track the discovered fallback nic for use in configuration generation. _fallback_interface = None @@ -192,21 +201,24 @@ class DataSource(object): local_hostname = self.get_hostname() instance_id = self.get_instance_id() availability_zone = self.availability_zone - cloud_name = self.cloud_name - # When adding new standard keys prefer underscore-delimited instead - # of hyphen-delimted to support simple variable references in jinja - # templates. + # In the event of upgrade from existing cloudinit, pickled datasource + # will not contain these new class attributes. So we need to recrawl + # metadata to discover that content. return { 'v1': { + '_beta_keys': ['subplatform'], 'availability-zone': availability_zone, 'availability_zone': availability_zone, - 'cloud-name': cloud_name, - 'cloud_name': cloud_name, + 'cloud-name': self.cloud_name, + 'cloud_name': self.cloud_name, + 'platform': self.platform_type, + 'public_ssh_keys': self.get_public_ssh_keys(), 'instance-id': instance_id, 'instance_id': instance_id, 'local-hostname': local_hostname, 'local_hostname': local_hostname, - 'region': self.region}} + 'region': self.region, + 'subplatform': self.subplatform}} def clear_cached_attrs(self, attr_defaults=()): """Reset any cached metadata attributes to datasource defaults. @@ -247,19 +259,27 @@ class DataSource(object): @return True on successful write, False otherwise. """ - instance_data = { - 'ds': {'_doc': EXPERIMENTAL_TEXT, - 'meta_data': self.metadata}} - if hasattr(self, 'network_json'): - network_json = getattr(self, 'network_json') - if network_json != UNSET: - instance_data['ds']['network_json'] = network_json - if hasattr(self, 'ec2_metadata'): - ec2_metadata = getattr(self, 'ec2_metadata') - if ec2_metadata != UNSET: - instance_data['ds']['ec2_metadata'] = ec2_metadata + if hasattr(self, '_crawled_metadata'): + # Any datasource with _crawled_metadata will best represent + # most recent, 'raw' metadata + crawled_metadata = copy.deepcopy( + getattr(self, '_crawled_metadata')) + crawled_metadata.pop('user-data', None) + crawled_metadata.pop('vendor-data', None) + instance_data = {'ds': crawled_metadata} + else: + instance_data = {'ds': {'meta_data': self.metadata}} + if hasattr(self, 'network_json'): + network_json = getattr(self, 'network_json') + if network_json != UNSET: + instance_data['ds']['network_json'] = network_json + if hasattr(self, 'ec2_metadata'): + ec2_metadata = getattr(self, 'ec2_metadata') + if ec2_metadata != UNSET: + instance_data['ds']['ec2_metadata'] = ec2_metadata instance_data.update( self._get_standardized_metadata()) + instance_data['ds']['_doc'] = EXPERIMENTAL_TEXT try: # Process content base64encoding unserializable values content = util.json_dumps(instance_data) @@ -346,6 +366,40 @@ class DataSource(object): self.cloud_name) return self._fallback_interface + @property + def platform_type(self): + if not hasattr(self, '_platform_type'): + # Handle upgrade path where pickled datasource has no _platform. + self._platform_type = self.dsname.lower() + if not self._platform_type: + self._platform_type = self.dsname.lower() + return self._platform_type + + @property + def subplatform(self): + """Return a string representing subplatform details for the datasource. + + This should be guidance for where the metadata is sourced. + Examples of this on different clouds: + ec2: metadata (http://169.254.169.254) + openstack: configdrive (/dev/path) + openstack: metadata (http://169.254.169.254) + nocloud: seed-dir (/seed/dir/path) + lxd: nocloud (/seed/dir/path) + """ + if not hasattr(self, '_subplatform'): + # Handle upgrade path where pickled datasource has no _platform. + self._subplatform = self._get_subplatform() + if not self._subplatform: + self._subplatform = self._get_subplatform() + return self._subplatform + + def _get_subplatform(self): + """Subclasses should implement to return a "slug (detail)" string.""" + if hasattr(self, 'metadata_address'): + return 'metadata (%s)' % getattr(self, 'metadata_address') + return METADATA_UNKNOWN + @property def cloud_name(self): """Return lowercase cloud name as determined by the datasource. @@ -359,9 +413,11 @@ class DataSource(object): cloud_name = self.metadata.get(METADATA_CLOUD_NAME_KEY) if isinstance(cloud_name, six.string_types): self._cloud_name = cloud_name.lower() - LOG.debug( - 'Ignoring metadata provided key %s: non-string type %s', - METADATA_CLOUD_NAME_KEY, type(cloud_name)) + else: + self._cloud_name = self._get_cloud_name().lower() + LOG.debug( + 'Ignoring metadata provided key %s: non-string type %s', + METADATA_CLOUD_NAME_KEY, type(cloud_name)) else: self._cloud_name = self._get_cloud_name().lower() return self._cloud_name diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 8082019e..391b3436 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -295,6 +295,7 @@ class TestDataSource(CiTestCase): 'base64_encoded_keys': [], 'sensitive_keys': [], 'v1': { + '_beta_keys': ['subplatform'], 'availability-zone': 'myaz', 'availability_zone': 'myaz', 'cloud-name': 'subclasscloudname', @@ -303,7 +304,10 @@ class TestDataSource(CiTestCase): 'instance_id': 'iid-datasource', 'local-hostname': 'test-subclass-hostname', 'local_hostname': 'test-subclass-hostname', - 'region': 'myregion'}, + 'platform': 'mytestsubclass', + 'public_ssh_keys': [], + 'region': 'myregion', + 'subplatform': 'unknown'}, 'ds': { '_doc': EXPERIMENTAL_TEXT, 'meta_data': {'availability_zone': 'myaz', @@ -339,6 +343,7 @@ class TestDataSource(CiTestCase): 'base64_encoded_keys': [], 'sensitive_keys': ['ds/meta_data/some/security-credentials'], 'v1': { + '_beta_keys': ['subplatform'], 'availability-zone': 'myaz', 'availability_zone': 'myaz', 'cloud-name': 'subclasscloudname', @@ -347,7 +352,10 @@ class TestDataSource(CiTestCase): 'instance_id': 'iid-datasource', 'local-hostname': 'test-subclass-hostname', 'local_hostname': 'test-subclass-hostname', - 'region': 'myregion'}, + 'platform': 'mytestsubclass', + 'public_ssh_keys': [], + 'region': 'myregion', + 'subplatform': 'unknown'}, 'ds': { '_doc': EXPERIMENTAL_TEXT, 'meta_data': { diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 7599126c..97d62947 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -71,6 +71,14 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertFalse(ds._get_data()) mocks._is_platform_viable.assert_called_once_with() + def test_platform_info(self): + """Return platform-related information for Oracle Datasource.""" + ds, _mocks = self._get_ds() + self.assertEqual('oracle', ds.cloud_name) + self.assertEqual('oracle', ds.platform_type) + self.assertEqual( + 'metadata (http://169.254.169.254/openstack/)', ds.subplatform) + @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) def test_without_userdata(self, m_is_iscsi_root): """If no user-data is provided, it should not be in return dict.""" diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index edb0c18f..749a3846 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -478,4 +478,20 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('foo', '1.1', 'aarch64'), dist) + +@mock.patch('os.path.exists') +class TestIsLXD(CiTestCase): + + def test_is_lxd_true_on_sock_device(self, m_exists): + """When lxd's /dev/lxd/sock exists, is_lxd returns true.""" + m_exists.return_value = True + self.assertTrue(util.is_lxd()) + m_exists.assert_called_once_with('/dev/lxd/sock') + + def test_is_lxd_false_when_sock_device_absent(self, m_exists): + """When lxd's /dev/lxd/sock is absent, is_lxd returns false.""" + m_exists.return_value = False + self.assertFalse(util.is_lxd()) + m_exists.assert_called_once_with('/dev/lxd/sock') + # vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 50680960..c67d6be6 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2171,6 +2171,11 @@ def is_container(): return False +def is_lxd(): + """Check to see if we are running in a lxd container.""" + return os.path.exists('/dev/lxd/sock') + + def get_proc_env(pid, encoding='utf-8', errors='replace'): """ Return the environment in a dict that a given process id was started with. diff --git a/doc/rtd/topics/instancedata.rst b/doc/rtd/topics/instancedata.rst index 634e1807..5d2dc948 100644 --- a/doc/rtd/topics/instancedata.rst +++ b/doc/rtd/topics/instancedata.rst @@ -90,24 +90,46 @@ There are three basic top-level keys: The standardized keys present: -+----------------------+-----------------------------------------------+---------------------------+ -| Key path | Description | Examples | -+======================+===============================================+===========================+ -| v1.cloud_name | The name of the cloud provided by metadata | aws, openstack, azure, | -| | key 'cloud-name' or the cloud-init datasource | configdrive, nocloud, | -| | name which was discovered. | ovf, etc. | -+----------------------+-----------------------------------------------+---------------------------+ -| v1.instance_id | Unique instance_id allocated by the cloud | i- | -+----------------------+-----------------------------------------------+---------------------------+ -| v1.local_hostname | The internal or local hostname of the system | ip-10-41-41-70, | -| | | | -+----------------------+-----------------------------------------------+---------------------------+ -| v1.region | The physical region/datacenter in which the | us-east-2 | -| | instance is deployed | | -+----------------------+-----------------------------------------------+---------------------------+ -| v1.availability_zone | The physical availability zone in which the | us-east-2b, nova, null | -| | instance is deployed | | -+----------------------+-----------------------------------------------+---------------------------+ ++----------------------+-----------------------------------------------+-----------------------------------+ +| Key path | Description | Examples | ++======================+===============================================+===================================+ +| v1._beta_keys | List of standardized keys still in 'beta'. | [subplatform] | +| | The format, intent or presence of these keys | | +| | can change. Do not consider them | | +| | production-ready. | | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.cloud_name | Where possible this will indicate the 'name' | aws, openstack, azure, | +| | of the cloud this system is running on. This | configdrive, nocloud, | +| | is specifically different than the 'platform' | ovf, etc. | +| | below. As an example, the name of Amazon Web | | +| | Services is 'aws' while the platform is 'ec2'.| | +| | | | +| | If no specific name is determinable or | | +| | provided in meta-data, then this field may | | +| | contain the same content as 'platform'. | | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.instance_id | Unique instance_id allocated by the cloud | i- | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.local_hostname | The internal or local hostname of the system | ip-10-41-41-70, | +| | | | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.platform | An attempt to identify the cloud platform | ec2, openstack, lxd, gce | +| | instance that the system is running on. | nocloud, ovf | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.subplatform | Additional platform details describing the | metadata (http://168.254.169.254),| +| | specific source or type of metadata used. | seed-dir (/path/to/seed-dir/), | +| | The format of subplatform will be: | config-disk (/dev/cd0), | +| | () | configdrive (/dev/sr0) | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.public_ssh_keys | A list of ssh keys provided to the instance | ['ssh-rsa AA...', ...] | +| | by the datasource metadata. | | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.region | The physical region/datacenter in which the | us-east-2 | +| | instance is deployed | | ++----------------------+-----------------------------------------------+-----------------------------------+ +| v1.availability_zone | The physical availability zone in which the | us-east-2b, nova, null | +| | instance is deployed | | ++----------------------+-----------------------------------------------+-----------------------------------+ Below is an example of ``/run/cloud-init/instance_data.json`` on an EC2 @@ -117,10 +139,75 @@ instance: { "base64_encoded_keys": [], - "sensitive_keys": [], "ds": { - "meta_data": { - "ami-id": "ami-014e1416b628b0cbf", + "_doc": "EXPERIMENTAL: The structure and format of content scoped under the 'ds' key may change in subsequent releases of cloud-init.", + "_metadata_api_version": "2016-09-02", + "dynamic": { + "instance-identity": { + "document": { + "accountId": "437526006925", + "architecture": "x86_64", + "availabilityZone": "us-east-2b", + "billingProducts": null, + "devpayProductCodes": null, + "imageId": "ami-079638aae7046bdd2", + "instanceId": "i-075f088c72ad3271c", + "instanceType": "t2.micro", + "kernelId": null, + "marketplaceProductCodes": null, + "pendingTime": "2018-10-05T20:10:43Z", + "privateIp": "10.41.41.95", + "ramdiskId": null, + "region": "us-east-2", + "version": "2017-09-30" + }, + "pkcs7": [ + "MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAaCAJIAEggHbewog", + "ICJkZXZwYXlQcm9kdWN0Q29kZXMiIDogbnVsbCwKICAibWFya2V0cGxhY2VQcm9kdWN0Q29kZXMi", + "IDogbnVsbCwKICAicHJpdmF0ZUlwIiA6ICIxMC40MS40MS45NSIsCiAgInZlcnNpb24iIDogIjIw", + "MTctMDktMzAiLAogICJpbnN0YW5jZUlkIiA6ICJpLTA3NWYwODhjNzJhZDMyNzFjIiwKICAiYmls", + "bGluZ1Byb2R1Y3RzIiA6IG51bGwsCiAgImluc3RhbmNlVHlwZSIgOiAidDIubWljcm8iLAogICJh", + "Y2NvdW50SWQiIDogIjQzNzUyNjAwNjkyNSIsCiAgImF2YWlsYWJpbGl0eVpvbmUiIDogInVzLWVh", + "c3QtMmIiLAogICJrZXJuZWxJZCIgOiBudWxsLAogICJyYW1kaXNrSWQiIDogbnVsbCwKICAiYXJj", + "aGl0ZWN0dXJlIiA6ICJ4ODZfNjQiLAogICJpbWFnZUlkIiA6ICJhbWktMDc5NjM4YWFlNzA0NmJk", + "ZDIiLAogICJwZW5kaW5nVGltZSIgOiAiMjAxOC0xMC0wNVQyMDoxMDo0M1oiLAogICJyZWdpb24i", + "IDogInVzLWVhc3QtMiIKfQAAAAAAADGCARcwggETAgEBMGkwXDELMAkGA1UEBhMCVVMxGTAXBgNV", + "BAgTEFdhc2hpbmd0b24gU3RhdGUxEDAOBgNVBAcTB1NlYXR0bGUxIDAeBgNVBAoTF0FtYXpvbiBX", + "ZWIgU2VydmljZXMgTExDAgkAlrpI2eVeGmcwCQYFKw4DAhoFAKBdMBgGCSqGSIb3DQEJAzELBgkq", + "hkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTE4MTAwNTIwMTA0OFowIwYJKoZIhvcNAQkEMRYEFK0k", + "Tz6n1A8/zU1AzFj0riNQORw2MAkGByqGSM44BAMELjAsAhRNrr174y98grPBVXUforN/6wZp8AIU", + "JLZBkrB2GJA8A4WJ1okq++jSrBIAAAAAAAA=" + ], + "rsa2048": [ + "MIAGCSqGSIb3DQEHAqCAMIACAQExDzANBglghkgBZQMEAgEFADCABgkqhkiG9w0BBwGggCSABIIB", + "23sKICAiZGV2cGF5UHJvZHVjdENvZGVzIiA6IG51bGwsCiAgIm1hcmtldHBsYWNlUHJvZHVjdENv", + "ZGVzIiA6IG51bGwsCiAgInByaXZhdGVJcCIgOiAiMTAuNDEuNDEuOTUiLAogICJ2ZXJzaW9uIiA6", + "ICIyMDE3LTA5LTMwIiwKICAiaW5zdGFuY2VJZCIgOiAiaS0wNzVmMDg4YzcyYWQzMjcxYyIsCiAg", + "ImJpbGxpbmdQcm9kdWN0cyIgOiBudWxsLAogICJpbnN0YW5jZVR5cGUiIDogInQyLm1pY3JvIiwK", + "ICAiYWNjb3VudElkIiA6ICI0Mzc1MjYwMDY5MjUiLAogICJhdmFpbGFiaWxpdHlab25lIiA6ICJ1", + "cy1lYXN0LTJiIiwKICAia2VybmVsSWQiIDogbnVsbCwKICAicmFtZGlza0lkIiA6IG51bGwsCiAg", + "ImFyY2hpdGVjdHVyZSIgOiAieDg2XzY0IiwKICAiaW1hZ2VJZCIgOiAiYW1pLTA3OTYzOGFhZTcw", + "NDZiZGQyIiwKICAicGVuZGluZ1RpbWUiIDogIjIwMTgtMTAtMDVUMjA6MTA6NDNaIiwKICAicmVn", + "aW9uIiA6ICJ1cy1lYXN0LTIiCn0AAAAAAAAxggH/MIIB+wIBATBpMFwxCzAJBgNVBAYTAlVTMRkw", + "FwYDVQQIExBXYXNoaW5ndG9uIFN0YXRlMRAwDgYDVQQHEwdTZWF0dGxlMSAwHgYDVQQKExdBbWF6", + "b24gV2ViIFNlcnZpY2VzIExMQwIJAM07oeX4xevdMA0GCWCGSAFlAwQCAQUAoGkwGAYJKoZIhvcN", + "AQkDMQsGCSqGSIb3DQEHATAcBgkqhkiG9w0BCQUxDxcNMTgxMDA1MjAxMDQ4WjAvBgkqhkiG9w0B", + "CQQxIgQgkYz0pZk3zJKBi4KP4egeOKJl/UYwu5UdE7id74pmPwMwDQYJKoZIhvcNAQEBBQAEggEA", + "dC3uIGGNul1OC1mJKSH3XoBWsYH20J/xhIdftYBoXHGf2BSFsrs9ZscXd2rKAKea4pSPOZEYMXgz", + "lPuT7W0WU89N3ZKviy/ReMSRjmI/jJmsY1lea6mlgcsJXreBXFMYucZvyeWGHdnCjamoKWXkmZlM", + "mSB1gshWy8Y7DzoKviYPQZi5aI54XK2Upt4kGme1tH1NI2Cq+hM4K+adxTbNhS3uzvWaWzMklUuU", + "QHX2GMmjAVRVc8vnA8IAsBCJJp+gFgYzi09IK+cwNgCFFPADoG6jbMHHf4sLB3MUGpiA+G9JlCnM", + "fmkjI2pNRB8spc0k4UG4egqLrqCz67WuK38tjwAAAAAAAA==" + ], + "signature": [ + "Tsw6h+V3WnxrNVSXBYIOs1V4j95YR1mLPPH45XnhX0/Ei3waJqf7/7EEKGYP1Cr4PTYEULtZ7Mvf", + "+xJpM50Ivs2bdF7o0c4vnplRWe3f06NI9pv50dr110j/wNzP4MZ1pLhJCqubQOaaBTF3LFutgRrt", + "r4B0mN3p7EcqD8G+ll0=" + ] + } + }, + "meta-data": { + "ami-id": "ami-079638aae7046bdd2", "ami-launch-index": "0", "ami-manifest-path": "(unknown)", "block-device-mapping": { @@ -129,31 +216,31 @@ instance: "ephemeral1": "sdc", "root": "/dev/sda1" }, - "hostname": "ip-10-41-41-70.us-east-2.compute.internal", + "hostname": "ip-10-41-41-95.us-east-2.compute.internal", "instance-action": "none", - "instance-id": "i-04fa31cfc55aa7976", + "instance-id": "i-075f088c72ad3271c", "instance-type": "t2.micro", - "local-hostname": "ip-10-41-41-70.us-east-2.compute.internal", - "local-ipv4": "10.41.41.70", - "mac": "06:b6:92:dd:9d:24", + "local-hostname": "ip-10-41-41-95.us-east-2.compute.internal", + "local-ipv4": "10.41.41.95", + "mac": "06:74:8f:39:cd:a6", "metrics": { "vhostmd": "" }, "network": { "interfaces": { "macs": { - "06:b6:92:dd:9d:24": { + "06:74:8f:39:cd:a6": { "device-number": "0", - "interface-id": "eni-08c0c9fdb99b6e6f4", + "interface-id": "eni-052058bbd7831eaae", "ipv4-associations": { - "18.224.22.43": "10.41.41.70" + "18.218.221.122": "10.41.41.95" }, - "local-hostname": "ip-10-41-41-70.us-east-2.compute.internal", - "local-ipv4s": "10.41.41.70", - "mac": "06:b6:92:dd:9d:24", + "local-hostname": "ip-10-41-41-95.us-east-2.compute.internal", + "local-ipv4s": "10.41.41.95", + "mac": "06:74:8f:39:cd:a6", "owner-id": "437526006925", - "public-hostname": "ec2-18-224-22-43.us-east-2.compute.amazonaws.com", - "public-ipv4s": "18.224.22.43", + "public-hostname": "ec2-18-218-221-122.us-east-2.compute.amazonaws.com", + "public-ipv4s": "18.218.221.122", "security-group-ids": "sg-828247e9", "security-groups": "Cloud-init integration test secgroup", "subnet-id": "subnet-282f3053", @@ -171,16 +258,14 @@ instance: "availability-zone": "us-east-2b" }, "profile": "default-hvm", - "public-hostname": "ec2-18-224-22-43.us-east-2.compute.amazonaws.com", - "public-ipv4": "18.224.22.43", + "public-hostname": "ec2-18-218-221-122.us-east-2.compute.amazonaws.com", + "public-ipv4": "18.218.221.122", "public-keys": { "cloud-init-integration": [ - "ssh-rsa - AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB - cloud-init-integration" + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB cloud-init-integration" ] }, - "reservation-id": "r-06ab75e9346f54333", + "reservation-id": "r-0594a20e31f6cfe46", "security-groups": "Cloud-init integration test secgroup", "services": { "domain": "amazonaws.com", @@ -188,16 +273,22 @@ instance: } } }, + "sensitive_keys": [], "v1": { + "_beta_keys": [ + "subplatform" + ], "availability-zone": "us-east-2b", "availability_zone": "us-east-2b", - "cloud-name": "aws", "cloud_name": "aws", - "instance-id": "i-04fa31cfc55aa7976", - "instance_id": "i-04fa31cfc55aa7976", - "local-hostname": "ip-10-41-41-70", - "local_hostname": "ip-10-41-41-70", - "region": "us-east-2" + "instance_id": "i-075f088c72ad3271c", + "local_hostname": "ip-10-41-41-95", + "platform": "ec2", + "public_ssh_keys": [ + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB cloud-init-integration" + ], + "region": "us-east-2", + "subplatform": "metadata (http://169.254.169.254)" } } diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index e18d601c..16b268ef 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -195,6 +195,9 @@ class CloudTestCase(unittest2.TestCase): self.assertIsNotNone( v1_data['availability_zone'], 'expected ec2 availability_zone') self.assertEqual('aws', v1_data['cloud_name']) + self.assertEqual('ec2', v1_data['platform']) + self.assertEqual( + 'metadata (http://169.254.169.254)', v1_data['subplatform']) self.assertIn('i-', v1_data['instance_id']) self.assertIn('ip-', v1_data['local_hostname']) self.assertIsNotNone(v1_data['region'], 'expected ec2 region') @@ -220,7 +223,11 @@ class CloudTestCase(unittest2.TestCase): instance_data = json.loads(out) v1_data = instance_data.get('v1', {}) self.assertItemsEqual([], sorted(instance_data['base64_encoded_keys'])) - self.assertEqual('nocloud', v1_data['cloud_name']) + self.assertEqual('unknown', v1_data['cloud_name']) + self.assertEqual('lxd', v1_data['platform']) + self.assertEqual( + 'seed-dir (/var/lib/cloud/seed/nocloud-net)', + v1_data['subplatform']) self.assertIsNone( v1_data['availability_zone'], 'found unexpected lxd availability_zone %s' % @@ -253,7 +260,9 @@ class CloudTestCase(unittest2.TestCase): instance_data = json.loads(out) v1_data = instance_data.get('v1', {}) self.assertItemsEqual([], instance_data['base64_encoded_keys']) - self.assertEqual('nocloud', v1_data['cloud_name']) + self.assertEqual('unknown', v1_data['cloud_name']) + self.assertEqual('nocloud', v1_data['platform']) + self.assertEqual('config-disk (/dev/vda)', v1_data['subplatform']) self.assertIsNone( v1_data['availability_zone'], 'found unexpected kvm availability_zone %s' % diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py index 1e77842f..e9213ca1 100644 --- a/tests/unittests/test_datasource/test_aliyun.py +++ b/tests/unittests/test_datasource/test_aliyun.py @@ -140,6 +140,10 @@ class TestAliYunDatasource(test_helpers.HttprettyTestCase): self._test_get_sshkey() self._test_get_iid() self._test_host_name() + self.assertEqual('aliyun', self.ds.cloud_name) + self.assertEqual('ec2', self.ds.platform) + self.assertEqual( + '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_altcloud.py b/tests/unittests/test_datasource/test_altcloud.py index ff35904e..3119bfac 100644 --- a/tests/unittests/test_datasource/test_altcloud.py +++ b/tests/unittests/test_datasource/test_altcloud.py @@ -10,7 +10,6 @@ This test file exercises the code in sources DataSourceAltCloud.py ''' -import mock import os import shutil import tempfile @@ -18,32 +17,13 @@ import tempfile from cloudinit import helpers from cloudinit import util -from cloudinit.tests.helpers import CiTestCase +from cloudinit.tests.helpers import CiTestCase, mock import cloudinit.sources.DataSourceAltCloud as dsac OS_UNAME_ORIG = getattr(os, 'uname') -def _write_cloud_info_file(value): - ''' - Populate the CLOUD_INFO_FILE which would be populated - with a cloud backend identifier ImageFactory when building - an image with ImageFactory. - ''' - cifile = open(dsac.CLOUD_INFO_FILE, 'w') - cifile.write(value) - cifile.close() - os.chmod(dsac.CLOUD_INFO_FILE, 0o664) - - -def _remove_cloud_info_file(): - ''' - Remove the test CLOUD_INFO_FILE - ''' - os.remove(dsac.CLOUD_INFO_FILE) - - def _write_user_data_files(mount_dir, value): ''' Populate the deltacloud_user_data_file the user_data_file @@ -98,13 +78,15 @@ def _dmi_data(expected): class TestGetCloudType(CiTestCase): - ''' - Test to exercise method: DataSourceAltCloud.get_cloud_type() - ''' + '''Test to exercise method: DataSourceAltCloud.get_cloud_type()''' + + with_logs = True def setUp(self): '''Set up.''' - self.paths = helpers.Paths({'cloud_dir': '/tmp'}) + super(TestGetCloudType, self).setUp() + self.tmp = self.tmp_dir() + self.paths = helpers.Paths({'cloud_dir': self.tmp}) self.dmi_data = util.read_dmi_data # We have a different code path for arm to deal with LP1243287 # We have to switch arch to x86_64 to avoid test failure @@ -115,6 +97,26 @@ class TestGetCloudType(CiTestCase): util.read_dmi_data = self.dmi_data force_arch() + def test_cloud_info_file_ioerror(self): + """Return UNKNOWN when /etc/sysconfig/cloud-info exists but errors.""" + self.assertEqual('/etc/sysconfig/cloud-info', dsac.CLOUD_INFO_FILE) + dsrc = dsac.DataSourceAltCloud({}, None, self.paths) + # Attempting to read the directory generates IOError + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.tmp): + self.assertEqual('UNKNOWN', dsrc.get_cloud_type()) + self.assertIn( + "[Errno 21] Is a directory: '%s'" % self.tmp, + self.logs.getvalue()) + + def test_cloud_info_file(self): + """Return uppercase stripped content from /etc/sysconfig/cloud-info.""" + dsrc = dsac.DataSourceAltCloud({}, None, self.paths) + cloud_info = self.tmp_path('cloud-info', dir=self.tmp) + util.write_file(cloud_info, ' OverRiDdeN CloudType ') + # Attempting to read the directory generates IOError + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', cloud_info): + self.assertEqual('OVERRIDDEN CLOUDTYPE', dsrc.get_cloud_type()) + def test_rhev(self): ''' Test method get_cloud_type() for RHEVm systems. @@ -153,60 +155,57 @@ class TestGetDataCloudInfoFile(CiTestCase): self.tmp = self.tmp_dir() self.paths = helpers.Paths( {'cloud_dir': self.tmp, 'run_dir': self.tmp}) - self.cloud_info_file = tempfile.mkstemp()[1] - self.dmi_data = util.read_dmi_data - dsac.CLOUD_INFO_FILE = self.cloud_info_file - - def tearDown(self): - # Reset - - # Attempt to remove the temp file ignoring errors - try: - os.remove(self.cloud_info_file) - except OSError: - pass - - util.read_dmi_data = self.dmi_data - dsac.CLOUD_INFO_FILE = '/etc/sysconfig/cloud-info' + self.cloud_info_file = self.tmp_path('cloud-info', dir=self.tmp) def test_rhev(self): '''Success Test module get_data() forcing RHEV.''' - _write_cloud_info_file('RHEV') + util.write_file(self.cloud_info_file, 'RHEV') dsrc = dsac.DataSourceAltCloud({}, None, self.paths) dsrc.user_data_rhevm = lambda: True - self.assertEqual(True, dsrc.get_data()) + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.cloud_info_file): + self.assertEqual(True, dsrc.get_data()) + self.assertEqual('altcloud', dsrc.cloud_name) + self.assertEqual('altcloud', dsrc.platform_type) + self.assertEqual('rhev (/dev/fd0)', dsrc.subplatform) def test_vsphere(self): '''Success Test module get_data() forcing VSPHERE.''' - _write_cloud_info_file('VSPHERE') + util.write_file(self.cloud_info_file, 'VSPHERE') dsrc = dsac.DataSourceAltCloud({}, None, self.paths) dsrc.user_data_vsphere = lambda: True - self.assertEqual(True, dsrc.get_data()) + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.cloud_info_file): + self.assertEqual(True, dsrc.get_data()) + self.assertEqual('altcloud', dsrc.cloud_name) + self.assertEqual('altcloud', dsrc.platform_type) + self.assertEqual('vsphere (unknown)', dsrc.subplatform) def test_fail_rhev(self): '''Failure Test module get_data() forcing RHEV.''' - _write_cloud_info_file('RHEV') + util.write_file(self.cloud_info_file, 'RHEV') dsrc = dsac.DataSourceAltCloud({}, None, self.paths) dsrc.user_data_rhevm = lambda: False - self.assertEqual(False, dsrc.get_data()) + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.cloud_info_file): + self.assertEqual(False, dsrc.get_data()) def test_fail_vsphere(self): '''Failure Test module get_data() forcing VSPHERE.''' - _write_cloud_info_file('VSPHERE') + util.write_file(self.cloud_info_file, 'VSPHERE') dsrc = dsac.DataSourceAltCloud({}, None, self.paths) dsrc.user_data_vsphere = lambda: False - self.assertEqual(False, dsrc.get_data()) + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.cloud_info_file): + self.assertEqual(False, dsrc.get_data()) def test_unrecognized(self): '''Failure Test module get_data() forcing unrecognized.''' - _write_cloud_info_file('unrecognized') + util.write_file(self.cloud_info_file, 'unrecognized') dsrc = dsac.DataSourceAltCloud({}, None, self.paths) - self.assertEqual(False, dsrc.get_data()) + with mock.patch.object(dsac, 'CLOUD_INFO_FILE', self.cloud_info_file): + self.assertEqual(False, dsrc.get_data()) class TestGetDataNoCloudInfoFile(CiTestCase): @@ -322,7 +321,8 @@ class TestUserDataVsphere(CiTestCase): ''' def setUp(self): '''Set up.''' - self.paths = helpers.Paths({'cloud_dir': '/tmp'}) + self.tmp = self.tmp_dir() + self.paths = helpers.Paths({'cloud_dir': self.tmp}) self.mount_dir = tempfile.mkdtemp() _write_user_data_files(self.mount_dir, 'test user data') @@ -363,6 +363,22 @@ class TestUserDataVsphere(CiTestCase): self.assertEqual(1, m_find_devs_with.call_count) self.assertEqual(1, m_mount_cb.call_count) + @mock.patch("cloudinit.sources.DataSourceAltCloud.util.find_devs_with") + @mock.patch("cloudinit.sources.DataSourceAltCloud.util.mount_cb") + def test_user_data_vsphere_success(self, m_mount_cb, m_find_devs_with): + """Test user_data_vsphere() where successful.""" + m_find_devs_with.return_value = ["/dev/mock/cdrom"] + m_mount_cb.return_value = 'raw userdata from cdrom' + dsrc = dsac.DataSourceAltCloud({}, None, self.paths) + cloud_info = self.tmp_path('cloud-info', dir=self.tmp) + util.write_file(cloud_info, 'VSPHERE') + self.assertEqual(True, dsrc.user_data_vsphere()) + m_find_devs_with.assert_called_once_with('LABEL=CDROM') + m_mount_cb.assert_called_once_with( + '/dev/mock/cdrom', dsac.read_user_data_callback) + with mock.patch.object(dsrc, 'get_cloud_type', return_value='VSPHERE'): + self.assertEqual('vsphere (/dev/mock/cdrom)', dsrc.subplatform) + class TestReadUserDataCallback(CiTestCase): ''' diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 4e428b71..0f4b7bf7 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -110,6 +110,8 @@ NETWORK_METADATA = { } } +MOCKPATH = 'cloudinit.sources.DataSourceAzure.' + class TestGetMetadataFromIMDS(HttprettyTestCase): @@ -119,9 +121,9 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): super(TestGetMetadataFromIMDS, self).setUp() self.network_md_url = dsaz.IMDS_URL + "instance?api-version=2017-12-01" - @mock.patch('cloudinit.sources.DataSourceAzure.readurl') - @mock.patch('cloudinit.sources.DataSourceAzure.EphemeralDHCPv4') - @mock.patch('cloudinit.sources.DataSourceAzure.net.is_up') + @mock.patch(MOCKPATH + 'readurl') + @mock.patch(MOCKPATH + 'EphemeralDHCPv4') + @mock.patch(MOCKPATH + 'net.is_up') def test_get_metadata_does_not_dhcp_if_network_is_up( self, m_net_is_up, m_dhcp, m_readurl): """Do not perform DHCP setup when nic is already up.""" @@ -138,9 +140,9 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): "Crawl of Azure Instance Metadata Service (IMDS) took", # log_time self.logs.getvalue()) - @mock.patch('cloudinit.sources.DataSourceAzure.readurl') - @mock.patch('cloudinit.sources.DataSourceAzure.EphemeralDHCPv4') - @mock.patch('cloudinit.sources.DataSourceAzure.net.is_up') + @mock.patch(MOCKPATH + 'readurl') + @mock.patch(MOCKPATH + 'EphemeralDHCPv4') + @mock.patch(MOCKPATH + 'net.is_up') def test_get_metadata_performs_dhcp_when_network_is_down( self, m_net_is_up, m_dhcp, m_readurl): """Perform DHCP setup when nic is not up.""" @@ -163,7 +165,7 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): headers={'Metadata': 'true'}, retries=2, timeout=1) @mock.patch('cloudinit.url_helper.time.sleep') - @mock.patch('cloudinit.sources.DataSourceAzure.net.is_up') + @mock.patch(MOCKPATH + 'net.is_up') def test_get_metadata_from_imds_empty_when_no_imds_present( self, m_net_is_up, m_sleep): """Return empty dict when IMDS network metadata is absent.""" @@ -380,7 +382,7 @@ fdescfs /dev/fd fdescfs rw 0 0 res = get_path_dev_freebsd('/etc', mnt_list) self.assertIsNotNone(res) - @mock.patch('cloudinit.sources.DataSourceAzure._is_platform_viable') + @mock.patch(MOCKPATH + '_is_platform_viable') def test_call_is_platform_viable_seed(self, m_is_platform_viable): """Check seed_dir using _is_platform_viable and return False.""" # Return a non-matching asset tag value @@ -401,6 +403,24 @@ fdescfs /dev/fd fdescfs rw 0 0 self.assertEqual(dsrc.metadata['local-hostname'], odata['HostName']) self.assertTrue(os.path.isfile( os.path.join(self.waagent_d, 'ovf-env.xml'))) + self.assertEqual('azure', dsrc.cloud_name) + self.assertEqual('azure', dsrc.platform_type) + self.assertEqual( + 'seed-dir (%s/seed/azure)' % self.tmp, dsrc.subplatform) + + def test_basic_dev_file(self): + """When a device path is used, present that in subplatform.""" + data = {'sys_cfg': {}, 'dsdevs': ['/dev/cd0']} + dsrc = self._get_ds(data) + with mock.patch(MOCKPATH + 'util.mount_cb') as m_mount_cb: + m_mount_cb.return_value = ( + {'local-hostname': 'me'}, 'ud', {'cfg': ''}, {}) + self.assertTrue(dsrc.get_data()) + self.assertEqual(dsrc.userdata_raw, 'ud') + self.assertEqual(dsrc.metadata['local-hostname'], 'me') + self.assertEqual('azure', dsrc.cloud_name) + self.assertEqual('azure', dsrc.platform_type) + self.assertEqual('config-disk (/dev/cd0)', dsrc.subplatform) def test_get_data_non_ubuntu_will_not_remove_network_scripts(self): """get_data on non-Ubuntu will not remove ubuntu net scripts.""" @@ -769,8 +789,8 @@ fdescfs /dev/fd fdescfs rw 0 0 ds.get_data() self.assertEqual(self.instance_id, ds.metadata['instance-id']) - @mock.patch("cloudinit.sources.DataSourceAzure.util.is_FreeBSD") - @mock.patch("cloudinit.sources.DataSourceAzure._check_freebsd_cdrom") + @mock.patch(MOCKPATH + 'util.is_FreeBSD') + @mock.patch(MOCKPATH + '_check_freebsd_cdrom') def test_list_possible_azure_ds_devs(self, m_check_fbsd_cdrom, m_is_FreeBSD): """On FreeBSD, possible devs should show /dev/cd0.""" @@ -885,17 +905,17 @@ fdescfs /dev/fd fdescfs rw 0 0 expected_config['config'].append(blacklist_config) self.assertEqual(netconfig, expected_config) - @mock.patch("cloudinit.sources.DataSourceAzure.util.subp") + @mock.patch(MOCKPATH + 'util.subp') def test_get_hostname_with_no_args(self, subp): dsaz.get_hostname() subp.assert_called_once_with(("hostname",), capture=True) - @mock.patch("cloudinit.sources.DataSourceAzure.util.subp") + @mock.patch(MOCKPATH + 'util.subp') def test_get_hostname_with_string_arg(self, subp): dsaz.get_hostname(hostname_command="hostname") subp.assert_called_once_with(("hostname",), capture=True) - @mock.patch("cloudinit.sources.DataSourceAzure.util.subp") + @mock.patch(MOCKPATH + 'util.subp') def test_get_hostname_with_iterable_arg(self, subp): dsaz.get_hostname(hostname_command=("hostname",)) subp.assert_called_once_with(("hostname",), capture=True) @@ -949,7 +969,7 @@ class TestAzureBounce(CiTestCase): self.set_hostname = self.patches.enter_context( mock.patch.object(dsaz, 'set_hostname')) self.subp = self.patches.enter_context( - mock.patch('cloudinit.sources.DataSourceAzure.util.subp')) + mock.patch(MOCKPATH + 'util.subp')) self.find_fallback_nic = self.patches.enter_context( mock.patch('cloudinit.net.find_fallback_nic', return_value='eth9')) @@ -989,7 +1009,7 @@ class TestAzureBounce(CiTestCase): ds.get_data() self.assertEqual(0, self.set_hostname.call_count) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_disabled_bounce_does_not_perform_bounce( self, perform_hostname_bounce): cfg = {'hostname_bounce': {'policy': 'off'}} @@ -1005,7 +1025,7 @@ class TestAzureBounce(CiTestCase): ds.get_data() self.assertEqual(0, self.set_hostname.call_count) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_unchanged_hostname_does_not_perform_bounce( self, perform_hostname_bounce): host_name = 'unchanged-host-name' @@ -1015,7 +1035,7 @@ class TestAzureBounce(CiTestCase): ds.get_data() self.assertEqual(0, perform_hostname_bounce.call_count) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_force_performs_bounce_regardless(self, perform_hostname_bounce): host_name = 'unchanged-host-name' self.get_hostname.return_value = host_name @@ -1032,7 +1052,7 @@ class TestAzureBounce(CiTestCase): cfg = {'hostname_bounce': {'policy': 'force'}} dsrc = self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg), agent_command=['not', '__builtin__']) - patch_path = 'cloudinit.sources.DataSourceAzure.util.which' + patch_path = MOCKPATH + 'util.which' with mock.patch(patch_path) as m_which: m_which.return_value = None ret = self._get_and_setup(dsrc) @@ -1053,7 +1073,7 @@ class TestAzureBounce(CiTestCase): self.assertEqual(expected_hostname, self.set_hostname.call_args_list[0][0][0]) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_different_hostnames_performs_bounce( self, perform_hostname_bounce): expected_hostname = 'azure-expected-host-name' @@ -1076,7 +1096,7 @@ class TestAzureBounce(CiTestCase): self.assertEqual(initial_host_name, self.set_hostname.call_args_list[-1][0][0]) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_failure_in_bounce_still_resets_host_name( self, perform_hostname_bounce): perform_hostname_bounce.side_effect = Exception @@ -1117,7 +1137,7 @@ class TestAzureBounce(CiTestCase): self.assertEqual( dsaz.BOUNCE_COMMAND_IFUP, bounce_args) - @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') + @mock.patch(MOCKPATH + 'perform_hostname_bounce') def test_set_hostname_option_can_disable_bounce( self, perform_hostname_bounce): cfg = {'set_hostname': False, 'hostname_bounce': {'policy': 'force'}} @@ -1218,12 +1238,12 @@ class TestCanDevBeReformatted(CiTestCase): def has_ntfs_fs(device): return bypath.get(device, {}).get('fs') == 'ntfs' - p = 'cloudinit.sources.DataSourceAzure' - self._domock(p + "._partitions_on_device", 'm_partitions_on_device') - self._domock(p + "._has_ntfs_filesystem", 'm_has_ntfs_filesystem') - self._domock(p + ".util.mount_cb", 'm_mount_cb') - self._domock(p + ".os.path.realpath", 'm_realpath') - self._domock(p + ".os.path.exists", 'm_exists') + p = MOCKPATH + self._domock(p + "_partitions_on_device", 'm_partitions_on_device') + self._domock(p + "_has_ntfs_filesystem", 'm_has_ntfs_filesystem') + self._domock(p + "util.mount_cb", 'm_mount_cb') + self._domock(p + "os.path.realpath", 'm_realpath') + self._domock(p + "os.path.exists", 'm_exists') self.m_exists.side_effect = lambda p: p in bypath self.m_realpath.side_effect = realpath @@ -1488,7 +1508,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): self.paths = helpers.Paths({'cloud_dir': tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d - @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') + @mock.patch(MOCKPATH + 'util.write_file') def test__should_reprovision_with_true_cfg(self, isfile, write_f): """The _should_reprovision method should return true with config flag present.""" @@ -1512,7 +1532,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) self.assertFalse(dsa._should_reprovision((None, None, {}, None))) - @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds') + @mock.patch(MOCKPATH + 'DataSourceAzure._poll_imds') def test_reprovision_calls__poll_imds(self, _poll_imds, isfile): """_reprovision will poll IMDS.""" isfile.return_value = False @@ -1528,8 +1548,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @mock.patch('requests.Session.request') -@mock.patch( - 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') +@mock.patch(MOCKPATH + 'DataSourceAzure._report_ready') class TestPreprovisioningPollIMDS(CiTestCase): def setUp(self): @@ -1539,7 +1558,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): self.paths = helpers.Paths({'cloud_dir': self.tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d - @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') + @mock.patch(MOCKPATH + 'util.write_file') def test_poll_imds_calls_report_ready(self, write_f, report_ready_func, fake_resp, m_dhcp, m_net): """The poll_imds will call report_ready after creating marker file.""" @@ -1550,8 +1569,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): 'unknown-245': '624c3620'} m_dhcp.return_value = [lease] dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) - mock_path = ( - 'cloudinit.sources.DataSourceAzure.REPORTED_READY_MARKER_FILE') + mock_path = (MOCKPATH + 'REPORTED_READY_MARKER_FILE') with mock.patch(mock_path, report_marker): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 1) @@ -1561,23 +1579,21 @@ class TestPreprovisioningPollIMDS(CiTestCase): fake_resp, m_dhcp, m_net): """The poll_imds should not call reporting ready when flag is false""" - report_marker = self.tmp_path('report_marker', self.tmp) - write_file(report_marker, content='dont run report_ready :)') + report_file = self.tmp_path('report_marker', self.tmp) + write_file(report_file, content='dont run report_ready :)') m_dhcp.return_value = [{ 'interface': 'eth9', 'fixed-address': '192.168.2.9', 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'}] dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) - mock_path = ( - 'cloudinit.sources.DataSourceAzure.REPORTED_READY_MARKER_FILE') - with mock.patch(mock_path, report_marker): + with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 0) -@mock.patch('cloudinit.sources.DataSourceAzure.util.subp') -@mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') -@mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD') +@mock.patch(MOCKPATH + 'util.subp') +@mock.patch(MOCKPATH + 'util.write_file') +@mock.patch(MOCKPATH + 'util.is_FreeBSD') @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @mock.patch('requests.Session.request') @@ -1688,7 +1704,7 @@ class TestRemoveUbuntuNetworkConfigScripts(CiTestCase): self.tmp_path('notfilehere', dir=self.tmp)]) self.assertNotIn('/not/a', self.logs.getvalue()) # No delete logs - @mock.patch('cloudinit.sources.DataSourceAzure.os.path.exists') + @mock.patch(MOCKPATH + '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.""" @@ -1704,14 +1720,14 @@ class TestWBIsPlatformViable(CiTestCase): """White box tests for _is_platform_viable.""" with_logs = True - @mock.patch('cloudinit.sources.DataSourceAzure.util.read_dmi_data') + @mock.patch(MOCKPATH + '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') + @mock.patch(MOCKPATH + 'os.path.exists') + @mock.patch(MOCKPATH + '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 @@ -1729,7 +1745,7 @@ class TestWBIsPlatformViable(CiTestCase): and no devices have a label starting with prefix 'rd_rdfe_'. """ self.assertFalse(wrap_and_call( - 'cloudinit.sources.DataSourceAzure', + MOCKPATH, {'os.path.exists': False, # Non-matching Azure chassis-asset-tag 'util.read_dmi_data': dsaz.AZURE_CHASSIS_ASSET_TAG + 'X', diff --git a/tests/unittests/test_datasource/test_cloudsigma.py b/tests/unittests/test_datasource/test_cloudsigma.py index 380ad1b5..3bf52e69 100644 --- a/tests/unittests/test_datasource/test_cloudsigma.py +++ b/tests/unittests/test_datasource/test_cloudsigma.py @@ -68,6 +68,12 @@ class DataSourceCloudSigmaTest(test_helpers.CiTestCase): self.assertEqual(SERVER_CONTEXT['uuid'], self.datasource.get_instance_id()) + def test_platform(self): + """All platform-related attributes are set.""" + self.assertEqual(self.datasource.cloud_name, 'cloudsigma') + self.assertEqual(self.datasource.platform_type, 'cloudsigma') + self.assertEqual(self.datasource.subplatform, 'cepko (/dev/ttyS1)') + def test_metadata(self): self.assertEqual(self.datasource.metadata, SERVER_CONTEXT) diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 231619c9..dcdabea5 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -478,6 +478,9 @@ class TestConfigDriveDataSource(CiTestCase): myds = cfg_ds_from_dir(self.tmp, files=CFG_DRIVE_FILES_V2) self.assertEqual(myds.get_public_ssh_keys(), [OSTACK_META['public_keys']['mykey']]) + self.assertEqual('configdrive', myds.cloud_name) + self.assertEqual('openstack', myds.platform) + self.assertEqual('seed-dir (%s/seed)' % self.tmp, myds.subplatform) class TestNetJson(CiTestCase): diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 497e7610..9f81255a 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -351,7 +351,9 @@ class TestEc2(test_helpers.HttprettyTestCase): m_get_interface_mac.return_value = mac1 nc = ds.network_config # Will re-crawl network metadata self.assertIsNotNone(nc) - self.assertIn('Re-crawl of metadata service', self.logs.getvalue()) + 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', @@ -386,7 +388,7 @@ class TestEc2(test_helpers.HttprettyTestCase): register_mock_metaserver( '{0}/{1}/dynamic/'.format(ds.metadata_address, all_versions[-1]), DYNAMIC_METADATA) - ds._cloud_platform = ec2.Platforms.AWS + ds._cloud_name = ec2.CloudNames.AWS # Setup cached metadata on the Datasource ds.metadata = DEFAULT_METADATA self.assertEqual('my-identity-id', ds.get_instance_id()) @@ -401,6 +403,9 @@ class TestEc2(test_helpers.HttprettyTestCase): ret = ds.get_data() self.assertTrue(ret) self.assertEqual(0, m_dhcp.call_count) + self.assertEqual('aws', ds.cloud_name) + self.assertEqual('ec2', ds.platform_type) + self.assertEqual('metadata (%s)' % ds.metadata_address, ds.subplatform) def test_valid_platform_with_strict_false(self): """Valid platform data should return true with strict_id false.""" @@ -439,16 +444,17 @@ class TestEc2(test_helpers.HttprettyTestCase): sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, md=DEFAULT_METADATA) platform_attrs = [ - attr for attr in ec2.Platforms.__dict__.keys() + attr for attr in ec2.CloudNames.__dict__.keys() if not attr.startswith('__')] for attr_name in platform_attrs: - platform_name = getattr(ec2.Platforms, attr_name) - if platform_name != 'AWS': - ds._cloud_platform = platform_name + platform_name = getattr(ec2.CloudNames, attr_name) + if platform_name != 'aws': + ds._cloud_name = platform_name ret = ds.get_data() + self.assertEqual('ec2', ds.platform_type) self.assertFalse(ret) message = ( - "Local Ec2 mode only supported on ('AWS',)," + "Local Ec2 mode only supported on ('aws',)," ' not {0}'.format(platform_name)) self.assertIn(message, self.logs.getvalue()) diff --git a/tests/unittests/test_datasource/test_ibmcloud.py b/tests/unittests/test_datasource/test_ibmcloud.py index e639ae47..0b54f585 100644 --- a/tests/unittests/test_datasource/test_ibmcloud.py +++ b/tests/unittests/test_datasource/test_ibmcloud.py @@ -1,14 +1,17 @@ # This file is part of cloud-init. See LICENSE file for license information. +from cloudinit.helpers import Paths from cloudinit.sources import DataSourceIBMCloud as ibm from cloudinit.tests import helpers as test_helpers +from cloudinit import util import base64 import copy import json -import mock from textwrap import dedent +mock = test_helpers.mock + D_PATH = "cloudinit.sources.DataSourceIBMCloud." @@ -309,4 +312,39 @@ class TestIsIBMProvisioning(test_helpers.FilesystemMockingTestCase): self.assertIn("no reference file", self.logs.getvalue()) +class TestDataSourceIBMCloud(test_helpers.CiTestCase): + + def setUp(self): + super(TestDataSourceIBMCloud, self).setUp() + self.tmp = self.tmp_dir() + self.cloud_dir = self.tmp_path('cloud', dir=self.tmp) + util.ensure_dir(self.cloud_dir) + paths = Paths({'run_dir': self.tmp, 'cloud_dir': self.cloud_dir}) + self.ds = ibm.DataSourceIBMCloud( + sys_cfg={}, distro=None, paths=paths) + + def test_get_data_false(self): + """When read_md returns None, get_data returns False.""" + with mock.patch(D_PATH + 'read_md', return_value=None): + self.assertFalse(self.ds.get_data()) + + def test_get_data_processes_read_md(self): + """get_data processes and caches content returned by read_md.""" + md = { + 'metadata': {}, 'networkdata': 'net', 'platform': 'plat', + 'source': 'src', 'system-uuid': 'uuid', 'userdata': 'ud', + 'vendordata': 'vd'} + with mock.patch(D_PATH + 'read_md', return_value=md): + self.assertTrue(self.ds.get_data()) + self.assertEqual('src', self.ds.source) + self.assertEqual('plat', self.ds.platform) + self.assertEqual({}, self.ds.metadata) + self.assertEqual('ud', self.ds.userdata_raw) + self.assertEqual('net', self.ds.network_json) + self.assertEqual('vd', self.ds.vendordata_pure) + self.assertEqual('uuid', self.ds.system_uuid) + self.assertEqual('ibmcloud', self.ds.cloud_name) + self.assertEqual('ibmcloud', self.ds.platform_type) + self.assertEqual('plat (src)', self.ds.subplatform) + # vi: ts=4 expandtab diff --git a/tests/unittests/test_datasource/test_nocloud.py b/tests/unittests/test_datasource/test_nocloud.py index 21931eb7..b6468b6d 100644 --- a/tests/unittests/test_datasource/test_nocloud.py +++ b/tests/unittests/test_datasource/test_nocloud.py @@ -10,6 +10,7 @@ import textwrap import yaml +@mock.patch('cloudinit.sources.DataSourceNoCloud.util.is_lxd') class TestNoCloudDataSource(CiTestCase): def setUp(self): @@ -28,10 +29,11 @@ class TestNoCloudDataSource(CiTestCase): self.mocks.enter_context( mock.patch.object(util, 'read_dmi_data', return_value=None)) - def test_nocloud_seed_dir(self): + def test_nocloud_seed_dir_on_lxd(self, m_is_lxd): md = {'instance-id': 'IID', 'dsmode': 'local'} ud = b"USER_DATA_HERE" - populate_dir(os.path.join(self.paths.seed_dir, "nocloud"), + seed_dir = os.path.join(self.paths.seed_dir, "nocloud") + populate_dir(seed_dir, {'user-data': ud, 'meta-data': yaml.safe_dump(md)}) sys_cfg = { @@ -44,9 +46,32 @@ class TestNoCloudDataSource(CiTestCase): ret = dsrc.get_data() self.assertEqual(dsrc.userdata_raw, ud) self.assertEqual(dsrc.metadata, md) + self.assertEqual(dsrc.platform_type, 'lxd') + self.assertEqual( + dsrc.subplatform, 'seed-dir (%s)' % seed_dir) self.assertTrue(ret) - def test_fs_label(self): + def test_nocloud_seed_dir_non_lxd_platform_is_nocloud(self, m_is_lxd): + """Non-lxd environments will list nocloud as the platform.""" + m_is_lxd.return_value = False + md = {'instance-id': 'IID', 'dsmode': 'local'} + seed_dir = os.path.join(self.paths.seed_dir, "nocloud") + populate_dir(seed_dir, + {'user-data': '', 'meta-data': yaml.safe_dump(md)}) + + sys_cfg = { + 'datasource': {'NoCloud': {'fs_label': None}} + } + + ds = DataSourceNoCloud.DataSourceNoCloud + + dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + self.assertTrue(dsrc.get_data()) + self.assertEqual(dsrc.platform_type, 'nocloud') + self.assertEqual( + dsrc.subplatform, 'seed-dir (%s)' % seed_dir) + + def test_fs_label(self, m_is_lxd): # find_devs_with should not be called ff fs_label is None ds = DataSourceNoCloud.DataSourceNoCloud @@ -68,7 +93,7 @@ class TestNoCloudDataSource(CiTestCase): ret = dsrc.get_data() self.assertFalse(ret) - def test_no_datasource_expected(self): + def test_no_datasource_expected(self, m_is_lxd): # no source should be found if no cmdline, config, and fs_label=None sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} @@ -76,7 +101,7 @@ class TestNoCloudDataSource(CiTestCase): dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) self.assertFalse(dsrc.get_data()) - def test_seed_in_config(self): + def test_seed_in_config(self, m_is_lxd): ds = DataSourceNoCloud.DataSourceNoCloud data = { @@ -92,7 +117,7 @@ class TestNoCloudDataSource(CiTestCase): self.assertEqual(dsrc.metadata.get('instance-id'), 'IID') self.assertTrue(ret) - def test_nocloud_seed_with_vendordata(self): + def test_nocloud_seed_with_vendordata(self, m_is_lxd): md = {'instance-id': 'IID', 'dsmode': 'local'} ud = b"USER_DATA_HERE" vd = b"THIS IS MY VENDOR_DATA" @@ -114,7 +139,7 @@ class TestNoCloudDataSource(CiTestCase): self.assertEqual(dsrc.vendordata_raw, vd) self.assertTrue(ret) - def test_nocloud_no_vendordata(self): + def test_nocloud_no_vendordata(self, m_is_lxd): populate_dir(os.path.join(self.paths.seed_dir, "nocloud"), {'user-data': b"ud", 'meta-data': "instance-id: IID\n"}) @@ -128,7 +153,7 @@ class TestNoCloudDataSource(CiTestCase): self.assertFalse(dsrc.vendordata) self.assertTrue(ret) - def test_metadata_network_interfaces(self): + def test_metadata_network_interfaces(self, m_is_lxd): gateway = "103.225.10.1" md = { 'instance-id': 'i-abcd', @@ -157,7 +182,7 @@ class TestNoCloudDataSource(CiTestCase): # very simple check just for the strings above self.assertIn(gateway, str(dsrc.network_config)) - def test_metadata_network_config(self): + def test_metadata_network_config(self, m_is_lxd): # network-config needs to get into network_config netconf = {'version': 1, 'config': [{'type': 'physical', 'name': 'interface0', @@ -177,7 +202,7 @@ class TestNoCloudDataSource(CiTestCase): self.assertTrue(ret) self.assertEqual(netconf, dsrc.network_config) - def test_metadata_network_config_over_interfaces(self): + def test_metadata_network_config_over_interfaces(self, m_is_lxd): # network-config should override meta-data/network-interfaces gateway = "103.225.10.1" md = { diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index 61591017..bb399f6d 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -123,6 +123,10 @@ class TestOpenNebulaDataSource(CiTestCase): self.assertTrue(ret) finally: util.find_devs_with = orig_find_devs_with + self.assertEqual('opennebula', dsrc.cloud_name) + self.assertEqual('opennebula', dsrc.platform_type) + self.assertEqual( + 'seed-dir (%s/seed/opennebula)' % self.tmp, dsrc.subplatform) def test_seed_dir_non_contextdisk(self): self.assertRaises(ds.NonContextDiskDir, ds.read_context_disk_dir, diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py index 9d52eb99..a226c032 100644 --- a/tests/unittests/test_datasource/test_ovf.py +++ b/tests/unittests/test_datasource/test_ovf.py @@ -11,7 +11,7 @@ from collections import OrderedDict from textwrap import dedent from cloudinit import util -from cloudinit.tests.helpers import CiTestCase, wrap_and_call +from cloudinit.tests.helpers import CiTestCase, mock, wrap_and_call from cloudinit.helpers import Paths from cloudinit.sources import DataSourceOVF as dsovf from cloudinit.sources.helpers.vmware.imc.config_custom_script import ( @@ -120,7 +120,7 @@ class TestDatasourceOVF(CiTestCase): def test_get_data_false_on_none_dmi_data(self): """When dmi for system-product-name is None, get_data returns False.""" - paths = Paths({'seed_dir': self.tdir}) + paths = Paths({'cloud_dir': self.tdir}) ds = self.datasource(sys_cfg={}, distro={}, paths=paths) retcode = wrap_and_call( 'cloudinit.sources.DataSourceOVF', @@ -134,7 +134,7 @@ class TestDatasourceOVF(CiTestCase): def test_get_data_no_vmware_customization_disabled(self): """When vmware customization is disabled via sys_cfg log a message.""" - paths = Paths({'seed_dir': self.tdir}) + paths = Paths({'cloud_dir': self.tdir}) ds = self.datasource( sys_cfg={'disable_vmware_customization': True}, distro={}, paths=paths) @@ -153,7 +153,7 @@ class TestDatasourceOVF(CiTestCase): """When cloud-init workflow for vmware is enabled via sys_cfg log a message. """ - paths = Paths({'seed_dir': self.tdir}) + paths = Paths({'cloud_dir': self.tdir}) ds = self.datasource( sys_cfg={'disable_vmware_customization': False}, distro={}, paths=paths) @@ -178,6 +178,50 @@ class TestDatasourceOVF(CiTestCase): self.assertIn('Script %s not found!!' % customscript, str(context.exception)) + def test_get_data_non_vmware_seed_platform_info(self): + """Platform info properly reports when on non-vmware platforms.""" + paths = Paths({'cloud_dir': self.tdir, 'run_dir': self.tdir}) + # Write ovf-env.xml seed file + seed_dir = self.tmp_path('seed', dir=self.tdir) + ovf_env = self.tmp_path('ovf-env.xml', dir=seed_dir) + util.write_file(ovf_env, OVF_ENV_CONTENT) + ds = self.datasource(sys_cfg={}, distro={}, paths=paths) + + self.assertEqual('ovf', ds.cloud_name) + self.assertEqual('ovf', ds.platform_type) + MPATH = 'cloudinit.sources.DataSourceOVF.' + with mock.patch(MPATH + 'util.read_dmi_data', return_value='!VMware'): + with mock.patch(MPATH + 'transport_vmware_guestd') as m_guestd: + with mock.patch(MPATH + 'transport_iso9660') as m_iso9660: + m_iso9660.return_value = (None, 'ignored', 'ignored') + m_guestd.return_value = (None, 'ignored', 'ignored') + self.assertTrue(ds.get_data()) + self.assertEqual( + 'ovf (%s/seed/ovf-env.xml)' % self.tdir, + ds.subplatform) + + def test_get_data_vmware_seed_platform_info(self): + """Platform info properly reports when on VMware platform.""" + paths = Paths({'cloud_dir': self.tdir, 'run_dir': self.tdir}) + # Write ovf-env.xml seed file + seed_dir = self.tmp_path('seed', dir=self.tdir) + ovf_env = self.tmp_path('ovf-env.xml', dir=seed_dir) + util.write_file(ovf_env, OVF_ENV_CONTENT) + ds = self.datasource(sys_cfg={}, distro={}, paths=paths) + + self.assertEqual('ovf', ds.cloud_name) + self.assertEqual('ovf', ds.platform_type) + MPATH = 'cloudinit.sources.DataSourceOVF.' + with mock.patch(MPATH + 'util.read_dmi_data', return_value='VMWare'): + with mock.patch(MPATH + 'transport_vmware_guestd') as m_guestd: + with mock.patch(MPATH + 'transport_iso9660') as m_iso9660: + m_iso9660.return_value = (None, 'ignored', 'ignored') + m_guestd.return_value = (None, 'ignored', 'ignored') + self.assertTrue(ds.get_data()) + self.assertEqual( + 'vmware (%s/seed/ovf-env.xml)' % self.tdir, + ds.subplatform) + class TestTransportIso9660(CiTestCase): diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py index 46d67b94..42ac6971 100644 --- a/tests/unittests/test_datasource/test_smartos.py +++ b/tests/unittests/test_datasource/test_smartos.py @@ -426,6 +426,13 @@ class TestSmartOSDataSource(FilesystemMockingTestCase): self.assertEqual(MOCK_RETURNS['sdc:uuid'], dsrc.metadata['instance-id']) + def test_platform_info(self): + """All platform-related attributes are properly set.""" + dsrc = self._get_ds(mockdata=MOCK_RETURNS) + self.assertEqual('joyent', dsrc.cloud_name) + self.assertEqual('joyent', dsrc.platform_type) + self.assertEqual('serial (/dev/ttyS1)', dsrc.subplatform) + def test_root_keys(self): dsrc = self._get_ds(mockdata=MOCK_RETURNS) ret = dsrc.get_data() -- cgit v1.2.3 From 9f88125a25338b70b53a2f5b0af59499f334f261 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 10 Oct 2018 19:29:55 +0000 Subject: tests: fix kvm integration test to assert flexible config-disk path Make integration test for flexible using regexp in case disk changes. LP: #1797199 --- tests/cloud_tests/testcases/base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 16b268ef..bbd80ae1 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -262,7 +262,10 @@ class CloudTestCase(unittest2.TestCase): self.assertItemsEqual([], instance_data['base64_encoded_keys']) self.assertEqual('unknown', v1_data['cloud_name']) self.assertEqual('nocloud', v1_data['platform']) - self.assertEqual('config-disk (/dev/vda)', v1_data['subplatform']) + subplatform = v1_data['subplatform'] + self.assertIsNotNone( + re.match(r'config-disk \(\/dev\/[a-z]{3}\)', subplatform), + 'kvm subplatform "%s" != "config-disk (/dev/...)"' % subplatform) self.assertIsNone( v1_data['availability_zone'], 'found unexpected kvm availability_zone %s' % -- cgit v1.2.3 From 4652b196e04b68d7c335e64766e3a0140157417c Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 10 Oct 2018 21:04:43 +0000 Subject: tests: meta_data key changed to meta-data in ec2 instance-data.json LP: #1797231 --- tests/cloud_tests/testcases/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index bbd80ae1..fd12d87b 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -177,7 +177,7 @@ class CloudTestCase(unittest2.TestCase): instance_data['base64_encoded_keys']) ds = instance_data.get('ds', {}) v1_data = instance_data.get('v1', {}) - metadata = ds.get('meta_data', {}) + metadata = ds.get('meta-data', {}) macs = metadata.get( 'network', {}).get('interfaces', {}).get('macs', {}) if not macs: -- cgit v1.2.3 From 1d5e9aefdab06a2574d78e644deed6c6fa1da171 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 17 Oct 2018 18:47:35 +0000 Subject: azure: Add apply_network_config option to disable network from IMDS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Azure generates network configuration from the IMDS service and removes any preexisting hotplug network scripts which exist in Azure cloud images. Add a datasource configuration option which allows for writing a default network configuration which sets up dhcp on eth0 and leave the hotplug handling to the cloud-image scripts. To disable network-config from Azure IMDS, add the following to /etc/cloud/cloud.cfg.d/99-azure-no-imds-network.cfg: datasource:   Azure:     apply_network_config: False LP: #1798424 --- cloudinit/sources/DataSourceAzure.py | 11 +++++- doc/rtd/topics/datasources/azure.rst | 46 ++++++++++++++++++++++ tests/unittests/test_datasource/test_azure.py | 56 +++++++++++++++++++++++++-- 3 files changed, 107 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 39391d01..d0358e96 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -207,7 +207,9 @@ BUILTIN_DS_CONFIG = { }, 'disk_aliases': {'ephemeral0': RESOURCE_DISK_PATH}, 'dhclient_lease_file': LEASE_FILE, + 'apply_network_config': True, # Use IMDS published network configuration } +# RELEASE_BLOCKER: Xenial and earlier apply_network_config default is False BUILTIN_CLOUD_CONFIG = { 'disk_setup': { @@ -458,7 +460,8 @@ class DataSourceAzure(sources.DataSource): except sources.InvalidMetaDataException as e: LOG.warning('Could not crawl Azure metadata: %s', e) return False - if self.distro and self.distro.name == 'ubuntu': + if (self.distro and self.distro.name == 'ubuntu' and + self.ds_cfg.get('apply_network_config')): maybe_remove_ubuntu_network_config_scripts() # Process crawled data and augment with various config defaults @@ -619,7 +622,11 @@ class DataSourceAzure(sources.DataSource): the blacklisted devices. """ if not self._network_config: - self._network_config = parse_network_config(self._metadata_imds) + if self.ds_cfg.get('apply_network_config'): + nc_src = self._metadata_imds + else: + nc_src = None + self._network_config = parse_network_config(nc_src) return self._network_config diff --git a/doc/rtd/topics/datasources/azure.rst b/doc/rtd/topics/datasources/azure.rst index 559011ef..f73c3694 100644 --- a/doc/rtd/topics/datasources/azure.rst +++ b/doc/rtd/topics/datasources/azure.rst @@ -57,6 +57,52 @@ in order to use waagent.conf with cloud-init, the following settings are recomme ResourceDisk.MountPoint=/mnt +Configuration +------------- +The following configuration can be set for the datasource in system +configuration (in `/etc/cloud/cloud.cfg` or `/etc/cloud/cloud.cfg.d/`). + +The settings that may be configured are: + + * **agent_command**: Either __builtin__ (default) or a command to run to getcw + metadata. If __builtin__, get metadata from walinuxagent. Otherwise run the + provided command to obtain metadata. + * **apply_network_config**: Boolean set to True to use network configuration + described by Azure's IMDS endpoint instead of fallback network config of + dhcp on eth0. Default is True. For Ubuntu 16.04 or earlier, default is False. + * **data_dir**: Path used to read metadata files and write crawled data. + * **dhclient_lease_file**: The fallback lease file to source when looking for + custom DHCP option 245 from Azure fabric. + * **disk_aliases**: A dictionary defining which device paths should be + interpreted as ephemeral images. See cc_disk_setup module for more info. + * **hostname_bounce**: A dictionary Azure hostname bounce behavior to react to + metadata changes. + * **hostname_bounce**: A dictionary Azure hostname bounce behavior to react to + metadata changes. Azure will throttle ifup/down in some cases after metadata + has been updated to inform dhcp server about updated hostnames. + * **set_hostname**: Boolean set to True when we want Azure to set the hostname + based on metadata. + +An example configuration with the default values is provided below: + +.. sourcecode:: yaml + + datasource: + Azure: + agent_command: __builtin__ + apply_network_config: true + data_dir: /var/lib/waagent + dhclient_lease_file: /var/lib/dhcp/dhclient.eth0.leases + disk_aliases: + ephemeral0: /dev/disk/cloud/azure_resource + hostname_bounce: + interface: eth0 + command: builtin + policy: true + hostname_command: hostname + set_hostname: true + + Userdata -------- Userdata is provided to cloud-init inside the ovf-env.xml file. Cloud-init diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 0f4b7bf7..cd6e7e74 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -256,7 +256,8 @@ scbus-1 on xpt0 bus 0 ]) return dsaz - def _get_ds(self, data, agent_command=None, distro=None): + def _get_ds(self, data, agent_command=None, distro=None, + apply_network=None): def dsdevs(): return data.get('dsdevs', []) @@ -312,6 +313,8 @@ scbus-1 on xpt0 bus 0 data.get('sys_cfg', {}), distro=distro, paths=self.paths) if agent_command is not None: dsrc.ds_cfg['agent_command'] = agent_command + if apply_network is not None: + dsrc.ds_cfg['apply_network_config'] = apply_network return dsrc @@ -434,14 +437,26 @@ fdescfs /dev/fd fdescfs rw 0 0 def test_get_data_on_ubuntu_will_remove_network_scripts(self): """get_data will remove ubuntu net scripts on Ubuntu distro.""" + sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}} odata = {'HostName': "myhost", 'UserName': "myuser"} data = {'ovfcontent': construct_valid_ovf_env(data=odata), - 'sys_cfg': {}} + 'sys_cfg': sys_cfg} dsrc = self._get_ds(data, distro='ubuntu') dsrc.get_data() self.m_remove_ubuntu_network_scripts.assert_called_once_with() + def test_get_data_on_ubuntu_will_not_remove_network_scripts_disabled(self): + """When apply_network_config false, do not remove scripts on Ubuntu.""" + sys_cfg = {'datasource': {'Azure': {'apply_network_config': False}}} + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': sys_cfg} + + dsrc = self._get_ds(data, distro='ubuntu') + dsrc.get_data() + self.m_remove_ubuntu_network_scripts.assert_not_called() + def test_crawl_metadata_returns_structured_data_and_caches_nothing(self): """Return all structured metadata and cache no class attributes.""" yaml_cfg = "{agent_command: my_command}\n" @@ -523,8 +538,10 @@ fdescfs /dev/fd fdescfs rw 0 0 def test_network_config_set_from_imds(self): """Datasource.network_config returns IMDS network data.""" + sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}} odata = {} - data = {'ovfcontent': construct_valid_ovf_env(data=odata)} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': sys_cfg} expected_network_config = { 'ethernets': { 'eth0': {'set-name': 'eth0', @@ -803,9 +820,10 @@ fdescfs /dev/fd fdescfs rw 0 0 @mock.patch('cloudinit.net.generate_fallback_config') def test_imds_network_config(self, mock_fallback): """Network config is generated from IMDS network data when present.""" + sys_cfg = {'datasource': {'Azure': {'apply_network_config': True}}} odata = {'HostName': "myhost", 'UserName': "myuser"} data = {'ovfcontent': construct_valid_ovf_env(data=odata), - 'sys_cfg': {}} + 'sys_cfg': sys_cfg} dsrc = self._get_ds(data) ret = dsrc.get_data() @@ -821,6 +839,36 @@ fdescfs /dev/fd fdescfs rw 0 0 self.assertEqual(expected_cfg, dsrc.network_config) mock_fallback.assert_not_called() + @mock.patch('cloudinit.net.get_interface_mac') + @mock.patch('cloudinit.net.get_devicelist') + @mock.patch('cloudinit.net.device_driver') + @mock.patch('cloudinit.net.generate_fallback_config') + def test_imds_network_ignored_when_apply_network_config_false( + self, mock_fallback, mock_dd, mock_devlist, mock_get_mac): + """When apply_network_config is False, use fallback instead of IMDS.""" + sys_cfg = {'datasource': {'Azure': {'apply_network_config': False}}} + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': sys_cfg} + fallback_config = { + 'version': 1, + 'config': [{ + 'type': 'physical', 'name': 'eth0', + 'mac_address': '00:11:22:33:44:55', + 'params': {'driver': 'hv_netsvc'}, + 'subnets': [{'type': 'dhcp'}], + }] + } + mock_fallback.return_value = fallback_config + + mock_devlist.return_value = ['eth0'] + mock_dd.return_value = ['hv_netsvc'] + mock_get_mac.return_value = '00:11:22:33:44:55' + + dsrc = self._get_ds(data) + self.assertTrue(dsrc.get_data()) + self.assertEqual(dsrc.network_config, fallback_config) + @mock.patch('cloudinit.net.get_interface_mac') @mock.patch('cloudinit.net.get_devicelist') @mock.patch('cloudinit.net.device_driver') -- cgit v1.2.3 From d74d3f0ff5c8d453f626b113f4e6065322f822fa Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 30 Oct 2018 20:02:38 +0000 Subject: query: better error when missing read permission on instance-data Emit a permissions error instead of "Missing instance-data.json" when non-root user doesn't have read-permission on /run/cloud-init/instance-data.json --- cloudinit/cmd/devel/render.py | 12 ++++++++---- cloudinit/cmd/query.py | 8 ++++++-- cloudinit/cmd/tests/test_query.py | 27 +++++++++++++++++++++++---- cloudinit/handlers/jinja_template.py | 10 +++++++++- doc/rtd/topics/network-config-format-v1.rst | 2 +- tests/unittests/test_builtin_handlers.py | 25 +++++++++++++++++++++++++ 6 files changed, 72 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/cloudinit/cmd/devel/render.py b/cloudinit/cmd/devel/render.py index 4d3ec958..1bc22406 100755 --- a/cloudinit/cmd/devel/render.py +++ b/cloudinit/cmd/devel/render.py @@ -71,10 +71,14 @@ def handle_args(name, args): except IOError: LOG.error('Missing user-data file: %s', args.user_data) return 1 - rendered_payload = render_jinja_payload_from_file( - payload=user_data, payload_fn=args.user_data, - instance_data_file=instance_data_fn, - debug=True if args.debug else False) + try: + rendered_payload = render_jinja_payload_from_file( + payload=user_data, payload_fn=args.user_data, + instance_data_file=instance_data_fn, + debug=True if args.debug else False) + except RuntimeError as e: + LOG.error('Cannot render from instance data: %s', str(e)) + return 1 if not rendered_payload: LOG.error('Unable to render user-data file: %s', args.user_data) return 1 diff --git a/cloudinit/cmd/query.py b/cloudinit/cmd/query.py index ff03de94..1d888b9d 100644 --- a/cloudinit/cmd/query.py +++ b/cloudinit/cmd/query.py @@ -3,6 +3,7 @@ """Query standardized instance metadata from the command line.""" import argparse +from errno import EACCES import os import six import sys @@ -106,8 +107,11 @@ def handle_args(name, args): try: instance_json = util.load_file(instance_data_fn) - except IOError: - LOG.error('Missing instance-data.json file: %s', instance_data_fn) + except (IOError, OSError) as e: + if e.errno == EACCES: + LOG.error("No read permission on '%s'. Try sudo", instance_data_fn) + else: + LOG.error('Missing instance-data file: %s', instance_data_fn) return 1 instance_data = util.load_json(instance_json) diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py index 241f5413..28738b1e 100644 --- a/cloudinit/cmd/tests/test_query.py +++ b/cloudinit/cmd/tests/test_query.py @@ -1,5 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. +import errno from six import StringIO from textwrap import dedent import os @@ -51,10 +52,28 @@ class TestQuery(CiTestCase): with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: self.assertEqual(1, query.handle_args('anyname', args)) self.assertIn( - 'ERROR: Missing instance-data.json file: %s' % absent_fn, + 'ERROR: Missing instance-data file: %s' % absent_fn, self.logs.getvalue()) self.assertIn( - 'ERROR: Missing instance-data.json file: %s' % absent_fn, + 'ERROR: Missing instance-data file: %s' % absent_fn, + m_stderr.getvalue()) + + def test_handle_args_error_when_no_read_permission_instance_data(self): + """When instance_data file is unreadable, log an error.""" + noread_fn = self.tmp_path('unreadable', dir=self.tmp) + write_file(noread_fn, 'thou shall not pass') + args = self.args( + debug=False, dump_all=True, format=None, instance_data=noread_fn, + list_keys=False, user_data='ud', vendor_data='vd', varname=None) + with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: + with mock.patch('cloudinit.cmd.query.util.load_file') as m_load: + m_load.side_effect = OSError(errno.EACCES, 'Not allowed') + self.assertEqual(1, query.handle_args('anyname', args)) + self.assertIn( + "ERROR: No read permission on '%s'. Try sudo" % noread_fn, + self.logs.getvalue()) + self.assertIn( + "ERROR: No read permission on '%s'. Try sudo" % noread_fn, m_stderr.getvalue()) def test_handle_args_defaults_instance_data(self): @@ -71,10 +90,10 @@ class TestQuery(CiTestCase): self.assertEqual(1, query.handle_args('anyname', args)) json_file = os.path.join(run_dir, INSTANCE_JSON_FILE) self.assertIn( - 'ERROR: Missing instance-data.json file: %s' % json_file, + 'ERROR: Missing instance-data file: %s' % json_file, self.logs.getvalue()) self.assertIn( - 'ERROR: Missing instance-data.json file: %s' % json_file, + 'ERROR: Missing instance-data file: %s' % json_file, m_stderr.getvalue()) def test_handle_args_root_fallsback_to_instance_data(self): diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py index 3fa4097e..ce3accf6 100644 --- a/cloudinit/handlers/jinja_template.py +++ b/cloudinit/handlers/jinja_template.py @@ -1,5 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. +from errno import EACCES import os import re @@ -76,7 +77,14 @@ def render_jinja_payload_from_file( raise RuntimeError( 'Cannot render jinja template vars. Instance data not yet' ' present at %s' % instance_data_file) - instance_data = load_json(load_file(instance_data_file)) + try: + instance_data = load_json(load_file(instance_data_file)) + except (IOError, OSError) as e: + if e.errno == EACCES: + raise RuntimeError( + 'Cannot render jinja template vars. No read permission on' + " '%s'. Try sudo" % instance_data_file) + rendered_payload = render_jinja_payload( payload, payload_fn, instance_data, debug) if not rendered_payload: diff --git a/doc/rtd/topics/network-config-format-v1.rst b/doc/rtd/topics/network-config-format-v1.rst index 83520000..3b0148ca 100644 --- a/doc/rtd/topics/network-config-format-v1.rst +++ b/doc/rtd/topics/network-config-format-v1.rst @@ -332,7 +332,7 @@ the following keys: - type: static address: 192.168.23.14/27 gateway: 192.168.23.1 - - type: nameserver + - type: nameserver: address: - 192.168.23.2 - 8.8.8.8 diff --git a/tests/unittests/test_builtin_handlers.py b/tests/unittests/test_builtin_handlers.py index abe820e1..b92ffc79 100644 --- a/tests/unittests/test_builtin_handlers.py +++ b/tests/unittests/test_builtin_handlers.py @@ -3,6 +3,7 @@ """Tests of the built-in user data handlers.""" import copy +import errno import os import shutil import tempfile @@ -202,6 +203,30 @@ class TestJinjaTemplatePartHandler(CiTestCase): os.path.exists(script_file), 'Unexpected file created %s' % script_file) + def test_jinja_template_handle_errors_on_unreadable_instance_data(self): + """If instance-data is unreadable, raise an error from handle_part.""" + script_handler = ShellScriptPartHandler(self.paths) + instance_json = os.path.join(self.run_dir, 'instance-data.json') + util.write_file(instance_json, util.json_dumps({})) + h = JinjaTemplatePartHandler( + self.paths, sub_handlers=[script_handler]) + with mock.patch(self.mpath + 'load_file') as m_load: + with self.assertRaises(RuntimeError) as context_manager: + m_load.side_effect = OSError(errno.EACCES, 'Not allowed') + h.handle_part( + data='data', ctype="!" + handlers.CONTENT_START, + filename='part01', + payload='## template: jinja \n#!/bin/bash\necho himom', + frequency='freq', headers='headers') + script_file = os.path.join(script_handler.script_dir, 'part01') + self.assertEqual( + 'Cannot render jinja template vars. No read permission on' + " '{rdir}/instance-data.json'. Try sudo".format(rdir=self.run_dir), + str(context_manager.exception)) + self.assertFalse( + os.path.exists(script_file), + 'Unexpected file created %s' % script_file) + @skipUnlessJinja() def test_jinja_template_handle_renders_jinja_content(self): """When present, render jinja variables from instance-data.json.""" -- cgit v1.2.3 From 907395104bb5850d221924365102cc5ab0eca2f1 Mon Sep 17 00:00:00 2001 From: asakkurr Date: Wed, 31 Oct 2018 20:19:15 +0000 Subject: azure: report ready to fabric after reprovision and reduce logging When reusing a preprovisioned VM, report ready to Azure fabric as soon as we get the reprovision data and the goal state so that we are not delayed by the cloud-init stage switch, saving 2-3 seconds. Also reduce logging when polling IMDS for reprovision data. LP: #1799594 --- cloudinit/sources/DataSourceAzure.py | 15 ++++++-- cloudinit/url_helper.py | 17 +++++---- tests/unittests/test_datasource/test_azure.py | 51 +++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index d0358e96..8642915e 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -267,6 +267,7 @@ class DataSourceAzure(sources.DataSource): dsname = 'Azure' _negotiated = False _metadata_imds = sources.UNSET + lease_info = None def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) @@ -406,8 +407,10 @@ class DataSourceAzure(sources.DataSource): LOG.warning("%s was not mountable", cdev) continue + should_report_ready_after_reprovision = False if reprovision or self._should_reprovision(ret): ret = self._reprovision() + should_report_ready_after_reprovision = True imds_md = get_metadata_from_imds( self.fallback_interface, retries=3) (md, userdata_raw, cfg, files) = ret @@ -434,6 +437,11 @@ class DataSourceAzure(sources.DataSource): crawled_data['metadata']['random_seed'] = seed crawled_data['metadata']['instance-id'] = util.read_dmi_data( 'system-uuid') + + if should_report_ready_after_reprovision: + LOG.info("Reporting ready to Azure after getting ReprovisionData") + self._report_ready(lease=self.lease_info) + return crawled_data def _is_platform_viable(self): @@ -522,6 +530,7 @@ class DataSourceAzure(sources.DataSource): while True: try: with EphemeralDHCPv4() as lease: + self.lease_info = lease if report_ready: path = REPORTED_READY_MARKER_FILE LOG.info( @@ -531,13 +540,13 @@ class DataSourceAzure(sources.DataSource): self._report_ready(lease=lease) report_ready = False return readurl(url, timeout=1, headers=headers, - exception_cb=exc_cb, infinite=True).contents + exception_cb=exc_cb, infinite=True, + log_req_resp=False).contents except UrlError: pass def _report_ready(self, lease): - """Tells the fabric provisioning has completed - before we go into our polling loop.""" + """Tells the fabric provisioning has completed """ try: get_metadata_from_fabric(None, lease['unknown-245']) except Exception: diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index 8067979e..cf57dbd5 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -199,7 +199,7 @@ def _get_ssl_args(url, ssl_details): def readurl(url, data=None, timeout=None, retries=0, sec_between=1, headers=None, headers_cb=None, ssl_details=None, check_status=True, allow_redirects=True, exception_cb=None, - session=None, infinite=False): + session=None, infinite=False, log_req_resp=True): url = _cleanurl(url) req_args = { 'url': url, @@ -256,9 +256,11 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1, continue filtered_req_args[k] = v try: - LOG.debug("[%s/%s] open '%s' with %s configuration", i, - "infinite" if infinite else manual_tries, url, - filtered_req_args) + + if log_req_resp: + LOG.debug("[%s/%s] open '%s' with %s configuration", i, + "infinite" if infinite else manual_tries, url, + filtered_req_args) if session is None: session = requests.Session() @@ -294,8 +296,11 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1, break if (infinite and sec_between > 0) or \ (i + 1 < manual_tries and sec_between > 0): - LOG.debug("Please wait %s seconds while we wait to try again", - sec_between) + + if log_req_resp: + LOG.debug( + "Please wait %s seconds while we wait to try again", + sec_between) time.sleep(sec_between) if excps: raise excps[-1] diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index cd6e7e74..4c5c6c12 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -513,6 +513,57 @@ fdescfs /dev/fd fdescfs rw 0 0 dsrc.crawl_metadata() self.assertEqual(str(cm.exception), error_msg) + @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') + @mock.patch( + 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') + @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds') + def test_crawl_metadata_on_reprovision_reports_ready( + self, poll_imds_func, + report_ready_func, + m_write): + """If reprovisioning, report ready at the end""" + ovfenv = construct_valid_ovf_env( + platform_settings={"PreprovisionedVm": "True"}) + + data = {'ovfcontent': ovfenv, + 'sys_cfg': {}} + dsrc = self._get_ds(data) + poll_imds_func.return_value = ovfenv + dsrc.crawl_metadata() + self.assertEqual(1, report_ready_func.call_count) + + @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') + @mock.patch( + 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') + @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') + @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') + @mock.patch('cloudinit.sources.DataSourceAzure.readurl') + def test_crawl_metadata_on_reprovision_reports_ready_using_lease( + self, m_readurl, m_dhcp, + m_net, report_ready_func, + m_write): + """If reprovisioning, report ready using the obtained lease""" + ovfenv = construct_valid_ovf_env( + platform_settings={"PreprovisionedVm": "True"}) + + data = {'ovfcontent': ovfenv, + 'sys_cfg': {}} + dsrc = self._get_ds(data) + + lease = { + 'interface': 'eth9', 'fixed-address': '192.168.2.9', + 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', + 'unknown-245': '624c3620'} + m_dhcp.return_value = [lease] + + reprovision_ovfenv = construct_valid_ovf_env() + m_readurl.return_value = url_helper.StringResponse( + reprovision_ovfenv.encode('utf-8')) + + dsrc.crawl_metadata() + self.assertEqual(2, report_ready_func.call_count) + report_ready_func.assert_called_with(lease=lease) + def test_waagent_d_has_0700_perms(self): # we expect /var/lib/waagent to be created 0700 dsrc = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) -- cgit v1.2.3 From 093f968013f418d591d9e778a57b2f050ac30db3 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 8 Nov 2018 04:19:09 +0000 Subject: tests: ec2 mock missing httpretty user-data and instance-identity routes --- tests/unittests/test_datasource/test_ec2.py | 40 +++++++++++++++++------------ 1 file changed, 24 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 9f81255a..1a5956d9 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -211,9 +211,9 @@ class TestEc2(test_helpers.HttprettyTestCase): self.metadata_addr = self.datasource.metadata_urls[0] self.tmp = self.tmp_dir() - def data_url(self, version): + def data_url(self, version, data_item='meta-data'): """Return a metadata url based on the version provided.""" - return '/'.join([self.metadata_addr, version, 'meta-data', '']) + return '/'.join([self.metadata_addr, version, data_item]) def _patch_add_cleanup(self, mpath, *args, **kwargs): p = mock.patch(mpath, *args, **kwargs) @@ -238,10 +238,18 @@ class TestEc2(test_helpers.HttprettyTestCase): all_versions = ( [ds.min_metadata_version] + ds.extended_metadata_versions) for version in all_versions: - metadata_url = self.data_url(version) + metadata_url = self.data_url(version) + '/' if version == md_version: # Register all metadata for desired version - register_mock_metaserver(metadata_url, md) + register_mock_metaserver( + metadata_url, md.get('md', DEFAULT_METADATA)) + userdata_url = self.data_url( + version, data_item='user-data') + register_mock_metaserver(userdata_url, md.get('ud', '')) + identity_url = self.data_url( + version, data_item='dynamic/instance-identity') + register_mock_metaserver( + identity_url, md.get('id', DYNAMIC_METADATA)) else: instance_id_url = metadata_url + 'instance-id' if version == ds.min_metadata_version: @@ -261,7 +269,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) find_fallback_path = ( 'cloudinit.sources.DataSourceEc2.net.find_fallback_nic') with mock.patch(find_fallback_path) as m_find_fallback: @@ -293,7 +301,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) find_fallback_path = ( 'cloudinit.sources.DataSourceEc2.net.find_fallback_nic') with mock.patch(find_fallback_path) as m_find_fallback: @@ -322,7 +330,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ds._network_config = {'cached': 'data'} self.assertEqual({'cached': 'data'}, ds.network_config) @@ -338,7 +346,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=old_metadata) + md={'md': old_metadata}) self.assertTrue(ds.get_data()) # Provide new revision of metadata that contains network data register_mock_metaserver( @@ -372,7 +380,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) # Mock 404s on all versions except latest all_versions = ( [ds.min_metadata_version] + ds.extended_metadata_versions) @@ -399,7 +407,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertTrue(ret) self.assertEqual(0, m_dhcp.call_count) @@ -412,7 +420,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertTrue(ret) @@ -422,7 +430,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''}, sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertFalse(ret) @@ -432,7 +440,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data={'uuid': uuid, 'uuid_source': 'dmi', 'serial': ''}, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertTrue(ret) @@ -442,7 +450,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) platform_attrs = [ attr for attr in ec2.CloudNames.__dict__.keys() if not attr.startswith('__')] @@ -469,7 +477,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertFalse(ret) self.assertIn( @@ -499,7 +507,7 @@ class TestEc2(test_helpers.HttprettyTestCase): ds = self._setup_ds( platform_data=self.valid_platform_data, sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, - md=DEFAULT_METADATA) + md={'md': DEFAULT_METADATA}) ret = ds.get_data() self.assertTrue(ret) -- cgit v1.2.3 From d910ecd15de642d73a36e935704e54370f93c45b Mon Sep 17 00:00:00 2001 From: asakkurr Date: Mon, 12 Nov 2018 17:16:09 +0000 Subject: azure: fix regression introduced when persisting ephemeral dhcp lease In commitish 9073951 azure datasource tried to leverage stale DHCP information obtained from EphemeralDHCPv4 context manager to report updated provisioning status to the fabric earlier in the boot process. Unfortunately the stale ephemeral network configuration had already been torn down in preparation to bring up IMDS network config so the report attempt failed on timeout. This branch introduces obtain_lease and clean_network public methods on EphemeralDHCPv4 to allow for setup and teardown of ephemeral network configuration without using a context manager. Azure datasource now uses this to persist ephemeral network configuration across multiple contexts during provisioning to avoid multiple DHCP roundtrips. --- cloudinit/net/dhcp.py | 42 ++++++++++++++++++------ cloudinit/sources/DataSourceAzure.py | 47 ++++++++++++++++----------- tests/unittests/test_datasource/test_azure.py | 3 +- 3 files changed, 62 insertions(+), 30 deletions(-) (limited to 'tests') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 12cf5097..bdc5799f 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -40,34 +40,56 @@ class EphemeralDHCPv4(object): def __init__(self, iface=None): self.iface = iface self._ephipv4 = None + self.lease = None def __enter__(self): + """Setup sandboxed dhcp context.""" + return self.obtain_lease() + + def __exit__(self, excp_type, excp_value, excp_traceback): + """Teardown sandboxed dhcp context.""" + self.clean_network() + + def clean_network(self): + """Exit _ephipv4 context to teardown of ip configuration performed.""" + if self.lease: + self.lease = None + if not self._ephipv4: + return + self._ephipv4.__exit__(None, None, None) + + def obtain_lease(self): + """Perform dhcp discovery in a sandboxed environment if possible. + + @return: A dict representing dhcp options on the most recent lease + obtained from the dhclient discovery if run, otherwise an error + is raised. + + @raises: NoDHCPLeaseError if no leases could be obtained. + """ + if self.lease: + return self.lease try: leases = maybe_perform_dhcp_discovery(self.iface) except InvalidDHCPLeaseFileError: raise NoDHCPLeaseError() if not leases: raise NoDHCPLeaseError() - lease = leases[-1] + self.lease = leases[-1] LOG.debug("Received dhcp lease on %s for %s/%s", - lease['interface'], lease['fixed-address'], - lease['subnet-mask']) + self.lease['interface'], self.lease['fixed-address'], + self.lease['subnet-mask']) nmap = {'interface': 'interface', 'ip': 'fixed-address', 'prefix_or_mask': 'subnet-mask', 'broadcast': 'broadcast-address', 'router': 'routers'} - kwargs = dict([(k, lease.get(v)) for k, v in nmap.items()]) + kwargs = dict([(k, self.lease.get(v)) for k, v in nmap.items()]) if not kwargs['broadcast']: kwargs['broadcast'] = bcip(kwargs['prefix_or_mask'], kwargs['ip']) ephipv4 = EphemeralIPv4Network(**kwargs) ephipv4.__enter__() self._ephipv4 = ephipv4 - return lease - - def __exit__(self, excp_type, excp_value, excp_traceback): - if not self._ephipv4: - return - self._ephipv4.__exit__(excp_type, excp_value, excp_traceback) + return self.lease def maybe_perform_dhcp_discovery(nic=None): diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 7bdd43d8..5ec6096f 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -267,7 +267,6 @@ class DataSourceAzure(sources.DataSource): dsname = 'Azure' _negotiated = False _metadata_imds = sources.UNSET - lease_info = None def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) @@ -281,6 +280,7 @@ class DataSourceAzure(sources.DataSource): self._network_config = None # Regenerate network config new_instance boot and every boot self.update_events['network'].add(EventType.BOOT) + self._ephemeral_dhcp_ctx = None def __str__(self): root = sources.DataSource.__str__(self) @@ -407,10 +407,9 @@ class DataSourceAzure(sources.DataSource): LOG.warning("%s was not mountable", cdev) continue - should_report_ready_after_reprovision = False - if reprovision or self._should_reprovision(ret): + perform_reprovision = reprovision or self._should_reprovision(ret) + if perform_reprovision: ret = self._reprovision() - should_report_ready_after_reprovision = True imds_md = get_metadata_from_imds( self.fallback_interface, retries=3) (md, userdata_raw, cfg, files) = ret @@ -438,9 +437,16 @@ class DataSourceAzure(sources.DataSource): crawled_data['metadata']['instance-id'] = util.read_dmi_data( 'system-uuid') - if should_report_ready_after_reprovision: + if perform_reprovision: LOG.info("Reporting ready to Azure after getting ReprovisionData") - self._report_ready(lease=self.lease_info) + use_cached_ephemeral = (net.is_up(self.fallback_interface) and + getattr(self, '_ephemeral_dhcp_ctx', None)) + if use_cached_ephemeral: + self._report_ready(lease=self._ephemeral_dhcp_ctx.lease) + self._ephemeral_dhcp_ctx.clean_network() # Teardown ephemeral + else: + with EphemeralDHCPv4() as lease: + self._report_ready(lease=lease) return crawled_data @@ -529,20 +535,23 @@ class DataSourceAzure(sources.DataSource): while True: try: - with EphemeralDHCPv4() as lease: - self.lease_info = lease - if report_ready: - path = REPORTED_READY_MARKER_FILE - LOG.info( - "Creating a marker file to report ready: %s", path) - util.write_file(path, "{pid}: {time}\n".format( - pid=os.getpid(), time=time())) - self._report_ready(lease=lease) - report_ready = False - return readurl(url, timeout=1, headers=headers, - exception_cb=exc_cb, infinite=True, - log_req_resp=False).contents + # Save our EphemeralDHCPv4 context so we avoid repeated dhcp + self._ephemeral_dhcp_ctx = EphemeralDHCPv4() + lease = self._ephemeral_dhcp_ctx.obtain_lease() + if report_ready: + path = REPORTED_READY_MARKER_FILE + LOG.info( + "Creating a marker file to report ready: %s", path) + util.write_file(path, "{pid}: {time}\n".format( + pid=os.getpid(), time=time())) + self._report_ready(lease=lease) + report_ready = False + return readurl(url, timeout=1, headers=headers, + exception_cb=exc_cb, infinite=True, + log_req_resp=False).contents except UrlError: + # Teardown our EphemeralDHCPv4 context on failure as we retry + self._ephemeral_dhcp_ctx.clean_network() pass def _report_ready(self, lease): diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 4c5c6c12..1dc69adb 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -513,6 +513,7 @@ fdescfs /dev/fd fdescfs rw 0 0 dsrc.crawl_metadata() self.assertEqual(str(cm.exception), error_msg) + @mock.patch('cloudinit.sources.DataSourceAzure.EphemeralDHCPv4') @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') @mock.patch( 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') @@ -520,7 +521,7 @@ fdescfs /dev/fd fdescfs rw 0 0 def test_crawl_metadata_on_reprovision_reports_ready( self, poll_imds_func, report_ready_func, - m_write): + m_write, m_dhcp): """If reprovisioning, report ready at the end""" ovfenv = construct_valid_ovf_env( platform_settings={"PreprovisionedVm": "True"}) -- cgit v1.2.3 From 6f9512049bbb594c3f01ffcd2ab25ae4e016f01e Mon Sep 17 00:00:00 2001 From: Jason Zions Date: Mon, 12 Nov 2018 18:43:42 +0000 Subject: azure: Accept variation in error msg from mount for ntfs volumes If Azure detects an ntfs filesystem type during mount attempt, it should still report the resource device as reformattable. There are slight differences in error message format on RedHat and SuSE. This patch simplifies the expected error match to work on both distributions. LP: #1799338 --- cloudinit/sources/DataSourceAzure.py | 2 +- tests/unittests/test_datasource/test_azure.py | 29 +++++++++++++-------------- 2 files changed, 15 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 5ec6096f..6e1797ea 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -725,7 +725,7 @@ def can_dev_be_reformatted(devpath, preserve_ntfs): file_count = util.mount_cb(cand_path, count_files, mtype="ntfs", update_env_for_mount={'LANG': 'C'}) except util.MountFailedError as e: - if "mount: unknown filesystem type 'ntfs'" in str(e): + if "unknown filesystem type 'ntfs'" in str(e): return True, (bmsg + ' but this system cannot mount NTFS,' ' assuming there are no important files.' ' Formatting allowed.') diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 1dc69adb..8ad4368c 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1511,21 +1511,20 @@ class TestCanDevBeReformatted(CiTestCase): '/dev/sda1': {'num': 1, 'fs': 'ntfs', 'files': []} }}}) - err = ("Unexpected error while running command.\n", - "Command: ['mount', '-o', 'ro,sync', '-t', 'auto', ", - "'/dev/sda1', '/fake-tmp/dir']\n" - "Exit code: 32\n" - "Reason: -\n" - "Stdout: -\n" - "Stderr: mount: unknown filesystem type 'ntfs'") - self.m_mount_cb.side_effect = MountFailedError( - 'Failed mounting %s to %s due to: %s' % - ('/dev/sda', '/fake-tmp/dir', err)) - - value, msg = dsaz.can_dev_be_reformatted('/dev/sda', - preserve_ntfs=False) - self.assertTrue(value) - self.assertIn('cannot mount NTFS, assuming', msg) + error_msgs = [ + "Stderr: mount: unknown filesystem type 'ntfs'", # RHEL + "Stderr: mount: /dev/sdb1: unknown filesystem type 'ntfs'" # SLES + ] + + for err_msg in error_msgs: + self.m_mount_cb.side_effect = MountFailedError( + "Failed mounting %s to %s due to: \nUnexpected.\n%s" % + ('/dev/sda', '/fake-tmp/dir', err_msg)) + + value, msg = dsaz.can_dev_be_reformatted('/dev/sda', + preserve_ntfs=False) + self.assertTrue(value) + self.assertIn('cannot mount NTFS, assuming', msg) def test_never_destroy_ntfs_config_false(self): """Normally formattable situation with never_destroy_ntfs set.""" -- cgit v1.2.3 From 6062595b83e08e0f12e1fe6d8e367d8db9d91ef8 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 13 Nov 2018 03:14:58 +0000 Subject: azure: retry imds polling on requests.Timeout There is an infrequent race when the booting instance can hit the IMDS service before it is fully available. This results in a requests.ConnectTimeout being raised. Azure's retry_callback logic now retries on either 404s or Timeouts. LP:1800223 --- cloudinit/sources/DataSourceAzure.py | 18 +++------------- cloudinit/tests/test_url_helper.py | 25 +++++++++++++++++++++- cloudinit/url_helper.py | 14 +++++++++++++ tests/unittests/test_datasource/test_azure.py | 30 +++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 6e1797ea..9e8a1a8b 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -22,7 +22,7 @@ from cloudinit.event import EventType from cloudinit.net.dhcp import EphemeralDHCPv4 from cloudinit import sources from cloudinit.sources.helpers.azure import get_metadata_from_fabric -from cloudinit.url_helper import readurl, UrlError +from cloudinit.url_helper import UrlError, readurl, retry_on_url_exc from cloudinit import util LOG = logging.getLogger(__name__) @@ -526,13 +526,6 @@ class DataSourceAzure(sources.DataSource): report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE)) LOG.debug("Start polling IMDS") - def exc_cb(msg, exception): - if isinstance(exception, UrlError) and exception.code == 404: - return True - # If we get an exception while trying to call IMDS, we - # call DHCP and setup the ephemeral network to acquire the new IP. - return False - while True: try: # Save our EphemeralDHCPv4 context so we avoid repeated dhcp @@ -547,7 +540,7 @@ class DataSourceAzure(sources.DataSource): self._report_ready(lease=lease) report_ready = False return readurl(url, timeout=1, headers=headers, - exception_cb=exc_cb, infinite=True, + exception_cb=retry_on_url_exc, infinite=True, log_req_resp=False).contents except UrlError: # Teardown our EphemeralDHCPv4 context on failure as we retry @@ -1187,17 +1180,12 @@ def get_metadata_from_imds(fallback_nic, 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 - 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) + exception_cb=retry_on_url_exc) except Exception as e: LOG.debug('Ignoring IMDS instance metadata: %s', e) return {} diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index 113249d9..aa9f3ec1 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -1,10 +1,12 @@ # This file is part of cloud-init. See LICENSE file for license information. -from cloudinit.url_helper import oauth_headers, read_file_or_url +from cloudinit.url_helper import ( + NOT_FOUND, UrlError, oauth_headers, read_file_or_url, retry_on_url_exc) from cloudinit.tests.helpers import CiTestCase, mock, skipIf from cloudinit import util import httpretty +import requests try: @@ -64,3 +66,24 @@ class TestReadFileOrUrl(CiTestCase): result = read_file_or_url(url) self.assertEqual(result.contents, data) self.assertEqual(str(result), data.decode('utf-8')) + + +class TestRetryOnUrlExc(CiTestCase): + + def test_do_not_retry_non_urlerror(self): + """When exception is not UrlError return False.""" + myerror = IOError('something unexcpected') + self.assertFalse(retry_on_url_exc(msg='', exc=myerror)) + + def test_perform_retries_on_not_found(self): + """When exception is UrlError with a 404 status code return True.""" + myerror = UrlError(cause=RuntimeError( + 'something was not found'), code=NOT_FOUND) + self.assertTrue(retry_on_url_exc(msg='', exc=myerror)) + + def test_perform_retries_on_timeout(self): + """When exception is a requests.Timout return True.""" + myerror = UrlError(cause=requests.Timeout('something timed out')) + self.assertTrue(retry_on_url_exc(msg='', exc=myerror)) + +# vi: ts=4 expandtab diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index cf57dbd5..396d69ae 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -554,4 +554,18 @@ def oauth_headers(url, consumer_key, token_key, token_secret, consumer_secret, _uri, signed_headers, _body = client.sign(url) return signed_headers + +def retry_on_url_exc(msg, exc): + """readurl exception_cb that will retry on NOT_FOUND and Timeout. + + Returns False to raise the exception from readurl, True to retry. + """ + if not isinstance(exc, UrlError): + return False + if exc.code == NOT_FOUND: + return True + if exc.cause and isinstance(exc.cause, requests.Timeout): + return True + return False + # vi: ts=4 expandtab diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 8ad4368c..56484b27 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -17,6 +17,7 @@ import crypt import httpretty import json import os +import requests import stat import xml.etree.ElementTree as ET import yaml @@ -184,6 +185,35 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): "Crawl of Azure Instance Metadata Service (IMDS) took", # log_time self.logs.getvalue()) + @mock.patch('requests.Session.request') + @mock.patch('cloudinit.url_helper.time.sleep') + @mock.patch(MOCKPATH + 'net.is_up') + def test_get_metadata_from_imds_retries_on_timeout( + self, m_net_is_up, m_sleep, m_request): + """Retry IMDS network metadata on timeout errors.""" + + self.attempt = 0 + m_request.side_effect = requests.Timeout('Fake Connection Timeout') + + def retry_callback(request, uri, headers): + self.attempt += 1 + raise requests.Timeout('Fake connection timeout') + + httpretty.register_uri( + httpretty.GET, + dsaz.IMDS_URL + 'instance?api-version=2017-12-01', + body=retry_callback) + + m_net_is_up.return_value = True # skips dhcp + + self.assertEqual({}, dsaz.get_metadata_from_imds('eth9', retries=3)) + + m_net_is_up.assert_called_with('eth9') + self.assertEqual([mock.call(1)]*3, m_sleep.call_args_list) + self.assertIn( + "Crawl of Azure Instance Metadata Service (IMDS) took", # log_time + self.logs.getvalue()) + class TestAzureDataSource(CiTestCase): -- cgit v1.2.3 From a3812a4ab4eeb2aa185eb4a2de186cc60ddd7c03 Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Wed, 14 Nov 2018 22:02:18 +0000 Subject: resizefs: Prefix discovered devpath with '/dev/' when path does not exist In some environments, like FreeBSD, gpart can return the device basename instead of the full path. If this discovered devpath does not exist and is missing the '/dev/' prefix, add that prefix in an attempt to find the device. --- cloudinit/config/cc_resizefs.py | 7 ++++ .../test_handler/test_handler_resizefs.py | 48 ++++++++++++++++++---- 2 files changed, 47 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index 2edddd0c..076b9d5a 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -197,6 +197,13 @@ def maybe_get_writable_device_path(devpath, info, log): if devpath.startswith('gpt/'): log.debug('We have a gpt label - just go ahead') return devpath + # Alternatively, our device could simply be a name as returned by gpart, + # such as da0p3 + if not devpath.startswith('/dev/') and not os.path.exists(devpath): + fulldevpath = '/dev/' + devpath.lstrip('/') + log.debug("'%s' doesn't appear to be a valid device path. Trying '%s'", + devpath, fulldevpath) + devpath = fulldevpath try: statret = os.stat(devpath) diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py index feca56c2..6ebacb1a 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -173,6 +173,38 @@ class TestResizefs(CiTestCase): self.assertEqual(('zpool', 'online', '-e', 'vmzroot', disk), ret) + @mock.patch('cloudinit.util.is_container', return_value=False) + @mock.patch('cloudinit.util.get_mount_info') + @mock.patch('cloudinit.util.get_device_info_from_zpool') + @mock.patch('cloudinit.util.parse_mount') + def test_handle_modern_zfsroot(self, mount_info, zpool_info, parse_mount, + is_container): + devpth = 'zroot/ROOT/default' + disk = 'da0p3' + fs_type = 'zfs' + mount_point = '/' + + mount_info.return_value = (devpth, fs_type, mount_point) + zpool_info.return_value = disk + parse_mount.return_value = (devpth, fs_type, mount_point) + + cfg = {'resize_rootfs': True} + + def fake_stat(devpath): + if devpath == disk: + raise OSError("not here") + FakeStat = namedtuple( + 'FakeStat', ['st_mode', 'st_size', 'st_mtime']) # minimal stat + return FakeStat(25008, 0, 1) # fake char block device + + with mock.patch('cloudinit.config.cc_resizefs.do_resize') as dresize: + with mock.patch('cloudinit.config.cc_resizefs.os.stat') as m_stat: + m_stat.side_effect = fake_stat + handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[]) + + self.assertEqual(('zpool', 'online', '-e', 'zroot', '/dev/' + disk), + dresize.call_args[0][0]) + class TestRootDevFromCmdline(CiTestCase): @@ -246,39 +278,39 @@ class TestMaybeGetDevicePathAsWritableBlock(CiTestCase): def test_maybe_get_writable_device_path_does_not_exist(self): """When devpath does not exist, a warning is logged.""" - info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none' devpath = wrap_and_call( 'cloudinit.config.cc_resizefs.util', {'is_container': {'return_value': False}}, - maybe_get_writable_device_path, '/I/dont/exist', info, LOG) + maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG) self.assertIsNone(devpath) self.assertIn( - "WARNING: Device '/I/dont/exist' did not exist." + "WARNING: Device '/dev/I/dont/exist' did not exist." ' cannot resize: %s' % info, self.logs.getvalue()) def test_maybe_get_writable_device_path_does_not_exist_in_container(self): """When devpath does not exist in a container, log a debug message.""" - info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none' devpath = wrap_and_call( 'cloudinit.config.cc_resizefs.util', {'is_container': {'return_value': True}}, - maybe_get_writable_device_path, '/I/dont/exist', info, LOG) + maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG) self.assertIsNone(devpath) self.assertIn( - "DEBUG: Device '/I/dont/exist' did not exist in container." + "DEBUG: Device '/dev/I/dont/exist' did not exist in container." ' cannot resize: %s' % info, self.logs.getvalue()) def test_maybe_get_writable_device_path_raises_oserror(self): """When unexpected OSError is raises by os.stat it is reraised.""" - info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + info = 'dev=/dev/I/dont/exist mnt_point=/ path=/dev/none' with self.assertRaises(OSError) as context_manager: wrap_and_call( 'cloudinit.config.cc_resizefs', {'util.is_container': {'return_value': True}, 'os.stat': {'side_effect': OSError('Something unexpected')}}, - maybe_get_writable_device_path, '/I/dont/exist', info, LOG) + maybe_get_writable_device_path, '/dev/I/dont/exist', info, LOG) self.assertEqual( 'Something unexpected', str(context_manager.exception)) -- cgit v1.2.3 From 8f812a15fde01173c0dd5b7e1a77b61031fd93e4 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 15 Nov 2018 22:55:42 +0000 Subject: azure: _poll_imds only retry on 404. Fail on Timeout Upon URL timeout, _poll_imds is expected to re-dhcp to get updated IP configuration. We don't want to indefinitely retry because the instance likely has invalid IP configuration. LP: #1803598 --- cloudinit/sources/DataSourceAzure.py | 9 ++++++- tests/unittests/test_datasource/test_azure.py | 34 ++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 9e8a1a8b..2a3e5677 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -526,6 +526,13 @@ class DataSourceAzure(sources.DataSource): report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE)) LOG.debug("Start polling IMDS") + def exc_cb(msg, exception): + if isinstance(exception, UrlError) and exception.code == 404: + return True + # If we get an exception while trying to call IMDS, we + # call DHCP and setup the ephemeral network to acquire the new IP. + return False + while True: try: # Save our EphemeralDHCPv4 context so we avoid repeated dhcp @@ -540,7 +547,7 @@ class DataSourceAzure(sources.DataSource): self._report_ready(lease=lease) report_ready = False return readurl(url, timeout=1, headers=headers, - exception_cb=retry_on_url_exc, infinite=True, + exception_cb=exc_cb, infinite=True, log_req_resp=False).contents except UrlError: # Teardown our EphemeralDHCPv4 context on failure as we retry diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 56484b27..5ea7ae5e 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1687,22 +1687,44 @@ class TestPreprovisioningPollIMDS(CiTestCase): self.paths = helpers.Paths({'cloud_dir': self.tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d - @mock.patch(MOCKPATH + 'util.write_file') - def test_poll_imds_calls_report_ready(self, write_f, report_ready_func, + @mock.patch(MOCKPATH + 'EphemeralDHCPv4') + def test_poll_imds_re_dhcp_on_timeout(self, m_dhcpv4, report_ready_func, fake_resp, m_dhcp, m_net): - """The poll_imds will call report_ready after creating marker file.""" - report_marker = self.tmp_path('report_marker', self.tmp) + """The poll_imds will retry DHCP on IMDS timeout.""" + report_file = self.tmp_path('report_marker', self.tmp) lease = { 'interface': 'eth9', 'fixed-address': '192.168.2.9', 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'} m_dhcp.return_value = [lease] + + dhcp_ctx = mock.MagicMock(lease=lease) + dhcp_ctx.obtain_lease.return_value = lease + m_dhcpv4.return_value = dhcp_ctx + + self.tries = 0 + + def fake_timeout_once(**kwargs): + self.tries += 1 + if self.tries == 1: + raise requests.Timeout('Fake connection timeout') + elif self.tries == 2: + response = requests.Response() + response.status_code = 404 + raise requests.exceptions.HTTPError( + "fake 404", response=response) + # Third try should succeed and stop retries or redhcp + return mock.MagicMock(status_code=200, text="good", content="good") + + fake_resp.side_effect = fake_timeout_once + dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) - mock_path = (MOCKPATH + 'REPORTED_READY_MARKER_FILE') - with mock.patch(mock_path, report_marker): + with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 1) report_ready_func.assert_called_with(lease=lease) + self.assertEqual(2, m_dhcpv4.call_count, 'Expected 2 DHCP calls') + self.assertEqual(3, self.tries, 'Expected 3 total reads from IMDS') def test_poll_imds_report_ready_false(self, report_ready_func, fake_resp, m_dhcp, m_net): -- cgit v1.2.3 From 530850f971e36f4b0ef216cec9d889a99474ca0e Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 27 Nov 2018 21:39:42 +0000 Subject: OVF: identify label iso9660 filesystems with label 'OVF ENV'. When deploying an OVA, at least some versions of vmware attach a cdrom with an ISO9660 filesystem label of 'OVF ENV'. This was seen on Vmware vCenter Server, 6.0.0, 2776510. In order to accomplish this we had to change the content of the DI_ISO9660_DEVS variable to be comma delimited rather than space delimited. --- tests/unittests/test_ds_identify.py | 2 +- tools/ds-identify | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index 46778e95..80640f19 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -499,7 +499,7 @@ class TestDsIdentify(DsIdentifyBase): # Add recognized labels valid_ovf_labels = ['ovf-transport', 'OVF-TRANSPORT', - "OVFENV", "ovfenv"] + "OVFENV", "ovfenv", "OVF ENV", "ovf env"] for valid_ovf_label in valid_ovf_labels: ovf_cdrom_by_label['mocks'][0]['out'] = blkid_out([ {'DEVNAME': 'sda1', 'TYPE': 'ext4', 'LABEL': 'rootfs'}, diff --git a/tools/ds-identify b/tools/ds-identify index 5afe5aa1..1acfeeb9 100755 --- a/tools/ds-identify +++ b/tools/ds-identify @@ -237,7 +237,7 @@ read_fs_info() { case "${line}" in DEVNAME=*) [ -n "$dev" -a "$ftype" = "iso9660" ] && - isodevs="${isodevs} ${dev}=$label" + isodevs="${isodevs},${dev}=$label" ftype=""; dev=""; label=""; dev=${line#DEVNAME=};; LABEL=*) label="${line#LABEL=}"; @@ -247,11 +247,11 @@ read_fs_info() { esac done [ -n "$dev" -a "$ftype" = "iso9660" ] && - isodevs="${isodevs} ${dev}=$label" + isodevs="${isodevs},${dev}=$label" DI_FS_LABELS="${labels%${delim}}" DI_FS_UUIDS="${uuids%${delim}}" - DI_ISO9660_DEVS="${isodevs# }" + DI_ISO9660_DEVS="${isodevs#,}" } cached() { @@ -735,9 +735,10 @@ is_cdrom_ovf() { return 1;; esac + debug 1 "got label=$label" # fast path known 'OVF' labels case "$label" in - OVF-TRANSPORT|ovf-transport|OVFENV|ovfenv) return 0;; + OVF-TRANSPORT|ovf-transport|OVFENV|ovfenv|OVF\ ENV|ovf\ env) return 0;; esac # explicitly skip known labels of other types. rd_rdfe is azure. @@ -757,9 +758,13 @@ dscheck_OVF() { # Azure provides ovf. Skip false positive by dis-allowing. is_azure_chassis && return $DS_NOT_FOUND - # DI_ISO9660_DEVS is =label, like /dev/sr0=OVF-TRANSPORT + # DI_ISO9660_DEVS is =label,=label2 + # like /dev/sr0=OVF-TRANSPORT,/dev/other=with spaces if [ "${DI_ISO9660_DEVS#${UNAVAILABLE}:}" = "${DI_ISO9660_DEVS}" ]; then - for tok in ${DI_ISO9660_DEVS}; do + local oifs="$IFS" + # shellcheck disable=2086 + { IFS=","; set -- ${DI_ISO9660_DEVS}; IFS="$oifs"; } + for tok in "$@"; do is_cdrom_ovf "${tok%%=*}" "${tok#*=}" && return $DS_FOUND done fi -- cgit v1.2.3 From 4ce8a2858dffcb1f9518b50d884b341f68ac5e63 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 28 Nov 2018 18:23:33 +0000 Subject: tests: fix incorrect order of mocks in test_handle_zfs_root. The order of parameters to test_handle_zfs_root did not match the order of the mocks applied. Thanks to Jason Zions for pointing this out. --- tests/unittests/test_handler/test_handler_resizefs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py index 6ebacb1a..35187847 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -151,9 +151,9 @@ class TestResizefs(CiTestCase): _resize_ufs(mount_point, devpth)) @mock.patch('cloudinit.util.is_container', return_value=False) - @mock.patch('cloudinit.util.get_mount_info') - @mock.patch('cloudinit.util.get_device_info_from_zpool') @mock.patch('cloudinit.util.parse_mount') + @mock.patch('cloudinit.util.get_device_info_from_zpool') + @mock.patch('cloudinit.util.get_mount_info') def test_handle_zfs_root(self, mount_info, zpool_info, parse_mount, is_container): devpth = 'vmzroot/ROOT/freebsd' -- cgit v1.2.3 From bf7917159dbb292c9fcdef82b004e0f5ecb32c16 Mon Sep 17 00:00:00 2001 From: Tamilmani Manoharan Date: Thu, 29 Nov 2018 21:53:18 +0000 Subject: azure: detect vnet migration via netlink media change event Replace Azure pre-provision polling on IMDS with a blocking call which watches for netlink link state change messages. The media change event happens when a pre-provisioned VM has been activated and is connected to the users virtual network and cloud-init can then resume operation to complete image instantiation. --- cloudinit/sources/DataSourceAzure.py | 31 +- cloudinit/sources/helpers/netlink.py | 250 ++++++++++++++++ cloudinit/sources/helpers/tests/test_netlink.py | 373 ++++++++++++++++++++++++ tests/unittests/test_datasource/test_azure.py | 40 ++- 4 files changed, 678 insertions(+), 16 deletions(-) create mode 100644 cloudinit/sources/helpers/netlink.py create mode 100644 cloudinit/sources/helpers/tests/test_netlink.py (limited to 'tests') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index be82ec4d..e076d5dc 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -22,6 +22,7 @@ from cloudinit.event import EventType from cloudinit.net.dhcp import EphemeralDHCPv4 from cloudinit import sources from cloudinit.sources.helpers.azure import get_metadata_from_fabric +from cloudinit.sources.helpers import netlink from cloudinit.url_helper import UrlError, readurl, retry_on_url_exc from cloudinit import util @@ -409,6 +410,10 @@ class DataSourceAzure(sources.DataSource): perform_reprovision = reprovision or self._should_reprovision(ret) if perform_reprovision: + if util.is_FreeBSD(): + msg = "Free BSD is not supported for PPS VMs" + LOG.error(msg) + raise sources.InvalidMetaDataException(msg) ret = self._reprovision() imds_md = get_metadata_from_imds( self.fallback_interface, retries=3) @@ -523,8 +528,8 @@ class DataSourceAzure(sources.DataSource): response. Then return the returned JSON object.""" url = IMDS_URL + "reprovisiondata?api-version=2017-04-02" headers = {"Metadata": "true"} + nl_sock = None report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE)) - LOG.debug("Start polling IMDS") def exc_cb(msg, exception): if isinstance(exception, UrlError) and exception.code == 404: @@ -533,12 +538,19 @@ class DataSourceAzure(sources.DataSource): # call DHCP and setup the ephemeral network to acquire the new IP. return False + LOG.debug("Wait for vnetswitch to happen") while True: try: # Save our EphemeralDHCPv4 context so we avoid repeated dhcp self._ephemeral_dhcp_ctx = EphemeralDHCPv4() lease = self._ephemeral_dhcp_ctx.obtain_lease() if report_ready: + try: + nl_sock = netlink.create_bound_netlink_socket() + except netlink.NetlinkCreateSocketError as e: + LOG.warning(e) + self._ephemeral_dhcp_ctx.clean_network() + return path = REPORTED_READY_MARKER_FILE LOG.info( "Creating a marker file to report ready: %s", path) @@ -546,13 +558,24 @@ class DataSourceAzure(sources.DataSource): pid=os.getpid(), time=time())) self._report_ready(lease=lease) report_ready = False - return readurl(url, timeout=1, headers=headers, - exception_cb=exc_cb, infinite=True, - log_req_resp=False).contents + try: + netlink.wait_for_media_disconnect_connect( + nl_sock, lease['interface']) + except AssertionError as error: + LOG.error(error) + return + self._ephemeral_dhcp_ctx.clean_network() + else: + return readurl(url, timeout=1, headers=headers, + exception_cb=exc_cb, infinite=True, + log_req_resp=False).contents except UrlError: # Teardown our EphemeralDHCPv4 context on failure as we retry self._ephemeral_dhcp_ctx.clean_network() pass + finally: + if nl_sock: + nl_sock.close() def _report_ready(self, lease): """Tells the fabric provisioning has completed """ diff --git a/cloudinit/sources/helpers/netlink.py b/cloudinit/sources/helpers/netlink.py new file mode 100644 index 00000000..d377ae3d --- /dev/null +++ b/cloudinit/sources/helpers/netlink.py @@ -0,0 +1,250 @@ +# Author: Tamilmani Manoharan +# +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit import log as logging +from cloudinit import util +from collections import namedtuple + +import os +import select +import socket +import struct + +LOG = logging.getLogger(__name__) + +# http://man7.org/linux/man-pages/man7/netlink.7.html +RTMGRP_LINK = 1 +NLMSG_NOOP = 1 +NLMSG_ERROR = 2 +NLMSG_DONE = 3 +RTM_NEWLINK = 16 +RTM_DELLINK = 17 +RTM_GETLINK = 18 +RTM_SETLINK = 19 +MAX_SIZE = 65535 +RTA_DATA_OFFSET = 32 +MSG_TYPE_OFFSET = 16 +SELECT_TIMEOUT = 60 + +NLMSGHDR_FMT = "IHHII" +IFINFOMSG_FMT = "BHiII" +NLMSGHDR_SIZE = struct.calcsize(NLMSGHDR_FMT) +IFINFOMSG_SIZE = struct.calcsize(IFINFOMSG_FMT) +RTATTR_START_OFFSET = NLMSGHDR_SIZE + IFINFOMSG_SIZE +RTA_DATA_START_OFFSET = 4 +PAD_ALIGNMENT = 4 + +IFLA_IFNAME = 3 +IFLA_OPERSTATE = 16 + +# https://www.kernel.org/doc/Documentation/networking/operstates.txt +OPER_UNKNOWN = 0 +OPER_NOTPRESENT = 1 +OPER_DOWN = 2 +OPER_LOWERLAYERDOWN = 3 +OPER_TESTING = 4 +OPER_DORMANT = 5 +OPER_UP = 6 + +RTAAttr = namedtuple('RTAAttr', ['length', 'rta_type', 'data']) +InterfaceOperstate = namedtuple('InterfaceOperstate', ['ifname', 'operstate']) +NetlinkHeader = namedtuple('NetlinkHeader', ['length', 'type', 'flags', 'seq', + 'pid']) + + +class NetlinkCreateSocketError(RuntimeError): + '''Raised if netlink socket fails during create or bind.''' + pass + + +def create_bound_netlink_socket(): + '''Creates netlink socket and bind on netlink group to catch interface + down/up events. The socket will bound only on RTMGRP_LINK (which only + includes RTM_NEWLINK/RTM_DELLINK/RTM_GETLINK events). The socket is set to + non-blocking mode since we're only receiving messages. + + :returns: netlink socket in non-blocking mode + :raises: NetlinkCreateSocketError + ''' + try: + netlink_socket = socket.socket(socket.AF_NETLINK, + socket.SOCK_RAW, + socket.NETLINK_ROUTE) + netlink_socket.bind((os.getpid(), RTMGRP_LINK)) + netlink_socket.setblocking(0) + except socket.error as e: + msg = "Exception during netlink socket create: %s" % e + raise NetlinkCreateSocketError(msg) + LOG.debug("Created netlink socket") + return netlink_socket + + +def get_netlink_msg_header(data): + '''Gets netlink message type and length + + :param: data read from netlink socket + :returns: netlink message type + :raises: AssertionError if data is None or data is not >= NLMSGHDR_SIZE + struct nlmsghdr { + __u32 nlmsg_len; /* Length of message including header */ + __u16 nlmsg_type; /* Type of message content */ + __u16 nlmsg_flags; /* Additional flags */ + __u32 nlmsg_seq; /* Sequence number */ + __u32 nlmsg_pid; /* Sender port ID */ + }; + ''' + assert (data is not None), ("data is none") + assert (len(data) >= NLMSGHDR_SIZE), ( + "data is smaller than netlink message header") + msg_len, msg_type, flags, seq, pid = struct.unpack(NLMSGHDR_FMT, + data[:MSG_TYPE_OFFSET]) + LOG.debug("Got netlink msg of type %d", msg_type) + return NetlinkHeader(msg_len, msg_type, flags, seq, pid) + + +def read_netlink_socket(netlink_socket, timeout=None): + '''Select and read from the netlink socket if ready. + + :param: netlink_socket: specify which socket object to read from + :param: timeout: specify a timeout value (integer) to wait while reading, + if none, it will block indefinitely until socket ready for read + :returns: string of data read (max length = ) from socket, + if no data read, returns None + :raises: AssertionError if netlink_socket is None + ''' + assert (netlink_socket is not None), ("netlink socket is none") + read_set, _, _ = select.select([netlink_socket], [], [], timeout) + # Incase of timeout,read_set doesn't contain netlink socket. + # just return from this function + if netlink_socket not in read_set: + return None + LOG.debug("netlink socket ready for read") + data = netlink_socket.recv(MAX_SIZE) + if data is None: + LOG.error("Reading from Netlink socket returned no data") + return data + + +def unpack_rta_attr(data, offset): + '''Unpack a single rta attribute. + + :param: data: string of data read from netlink socket + :param: offset: starting offset of RTA Attribute + :return: RTAAttr object with length, type and data. On error, return None. + :raises: AssertionError if data is None or offset is not integer. + ''' + assert (data is not None), ("data is none") + assert (type(offset) == int), ("offset is not integer") + assert (offset >= RTATTR_START_OFFSET), ( + "rta offset is less than expected length") + length = rta_type = 0 + attr_data = None + try: + length = struct.unpack_from("H", data, offset=offset)[0] + rta_type = struct.unpack_from("H", data, offset=offset+2)[0] + except struct.error: + return None # Should mean our offset is >= remaining data + + # Unpack just the attribute's data. Offset by 4 to skip length/type header + attr_data = data[offset+RTA_DATA_START_OFFSET:offset+length] + return RTAAttr(length, rta_type, attr_data) + + +def read_rta_oper_state(data): + '''Reads Interface name and operational state from RTA Data. + + :param: data: string of data read from netlink socket + :returns: InterfaceOperstate object containing if_name and oper_state. + None if data does not contain valid IFLA_OPERSTATE and + IFLA_IFNAME messages. + :raises: AssertionError if data is None or length of data is + smaller than RTATTR_START_OFFSET. + ''' + assert (data is not None), ("data is none") + assert (len(data) > RTATTR_START_OFFSET), ( + "length of data is smaller than RTATTR_START_OFFSET") + ifname = operstate = None + offset = RTATTR_START_OFFSET + while offset <= len(data): + attr = unpack_rta_attr(data, offset) + if not attr or attr.length == 0: + break + # Each attribute is 4-byte aligned. Determine pad length. + padlen = (PAD_ALIGNMENT - + (attr.length % PAD_ALIGNMENT)) % PAD_ALIGNMENT + offset += attr.length + padlen + + if attr.rta_type == IFLA_OPERSTATE: + operstate = ord(attr.data) + elif attr.rta_type == IFLA_IFNAME: + interface_name = util.decode_binary(attr.data, 'utf-8') + ifname = interface_name.strip('\0') + if not ifname or operstate is None: + return None + LOG.debug("rta attrs: ifname %s operstate %d", ifname, operstate) + return InterfaceOperstate(ifname, operstate) + + +def wait_for_media_disconnect_connect(netlink_socket, ifname): + '''Block until media disconnect and connect has happened on an interface. + Listens on netlink socket to receive netlink events and when the carrier + changes from 0 to 1, it considers event has happened and + return from this function + + :param: netlink_socket: netlink_socket to receive events + :param: ifname: Interface name to lookout for netlink events + :raises: AssertionError if netlink_socket is None or ifname is None. + ''' + assert (netlink_socket is not None), ("netlink socket is none") + assert (ifname is not None), ("interface name is none") + assert (len(ifname) > 0), ("interface name cannot be empty") + carrier = OPER_UP + prevCarrier = OPER_UP + data = bytes() + LOG.debug("Wait for media disconnect and reconnect to happen") + while True: + recv_data = read_netlink_socket(netlink_socket, SELECT_TIMEOUT) + if recv_data is None: + continue + LOG.debug('read %d bytes from socket', len(recv_data)) + data += recv_data + LOG.debug('Length of data after concat %d', len(data)) + offset = 0 + datalen = len(data) + while offset < datalen: + nl_msg = data[offset:] + if len(nl_msg) < NLMSGHDR_SIZE: + LOG.debug("Data is smaller than netlink header") + break + nlheader = get_netlink_msg_header(nl_msg) + if len(nl_msg) < nlheader.length: + LOG.debug("Partial data. Smaller than netlink message") + break + padlen = (nlheader.length+PAD_ALIGNMENT-1) & ~(PAD_ALIGNMENT-1) + offset = offset + padlen + LOG.debug('offset to next netlink message: %d', offset) + # Ignore any messages not new link or del link + if nlheader.type not in [RTM_NEWLINK, RTM_DELLINK]: + continue + interface_state = read_rta_oper_state(nl_msg) + if interface_state is None: + LOG.debug('Failed to read rta attributes: %s', interface_state) + continue + if interface_state.ifname != ifname: + LOG.debug( + "Ignored netlink event on interface %s. Waiting for %s.", + interface_state.ifname, ifname) + continue + if interface_state.operstate not in [OPER_UP, OPER_DOWN]: + continue + prevCarrier = carrier + carrier = interface_state.operstate + # check for carrier down, up sequence + isVnetSwitch = (prevCarrier == OPER_DOWN) and (carrier == OPER_UP) + if isVnetSwitch: + LOG.debug("Media switch happened on %s.", ifname) + return + data = data[offset:] + +# vi: ts=4 expandtab diff --git a/cloudinit/sources/helpers/tests/test_netlink.py b/cloudinit/sources/helpers/tests/test_netlink.py new file mode 100644 index 00000000..c2898a16 --- /dev/null +++ b/cloudinit/sources/helpers/tests/test_netlink.py @@ -0,0 +1,373 @@ +# Author: Tamilmani Manoharan +# +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit.tests.helpers import CiTestCase, mock +import socket +import struct +import codecs +from cloudinit.sources.helpers.netlink import ( + NetlinkCreateSocketError, create_bound_netlink_socket, read_netlink_socket, + read_rta_oper_state, unpack_rta_attr, wait_for_media_disconnect_connect, + OPER_DOWN, OPER_UP, OPER_DORMANT, OPER_LOWERLAYERDOWN, OPER_NOTPRESENT, + OPER_TESTING, OPER_UNKNOWN, RTATTR_START_OFFSET, RTM_NEWLINK, RTM_SETLINK, + RTM_GETLINK, MAX_SIZE) + + +def int_to_bytes(i): + '''convert integer to binary: eg: 1 to \x01''' + hex_value = '{0:x}'.format(i) + hex_value = '0' * (len(hex_value) % 2) + hex_value + return codecs.decode(hex_value, 'hex_codec') + + +class TestCreateBoundNetlinkSocket(CiTestCase): + + @mock.patch('cloudinit.sources.helpers.netlink.socket.socket') + def test_socket_error_on_create(self, m_socket): + '''create_bound_netlink_socket catches socket creation exception''' + + """NetlinkCreateSocketError is raised when socket creation errors.""" + m_socket.side_effect = socket.error("Fake socket failure") + with self.assertRaises(NetlinkCreateSocketError) as ctx_mgr: + create_bound_netlink_socket() + self.assertEqual( + 'Exception during netlink socket create: Fake socket failure', + str(ctx_mgr.exception)) + + +class TestReadNetlinkSocket(CiTestCase): + + @mock.patch('cloudinit.sources.helpers.netlink.socket.socket') + @mock.patch('cloudinit.sources.helpers.netlink.select.select') + def test_read_netlink_socket(self, m_select, m_socket): + '''read_netlink_socket able to receive data''' + data = 'netlinktest' + m_select.return_value = [m_socket], None, None + m_socket.recv.return_value = data + recv_data = read_netlink_socket(m_socket, 2) + m_select.assert_called_with([m_socket], [], [], 2) + m_socket.recv.assert_called_with(MAX_SIZE) + self.assertIsNotNone(recv_data) + self.assertEqual(recv_data, data) + + @mock.patch('cloudinit.sources.helpers.netlink.socket.socket') + @mock.patch('cloudinit.sources.helpers.netlink.select.select') + def test_netlink_read_timeout(self, m_select, m_socket): + '''read_netlink_socket should timeout if nothing to read''' + m_select.return_value = [], None, None + data = read_netlink_socket(m_socket, 1) + m_select.assert_called_with([m_socket], [], [], 1) + self.assertEqual(m_socket.recv.call_count, 0) + self.assertIsNone(data) + + def test_read_invalid_socket(self): + '''read_netlink_socket raises assert error if socket is invalid''' + socket = None + with self.assertRaises(AssertionError) as context: + read_netlink_socket(socket, 1) + self.assertTrue('netlink socket is none' in str(context.exception)) + + +class TestParseNetlinkMessage(CiTestCase): + + def test_read_rta_oper_state(self): + '''read_rta_oper_state could parse netlink message and extract data''' + ifname = "eth0" + bytes = ifname.encode("utf-8") + buf = bytearray(48) + struct.pack_into("HH4sHHc", buf, RTATTR_START_OFFSET, 8, 3, bytes, 5, + 16, int_to_bytes(OPER_DOWN)) + interface_state = read_rta_oper_state(buf) + self.assertEqual(interface_state.ifname, ifname) + self.assertEqual(interface_state.operstate, OPER_DOWN) + + def test_read_none_data(self): + '''read_rta_oper_state raises assert error if data is none''' + data = None + with self.assertRaises(AssertionError) as context: + read_rta_oper_state(data) + self.assertTrue('data is none', str(context.exception)) + + def test_read_invalid_rta_operstate_none(self): + '''read_rta_oper_state returns none if operstate is none''' + ifname = "eth0" + buf = bytearray(40) + bytes = ifname.encode("utf-8") + struct.pack_into("HH4s", buf, RTATTR_START_OFFSET, 8, 3, bytes) + interface_state = read_rta_oper_state(buf) + self.assertIsNone(interface_state) + + def test_read_invalid_rta_ifname_none(self): + '''read_rta_oper_state returns none if ifname is none''' + buf = bytearray(40) + struct.pack_into("HHc", buf, RTATTR_START_OFFSET, 5, 16, + int_to_bytes(OPER_DOWN)) + interface_state = read_rta_oper_state(buf) + self.assertIsNone(interface_state) + + def test_read_invalid_data_len(self): + '''raise assert error if data size is smaller than required size''' + buf = bytearray(32) + with self.assertRaises(AssertionError) as context: + read_rta_oper_state(buf) + self.assertTrue('length of data is smaller than RTATTR_START_OFFSET' in + str(context.exception)) + + def test_unpack_rta_attr_none_data(self): + '''unpack_rta_attr raises assert error if data is none''' + data = None + with self.assertRaises(AssertionError) as context: + unpack_rta_attr(data, RTATTR_START_OFFSET) + self.assertTrue('data is none' in str(context.exception)) + + def test_unpack_rta_attr_invalid_offset(self): + '''unpack_rta_attr raises assert error if offset is invalid''' + data = bytearray(48) + with self.assertRaises(AssertionError) as context: + unpack_rta_attr(data, "offset") + self.assertTrue('offset is not integer' in str(context.exception)) + with self.assertRaises(AssertionError) as context: + unpack_rta_attr(data, 31) + self.assertTrue('rta offset is less than expected length' in + str(context.exception)) + + +@mock.patch('cloudinit.sources.helpers.netlink.socket.socket') +@mock.patch('cloudinit.sources.helpers.netlink.read_netlink_socket') +class TestWaitForMediaDisconnectConnect(CiTestCase): + with_logs = True + + def _media_switch_data(self, ifname, msg_type, operstate): + '''construct netlink data with specified fields''' + if ifname and operstate is not None: + data = bytearray(48) + bytes = ifname.encode("utf-8") + struct.pack_into("HH4sHHc", data, RTATTR_START_OFFSET, 8, 3, + bytes, 5, 16, int_to_bytes(operstate)) + elif ifname: + data = bytearray(40) + bytes = ifname.encode("utf-8") + struct.pack_into("HH4s", data, RTATTR_START_OFFSET, 8, 3, bytes) + elif operstate: + data = bytearray(40) + struct.pack_into("HHc", data, RTATTR_START_OFFSET, 5, 16, + int_to_bytes(operstate)) + struct.pack_into("=LHHLL", data, 0, len(data), msg_type, 0, 0, 0) + return data + + def test_media_down_up_scenario(self, m_read_netlink_socket, + m_socket): + '''Test for media down up sequence for required interface name''' + ifname = "eth0" + # construct data for Oper State down + data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) + # construct data for Oper State up + data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [data_op_down, data_op_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 2) + + def test_wait_for_media_switch_diff_interface(self, m_read_netlink_socket, + m_socket): + '''wait_for_media_disconnect_connect ignores unexpected interfaces. + + The first two messages are for other interfaces and last two are for + expected interface. So the function exit only after receiving last + 2 messages and therefore the call count for m_read_netlink_socket + has to be 4 + ''' + other_ifname = "eth1" + expected_ifname = "eth0" + data_op_down_eth1 = self._media_switch_data( + other_ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up_eth1 = self._media_switch_data( + other_ifname, RTM_NEWLINK, OPER_UP) + data_op_down_eth0 = self._media_switch_data( + expected_ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up_eth0 = self._media_switch_data( + expected_ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [data_op_down_eth1, + data_op_up_eth1, + data_op_down_eth0, + data_op_up_eth0] + wait_for_media_disconnect_connect(m_socket, expected_ifname) + self.assertIn('Ignored netlink event on interface %s' % other_ifname, + self.logs.getvalue()) + self.assertEqual(m_read_netlink_socket.call_count, 4) + + def test_invalid_msgtype_getlink(self, m_read_netlink_socket, m_socket): + '''wait_for_media_disconnect_connect ignores GETLINK events. + + The first two messages are for oper down and up for RTM_GETLINK type + which netlink module will ignore. The last 2 messages are RTM_NEWLINK + with oper state down and up messages. Therefore the call count for + m_read_netlink_socket has to be 4 ignoring first 2 messages + of RTM_GETLINK + ''' + ifname = "eth0" + data_getlink_down = self._media_switch_data( + ifname, RTM_GETLINK, OPER_DOWN) + data_getlink_up = self._media_switch_data( + ifname, RTM_GETLINK, OPER_UP) + data_newlink_down = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_DOWN) + data_newlink_up = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [data_getlink_down, + data_getlink_up, + data_newlink_down, + data_newlink_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 4) + + def test_invalid_msgtype_setlink(self, m_read_netlink_socket, m_socket): + '''wait_for_media_disconnect_connect ignores SETLINK events. + + The first two messages are for oper down and up for RTM_GETLINK type + which it will ignore. 3rd and 4th messages are RTM_NEWLINK with down + and up messages. This function should exit after 4th messages since it + sees down->up scenario. So the call count for m_read_netlink_socket + has to be 4 ignoring first 2 messages of RTM_GETLINK and + last 2 messages of RTM_NEWLINK + ''' + ifname = "eth0" + data_setlink_down = self._media_switch_data( + ifname, RTM_SETLINK, OPER_DOWN) + data_setlink_up = self._media_switch_data( + ifname, RTM_SETLINK, OPER_UP) + data_newlink_down = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_DOWN) + data_newlink_up = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [data_setlink_down, + data_setlink_up, + data_newlink_down, + data_newlink_up, + data_newlink_down, + data_newlink_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 4) + + def test_netlink_invalid_switch_scenario(self, m_read_netlink_socket, + m_socket): + '''returns only if it receives UP event after a DOWN event''' + ifname = "eth0" + data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) + data_op_dormant = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_DORMANT) + data_op_notpresent = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_NOTPRESENT) + data_op_lowerdown = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_LOWERLAYERDOWN) + data_op_testing = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_TESTING) + data_op_unknown = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_UNKNOWN) + m_read_netlink_socket.side_effect = [data_op_up, data_op_up, + data_op_dormant, data_op_up, + data_op_notpresent, data_op_up, + data_op_lowerdown, data_op_up, + data_op_testing, data_op_up, + data_op_unknown, data_op_up, + data_op_down, data_op_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 14) + + def test_netlink_valid_inbetween_transitions(self, m_read_netlink_socket, + m_socket): + '''wait_for_media_disconnect_connect handles in between transitions''' + ifname = "eth0" + data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) + data_op_dormant = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_DORMANT) + data_op_unknown = self._media_switch_data(ifname, RTM_NEWLINK, + OPER_UNKNOWN) + m_read_netlink_socket.side_effect = [data_op_down, data_op_dormant, + data_op_unknown, data_op_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 4) + + def test_netlink_invalid_operstate(self, m_read_netlink_socket, m_socket): + '''wait_for_media_disconnect_connect should handle invalid operstates. + + The function should not fail and return even if it receives invalid + operstates. It always should wait for down up sequence. + ''' + ifname = "eth0" + data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) + data_op_invalid = self._media_switch_data(ifname, RTM_NEWLINK, 7) + m_read_netlink_socket.side_effect = [data_op_invalid, data_op_up, + data_op_down, data_op_invalid, + data_op_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 5) + + def test_wait_invalid_socket(self, m_read_netlink_socket, m_socket): + '''wait_for_media_disconnect_connect handle none netlink socket.''' + socket = None + ifname = "eth0" + with self.assertRaises(AssertionError) as context: + wait_for_media_disconnect_connect(socket, ifname) + self.assertTrue('netlink socket is none' in str(context.exception)) + + def test_wait_invalid_ifname(self, m_read_netlink_socket, m_socket): + '''wait_for_media_disconnect_connect handle none interface name''' + ifname = None + with self.assertRaises(AssertionError) as context: + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertTrue('interface name is none' in str(context.exception)) + ifname = "" + with self.assertRaises(AssertionError) as context: + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertTrue('interface name cannot be empty' in + str(context.exception)) + + def test_wait_invalid_rta_attr(self, m_read_netlink_socket, m_socket): + ''' wait_for_media_disconnect_connect handles invalid rta data''' + ifname = "eth0" + data_invalid1 = self._media_switch_data(None, RTM_NEWLINK, OPER_DOWN) + data_invalid2 = self._media_switch_data(ifname, RTM_NEWLINK, None) + data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) + data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [data_invalid1, data_invalid2, + data_op_down, data_op_up] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 4) + + def test_read_multiple_netlink_msgs(self, m_read_netlink_socket, m_socket): + '''Read multiple messages in single receive call''' + ifname = "eth0" + bytes = ifname.encode("utf-8") + data = bytearray(96) + struct.pack_into("=LHHLL", data, 0, 48, RTM_NEWLINK, 0, 0, 0) + struct.pack_into("HH4sHHc", data, RTATTR_START_OFFSET, 8, 3, + bytes, 5, 16, int_to_bytes(OPER_DOWN)) + struct.pack_into("=LHHLL", data, 48, 48, RTM_NEWLINK, 0, 0, 0) + struct.pack_into("HH4sHHc", data, 48 + RTATTR_START_OFFSET, 8, + 3, bytes, 5, 16, int_to_bytes(OPER_UP)) + m_read_netlink_socket.return_value = data + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 1) + + def test_read_partial_netlink_msgs(self, m_read_netlink_socket, m_socket): + '''Read partial messages in receive call''' + ifname = "eth0" + bytes = ifname.encode("utf-8") + data1 = bytearray(112) + data2 = bytearray(32) + struct.pack_into("=LHHLL", data1, 0, 48, RTM_NEWLINK, 0, 0, 0) + struct.pack_into("HH4sHHc", data1, RTATTR_START_OFFSET, 8, 3, + bytes, 5, 16, int_to_bytes(OPER_DOWN)) + struct.pack_into("=LHHLL", data1, 48, 48, RTM_NEWLINK, 0, 0, 0) + struct.pack_into("HH4sHHc", data1, 80, 8, 3, bytes, 5, 16, + int_to_bytes(OPER_DOWN)) + struct.pack_into("=LHHLL", data1, 96, 48, RTM_NEWLINK, 0, 0, 0) + struct.pack_into("HH4sHHc", data2, 16, 8, 3, bytes, 5, 16, + int_to_bytes(OPER_UP)) + m_read_netlink_socket.side_effect = [data1, data2] + wait_for_media_disconnect_connect(m_socket, ifname) + self.assertEqual(m_read_netlink_socket.call_count, 2) diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 5ea7ae5e..417d86a9 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -564,6 +564,8 @@ fdescfs /dev/fd fdescfs rw 0 0 self.assertEqual(1, report_ready_func.call_count) @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file') + @mock.patch('cloudinit.sources.helpers.netlink.' + 'wait_for_media_disconnect_connect') @mock.patch( 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @@ -572,7 +574,7 @@ fdescfs /dev/fd fdescfs rw 0 0 def test_crawl_metadata_on_reprovision_reports_ready_using_lease( self, m_readurl, m_dhcp, m_net, report_ready_func, - m_write): + m_media_switch, m_write): """If reprovisioning, report ready using the obtained lease""" ovfenv = construct_valid_ovf_env( platform_settings={"PreprovisionedVm": "True"}) @@ -586,6 +588,7 @@ fdescfs /dev/fd fdescfs rw 0 0 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'} m_dhcp.return_value = [lease] + m_media_switch.return_value = None reprovision_ovfenv = construct_valid_ovf_env() m_readurl.return_value = url_helper.StringResponse( @@ -1676,6 +1679,8 @@ class TestPreprovisioningShouldReprovision(CiTestCase): @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') +@mock.patch('cloudinit.sources.helpers.netlink.' + 'wait_for_media_disconnect_connect') @mock.patch('requests.Session.request') @mock.patch(MOCKPATH + 'DataSourceAzure._report_ready') class TestPreprovisioningPollIMDS(CiTestCase): @@ -1689,7 +1694,8 @@ class TestPreprovisioningPollIMDS(CiTestCase): @mock.patch(MOCKPATH + 'EphemeralDHCPv4') def test_poll_imds_re_dhcp_on_timeout(self, m_dhcpv4, report_ready_func, - fake_resp, m_dhcp, m_net): + fake_resp, m_media_switch, m_dhcp, + m_net): """The poll_imds will retry DHCP on IMDS timeout.""" report_file = self.tmp_path('report_marker', self.tmp) lease = { @@ -1697,7 +1703,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'} m_dhcp.return_value = [lease] - + m_media_switch.return_value = None dhcp_ctx = mock.MagicMock(lease=lease) dhcp_ctx.obtain_lease.return_value = lease m_dhcpv4.return_value = dhcp_ctx @@ -1723,11 +1729,12 @@ class TestPreprovisioningPollIMDS(CiTestCase): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 1) report_ready_func.assert_called_with(lease=lease) - self.assertEqual(2, m_dhcpv4.call_count, 'Expected 2 DHCP calls') + self.assertEqual(3, m_dhcpv4.call_count, 'Expected 3 DHCP calls') self.assertEqual(3, self.tries, 'Expected 3 total reads from IMDS') - def test_poll_imds_report_ready_false(self, report_ready_func, - fake_resp, m_dhcp, m_net): + def test_poll_imds_report_ready_false(self, + report_ready_func, fake_resp, + m_media_switch, m_dhcp, m_net): """The poll_imds should not call reporting ready when flag is false""" report_file = self.tmp_path('report_marker', self.tmp) @@ -1736,6 +1743,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): 'interface': 'eth9', 'fixed-address': '192.168.2.9', 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'}] + m_media_switch.return_value = None dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file): dsa._poll_imds() @@ -1745,6 +1753,8 @@ class TestPreprovisioningPollIMDS(CiTestCase): @mock.patch(MOCKPATH + 'util.subp') @mock.patch(MOCKPATH + 'util.write_file') @mock.patch(MOCKPATH + 'util.is_FreeBSD') +@mock.patch('cloudinit.sources.helpers.netlink.' + 'wait_for_media_disconnect_connect') @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @mock.patch('requests.Session.request') @@ -1757,10 +1767,13 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): self.paths = helpers.Paths({'cloud_dir': tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d - def test_poll_imds_returns_ovf_env(self, fake_resp, m_dhcp, m_net, + def test_poll_imds_returns_ovf_env(self, fake_resp, + m_dhcp, m_net, + m_media_switch, m_is_bsd, write_f, subp): """The _poll_imds method should return the ovf_env.xml.""" m_is_bsd.return_value = False + m_media_switch.return_value = None m_dhcp.return_value = [{ 'interface': 'eth9', 'fixed-address': '192.168.2.9', 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0'}] @@ -1778,16 +1791,19 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): 'Cloud-Init/%s' % vs() }, method='GET', timeout=1, url=full_url)]) - self.assertEqual(m_dhcp.call_count, 1) + self.assertEqual(m_dhcp.call_count, 2) m_net.assert_any_call( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', prefix_or_mask='255.255.255.0', router='192.168.2.1') - self.assertEqual(m_net.call_count, 1) + self.assertEqual(m_net.call_count, 2) - def test__reprovision_calls__poll_imds(self, fake_resp, m_dhcp, m_net, + def test__reprovision_calls__poll_imds(self, fake_resp, + m_dhcp, m_net, + m_media_switch, m_is_bsd, write_f, subp): """The _reprovision method should call poll IMDS.""" m_is_bsd.return_value = False + m_media_switch.return_value = None m_dhcp.return_value = [{ 'interface': 'eth9', 'fixed-address': '192.168.2.9', 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', @@ -1811,11 +1827,11 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): 'User-Agent': 'Cloud-Init/%s' % vs()}, method='GET', timeout=1, url=full_url)]) - self.assertEqual(m_dhcp.call_count, 1) + self.assertEqual(m_dhcp.call_count, 2) m_net.assert_any_call( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', prefix_or_mask='255.255.255.0', router='192.168.2.1') - self.assertEqual(m_net.call_count, 1) + self.assertEqual(m_net.call_count, 2) class TestRemoveUbuntuNetworkConfigScripts(CiTestCase): -- cgit v1.2.3 From cb44ad6f42ac015d7d8eaf2ab0bb5ab125ed04b6 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Mon, 3 Dec 2018 18:43:21 +0000 Subject: ovf: Fix ovf network config generation gateway/routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move routes under the nic's subnet rather than use top-level ("global") route config ensuring all net renderers will provide the configured route. Also updated cloudinit/cmd/devel/net_convert.py:  - Add input type 'vmware-imc' for OVF customization config files  - Fix bug when output-type was netplan which invoked netplan   generate/apply and attempted to write to   /etc/netplan/50-cloud-init.yaml instead of joining with the   output directory. LP: #1806103 --- cloudinit/cmd/devel/net_convert.py | 15 ++++-- cloudinit/sources/helpers/vmware/imc/config_nic.py | 5 +- tests/unittests/test_vmware_config_file.py | 58 +++++++++++++++++++--- 3 files changed, 64 insertions(+), 14 deletions(-) (limited to 'tests') diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py index a0f58a0a..1ad7e0bd 100755 --- a/cloudinit/cmd/devel/net_convert.py +++ b/cloudinit/cmd/devel/net_convert.py @@ -9,6 +9,7 @@ import yaml from cloudinit.sources.helpers import openstack from cloudinit.sources import DataSourceAzure as azure +from cloudinit.sources import DataSourceOVF as ovf from cloudinit import distros from cloudinit.net import eni, netplan, network_state, sysconfig @@ -31,7 +32,7 @@ def get_parser(parser=None): metavar="PATH", required=True) parser.add_argument("-k", "--kind", choices=['eni', 'network_data.json', 'yaml', - 'azure-imds'], + 'azure-imds', 'vmware-imc'], required=True) parser.add_argument("-d", "--directory", metavar="PATH", @@ -76,7 +77,6 @@ def handle_args(name, args): net_data = args.network_data.read() if args.kind == "eni": pre_ns = eni.convert_eni_data(net_data) - ns = network_state.parse_net_config_data(pre_ns) elif args.kind == "yaml": pre_ns = yaml.load(net_data) if 'network' in pre_ns: @@ -85,15 +85,16 @@ def handle_args(name, args): sys.stderr.write('\n'.join( ["Input YAML", yaml.dump(pre_ns, default_flow_style=False, indent=4), ""])) - ns = network_state.parse_net_config_data(pre_ns) elif args.kind == 'network_data.json': pre_ns = openstack.convert_net_json( json.loads(net_data), known_macs=known_macs) - ns = network_state.parse_net_config_data(pre_ns) elif args.kind == 'azure-imds': pre_ns = azure.parse_network_config(json.loads(net_data)) - ns = network_state.parse_net_config_data(pre_ns) + elif args.kind == 'vmware-imc': + config = ovf.Config(ovf.ConfigFile(args.network_data.name)) + pre_ns = ovf.get_network_config_from_conf(config, False) + ns = network_state.parse_net_config_data(pre_ns) if not ns: raise RuntimeError("No valid network_state object created from" "input data") @@ -111,6 +112,10 @@ def handle_args(name, args): elif args.output_kind == "netplan": r_cls = netplan.Renderer config = distro.renderer_configs.get('netplan') + # don't run netplan generate/apply + config['postcmds'] = False + # trim leading slash + config['netplan_path'] = config['netplan_path'][1:] else: r_cls = sysconfig.Renderer config = distro.renderer_configs.get('sysconfig') diff --git a/cloudinit/sources/helpers/vmware/imc/config_nic.py b/cloudinit/sources/helpers/vmware/imc/config_nic.py index e1890e23..77cbf3b6 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_nic.py +++ b/cloudinit/sources/helpers/vmware/imc/config_nic.py @@ -165,9 +165,8 @@ class NicConfigurator(object): # Add routes if there is no primary nic if not self._primaryNic and v4.gateways: - route_list.extend(self.gen_ipv4_route(nic, - v4.gateways, - v4.netmask)) + subnet.update( + {'routes': self.gen_ipv4_route(nic, v4.gateways, v4.netmask)}) return ([subnet], route_list) diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py index 602dedb0..f47335ea 100644 --- a/tests/unittests/test_vmware_config_file.py +++ b/tests/unittests/test_vmware_config_file.py @@ -263,7 +263,7 @@ class TestVmwareConfigFile(CiTestCase): nicConfigurator = NicConfigurator(config.nics, False) nics_cfg_list = nicConfigurator.generate() - self.assertEqual(5, len(nics_cfg_list), "number of elements") + self.assertEqual(2, len(nics_cfg_list), "number of elements") nic1 = {'name': 'NIC1'} nic2 = {'name': 'NIC2'} @@ -275,8 +275,6 @@ class TestVmwareConfigFile(CiTestCase): nic1.update(cfg) elif cfg.get('name') == nic2.get('name'): nic2.update(cfg) - elif cfg_type == 'route': - route_list.append(cfg) self.assertEqual('physical', nic1.get('type'), 'type of NIC1') self.assertEqual('NIC1', nic1.get('name'), 'name of NIC1') @@ -297,6 +295,9 @@ class TestVmwareConfigFile(CiTestCase): static6_subnet.append(subnet) else: self.assertEqual(True, False, 'Unknown type') + if 'route' in subnet: + for route in subnet.get('routes'): + route_list.append(route) self.assertEqual(1, len(static_subnet), 'Number of static subnet') self.assertEqual(1, len(static6_subnet), 'Number of static6 subnet') @@ -351,6 +352,8 @@ class TestVmwareConfigFile(CiTestCase): class TestVmwareNetConfig(CiTestCase): """Test conversion of vmware config to cloud-init config.""" + maxDiff = None + def _get_NicConfigurator(self, text): fp = None try: @@ -420,9 +423,52 @@ class TestVmwareNetConfig(CiTestCase): 'mac_address': '00:50:56:a6:8c:08', 'subnets': [ {'control': 'auto', 'type': 'static', - 'address': '10.20.87.154', 'netmask': '255.255.252.0'}]}, - {'type': 'route', 'destination': '10.20.84.0/22', - 'gateway': '10.20.87.253', 'metric': 10000}], + 'address': '10.20.87.154', 'netmask': '255.255.252.0', + 'routes': + [{'type': 'route', 'destination': '10.20.84.0/22', + 'gateway': '10.20.87.253', 'metric': 10000}]}]}], + nc.generate()) + + def test_cust_non_primary_nic_with_gateway_(self): + """A customer non primary nic set can have a gateway.""" + config = textwrap.dedent("""\ + [NETWORK] + NETWORKING = yes + BOOTPROTO = dhcp + HOSTNAME = static-debug-vm + DOMAINNAME = cluster.local + + [NIC-CONFIG] + NICS = NIC1 + + [NIC1] + MACADDR = 00:50:56:ac:d1:8a + ONBOOT = yes + IPv4_MODE = BACKWARDS_COMPATIBLE + BOOTPROTO = static + IPADDR = 100.115.223.75 + NETMASK = 255.255.255.0 + GATEWAY = 100.115.223.254 + + + [DNS] + DNSFROMDHCP=no + + NAMESERVER|1 = 8.8.8.8 + + [DATETIME] + UTC = yes + """) + nc = self._get_NicConfigurator(config) + self.assertEqual( + [{'type': 'physical', 'name': 'NIC1', + 'mac_address': '00:50:56:ac:d1:8a', + 'subnets': [ + {'control': 'auto', 'type': 'static', + 'address': '100.115.223.75', 'netmask': '255.255.255.0', + 'routes': + [{'type': 'route', 'destination': '100.115.223.0/24', + 'gateway': '100.115.223.254', 'metric': 10000}]}]}], nc.generate()) def test_a_primary_nic_with_gateway(self): -- cgit v1.2.3 From adbd950af07a4b613a14bd83049915abdd6ca348 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 3 Dec 2018 22:06:47 +0000 Subject: NoCloud: Allow top level 'network' key in network-config. NoCloud's 'network-config' file was originally expected to contain network configuration without the top level 'network' key. This was because the file was named 'network-config' so specifying 'network' seemed redundant. However, JuJu is currently providing a top level 'network' config when it tries to disable networking ({"network": {"config": "disabled"}). Other users have also been surprised/confused by the fact that a network config in /etc/cloud/cloud.cfg.d/network.cfg differed from what was expected in 'network-config'. LP: #1798117 --- cloudinit/sources/DataSourceNoCloud.py | 32 +++++++- tests/unittests/test_datasource/test_nocloud.py | 100 ++++++++++++++++-------- 2 files changed, 97 insertions(+), 35 deletions(-) (limited to 'tests') diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py index 9010f06c..6860f0cc 100644 --- a/cloudinit/sources/DataSourceNoCloud.py +++ b/cloudinit/sources/DataSourceNoCloud.py @@ -311,6 +311,35 @@ def parse_cmdline_data(ds_id, fill, cmdline=None): return True +def _maybe_remove_top_network(cfg): + """If network-config contains top level 'network' key, then remove it. + + Some providers of network configuration may provide a top level + 'network' key (LP: #1798117) even though it is not necessary. + + Be friendly and remove it if it really seems so. + + Return the original value if no change or the updated value if changed.""" + nullval = object() + network_val = cfg.get('network', nullval) + if network_val is nullval: + return cfg + bmsg = 'Top level network key in network-config %s: %s' + if not isinstance(network_val, dict): + LOG.debug(bmsg, "was not a dict", cfg) + return cfg + if len(list(cfg.keys())) != 1: + LOG.debug(bmsg, "had multiple top level keys", cfg) + return cfg + if network_val.get('config') == "disabled": + LOG.debug(bmsg, "was config/disabled", cfg) + elif not all(('config' in network_val, 'version' in network_val)): + LOG.debug(bmsg, "but missing 'config' or 'version'", cfg) + return cfg + LOG.debug(bmsg, "fixed by removing shifting network.", cfg) + return network_val + + def _merge_new_seed(cur, seeded): ret = cur.copy() @@ -320,7 +349,8 @@ def _merge_new_seed(cur, seeded): ret['meta-data'] = util.mergemanydict([cur['meta-data'], newmd]) if seeded.get('network-config'): - ret['network-config'] = util.load_yaml(seeded['network-config']) + ret['network-config'] = _maybe_remove_top_network( + util.load_yaml(seeded.get('network-config'))) if 'user-data' in seeded: ret['user-data'] = seeded['user-data'] diff --git a/tests/unittests/test_datasource/test_nocloud.py b/tests/unittests/test_datasource/test_nocloud.py index b6468b6d..3429272c 100644 --- a/tests/unittests/test_datasource/test_nocloud.py +++ b/tests/unittests/test_datasource/test_nocloud.py @@ -1,7 +1,10 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit import helpers -from cloudinit.sources import DataSourceNoCloud +from cloudinit.sources.DataSourceNoCloud import ( + DataSourceNoCloud as dsNoCloud, + _maybe_remove_top_network, + parse_cmdline_data) from cloudinit import util from cloudinit.tests.helpers import CiTestCase, populate_dir, mock, ExitStack @@ -40,9 +43,7 @@ class TestNoCloudDataSource(CiTestCase): 'datasource': {'NoCloud': {'fs_label': None}} } - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertEqual(dsrc.userdata_raw, ud) self.assertEqual(dsrc.metadata, md) @@ -63,9 +64,7 @@ class TestNoCloudDataSource(CiTestCase): 'datasource': {'NoCloud': {'fs_label': None}} } - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) self.assertTrue(dsrc.get_data()) self.assertEqual(dsrc.platform_type, 'nocloud') self.assertEqual( @@ -73,8 +72,6 @@ class TestNoCloudDataSource(CiTestCase): def test_fs_label(self, m_is_lxd): # find_devs_with should not be called ff fs_label is None - ds = DataSourceNoCloud.DataSourceNoCloud - class PsuedoException(Exception): pass @@ -84,12 +81,12 @@ class TestNoCloudDataSource(CiTestCase): # by default, NoCloud should search for filesystems by label sys_cfg = {'datasource': {'NoCloud': {}}} - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) self.assertRaises(PsuedoException, dsrc.get_data) # but disabling searching should just end up with None found sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertFalse(ret) @@ -97,13 +94,10 @@ class TestNoCloudDataSource(CiTestCase): # no source should be found if no cmdline, config, and fs_label=None sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - ds = DataSourceNoCloud.DataSourceNoCloud - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) self.assertFalse(dsrc.get_data()) def test_seed_in_config(self, m_is_lxd): - ds = DataSourceNoCloud.DataSourceNoCloud - data = { 'fs_label': None, 'meta-data': yaml.safe_dump({'instance-id': 'IID'}), @@ -111,7 +105,7 @@ class TestNoCloudDataSource(CiTestCase): } sys_cfg = {'datasource': {'NoCloud': data}} - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertEqual(dsrc.userdata_raw, b"USER_DATA_RAW") self.assertEqual(dsrc.metadata.get('instance-id'), 'IID') @@ -130,9 +124,7 @@ class TestNoCloudDataSource(CiTestCase): 'datasource': {'NoCloud': {'fs_label': None}} } - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertEqual(dsrc.userdata_raw, ud) self.assertEqual(dsrc.metadata, md) @@ -145,9 +137,7 @@ class TestNoCloudDataSource(CiTestCase): sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertEqual(dsrc.userdata_raw, b"ud") self.assertFalse(dsrc.vendordata) @@ -174,9 +164,7 @@ class TestNoCloudDataSource(CiTestCase): sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertTrue(ret) # very simple check just for the strings above @@ -195,9 +183,23 @@ class TestNoCloudDataSource(CiTestCase): sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - ds = DataSourceNoCloud.DataSourceNoCloud + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) + ret = dsrc.get_data() + self.assertTrue(ret) + self.assertEqual(netconf, dsrc.network_config) + + def test_metadata_network_config_with_toplevel_network(self, m_is_lxd): + """network-config may have 'network' top level key.""" + netconf = {'config': 'disabled'} + populate_dir( + os.path.join(self.paths.seed_dir, "nocloud"), + {'user-data': b"ud", + 'meta-data': "instance-id: IID\n", + 'network-config': yaml.dump({'network': netconf}) + "\n"}) + + sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertTrue(ret) self.assertEqual(netconf, dsrc.network_config) @@ -228,9 +230,7 @@ class TestNoCloudDataSource(CiTestCase): sys_cfg = {'datasource': {'NoCloud': {'fs_label': None}}} - ds = DataSourceNoCloud.DataSourceNoCloud - - dsrc = ds(sys_cfg=sys_cfg, distro=None, paths=self.paths) + dsrc = dsNoCloud(sys_cfg=sys_cfg, distro=None, paths=self.paths) ret = dsrc.get_data() self.assertTrue(ret) self.assertEqual(netconf, dsrc.network_config) @@ -258,8 +258,7 @@ class TestParseCommandLineData(CiTestCase): for (fmt, expected) in pairs: fill = {} cmdline = fmt % {'ds_id': ds_id} - ret = DataSourceNoCloud.parse_cmdline_data(ds_id=ds_id, fill=fill, - cmdline=cmdline) + ret = parse_cmdline_data(ds_id=ds_id, fill=fill, cmdline=cmdline) self.assertEqual(expected, fill) self.assertTrue(ret) @@ -276,10 +275,43 @@ class TestParseCommandLineData(CiTestCase): for cmdline in cmdlines: fill = {} - ret = DataSourceNoCloud.parse_cmdline_data(ds_id=ds_id, fill=fill, - cmdline=cmdline) + ret = parse_cmdline_data(ds_id=ds_id, fill=fill, cmdline=cmdline) self.assertEqual(fill, {}) self.assertFalse(ret) +class TestMaybeRemoveToplevelNetwork(CiTestCase): + """test _maybe_remove_top_network function.""" + basecfg = [{'type': 'physical', 'name': 'interface0', + 'subnets': [{'type': 'dhcp'}]}] + + def test_should_remove_safely(self): + mcfg = {'config': self.basecfg, 'version': 1} + self.assertEqual(mcfg, _maybe_remove_top_network({'network': mcfg})) + + def test_no_remove_if_other_keys(self): + """should not shift if other keys at top level.""" + mcfg = {'network': {'config': self.basecfg, 'version': 1}, + 'unknown_keyname': 'keyval'} + self.assertEqual(mcfg, _maybe_remove_top_network(mcfg)) + + def test_no_remove_if_non_dict(self): + """should not shift if not a dict.""" + mcfg = {'network': '"content here'} + self.assertEqual(mcfg, _maybe_remove_top_network(mcfg)) + + def test_no_remove_if_missing_config_or_version(self): + """should not shift unless network entry has config and version.""" + mcfg = {'network': {'config': self.basecfg}} + self.assertEqual(mcfg, _maybe_remove_top_network(mcfg)) + + mcfg = {'network': {'version': 1}} + self.assertEqual(mcfg, _maybe_remove_top_network(mcfg)) + + def test_remove_with_config_disabled(self): + """network/config=disabled should be shifted.""" + mcfg = {'config': 'disabled'} + self.assertEqual(mcfg, _maybe_remove_top_network({'network': mcfg})) + + # vi: ts=4 expandtab -- cgit v1.2.3 From 230e67ebd489c0d895243a2fc66ca95af2cea13b Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 3 Dec 2018 22:07:59 +0000 Subject: dhclient-hook: cleanups, tests and fix a bug on 'down' event. I noticed a bug in dhclient_hook on the 'down' event, using 'is' operator rather than '==' (if self.net_action is 'down'). This refactors/simplifies the code a bit for easier testing and adds tests. The reason for the rename of 'action' to 'event' is to just be internally consistent. The word and Namespace 'action' is used by cloud-init main, so it was not really usable here. Also adds a main which can easily be debugged with: CI_DHCP_HOOK_DATA_D=./my.d python -m cloudinit.dhclient_hook up eth0 --- bash_completion/cloud-init | 5 +- cloudinit/cmd/main.py | 20 ++----- cloudinit/dhclient_hook.py | 110 ++++++++++++++++++++++------------ cloudinit/tests/test_dhclient_hook.py | 105 ++++++++++++++++++++++++++++++++ tests/unittests/test_cli.py | 16 ++--- 5 files changed, 193 insertions(+), 63 deletions(-) create mode 100644 cloudinit/tests/test_dhclient_hook.py (limited to 'tests') diff --git a/bash_completion/cloud-init b/bash_completion/cloud-init index 8c25032f..a9577e9d 100644 --- a/bash_completion/cloud-init +++ b/bash_completion/cloud-init @@ -30,7 +30,10 @@ _cloudinit_complete() devel) COMPREPLY=($(compgen -W "--help schema net-convert" -- $cur_word)) ;; - dhclient-hook|features) + dhclient-hook) + COMPREPLY=($(compgen -W "--help up down" -- $cur_word)) + ;; + features) COMPREPLY=($(compgen -W "--help" -- $cur_word)) ;; init) diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index 5a437020..933c019a 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -41,7 +41,7 @@ from cloudinit.settings import (PER_INSTANCE, PER_ALWAYS, PER_ONCE, from cloudinit import atomic_helper from cloudinit.config import cc_set_hostname -from cloudinit.dhclient_hook import LogDhclient +from cloudinit import dhclient_hook # Welcome message template @@ -586,12 +586,6 @@ def main_single(name, args): return 0 -def dhclient_hook(name, args): - record = LogDhclient(args) - record.check_hooks_dir() - record.record() - - def status_wrapper(name, args, data_d=None, link_d=None): if data_d is None: data_d = os.path.normpath("/var/lib/cloud/data") @@ -795,15 +789,9 @@ def main(sysv_args=None): 'query', help='Query standardized instance metadata from the command line.') - parser_dhclient = subparsers.add_parser('dhclient-hook', - help=('run the dhclient hook' - 'to record network info')) - parser_dhclient.add_argument("net_action", - help=('action taken on the interface')) - parser_dhclient.add_argument("net_interface", - help=('the network interface being acted' - ' upon')) - parser_dhclient.set_defaults(action=('dhclient_hook', dhclient_hook)) + parser_dhclient = subparsers.add_parser( + dhclient_hook.NAME, help=dhclient_hook.__doc__) + dhclient_hook.get_parser(parser_dhclient) parser_features = subparsers.add_parser('features', help=('list defined features')) diff --git a/cloudinit/dhclient_hook.py b/cloudinit/dhclient_hook.py index 7f02d7fa..72b51b6a 100644 --- a/cloudinit/dhclient_hook.py +++ b/cloudinit/dhclient_hook.py @@ -1,5 +1,8 @@ # This file is part of cloud-init. See LICENSE file for license information. +"""Run the dhclient hook to record network info.""" + +import argparse import os from cloudinit import atomic_helper @@ -8,44 +11,75 @@ from cloudinit import stages LOG = logging.getLogger(__name__) +NAME = "dhclient-hook" +UP = "up" +DOWN = "down" +EVENTS = (UP, DOWN) + + +def _get_hooks_dir(): + i = stages.Init() + return os.path.join(i.paths.get_runpath(), 'dhclient.hooks') + + +def _filter_env_vals(info): + """Given info (os.environ), return a dictionary with + lower case keys for each entry starting with DHCP4_ or new_.""" + new_info = {} + for k, v in info.items(): + if k.startswith("DHCP4_") or k.startswith("new_"): + key = (k.replace('DHCP4_', '').replace('new_', '')).lower() + new_info[key] = v + return new_info + + +def run_hook(interface, event, data_d=None, env=None): + if event not in EVENTS: + raise ValueError("Unexpected event '%s'. Expected one of: %s" % + (event, EVENTS)) + if data_d is None: + data_d = _get_hooks_dir() + if env is None: + env = os.environ + hook_file = os.path.join(data_d, interface + ".json") + + if event == UP: + if not os.path.exists(data_d): + os.makedirs(data_d) + atomic_helper.write_json(hook_file, _filter_env_vals(env)) + LOG.debug("Wrote dhclient options in %s", hook_file) + elif event == DOWN: + if os.path.exists(hook_file): + os.remove(hook_file) + LOG.debug("Removed dhclient options file %s", hook_file) + + +def get_parser(parser=None): + if parser is None: + parser = argparse.ArgumentParser(prog=NAME, description=__doc__) + parser.add_argument( + "event", help='event taken on the interface', choices=EVENTS) + parser.add_argument( + "interface", help='the network interface being acted upon') + # cloud-init main uses 'action' + parser.set_defaults(action=(NAME, handle_args)) + return parser + + +def handle_args(name, args, data_d=None): + """Handle the Namespace args. + Takes 'name' as passed by cloud-init main. not used here.""" + return run_hook(interface=args.interface, event=args.event, data_d=data_d) + + +if __name__ == '__main__': + import sys + parser = get_parser() + args = parser.parse_args(args=sys.argv[1:]) + return_value = handle_args( + NAME, args, data_d=os.environ.get('_CI_DHCP_HOOK_DATA_D')) + if return_value: + sys.exit(return_value) -class LogDhclient(object): - - def __init__(self, cli_args): - self.hooks_dir = self._get_hooks_dir() - self.net_interface = cli_args.net_interface - self.net_action = cli_args.net_action - self.hook_file = os.path.join(self.hooks_dir, - self.net_interface + ".json") - - @staticmethod - def _get_hooks_dir(): - i = stages.Init() - return os.path.join(i.paths.get_runpath(), 'dhclient.hooks') - - def check_hooks_dir(self): - if not os.path.exists(self.hooks_dir): - os.makedirs(self.hooks_dir) - else: - # If the action is down and the json file exists, we need to - # delete the file - if self.net_action is 'down' and os.path.exists(self.hook_file): - os.remove(self.hook_file) - - @staticmethod - def get_vals(info): - new_info = {} - for k, v in info.items(): - if k.startswith("DHCP4_") or k.startswith("new_"): - key = (k.replace('DHCP4_', '').replace('new_', '')).lower() - new_info[key] = v - return new_info - - def record(self): - envs = os.environ - if self.hook_file is None: - return - atomic_helper.write_json(self.hook_file, self.get_vals(envs)) - LOG.debug("Wrote dhclient options in %s", self.hook_file) # vi: ts=4 expandtab diff --git a/cloudinit/tests/test_dhclient_hook.py b/cloudinit/tests/test_dhclient_hook.py new file mode 100644 index 00000000..7aab8dd5 --- /dev/null +++ b/cloudinit/tests/test_dhclient_hook.py @@ -0,0 +1,105 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +"""Tests for cloudinit.dhclient_hook.""" + +from cloudinit import dhclient_hook as dhc +from cloudinit.tests.helpers import CiTestCase, dir2dict, populate_dir + +import argparse +import json +import mock +import os + + +class TestDhclientHook(CiTestCase): + + ex_env = { + 'interface': 'eth0', + 'new_dhcp_lease_time': '3600', + 'new_host_name': 'x1', + 'new_ip_address': '10.145.210.163', + 'new_subnet_mask': '255.255.255.0', + 'old_host_name': 'x1', + 'PATH': '/usr/sbin:/usr/bin:/sbin:/bin', + 'pid': '614', + 'reason': 'BOUND', + } + + # some older versions of dhclient put the same content, + # but in upper case with DHCP4_ instead of new_ + ex_env_dhcp4 = { + 'REASON': 'BOUND', + 'DHCP4_dhcp_lease_time': '3600', + 'DHCP4_host_name': 'x1', + 'DHCP4_ip_address': '10.145.210.163', + 'DHCP4_subnet_mask': '255.255.255.0', + 'INTERFACE': 'eth0', + 'PATH': '/usr/sbin:/usr/bin:/sbin:/bin', + 'pid': '614', + } + + expected = { + 'dhcp_lease_time': '3600', + 'host_name': 'x1', + 'ip_address': '10.145.210.163', + 'subnet_mask': '255.255.255.0'} + + def setUp(self): + super(TestDhclientHook, self).setUp() + self.tmp = self.tmp_dir() + + def test_handle_args(self): + """quick test of call to handle_args.""" + nic = 'eth0' + args = argparse.Namespace(event=dhc.UP, interface=nic) + with mock.patch.dict("os.environ", clear=True, values=self.ex_env): + dhc.handle_args(dhc.NAME, args, data_d=self.tmp) + found = dir2dict(self.tmp + os.path.sep) + self.assertEqual([nic + ".json"], list(found.keys())) + self.assertEqual(self.expected, json.loads(found[nic + ".json"])) + + def test_run_hook_up_creates_dir(self): + """If dir does not exist, run_hook should create it.""" + subd = self.tmp_path("subdir", self.tmp) + nic = 'eth1' + dhc.run_hook(nic, 'up', data_d=subd, env=self.ex_env) + self.assertEqual( + set([nic + ".json"]), set(dir2dict(subd + os.path.sep))) + + def test_run_hook_up(self): + """Test expected use of run_hook_up.""" + nic = 'eth0' + dhc.run_hook(nic, 'up', data_d=self.tmp, env=self.ex_env) + found = dir2dict(self.tmp + os.path.sep) + self.assertEqual([nic + ".json"], list(found.keys())) + self.assertEqual(self.expected, json.loads(found[nic + ".json"])) + + def test_run_hook_up_dhcp4_prefix(self): + """Test run_hook filters correctly with older DHCP4_ data.""" + nic = 'eth0' + dhc.run_hook(nic, 'up', data_d=self.tmp, env=self.ex_env_dhcp4) + found = dir2dict(self.tmp + os.path.sep) + self.assertEqual([nic + ".json"], list(found.keys())) + self.assertEqual(self.expected, json.loads(found[nic + ".json"])) + + def test_run_hook_down_deletes(self): + """down should delete the created json file.""" + nic = 'eth1' + populate_dir( + self.tmp, {nic + ".json": "{'abcd'}", 'myfile.txt': 'text'}) + dhc.run_hook(nic, 'down', data_d=self.tmp, env={'old_host_name': 'x1'}) + self.assertEqual( + set(['myfile.txt']), + set(dir2dict(self.tmp + os.path.sep))) + + def test_get_parser(self): + """Smoke test creation of get_parser.""" + # cloud-init main uses 'action'. + event, interface = (dhc.UP, 'mynic0') + self.assertEqual( + argparse.Namespace(event=event, interface=interface, + action=(dhc.NAME, dhc.handle_args)), + dhc.get_parser().parse_args([event, interface])) + + +# vi: ts=4 expandtab diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py index 199d69b0..d283f136 100644 --- a/tests/unittests/test_cli.py +++ b/tests/unittests/test_cli.py @@ -246,18 +246,18 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): self.assertEqual('cc_ntp', parseargs.name) self.assertFalse(parseargs.report) - @mock.patch('cloudinit.cmd.main.dhclient_hook') - def test_dhclient_hook_subcommand(self, m_dhclient_hook): + @mock.patch('cloudinit.cmd.main.dhclient_hook.handle_args') + def test_dhclient_hook_subcommand(self, m_handle_args): """The subcommand 'dhclient-hook' calls dhclient_hook with args.""" - self._call_main(['cloud-init', 'dhclient-hook', 'net_action', 'eth0']) - (name, parseargs) = m_dhclient_hook.call_args_list[0][0] - self.assertEqual('dhclient_hook', name) + self._call_main(['cloud-init', 'dhclient-hook', 'up', 'eth0']) + (name, parseargs) = m_handle_args.call_args_list[0][0] + self.assertEqual('dhclient-hook', name) self.assertEqual('dhclient-hook', parseargs.subcommand) - self.assertEqual('dhclient_hook', parseargs.action[0]) + self.assertEqual('dhclient-hook', parseargs.action[0]) self.assertFalse(parseargs.debug) self.assertFalse(parseargs.force) - self.assertEqual('net_action', parseargs.net_action) - self.assertEqual('eth0', parseargs.net_interface) + self.assertEqual('up', parseargs.event) + self.assertEqual('eth0', parseargs.interface) @mock.patch('cloudinit.cmd.main.main_features') def test_features_hook_subcommand(self, m_features): -- cgit v1.2.3 From a4007d063f96b82545aa678ef2cb472ea3b48b1e Mon Sep 17 00:00:00 2001 From: James Baxter Date: Thu, 6 Dec 2018 18:26:32 +0000 Subject: write_files: add support for appending to files. Add 'append: true' to write_files entries to append 'content' to file specified by 'path' key. This modifies the file open mode to append. --- cloudinit/config/cc_write_files.py | 7 ++++++- tests/unittests/test_handler/test_handler_write_files.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py index 31d1db61..0b6546e2 100644 --- a/cloudinit/config/cc_write_files.py +++ b/cloudinit/config/cc_write_files.py @@ -49,6 +49,10 @@ binary gzip data can be specified and will be decoded before being written. ... path: /bin/arch permissions: '0555' + - content: | + 15 * * * * root ship_logs + path: /etc/crontab + append: true """ import base64 @@ -113,7 +117,8 @@ def write_files(name, files): contents = extract_contents(f_info.get('content', ''), extractions) (u, g) = util.extract_usergroup(f_info.get('owner', DEFAULT_OWNER)) perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS) - util.write_file(path, contents, mode=perms) + omode = 'ab' if util.get_cfg_option_bool(f_info, 'append') else 'wb' + util.write_file(path, contents, omode=omode, mode=perms) util.chownbyname(path, u, g) diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py index 7fa8fd21..bc8756ca 100644 --- a/tests/unittests/test_handler/test_handler_write_files.py +++ b/tests/unittests/test_handler/test_handler_write_files.py @@ -52,6 +52,18 @@ class TestWriteFiles(FilesystemMockingTestCase): "test_simple", [{"content": expected, "path": filename}]) self.assertEqual(util.load_file(filename), expected) + def test_append(self): + self.patchUtils(self.tmp) + existing = "hello " + added = "world\n" + expected = existing + added + filename = "/tmp/append.file" + util.write_file(filename, existing) + write_files( + "test_append", + [{"content": added, "path": filename, "append": "true"}]) + self.assertEqual(util.load_file(filename), expected) + def test_yaml_binary(self): self.patchUtils(self.tmp) data = util.load_yaml(YAML_TEXT) -- cgit v1.2.3 From 6aef6c7d402b17ebc04516a088a91f8e6ed86510 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Tue, 11 Dec 2018 17:24:11 +0000 Subject: net: render 'metric' values in per-subnet routes It is possible to have a metric value in a per-subnet route. This is currently missing in all renderers. Update each renderer to emit the correct metric value from the config. LP: #1805871 --- cloudinit/net/eni.py | 29 +++++++++++++------------- cloudinit/net/netplan.py | 6 +++--- cloudinit/net/sysconfig.py | 25 +++++++++++++++++++---- tests/unittests/test_net.py | 50 +++++++++++++++++++++++++++++++++++++++------ 4 files changed, 83 insertions(+), 27 deletions(-) (limited to 'tests') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index c6f631a9..64236320 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -371,22 +371,23 @@ class Renderer(renderer.Renderer): 'gateway': 'gw', 'metric': 'metric', } + + default_gw = '' if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0': - default_gw = " default gw %s" % route['gateway'] - content.append(up + default_gw + or_true) - content.append(down + default_gw + or_true) + default_gw = ' default' elif route['network'] == '::' and route['prefix'] == 0: - # ipv6! - default_gw = " -A inet6 default gw %s" % route['gateway'] - content.append(up + default_gw + or_true) - content.append(down + default_gw + or_true) - else: - route_line = "" - for k in ['network', 'netmask', 'gateway', 'metric']: - if k in route: - route_line += " %s %s" % (mapping[k], route[k]) - content.append(up + route_line + or_true) - content.append(down + route_line + or_true) + default_gw = ' -A inet6 default' + + route_line = '' + for k in ['network', 'netmask', 'gateway', 'metric']: + if default_gw and k in ['network', 'netmask']: + continue + if k == 'gateway': + route_line += '%s %s %s' % (default_gw, mapping[k], route[k]) + elif k in route: + route_line += ' %s %s' % (mapping[k], route[k]) + content.append(up + route_line + or_true) + content.append(down + route_line + or_true) return content def _render_iface(self, iface, render_hwaddress=False): diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index bc1087f9..21517fda 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -114,13 +114,13 @@ def _extract_addresses(config, entry, ifname): for route in subnet.get('routes', []): to_net = "%s/%s" % (route.get('network'), route.get('prefix')) - route = { + new_route = { 'via': route.get('gateway'), 'to': to_net, } if 'metric' in route: - route.update({'metric': route.get('metric', 100)}) - routes.append(route) + new_route.update({'metric': route.get('metric', 100)}) + routes.append(new_route) addresses.append(addr) diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 9c16d3a7..17293e1d 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -156,13 +156,23 @@ class Route(ConfigMap): _quote_value(gateway_value))) buf.write("%s=%s\n" % ('NETMASK' + str(reindex), _quote_value(netmask_value))) + metric_key = 'METRIC' + index + if metric_key in self._conf: + metric_value = str(self._conf['METRIC' + index]) + buf.write("%s=%s\n" % ('METRIC' + str(reindex), + _quote_value(metric_value))) elif proto == "ipv6" and self.is_ipv6_route(address_value): netmask_value = str(self._conf['NETMASK' + index]) gateway_value = str(self._conf['GATEWAY' + index]) - buf.write("%s/%s via %s dev %s\n" % (address_value, - netmask_value, - gateway_value, - self._route_name)) + metric_value = ( + 'metric ' + str(self._conf['METRIC' + index]) + if 'METRIC' + index in self._conf else '') + buf.write( + "%s/%s via %s %s dev %s\n" % (address_value, + netmask_value, + gateway_value, + metric_value, + self._route_name)) return buf.getvalue() @@ -370,6 +380,9 @@ class Renderer(renderer.Renderer): else: iface_cfg['GATEWAY'] = subnet['gateway'] + if 'metric' in subnet: + iface_cfg['METRIC'] = subnet['metric'] + if 'dns_search' in subnet: iface_cfg['DOMAIN'] = ' '.join(subnet['dns_search']) @@ -414,15 +427,19 @@ class Renderer(renderer.Renderer): else: iface_cfg['GATEWAY'] = route['gateway'] route_cfg.has_set_default_ipv4 = True + if 'metric' in route: + iface_cfg['METRIC'] = route['metric'] else: gw_key = 'GATEWAY%s' % route_cfg.last_idx nm_key = 'NETMASK%s' % route_cfg.last_idx addr_key = 'ADDRESS%s' % route_cfg.last_idx + metric_key = 'METRIC%s' % route_cfg.last_idx route_cfg.last_idx += 1 # add default routes only to ifcfg files, not # to route-* or route6-* for (old_key, new_key) in [('gateway', gw_key), + ('metric', metric_key), ('netmask', nm_key), ('network', addr_key)]: if old_key in route: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 8e383739..195f261c 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -488,8 +488,8 @@ NETWORK_CONFIGS = { address 192.168.21.3/24 dns-nameservers 8.8.8.8 8.8.4.4 dns-search barley.maas sach.maas - post-up route add default gw 65.61.151.37 || true - pre-down route del default gw 65.61.151.37 || true + post-up route add default gw 65.61.151.37 metric 10000 || true + pre-down route del default gw 65.61.151.37 metric 10000 || true """).rstrip(' '), 'expected_netplan': textwrap.dedent(""" network: @@ -513,7 +513,8 @@ NETWORK_CONFIGS = { - barley.maas - sach.maas routes: - - to: 0.0.0.0/0 + - metric: 10000 + to: 0.0.0.0/0 via: 65.61.151.37 set-name: eth99 """).rstrip(' '), @@ -537,6 +538,7 @@ NETWORK_CONFIGS = { HWADDR=c0:d6:9f:2c:e8:80 IPADDR=192.168.21.3 NETMASK=255.255.255.0 + METRIC=10000 NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet @@ -561,7 +563,7 @@ NETWORK_CONFIGS = { - gateway: 65.61.151.37 netmask: 0.0.0.0 network: 0.0.0.0 - metric: 2 + metric: 10000 - type: physical name: eth1 mac_address: "cf:d6:af:48:e8:80" @@ -1161,6 +1163,13 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true - gateway: 192.168.0.3 netmask: 255.255.255.0 network: 10.1.3.0 + - gateway: 2001:67c:1562:1 + network: 2001:67c:1 + netmask: ffff:ffff:0 + - gateway: 3001:67c:1562:1 + network: 3001:67c:1 + netmask: ffff:ffff:0 + metric: 10000 - type: static address: 192.168.1.2/24 - type: static @@ -1197,6 +1206,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true routes: - to: 10.1.3.0/24 via: 192.168.0.3 + - to: 2001:67c:1/32 + via: 2001:67c:1562:1 + - metric: 10000 + to: 3001:67c:1/32 + via: 3001:67c:1562:1 """), 'yaml-v2': textwrap.dedent(""" version: 2 @@ -1228,6 +1242,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true routes: - to: 10.1.3.0/24 via: 192.168.0.3 + - to: 2001:67c:1562:8007::1/64 + via: 2001:67c:1562:8007::aac:40b2 + - metric: 10000 + to: 3001:67c:1562:8007::1/64 + via: 3001:67c:1562:8007::aac:40b2 """), 'expected_netplan-v2': textwrap.dedent(""" network: @@ -1249,6 +1268,11 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true routes: - to: 10.1.3.0/24 via: 192.168.0.3 + - to: 2001:67c:1562:8007::1/64 + via: 2001:67c:1562:8007::aac:40b2 + - metric: 10000 + to: 3001:67c:1562:8007::1/64 + via: 3001:67c:1562:8007::aac:40b2 ethernets: eth0: match: @@ -1349,6 +1373,10 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true USERCTL=no """), 'route6-bond0': textwrap.dedent("""\ + # Created by cloud-init on instance boot automatically, do not edit. + # + 2001:67c:1/ffff:ffff:0 via 2001:67c:1562:1 dev bond0 + 3001:67c:1/ffff:ffff:0 via 3001:67c:1562:1 metric 10000 dev bond0 """), 'route-bond0': textwrap.dedent("""\ ADDRESS0=10.1.3.0 @@ -1879,14 +1907,24 @@ class TestRhelSysConfigRendering(CiTestCase): return dir2dict(dir) def _compare_files_to_expected(self, expected, found): + + def _try_load(f): + ''' Attempt to load shell content, otherwise return as-is ''' + try: + return util.load_shell_content(f) + except ValueError: + pass + # route6- * files aren't shell content, but iproute2 params + return f + orig_maxdiff = self.maxDiff expected_d = dict( - (os.path.join(self.scripts_dir, k), util.load_shell_content(v)) + (os.path.join(self.scripts_dir, k), _try_load(v)) for k, v in expected.items()) # only compare the files in scripts_dir scripts_found = dict( - (k, util.load_shell_content(v)) for k, v in found.items() + (k, _try_load(v)) for k, v in found.items() if k.startswith(self.scripts_dir)) try: self.maxDiff = None -- cgit v1.2.3 From e1da348a968567a765c34a8c6f04f6ea9930721b Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 11 Dec 2018 23:05:39 +0000 Subject: tests: add Disco release --- tests/cloud_tests/releases.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'tests') diff --git a/tests/cloud_tests/releases.yaml b/tests/cloud_tests/releases.yaml index defae02b..ec5da724 100644 --- a/tests/cloud_tests/releases.yaml +++ b/tests/cloud_tests/releases.yaml @@ -129,6 +129,22 @@ features: releases: # UBUNTU ================================================================= + disco: + # EOL: Jan 2020 + default: + enabled: true + release: disco + version: 19.04 + os: ubuntu + feature_groups: + - base + - debian_base + - ubuntu_specific + lxd: + sstreams_server: https://cloud-images.ubuntu.com/daily + alias: disco + setup_overrides: null + override_templates: false cosmic: # EOL: Jul 2019 default: -- cgit v1.2.3