From 30b4d15764a1a9644379cf95770e8b2480856882 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Dec 2017 16:25:11 -0700 Subject: cli: Add clean and status subcommands The 'cloud-init clean' command allows a user or script to clear cloud-init artifacts from the system so that cloud-init sees the system as unconfigured upon reboot. Optional parameters can be provided to remove cloud-init logs and reboot after clean. The 'cloud-init status' command allows the user or script to check whether cloud-init has finished all configuration stages and whether errors occurred. An optional --wait argument will poll on a 0.25 second interval until cloud-init configuration is complete. The benefit here is scripts can block on cloud-init completion before performing post-config tasks. --- cloudinit/util.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 6c014ba5..320d64e0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1398,6 +1398,32 @@ def get_output_cfg(cfg, mode): return ret +def get_config_logfiles(cfg): + """Return a list of log file paths from the configuration dictionary. + + @param cfg: The cloud-init merged configuration dictionary. + """ + logs = [] + if not cfg or not isinstance(cfg, dict): + return logs + default_log = cfg.get('def_log_file') + if default_log: + logs.append(default_log) + for fmt in get_output_cfg(cfg, None): + if not fmt: + continue + match = re.match('(?P\||>+)\s*(?P.*)', fmt) + if not match: + continue + target = match.group('target') + parts = target.split() + if len(parts) == 1: + logs.append(target) + elif ['tee', '-a'] == parts[:2]: + logs.append(parts[2]) + return list(set(logs)) + + def logexc(log, msg, *args): # Setting this here allows this to change # levels easily (not always error level) -- cgit v1.2.3 From 0cf6db3617e0cebeb89c4809396f84360827e96c Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 5 Dec 2017 16:42:35 -0700 Subject: Datasources: Formalize DataSource get_data and related properties. Each DataSource subclass must define its own get_data method. This branch formalizes our DataSource class to require that subclasses define an explicit dsname for sourcing cloud-config datasource configuration. Subclasses must also override the _get_data method or a NotImplementedError is raised. The branch also writes /run/cloud-init/instance-data.json. This file contains all meta-data, user-data and vendor-data and a standardized set of metadata keys in a json blob which other utilities with root-access could make use of. Because some meta-data or user-data is potentially sensitive the file is only readable by root. Generally most metadata content types should be json serializable. If specific keys or values are not serializable, those specific values will be base64encoded and the key path will be listed under the top-level key 'base64-encoded-keys' in instance-data.json. If json writing fails due to other TypeErrors or UnicodeDecodeErrors, a warning log will be emitted to /var/log/cloud-init.log and no instance-data.json will be created. --- cloudinit/analyze/__main__.py | 4 +- cloudinit/analyze/dump.py | 8 +- cloudinit/sources/DataSourceAliYun.py | 1 + cloudinit/sources/DataSourceAltCloud.py | 5 +- cloudinit/sources/DataSourceAzure.py | 4 +- cloudinit/sources/DataSourceBigstep.py | 5 +- cloudinit/sources/DataSourceCloudSigma.py | 5 +- cloudinit/sources/DataSourceCloudStack.py | 5 +- cloudinit/sources/DataSourceConfigDrive.py | 5 +- cloudinit/sources/DataSourceDigitalOcean.py | 5 +- cloudinit/sources/DataSourceEc2.py | 12 +- cloudinit/sources/DataSourceGCE.py | 5 +- cloudinit/sources/DataSourceMAAS.py | 5 +- cloudinit/sources/DataSourceNoCloud.py | 5 +- cloudinit/sources/DataSourceNone.py | 5 +- cloudinit/sources/DataSourceOVF.py | 5 +- cloudinit/sources/DataSourceOpenNebula.py | 5 +- cloudinit/sources/DataSourceOpenStack.py | 5 +- cloudinit/sources/DataSourceScaleway.py | 4 +- cloudinit/sources/DataSourceSmartOS.py | 5 +- cloudinit/sources/__init__.py | 129 +++++++++++-- cloudinit/sources/tests/__init__.py | 0 cloudinit/sources/tests/test_init.py | 202 +++++++++++++++++++++ cloudinit/tests/helpers.py | 7 - cloudinit/util.py | 33 +++- tests/unittests/test_datasource/test_aliyun.py | 2 +- tests/unittests/test_datasource/test_altcloud.py | 22 ++- tests/unittests/test_datasource/test_azure.py | 28 +-- tests/unittests/test_datasource/test_cloudsigma.py | 13 +- tests/unittests/test_datasource/test_cloudstack.py | 19 +- .../unittests/test_datasource/test_configdrive.py | 3 +- .../unittests/test_datasource/test_digitalocean.py | 11 +- tests/unittests/test_datasource/test_ec2.py | 3 +- tests/unittests/test_datasource/test_gce.py | 3 +- tests/unittests/test_datasource/test_nocloud.py | 14 +- tests/unittests/test_datasource/test_opennebula.py | 12 +- tests/unittests/test_datasource/test_openstack.py | 12 +- tests/unittests/test_datasource/test_scaleway.py | 13 +- tests/unittests/test_datasource/test_smartos.py | 3 +- tests/unittests/test_ds_identify.py | 4 +- tests/unittests/test_runs/test_merge_run.py | 1 + tests/unittests/test_runs/test_simple_run.py | 3 +- 42 files changed, 517 insertions(+), 123 deletions(-) create mode 100644 cloudinit/sources/tests/__init__.py create mode 100644 cloudinit/sources/tests/test_init.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/analyze/__main__.py b/cloudinit/analyze/__main__.py index 69b9e43e..3ba5903f 100644 --- a/cloudinit/analyze/__main__.py +++ b/cloudinit/analyze/__main__.py @@ -6,6 +6,8 @@ import argparse import re import sys +from cloudinit.util import json_dumps + from . import dump from . import show @@ -112,7 +114,7 @@ def analyze_show(name, args): def analyze_dump(name, args): """Dump cloud-init events in json format""" (infh, outfh) = configure_io(args) - outfh.write(dump.json_dumps(_get_events(infh)) + '\n') + outfh.write(json_dumps(_get_events(infh)) + '\n') def _get_events(infile): diff --git a/cloudinit/analyze/dump.py b/cloudinit/analyze/dump.py index ca4da496..b071aa19 100644 --- a/cloudinit/analyze/dump.py +++ b/cloudinit/analyze/dump.py @@ -2,7 +2,6 @@ import calendar from datetime import datetime -import json import sys from cloudinit import util @@ -132,11 +131,6 @@ def parse_ci_logline(line): return event -def json_dumps(data): - return json.dumps(data, indent=1, sort_keys=True, - separators=(',', ': ')) - - def dump_events(cisource=None, rawdata=None): events = [] event = None @@ -169,7 +163,7 @@ def main(): else: cisource = sys.stdin - return json_dumps(dump_events(cisource)) + return util.json_dumps(dump_events(cisource)) if __name__ == "__main__": diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py index 43a7e42c..7ac8288d 100644 --- a/cloudinit/sources/DataSourceAliYun.py +++ b/cloudinit/sources/DataSourceAliYun.py @@ -11,6 +11,7 @@ ALIYUN_PRODUCT = "Alibaba Cloud ECS" class DataSourceAliYun(EC2.DataSourceEc2): + dsname = 'AliYun' metadata_urls = ['http://100.100.100.200'] # The minimum supported metadata_version from the ec2 metadata apis diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py index c78ad9eb..be2d6cf8 100644 --- a/cloudinit/sources/DataSourceAltCloud.py +++ b/cloudinit/sources/DataSourceAltCloud.py @@ -74,6 +74,9 @@ def read_user_data_callback(mount_dir): class DataSourceAltCloud(sources.DataSource): + + dsname = 'AltCloud' + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed = None @@ -112,7 +115,7 @@ class DataSourceAltCloud(sources.DataSource): return 'UNKNOWN' - def get_data(self): + def _get_data(self): ''' Description: User Data is passed to the launching instance which diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 14367e9c..6978d4e5 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -246,6 +246,8 @@ def temporary_hostname(temp_hostname, cfg, hostname_command='hostname'): class DataSourceAzure(sources.DataSource): + + dsname = 'Azure' _negotiated = False def __init__(self, sys_cfg, distro, paths): @@ -330,7 +332,7 @@ class DataSourceAzure(sources.DataSource): metadata['public-keys'] = key_value or pubkeys_from_crt_files(fp_files) return metadata - def get_data(self): + def _get_data(self): # azure removes/ejects the cdrom containing the ovf-env.xml # file on reboot. So, in order to successfully reboot we # need to look in the datadir and consider that valid diff --git a/cloudinit/sources/DataSourceBigstep.py b/cloudinit/sources/DataSourceBigstep.py index d7fcd45a..699a85b5 100644 --- a/cloudinit/sources/DataSourceBigstep.py +++ b/cloudinit/sources/DataSourceBigstep.py @@ -16,13 +16,16 @@ LOG = logging.getLogger(__name__) class DataSourceBigstep(sources.DataSource): + + dsname = 'Bigstep' + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.metadata = {} self.vendordata_raw = "" self.userdata_raw = "" - def get_data(self, apply_filter=False): + def _get_data(self, apply_filter=False): url = get_url_from_file() if url is None: return False diff --git a/cloudinit/sources/DataSourceCloudSigma.py b/cloudinit/sources/DataSourceCloudSigma.py index 19df16b1..4eaad475 100644 --- a/cloudinit/sources/DataSourceCloudSigma.py +++ b/cloudinit/sources/DataSourceCloudSigma.py @@ -23,6 +23,9 @@ class DataSourceCloudSigma(sources.DataSource): For more information about CloudSigma's Server Context: http://cloudsigma-docs.readthedocs.org/en/latest/server_context.html """ + + dsname = 'CloudSigma' + def __init__(self, sys_cfg, distro, paths): self.cepko = Cepko() self.ssh_public_key = '' @@ -46,7 +49,7 @@ class DataSourceCloudSigma(sources.DataSource): LOG.warning("failed to query dmi data for system product name") return False - def get_data(self): + def _get_data(self): """ Metadata is the whole server context and /meta/cloud-config is used as userdata. diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py index 9dc473fc..0df545fc 100644 --- a/cloudinit/sources/DataSourceCloudStack.py +++ b/cloudinit/sources/DataSourceCloudStack.py @@ -65,6 +65,9 @@ class CloudStackPasswordServerClient(object): class DataSourceCloudStack(sources.DataSource): + + dsname = 'CloudStack' + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed_dir = os.path.join(paths.seed_dir, 'cs') @@ -117,7 +120,7 @@ class DataSourceCloudStack(sources.DataSource): def get_config_obj(self): return self.cfg - def get_data(self): + def _get_data(self): seed_ret = {} if util.read_optional_seed(seed_ret, base=(self.seed_dir + "/")): self.userdata_raw = seed_ret['user-data'] diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index ef374f3f..870b3688 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -32,6 +32,9 @@ OPTICAL_DEVICES = tuple(('/dev/%s%s' % (z, i) for z in POSSIBLE_MOUNTS class DataSourceConfigDrive(openstack.SourceMixin, sources.DataSource): + + dsname = 'ConfigDrive' + def __init__(self, sys_cfg, distro, paths): super(DataSourceConfigDrive, self).__init__(sys_cfg, distro, paths) self.source = None @@ -50,7 +53,7 @@ class DataSourceConfigDrive(openstack.SourceMixin, sources.DataSource): mstr += "[source=%s]" % (self.source) return mstr - def get_data(self): + def _get_data(self): found = None md = {} results = {} diff --git a/cloudinit/sources/DataSourceDigitalOcean.py b/cloudinit/sources/DataSourceDigitalOcean.py index 5e7e66be..e0ef665e 100644 --- a/cloudinit/sources/DataSourceDigitalOcean.py +++ b/cloudinit/sources/DataSourceDigitalOcean.py @@ -27,6 +27,9 @@ MD_USE_IPV4LL = True class DataSourceDigitalOcean(sources.DataSource): + + dsname = 'DigitalOcean' + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.distro = distro @@ -44,7 +47,7 @@ class DataSourceDigitalOcean(sources.DataSource): def _get_sysinfo(self): return do_helper.read_sysinfo() - def get_data(self): + def _get_data(self): (is_do, droplet_id) = self._get_sysinfo() # only proceed if we know we are on DigitalOcean diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 7bbbfb63..e5c88334 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -31,6 +31,7 @@ _unset = "_unset" class Platforms(object): + # TODO Rename and move to cloudinit.cloud.CloudNames ALIYUN = "AliYun" AWS = "AWS" BRIGHTBOX = "Brightbox" @@ -45,6 +46,7 @@ class Platforms(object): class DataSourceEc2(sources.DataSource): + dsname = 'Ec2' # Default metadata urls that will be used if none are provided # They will be checked for 'resolveability' and some of the # following may be discarded if they do not resolve @@ -68,11 +70,15 @@ class DataSourceEc2(sources.DataSource): _fallback_interface = None def __init__(self, sys_cfg, distro, paths): - sources.DataSource.__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_data(self): + def _get_cloud_name(self): + """Return the cloud name as identified during _get_data.""" + return self.cloud_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'] @@ -274,7 +280,7 @@ class DataSourceEc2(sources.DataSource): return None @property - def cloud_platform(self): + def cloud_platform(self): # TODO rename cloud_name if self._cloud_platform is None: self._cloud_platform = identify_platform() return self._cloud_platform diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py index ccae4200..ad6dae37 100644 --- a/cloudinit/sources/DataSourceGCE.py +++ b/cloudinit/sources/DataSourceGCE.py @@ -42,6 +42,9 @@ class GoogleMetadataFetcher(object): class DataSourceGCE(sources.DataSource): + + dsname = 'GCE' + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.metadata = dict() @@ -50,7 +53,7 @@ class DataSourceGCE(sources.DataSource): BUILTIN_DS_CONFIG]) self.metadata_address = self.ds_cfg['metadata_url'] - def get_data(self): + def _get_data(self): ret = util.log_time( LOG.debug, 'Crawl of GCE metadata service', read_md, kwargs={'address': self.metadata_address}) diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py index 77df5a51..496bd06a 100644 --- a/cloudinit/sources/DataSourceMAAS.py +++ b/cloudinit/sources/DataSourceMAAS.py @@ -39,6 +39,9 @@ class DataSourceMAAS(sources.DataSource): hostname vendor-data """ + + dsname = "MAAS" + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.base_url = None @@ -62,7 +65,7 @@ class DataSourceMAAS(sources.DataSource): root = sources.DataSource.__str__(self) return "%s [%s]" % (root, self.base_url) - def get_data(self): + def _get_data(self): mcfg = self.ds_cfg try: diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py index e641244d..5d3a8ddb 100644 --- a/cloudinit/sources/DataSourceNoCloud.py +++ b/cloudinit/sources/DataSourceNoCloud.py @@ -20,6 +20,9 @@ LOG = logging.getLogger(__name__) class DataSourceNoCloud(sources.DataSource): + + dsname = "NoCloud" + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed = None @@ -32,7 +35,7 @@ class DataSourceNoCloud(sources.DataSource): root = sources.DataSource.__str__(self) return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode) - def get_data(self): + def _get_data(self): defaults = { "instance-id": "nocloud", "dsmode": self.dsmode, diff --git a/cloudinit/sources/DataSourceNone.py b/cloudinit/sources/DataSourceNone.py index 906bb278..e63a7e39 100644 --- a/cloudinit/sources/DataSourceNone.py +++ b/cloudinit/sources/DataSourceNone.py @@ -11,12 +11,15 @@ LOG = logging.getLogger(__name__) class DataSourceNone(sources.DataSource): + + dsname = "None" + def __init__(self, sys_cfg, distro, paths, ud_proc=None): sources.DataSource.__init__(self, sys_cfg, distro, paths, ud_proc) self.metadata = {} self.userdata_raw = '' - def get_data(self): + def _get_data(self): # If the datasource config has any provided 'fallback' # userdata or metadata, use it... if 'userdata_raw' in self.ds_cfg: diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index ccebf11a..6ac621f2 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -43,6 +43,9 @@ LOG = logging.getLogger(__name__) class DataSourceOVF(sources.DataSource): + + dsname = "OVF" + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed = None @@ -60,7 +63,7 @@ class DataSourceOVF(sources.DataSource): root = sources.DataSource.__str__(self) return "%s [seed=%s]" % (root, self.seed) - def get_data(self): + def _get_data(self): found = [] md = {} ud = "" diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 5fdac192..5da11847 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -31,6 +31,9 @@ CONTEXT_DISK_FILES = ["context.sh"] class DataSourceOpenNebula(sources.DataSource): + + dsname = "OpenNebula" + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed = None @@ -40,7 +43,7 @@ class DataSourceOpenNebula(sources.DataSource): root = sources.DataSource.__str__(self) return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode) - def get_data(self): + def _get_data(self): defaults = {"instance-id": DEFAULT_IID} results = None seed = None diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py index b64a7f24..e55a7638 100644 --- a/cloudinit/sources/DataSourceOpenStack.py +++ b/cloudinit/sources/DataSourceOpenStack.py @@ -24,6 +24,9 @@ DEFAULT_METADATA = { class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource): + + dsname = "OpenStack" + def __init__(self, sys_cfg, distro, paths): super(DataSourceOpenStack, self).__init__(sys_cfg, distro, paths) self.metadata_address = None @@ -96,7 +99,7 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource): self.metadata_address = url2base.get(avail_url) return bool(avail_url) - def get_data(self): + def _get_data(self): try: if not self.wait_for_metadata_service(): return False diff --git a/cloudinit/sources/DataSourceScaleway.py b/cloudinit/sources/DataSourceScaleway.py index 3a8a8e8f..b0b19c93 100644 --- a/cloudinit/sources/DataSourceScaleway.py +++ b/cloudinit/sources/DataSourceScaleway.py @@ -169,6 +169,8 @@ def query_data_api(api_type, api_address, retries, timeout): class DataSourceScaleway(sources.DataSource): + dsname = "Scaleway" + def __init__(self, sys_cfg, distro, paths): super(DataSourceScaleway, self).__init__(sys_cfg, distro, paths) @@ -184,7 +186,7 @@ class DataSourceScaleway(sources.DataSource): self.retries = int(self.ds_cfg.get('retries', DEF_MD_RETRIES)) self.timeout = int(self.ds_cfg.get('timeout', DEF_MD_TIMEOUT)) - def get_data(self): + def _get_data(self): if not on_scaleway(): return False diff --git a/cloudinit/sources/DataSourceSmartOS.py b/cloudinit/sources/DataSourceSmartOS.py index 6c6902fd..86bfa5d8 100644 --- a/cloudinit/sources/DataSourceSmartOS.py +++ b/cloudinit/sources/DataSourceSmartOS.py @@ -159,6 +159,9 @@ LEGACY_USER_D = "/var/db" class DataSourceSmartOS(sources.DataSource): + + dsname = "Joyent" + _unset = "_unset" smartos_type = _unset md_client = _unset @@ -211,7 +214,7 @@ class DataSourceSmartOS(sources.DataSource): os.rename('/'.join([svc_path, 'provisioning']), '/'.join([svc_path, 'provision_success'])) - def get_data(self): + def _get_data(self): self._init() md = {} diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 9a43fbee..4b819ce6 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -10,9 +10,11 @@ import abc import copy +import json import os import six +from cloudinit.atomic_helper import write_json from cloudinit import importer from cloudinit import log as logging from cloudinit import type_utils @@ -33,6 +35,12 @@ DEP_FILESYSTEM = "FILESYSTEM" DEP_NETWORK = "NETWORK" DS_PREFIX = 'DataSource' +# File in which instance meta-data, user-data and vendor-data is written +INSTANCE_JSON_FILE = 'instance-data.json' + +# Key which can be provide a cloud's official product name to cloud-init +METADATA_CLOUD_NAME_KEY = 'cloud-name' + LOG = logging.getLogger(__name__) @@ -40,12 +48,39 @@ class DataSourceNotFoundException(Exception): pass +def process_base64_metadata(metadata, key_path=''): + """Strip ci-b64 prefix and return metadata with base64-encoded-keys set.""" + md_copy = copy.deepcopy(metadata) + md_copy['base64-encoded-keys'] = [] + for key, val in metadata.items(): + if key_path: + sub_key_path = key_path + '/' + key + else: + sub_key_path = key + if isinstance(val, str) and val.startswith('ci-b64:'): + md_copy['base64-encoded-keys'].append(sub_key_path) + md_copy[key] = val.replace('ci-b64:', '') + if isinstance(val, dict): + return_val = process_base64_metadata(val, sub_key_path) + md_copy['base64-encoded-keys'].extend( + return_val.pop('base64-encoded-keys')) + md_copy[key] = return_val + return md_copy + + @six.add_metaclass(abc.ABCMeta) class DataSource(object): dsmode = DSMODE_NETWORK default_locale = 'en_US.UTF-8' + # Datasource name needs to be set by subclasses to determine which + # cloud-config datasource key is loaded + dsname = '_undef' + + # Cached cloud_name as determined by _get_cloud_name + _cloud_name = None + def __init__(self, sys_cfg, distro, paths, ud_proc=None): self.sys_cfg = sys_cfg self.distro = distro @@ -56,17 +91,8 @@ class DataSource(object): self.vendordata = None self.vendordata_raw = None - # find the datasource config name. - # remove 'DataSource' from classname on front, and remove 'Net' on end. - # Both Foo and FooNet sources expect config in cfg['sources']['Foo'] - name = type_utils.obj_name(self) - if name.startswith(DS_PREFIX): - name = name[len(DS_PREFIX):] - if name.endswith('Net'): - name = name[0:-3] - - self.ds_cfg = util.get_cfg_by_path(self.sys_cfg, - ("datasource", name), {}) + self.ds_cfg = util.get_cfg_by_path( + self.sys_cfg, ("datasource", self.dsname), {}) if not self.ds_cfg: self.ds_cfg = {} @@ -78,6 +104,51 @@ class DataSource(object): def __str__(self): return type_utils.obj_name(self) + def _get_standardized_metadata(self): + """Return a dictionary of standardized metadata keys.""" + return {'v1': { + 'local-hostname': self.get_hostname(), + 'instance-id': self.get_instance_id(), + 'cloud-name': self.cloud_name, + 'region': self.region, + 'availability-zone': self.availability_zone}} + + def get_data(self): + """Datasources implement _get_data to setup metadata and userdata_raw. + + Minimally, the datasource should return a boolean True on success. + """ + return_value = self._get_data() + json_file = os.path.join(self.paths.run_dir, INSTANCE_JSON_FILE) + if not return_value: + return return_value + + instance_data = { + 'ds': { + 'meta-data': self.metadata, + 'user-data': self.get_userdata_raw(), + 'vendor-data': self.get_vendordata_raw()}} + instance_data.update( + self._get_standardized_metadata()) + try: + # Process content base64encoding unserializable values + content = util.json_dumps(instance_data) + # Strip base64: prefix and return base64-encoded-keys + processed_data = process_base64_metadata(json.loads(content)) + except TypeError as e: + LOG.warning('Error persisting instance-data.json: %s', str(e)) + return return_value + except UnicodeDecodeError as e: + LOG.warning('Error persisting instance-data.json: %s', str(e)) + return return_value + write_json(json_file, processed_data, mode=0o600) + return return_value + + def _get_data(self): + raise NotImplementedError( + 'Subclasses of DataSource must implement _get_data which' + ' sets self.metadata, vendordata_raw and userdata_raw.') + def get_userdata(self, apply_filter=False): if self.userdata is None: self.userdata = self.ud_proc.process(self.get_userdata_raw()) @@ -90,6 +161,34 @@ class DataSource(object): self.vendordata = self.ud_proc.process(self.get_vendordata_raw()) return self.vendordata + @property + def cloud_name(self): + """Return lowercase cloud name as determined by the datasource. + + Datasource can determine or define its own cloud product name in + metadata. + """ + if self._cloud_name: + return self._cloud_name + if self.metadata and self.metadata.get(METADATA_CLOUD_NAME_KEY): + 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() + return self._cloud_name + + def _get_cloud_name(self): + """Return the datasource name as it frequently matches cloud name. + + Should be overridden in subclasses which can run on multiple + cloud names, such as DatasourceEc2. + """ + return self.dsname + @property def launch_index(self): if not self.metadata: @@ -161,8 +260,11 @@ class DataSource(object): @property def availability_zone(self): - return self.metadata.get('availability-zone', - self.metadata.get('availability_zone')) + top_level_az = self.metadata.get( + 'availability-zone', self.metadata.get('availability_zone')) + if top_level_az: + return top_level_az + return self.metadata.get('placement', {}).get('availability-zone') @property def region(self): @@ -417,4 +519,5 @@ def list_from_depends(depends, ds_list): ret_list.append(cls) return ret_list + # vi: ts=4 expandtab diff --git a/cloudinit/sources/tests/__init__.py b/cloudinit/sources/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py new file mode 100644 index 00000000..af151154 --- /dev/null +++ b/cloudinit/sources/tests/test_init.py @@ -0,0 +1,202 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import os +import six +import stat + +from cloudinit.helpers import Paths +from cloudinit.sources import ( + INSTANCE_JSON_FILE, DataSource) +from cloudinit.tests.helpers import CiTestCase, skipIf +from cloudinit.user_data import UserDataProcessor +from cloudinit import util + + +class DataSourceTestSubclassNet(DataSource): + + dsname = 'MyTestSubclass' + + def __init__(self, sys_cfg, distro, paths, custom_userdata=None): + super(DataSourceTestSubclassNet, self).__init__( + sys_cfg, distro, paths) + self._custom_userdata = custom_userdata + + def _get_cloud_name(self): + return 'SubclassCloudName' + + def _get_data(self): + self.metadata = {'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion'} + if self._custom_userdata: + self.userdata_raw = self._custom_userdata + else: + self.userdata_raw = 'userdata_raw' + self.vendordata_raw = 'vendordata_raw' + return True + + +class InvalidDataSourceTestSubclassNet(DataSource): + pass + + +class TestDataSource(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestDataSource, self).setUp() + self.sys_cfg = {'datasource': {'_undef': {'key1': False}}} + self.distro = 'distrotest' # generally should be a Distro object + self.paths = Paths({}) + self.datasource = DataSource(self.sys_cfg, self.distro, self.paths) + + def test_datasource_init(self): + """DataSource initializes metadata attributes, ds_cfg and ud_proc.""" + self.assertEqual(self.paths, self.datasource.paths) + self.assertEqual(self.sys_cfg, self.datasource.sys_cfg) + self.assertEqual(self.distro, self.datasource.distro) + self.assertIsNone(self.datasource.userdata) + self.assertEqual({}, self.datasource.metadata) + self.assertIsNone(self.datasource.userdata_raw) + self.assertIsNone(self.datasource.vendordata) + self.assertIsNone(self.datasource.vendordata_raw) + self.assertEqual({'key1': False}, self.datasource.ds_cfg) + self.assertIsInstance(self.datasource.ud_proc, UserDataProcessor) + + def test_datasource_init_gets_ds_cfg_using_dsname(self): + """Init uses DataSource.dsname for sourcing ds_cfg.""" + sys_cfg = {'datasource': {'MyTestSubclass': {'key2': False}}} + distro = 'distrotest' # generally should be a Distro object + paths = Paths({}) + datasource = DataSourceTestSubclassNet(sys_cfg, distro, paths) + self.assertEqual({'key2': False}, datasource.ds_cfg) + + def test_str_is_classname(self): + """The string representation of the datasource is the classname.""" + self.assertEqual('DataSource', str(self.datasource)) + self.assertEqual( + 'DataSourceTestSubclassNet', + str(DataSourceTestSubclassNet('', '', self.paths))) + + def test__get_data_unimplemented(self): + """Raise an error when _get_data is not implemented.""" + with self.assertRaises(NotImplementedError) as context_manager: + self.datasource.get_data() + self.assertIn( + 'Subclasses of DataSource must implement _get_data', + str(context_manager.exception)) + datasource2 = InvalidDataSourceTestSubclassNet( + self.sys_cfg, self.distro, self.paths) + with self.assertRaises(NotImplementedError) as context_manager: + datasource2.get_data() + self.assertIn( + 'Subclasses of DataSource must implement _get_data', + str(context_manager.exception)) + + def test_get_data_calls_subclass__get_data(self): + """Datasource.get_data uses the subclass' version of _get_data.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp})) + self.assertTrue(datasource.get_data()) + self.assertEqual( + {'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion'}, + datasource.metadata) + self.assertEqual('userdata_raw', datasource.userdata_raw) + self.assertEqual('vendordata_raw', datasource.vendordata_raw) + + def test_get_data_write_json_instance_data(self): + """get_data writes INSTANCE_JSON_FILE to run_dir as readonly root.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp})) + datasource.get_data() + json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) + content = util.load_file(json_file) + expected = { + 'base64-encoded-keys': [], + 'v1': { + 'availability-zone': 'myaz', + 'cloud-name': 'subclasscloudname', + 'instance-id': 'iid-datasource', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion'}, + 'ds': { + 'meta-data': {'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion'}, + 'user-data': 'userdata_raw', + 'vendor-data': 'vendordata_raw'}} + self.assertEqual(expected, util.load_json(content)) + file_stat = os.stat(json_file) + self.assertEqual(0o600, stat.S_IMODE(file_stat.st_mode)) + + def test_get_data_handles_redacted_unserializable_content(self): + """get_data warns unserializable content in INSTANCE_JSON_FILE.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp}), + custom_userdata={'key1': 'val1', 'key2': {'key2.1': self.paths}}) + self.assertTrue(datasource.get_data()) + json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) + content = util.load_file(json_file) + expected_userdata = { + 'key1': 'val1', + 'key2': { + 'key2.1': "Warning: redacted unserializable type "}} + instance_json = util.load_json(content) + self.assertEqual( + expected_userdata, instance_json['ds']['user-data']) + + @skipIf(not six.PY3, "json serialization on <= py2.7 handles bytes") + def test_get_data_base64encodes_unserializable_bytes(self): + """On py3, get_data base64encodes any unserializable content.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp}), + custom_userdata={'key1': 'val1', 'key2': {'key2.1': b'\x123'}}) + self.assertTrue(datasource.get_data()) + json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) + content = util.load_file(json_file) + instance_json = util.load_json(content) + self.assertEqual( + ['ds/user-data/key2/key2.1'], + instance_json['base64-encoded-keys']) + self.assertEqual( + {'key1': 'val1', 'key2': {'key2.1': 'EjM='}}, + instance_json['ds']['user-data']) + + @skipIf(not six.PY2, "json serialization on <= py2.7 handles bytes") + def test_get_data_handles_bytes_values(self): + """On py2 get_data handles bytes values without having to b64encode.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp}), + custom_userdata={'key1': 'val1', 'key2': {'key2.1': b'\x123'}}) + self.assertTrue(datasource.get_data()) + json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) + content = util.load_file(json_file) + instance_json = util.load_json(content) + self.assertEqual([], instance_json['base64-encoded-keys']) + self.assertEqual( + {'key1': 'val1', 'key2': {'key2.1': '\x123'}}, + instance_json['ds']['user-data']) + + @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8") + def test_non_utf8_encoding_logs_warning(self): + """When non-utf-8 values exist in py2 instance-data is not written.""" + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp}), + custom_userdata={'key1': 'val1', 'key2': {'key2.1': b'ab\xaadef'}}) + self.assertTrue(datasource.get_data()) + json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) + self.assertFalse(os.path.exists(json_file)) + self.assertIn( + "WARNING: Error persisting instance-data.json: 'utf8' codec can't" + " decode byte 0xaa in position 2: invalid start byte", + self.logs.getvalue()) diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 6f88a5b7..feb884ab 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -3,7 +3,6 @@ from __future__ import print_function import functools -import json import logging import os import shutil @@ -337,12 +336,6 @@ def dir2dict(startdir, prefix=None): return flist -def json_dumps(data): - # print data in nicely formatted json. - return json.dumps(data, indent=1, sort_keys=True, - separators=(',', ': ')) - - def wrap_and_call(prefix, mocks, func, *args, **kwargs): """ call func(args, **kwargs) with mocks applied, then unapplies mocks diff --git a/cloudinit/util.py b/cloudinit/util.py index 320d64e0..11e96a77 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -533,15 +533,6 @@ def multi_log(text, console=True, stderr=True, log.log(log_level, text) -def load_json(text, root_types=(dict,)): - decoded = json.loads(decode_binary(text)) - if not isinstance(decoded, tuple(root_types)): - expected_types = ", ".join([str(t) for t in root_types]) - raise TypeError("(%s) root types expected, got %s instead" - % (expected_types, type(decoded))) - return decoded - - def is_ipv4(instr): """determine if input string is a ipv4 address. return boolean.""" toks = instr.split('.') @@ -1480,7 +1471,31 @@ def ensure_dirs(dirlist, mode=0o755): ensure_dir(d, mode) +def load_json(text, root_types=(dict,)): + decoded = json.loads(decode_binary(text)) + if not isinstance(decoded, tuple(root_types)): + expected_types = ", ".join([str(t) for t in root_types]) + raise TypeError("(%s) root types expected, got %s instead" + % (expected_types, type(decoded))) + return decoded + + +def json_serialize_default(_obj): + """Handler for types which aren't json serializable.""" + try: + return 'ci-b64:{0}'.format(b64e(_obj)) + except AttributeError: + return 'Warning: redacted unserializable type {0}'.format(type(_obj)) + + +def json_dumps(data): + """Return data in nicely formatted json.""" + return json.dumps(data, indent=1, sort_keys=True, + separators=(',', ': '), default=json_serialize_default) + + def yaml_dumps(obj, explicit_start=True, explicit_end=True): + """Return data in nicely formatted yaml.""" return yaml.safe_dump(obj, line_break="\n", indent=4, diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py index 82ee9714..714f5dac 100644 --- a/tests/unittests/test_datasource/test_aliyun.py +++ b/tests/unittests/test_datasource/test_aliyun.py @@ -67,7 +67,7 @@ class TestAliYunDatasource(test_helpers.HttprettyTestCase): super(TestAliYunDatasource, self).setUp() cfg = {'datasource': {'AliYun': {'timeout': '1', 'max_wait': '1'}}} distro = {} - paths = helpers.Paths({}) + paths = helpers.Paths({'run_dir': self.tmp_dir()}) self.ds = ay.DataSourceAliYun(cfg, distro, paths) self.metadata_address = self.ds.metadata_urls[0] diff --git a/tests/unittests/test_datasource/test_altcloud.py b/tests/unittests/test_datasource/test_altcloud.py index a4dfb540..3253f3ad 100644 --- a/tests/unittests/test_datasource/test_altcloud.py +++ b/tests/unittests/test_datasource/test_altcloud.py @@ -18,7 +18,7 @@ import tempfile from cloudinit import helpers from cloudinit import util -from cloudinit.tests.helpers import TestCase +from cloudinit.tests.helpers import CiTestCase import cloudinit.sources.DataSourceAltCloud as dsac @@ -97,7 +97,7 @@ def _dmi_data(expected): return _data -class TestGetCloudType(TestCase): +class TestGetCloudType(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.get_cloud_type() ''' @@ -143,14 +143,16 @@ class TestGetCloudType(TestCase): self.assertEqual('UNKNOWN', dsrc.get_cloud_type()) -class TestGetDataCloudInfoFile(TestCase): +class TestGetDataCloudInfoFile(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.get_data() With a contrived CLOUD_INFO_FILE ''' 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, '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 @@ -207,14 +209,16 @@ class TestGetDataCloudInfoFile(TestCase): self.assertEqual(False, dsrc.get_data()) -class TestGetDataNoCloudInfoFile(TestCase): +class TestGetDataNoCloudInfoFile(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.get_data() Without a CLOUD_INFO_FILE ''' 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, 'run_dir': self.tmp}) self.dmi_data = util.read_dmi_data dsac.CLOUD_INFO_FILE = \ 'no such file' @@ -254,7 +258,7 @@ class TestGetDataNoCloudInfoFile(TestCase): self.assertEqual(False, dsrc.get_data()) -class TestUserDataRhevm(TestCase): +class TestUserDataRhevm(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.user_data_rhevm() ''' @@ -320,7 +324,7 @@ class TestUserDataRhevm(TestCase): self.assertEqual(False, dsrc.user_data_rhevm()) -class TestUserDataVsphere(TestCase): +class TestUserDataVsphere(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.user_data_vsphere() ''' @@ -368,7 +372,7 @@ class TestUserDataVsphere(TestCase): self.assertEqual(1, m_mount_cb.call_count) -class TestReadUserDataCallback(TestCase): +class TestReadUserDataCallback(CiTestCase): ''' Test to exercise method: DataSourceAltCloud.read_user_data_callback() ''' diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 7cb1812a..226c214a 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -11,9 +11,7 @@ from cloudinit.tests.helpers import (CiTestCase, TestCase, populate_dir, mock, import crypt import os -import shutil import stat -import tempfile import xml.etree.ElementTree as ET import yaml @@ -84,11 +82,11 @@ class TestAzureDataSource(CiTestCase): super(TestAzureDataSource, self).setUp() if PY26: raise SkipTest("Does not work on python 2.6") - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp) + self.tmp = self.tmp_dir() # patch cloud_dir, so our 'seed_dir' is guaranteed empty - self.paths = helpers.Paths({'cloud_dir': self.tmp}) + self.paths = helpers.Paths( + {'cloud_dir': self.tmp, 'run_dir': self.tmp}) self.waagent_d = os.path.join(self.tmp, 'var', 'lib', 'waagent') self.patches = ExitStack() @@ -642,7 +640,7 @@ fdescfs /dev/fd fdescfs rw 0 0 self.assertEqual(netconfig, expected_config) -class TestAzureBounce(TestCase): +class TestAzureBounce(CiTestCase): def mock_out_azure_moving_parts(self): self.patches.enter_context( @@ -669,10 +667,10 @@ class TestAzureBounce(TestCase): def setUp(self): super(TestAzureBounce, self).setUp() - self.tmp = tempfile.mkdtemp() + self.tmp = self.tmp_dir() self.waagent_d = os.path.join(self.tmp, 'var', 'lib', 'waagent') - self.paths = helpers.Paths({'cloud_dir': self.tmp}) - self.addCleanup(shutil.rmtree, self.tmp) + self.paths = helpers.Paths( + {'cloud_dir': self.tmp, 'run_dir': self.tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d self.patches = ExitStack() self.mock_out_azure_moving_parts() @@ -714,21 +712,24 @@ class TestAzureBounce(TestCase): def test_disabled_bounce_does_not_change_hostname(self): cfg = {'hostname_bounce': {'policy': 'off'}} - self._get_ds(self.get_ovf_env_with_dscfg('test-host', cfg)).get_data() + ds = self._get_ds(self.get_ovf_env_with_dscfg('test-host', cfg)) + ds.get_data() self.assertEqual(0, self.set_hostname.call_count) @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') def test_disabled_bounce_does_not_perform_bounce( self, perform_hostname_bounce): cfg = {'hostname_bounce': {'policy': 'off'}} - self._get_ds(self.get_ovf_env_with_dscfg('test-host', cfg)).get_data() + ds = self._get_ds(self.get_ovf_env_with_dscfg('test-host', cfg)) + ds.get_data() self.assertEqual(0, perform_hostname_bounce.call_count) def test_same_hostname_does_not_change_hostname(self): host_name = 'unchanged-host-name' self.get_hostname.return_value = host_name cfg = {'hostname_bounce': {'policy': 'yes'}} - self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg)).get_data() + ds = self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg)) + ds.get_data() self.assertEqual(0, self.set_hostname.call_count) @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') @@ -737,7 +738,8 @@ class TestAzureBounce(TestCase): host_name = 'unchanged-host-name' self.get_hostname.return_value = host_name cfg = {'hostname_bounce': {'policy': 'yes'}} - self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg)).get_data() + ds = self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg)) + ds.get_data() self.assertEqual(0, perform_hostname_bounce.call_count) @mock.patch('cloudinit.sources.DataSourceAzure.perform_hostname_bounce') diff --git a/tests/unittests/test_datasource/test_cloudsigma.py b/tests/unittests/test_datasource/test_cloudsigma.py index e4c59907..f6a59b6b 100644 --- a/tests/unittests/test_datasource/test_cloudsigma.py +++ b/tests/unittests/test_datasource/test_cloudsigma.py @@ -3,6 +3,7 @@ import copy from cloudinit.cs_utils import Cepko +from cloudinit import helpers from cloudinit import sources from cloudinit.sources import DataSourceCloudSigma @@ -38,10 +39,12 @@ class CepkoMock(Cepko): return self -class DataSourceCloudSigmaTest(test_helpers.TestCase): +class DataSourceCloudSigmaTest(test_helpers.CiTestCase): def setUp(self): super(DataSourceCloudSigmaTest, self).setUp() - self.datasource = DataSourceCloudSigma.DataSourceCloudSigma("", "", "") + self.paths = helpers.Paths({'run_dir': self.tmp_dir()}) + self.datasource = DataSourceCloudSigma.DataSourceCloudSigma( + "", "", paths=self.paths) self.datasource.is_running_in_cloudsigma = lambda: True self.datasource.cepko = CepkoMock(SERVER_CONTEXT) self.datasource.get_data() @@ -85,7 +88,8 @@ class DataSourceCloudSigmaTest(test_helpers.TestCase): def test_lack_of_vendor_data(self): stripped_context = copy.deepcopy(SERVER_CONTEXT) del stripped_context["vendor_data"] - self.datasource = DataSourceCloudSigma.DataSourceCloudSigma("", "", "") + self.datasource = DataSourceCloudSigma.DataSourceCloudSigma( + "", "", paths=self.paths) self.datasource.cepko = CepkoMock(stripped_context) self.datasource.get_data() @@ -94,7 +98,8 @@ class DataSourceCloudSigmaTest(test_helpers.TestCase): def test_lack_of_cloudinit_key_in_vendor_data(self): stripped_context = copy.deepcopy(SERVER_CONTEXT) del stripped_context["vendor_data"]["cloudinit"] - self.datasource = DataSourceCloudSigma.DataSourceCloudSigma("", "", "") + self.datasource = DataSourceCloudSigma.DataSourceCloudSigma( + "", "", paths=self.paths) self.datasource.cepko = CepkoMock(stripped_context) self.datasource.get_data() diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py index 96144b64..d6d2d6b2 100644 --- a/tests/unittests/test_datasource/test_cloudstack.py +++ b/tests/unittests/test_datasource/test_cloudstack.py @@ -33,6 +33,7 @@ class TestCloudStackPasswordFetching(CiTestCase): self.patches.enter_context(mock.patch( mod_name + '.dhcp.networkd_get_option_from_leases', get_networkd_server_address)) + self.tmp = self.tmp_dir() def _set_password_server_response(self, response_string): subp = mock.MagicMock(return_value=(response_string, '')) @@ -43,26 +44,30 @@ class TestCloudStackPasswordFetching(CiTestCase): def test_empty_password_doesnt_create_config(self): self._set_password_server_response('') - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) ds.get_data() self.assertEqual({}, ds.get_config_obj()) def test_saved_password_doesnt_create_config(self): self._set_password_server_response('saved_password') - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) ds.get_data() self.assertEqual({}, ds.get_config_obj()) def test_password_sets_password(self): password = 'SekritSquirrel' self._set_password_server_response(password) - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) ds.get_data() self.assertEqual(password, ds.get_config_obj()['password']) def test_bad_request_doesnt_stop_ds_from_working(self): self._set_password_server_response('bad_request') - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) self.assertTrue(ds.get_data()) def assertRequestTypesSent(self, subp, expected_request_types): @@ -77,14 +82,16 @@ class TestCloudStackPasswordFetching(CiTestCase): def test_valid_response_means_password_marked_as_saved(self): password = 'SekritSquirrel' subp = self._set_password_server_response(password) - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) ds.get_data() self.assertRequestTypesSent(subp, ['send_my_password', 'saved_password']) def _check_password_not_saved_for(self, response_string): subp = self._set_password_server_response(response_string) - ds = DataSourceCloudStack({}, None, helpers.Paths({})) + ds = DataSourceCloudStack( + {}, None, helpers.Paths({'run_dir': self.tmp})) ds.get_data() self.assertRequestTypesSent(subp, ['send_my_password']) diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 237c189b..98497886 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -725,8 +725,9 @@ class TestConvertNetworkData(TestCase): def cfg_ds_from_dir(seed_d): + tmp = tempfile.mkdtemp() cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': tmp})) cfg_ds.seed_dir = seed_d cfg_ds.known_macs = KNOWN_MACS.copy() if not cfg_ds.get_data(): diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py index f264f361..ec321733 100644 --- a/tests/unittests/test_datasource/test_digitalocean.py +++ b/tests/unittests/test_datasource/test_digitalocean.py @@ -13,7 +13,7 @@ from cloudinit import settings from cloudinit.sources import DataSourceDigitalOcean from cloudinit.sources.helpers import digitalocean -from cloudinit.tests.helpers import mock, TestCase +from cloudinit.tests.helpers import mock, CiTestCase DO_MULTIPLE_KEYS = ["ssh-rsa AAAAB3NzaC1yc2EAAAA... test1@do.co", "ssh-rsa AAAAB3NzaC1yc2EAAAA... test2@do.co"] @@ -135,14 +135,17 @@ def _mock_dmi(): return (True, DO_META.get('id')) -class TestDataSourceDigitalOcean(TestCase): +class TestDataSourceDigitalOcean(CiTestCase): """ Test reading the meta-data """ + def setUp(self): + super(TestDataSourceDigitalOcean, self).setUp() + self.tmp = self.tmp_dir() def get_ds(self, get_sysinfo=_mock_dmi): ds = DataSourceDigitalOcean.DataSourceDigitalOcean( - settings.CFG_BUILTIN, None, helpers.Paths({})) + settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp})) ds.use_ip4LL = False if get_sysinfo is not None: ds._get_sysinfo = get_sysinfo @@ -194,7 +197,7 @@ class TestDataSourceDigitalOcean(TestCase): self.assertIsInstance(ds.get_public_ssh_keys(), list) -class TestNetworkConvert(TestCase): +class TestNetworkConvert(CiTestCase): @mock.patch('cloudinit.net.get_interfaces_by_mac') def _get_networking(self, m_get_by_mac): diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index ba328ee9..ba042eac 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -186,6 +186,7 @@ class TestEc2(test_helpers.HttprettyTestCase): super(TestEc2, self).setUp() self.datasource = ec2.DataSourceEc2 self.metadata_addr = self.datasource.metadata_urls[0] + self.tmp = self.tmp_dir() def data_url(self, version): """Return a metadata url based on the version provided.""" @@ -199,7 +200,7 @@ class TestEc2(test_helpers.HttprettyTestCase): def _setup_ds(self, sys_cfg, platform_data, md, md_version=None): self.uris = [] distro = {} - paths = helpers.Paths({}) + paths = helpers.Paths({'run_dir': self.tmp}) if sys_cfg is None: sys_cfg = {} ds = self.datasource(sys_cfg=sys_cfg, distro=distro, paths=paths) diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py index d399ae7a..82c788dc 100644 --- a/tests/unittests/test_datasource/test_gce.py +++ b/tests/unittests/test_datasource/test_gce.py @@ -70,9 +70,10 @@ def _set_mock_metadata(gce_meta=None): class TestDataSourceGCE(test_helpers.HttprettyTestCase): def setUp(self): + tmp = self.tmp_dir() self.ds = DataSourceGCE.DataSourceGCE( settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': tmp})) ppatch = self.m_platform_reports_gce = mock.patch( 'cloudinit.sources.DataSourceGCE.platform_reports_gce') self.m_platform_reports_gce = ppatch.start() diff --git a/tests/unittests/test_datasource/test_nocloud.py b/tests/unittests/test_datasource/test_nocloud.py index fea9156b..70d50de4 100644 --- a/tests/unittests/test_datasource/test_nocloud.py +++ b/tests/unittests/test_datasource/test_nocloud.py @@ -3,22 +3,20 @@ from cloudinit import helpers from cloudinit.sources import DataSourceNoCloud from cloudinit import util -from cloudinit.tests.helpers import TestCase, populate_dir, mock, ExitStack +from cloudinit.tests.helpers import CiTestCase, populate_dir, mock, ExitStack import os -import shutil -import tempfile import textwrap import yaml -class TestNoCloudDataSource(TestCase): +class TestNoCloudDataSource(CiTestCase): def setUp(self): super(TestNoCloudDataSource, self).setUp() - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp) - self.paths = helpers.Paths({'cloud_dir': self.tmp}) + self.tmp = self.tmp_dir() + self.paths = helpers.Paths( + {'cloud_dir': self.tmp, 'run_dir': self.tmp}) self.cmdline = "root=TESTCMDLINE" @@ -215,7 +213,7 @@ class TestNoCloudDataSource(TestCase): self.assertNotIn(gateway, str(dsrc.network_config)) -class TestParseCommandLineData(TestCase): +class TestParseCommandLineData(CiTestCase): def test_parse_cmdline_data_valid(self): ds_id = "ds=nocloud" diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index e7d55692..2326dd58 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -3,12 +3,10 @@ from cloudinit import helpers from cloudinit.sources import DataSourceOpenNebula as ds from cloudinit import util -from cloudinit.tests.helpers import mock, populate_dir, TestCase +from cloudinit.tests.helpers import mock, populate_dir, CiTestCase import os import pwd -import shutil -import tempfile import unittest @@ -36,14 +34,14 @@ PUBLIC_IP = '10.0.0.3' DS_PATH = "cloudinit.sources.DataSourceOpenNebula" -class TestOpenNebulaDataSource(TestCase): +class TestOpenNebulaDataSource(CiTestCase): parsed_user = None def setUp(self): super(TestOpenNebulaDataSource, self).setUp() - self.tmp = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, self.tmp) - self.paths = helpers.Paths({'cloud_dir': self.tmp}) + self.tmp = self.tmp_dir() + self.paths = helpers.Paths( + {'cloud_dir': self.tmp, 'run_dir': self.tmp}) # defaults for few tests self.ds = ds.DataSourceOpenNebula diff --git a/tests/unittests/test_datasource/test_openstack.py b/tests/unittests/test_datasource/test_openstack.py index ed367e05..42c31554 100644 --- a/tests/unittests/test_datasource/test_openstack.py +++ b/tests/unittests/test_datasource/test_openstack.py @@ -131,6 +131,10 @@ def _read_metadata_service(): class TestOpenStackDataSource(test_helpers.HttprettyTestCase): VERSION = 'latest' + def setUp(self): + super(TestOpenStackDataSource, self).setUp() + self.tmp = self.tmp_dir() + @hp.activate def test_successful(self): _register_uris(self.VERSION, EC2_FILES, EC2_META, OS_FILES) @@ -232,7 +236,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): _register_uris(self.VERSION, EC2_FILES, EC2_META, OS_FILES) ds_os = ds.DataSourceOpenStack(settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': self.tmp})) self.assertIsNone(ds_os.version) found = ds_os.get_data() self.assertTrue(found) @@ -256,7 +260,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): _register_uris(self.VERSION, {}, {}, os_files) ds_os = ds.DataSourceOpenStack(settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': self.tmp})) self.assertIsNone(ds_os.version) found = ds_os.get_data() self.assertFalse(found) @@ -271,7 +275,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): _register_uris(self.VERSION, {}, {}, os_files) ds_os = ds.DataSourceOpenStack(settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': self.tmp})) ds_os.ds_cfg = { 'max_wait': 0, 'timeout': 0, @@ -294,7 +298,7 @@ class TestOpenStackDataSource(test_helpers.HttprettyTestCase): _register_uris(self.VERSION, {}, {}, os_files) ds_os = ds.DataSourceOpenStack(settings.CFG_BUILTIN, None, - helpers.Paths({})) + helpers.Paths({'run_dir': self.tmp})) ds_os.ds_cfg = { 'max_wait': 0, 'timeout': 0, diff --git a/tests/unittests/test_datasource/test_scaleway.py b/tests/unittests/test_datasource/test_scaleway.py index 436df9ee..8dec06b1 100644 --- a/tests/unittests/test_datasource/test_scaleway.py +++ b/tests/unittests/test_datasource/test_scaleway.py @@ -9,7 +9,7 @@ from cloudinit import helpers from cloudinit import settings from cloudinit.sources import DataSourceScaleway -from cloudinit.tests.helpers import mock, HttprettyTestCase, TestCase +from cloudinit.tests.helpers import mock, HttprettyTestCase, CiTestCase class DataResponses(object): @@ -63,7 +63,11 @@ class MetadataResponses(object): return 200, headers, json.dumps(cls.FAKE_METADATA) -class TestOnScaleway(TestCase): +class TestOnScaleway(CiTestCase): + + def setUp(self): + super(TestOnScaleway, self).setUp() + self.tmp = self.tmp_dir() def install_mocks(self, fake_dmi, fake_file_exists, fake_cmdline): mock, faked = fake_dmi @@ -91,7 +95,7 @@ class TestOnScaleway(TestCase): # When not on Scaleway, get_data() returns False. datasource = DataSourceScaleway.DataSourceScaleway( - settings.CFG_BUILTIN, None, helpers.Paths({}) + settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': self.tmp}) ) self.assertFalse(datasource.get_data()) @@ -159,8 +163,9 @@ def get_source_address_adapter(*args, **kwargs): class TestDataSourceScaleway(HttprettyTestCase): def setUp(self): + tmp = self.tmp_dir() self.datasource = DataSourceScaleway.DataSourceScaleway( - settings.CFG_BUILTIN, None, helpers.Paths({}) + settings.CFG_BUILTIN, None, helpers.Paths({'run_dir': tmp}) ) super(TestDataSourceScaleway, self).setUp() diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py index 933d5b63..88bae5f9 100644 --- a/tests/unittests/test_datasource/test_smartos.py +++ b/tests/unittests/test_datasource/test_smartos.py @@ -359,7 +359,8 @@ class TestSmartOSDataSource(FilesystemMockingTestCase): self.tmp = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, self.tmp) - self.paths = c_helpers.Paths({'cloud_dir': self.tmp}) + self.paths = c_helpers.Paths( + {'cloud_dir': self.tmp, 'run_dir': self.tmp}) self.legacy_user_d = os.path.join(self.tmp, 'legacy_user_tmp') os.mkdir(self.legacy_user_d) diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index 1284e755..7a920d42 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -7,7 +7,7 @@ from uuid import uuid4 from cloudinit import safeyaml from cloudinit import util from cloudinit.tests.helpers import ( - CiTestCase, dir2dict, json_dumps, populate_dir) + CiTestCase, dir2dict, populate_dir) UNAME_MYSYS = ("Linux bart 4.4.0-62-generic #83-Ubuntu " "SMP Wed Jan 18 14:10:15 UTC 2017 x86_64 GNU/Linux") @@ -319,7 +319,7 @@ def _print_run_output(rc, out, err, cfg, files): '-- rc = %s --' % rc, '-- out --', str(out), '-- err --', str(err), - '-- cfg --', json_dumps(cfg)])) + '-- cfg --', util.json_dumps(cfg)])) print('-- files --') for k, v in files.items(): if "/_shwrap" in k: diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py index add93653..5d3f1ca3 100644 --- a/tests/unittests/test_runs/test_merge_run.py +++ b/tests/unittests/test_runs/test_merge_run.py @@ -23,6 +23,7 @@ class TestMergeRun(helpers.FilesystemMockingTestCase): cfg = { 'datasource_list': ['None'], 'cloud_init_modules': ['write-files'], + 'system_info': {'paths': {'run_dir': new_root}} } ud = self.readResource('user_data.1.txt') cloud_cfg = util.yaml_dumps(cfg) diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py index b8fb4794..762974e9 100644 --- a/tests/unittests/test_runs/test_simple_run.py +++ b/tests/unittests/test_runs/test_simple_run.py @@ -2,10 +2,10 @@ import os -from cloudinit.tests import helpers from cloudinit.settings import PER_INSTANCE from cloudinit import stages +from cloudinit.tests import helpers from cloudinit import util @@ -23,6 +23,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): 'datasource_list': ['None'], 'runcmd': ['ls /etc'], # test ALL_DISTROS 'spacewalk': {}, # test non-ubuntu distros module definition + 'system_info': {'paths': {'run_dir': self.new_root}}, 'write_files': [ { 'path': '/etc/blah.ini', -- cgit v1.2.3 From c6a6f59e80f1fd62562b1fe9acfd45e1cee3cbe8 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 15 Dec 2017 15:24:53 -0700 Subject: lint: Fix lints seen by pylint version 1.8.1. This branch resolves lints seen by pylint revision 1.8.1 and updates our pinned tox pylint dependency used by our tox pylint target. --- cloudinit/config/cc_apt_configure.py | 5 +++-- cloudinit/config/cc_disk_setup.py | 8 +++++--- cloudinit/config/cc_landscape.py | 8 ++++---- cloudinit/config/cc_ntp.py | 10 +++++----- cloudinit/config/cc_seed_random.py | 3 ++- cloudinit/config/cc_snap_config.py | 7 +++++-- cloudinit/net/cmdline.py | 9 +++++---- cloudinit/net/network_state.py | 8 +++++--- cloudinit/sources/DataSourceAltCloud.py | 2 +- cloudinit/sources/DataSourceAzure.py | 6 +++--- cloudinit/sources/DataSourceOpenNebula.py | 5 +++-- cloudinit/sources/helpers/azure.py | 3 ++- cloudinit/util.py | 3 ++- tox.ini | 2 +- 14 files changed, 46 insertions(+), 33 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 177cbcf7..5b9cbca0 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -275,8 +275,9 @@ def handle(name, ocfg, cloud, log, _): cfg = ocfg.get('apt', {}) if not isinstance(cfg, dict): - raise ValueError("Expected dictionary for 'apt' config, found %s", - type(cfg)) + raise ValueError( + "Expected dictionary for 'apt' config, found {config_type}".format( + config_type=type(cfg))) apply_debconf_selections(cfg, target) apply_apt(cfg, cloud, target) diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py index c2b83aea..c3e8c484 100644 --- a/cloudinit/config/cc_disk_setup.py +++ b/cloudinit/config/cc_disk_setup.py @@ -788,7 +788,8 @@ def mkpart(device, definition): # This prevents you from overwriting the device LOG.debug("Checking if device %s is a valid device", device) if not is_device_valid(device): - raise Exception("Device %s is not a disk device!", device) + raise Exception( + 'Device {device} is not a disk device!'.format(device=device)) # Remove the partition table entries if isinstance(layout, str) and layout.lower() == "remove": @@ -945,8 +946,9 @@ def mkfs(fs_cfg): # Check that we can create the FS if not (fs_type or fs_cmd): - raise Exception("No way to create filesystem '%s'. fs_type or fs_cmd " - "must be set.", label) + raise Exception( + "No way to create filesystem '{label}'. fs_type or fs_cmd " + "must be set.".format(label=label)) # Create the commands shell = False diff --git a/cloudinit/config/cc_landscape.py b/cloudinit/config/cc_landscape.py index 8f9f1abd..eaf1e940 100644 --- a/cloudinit/config/cc_landscape.py +++ b/cloudinit/config/cc_landscape.py @@ -94,10 +94,10 @@ def handle(_name, cfg, cloud, log, _args): ls_cloudcfg = cfg.get("landscape", {}) if not isinstance(ls_cloudcfg, (dict)): - raise RuntimeError(("'landscape' key existed in config," - " but not a dictionary type," - " is a %s instead"), - type_utils.obj_name(ls_cloudcfg)) + raise RuntimeError( + "'landscape' key existed in config, but not a dictionary type," + " is a {_type} instead".format( + _type=type_utils.obj_name(ls_cloudcfg))) if not ls_cloudcfg: return diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index f50bcb35..cbd0237d 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -106,9 +106,9 @@ def handle(name, cfg, cloud, log, _args): # TODO drop this when validate_cloudconfig_schema is strict=True if not isinstance(ntp_cfg, (dict)): - raise RuntimeError(("'ntp' key existed in config," - " but not a dictionary type," - " is a %s %instead"), type_utils.obj_name(ntp_cfg)) + raise RuntimeError( + "'ntp' key existed in config, but not a dictionary type," + " is a {_type} instead".format(_type=type_utils.obj_name(ntp_cfg))) validate_cloudconfig_schema(cfg, schema) if ntp_installable(): @@ -206,8 +206,8 @@ def write_ntp_config_template(cfg, cloud, path, template=None): if not template_fn: template_fn = cloud.get_template_filename('ntp.conf') if not template_fn: - raise RuntimeError(("No template found, " - "not rendering %s"), path) + raise RuntimeError( + 'No template found, not rendering {path}'.format(path=path)) templater.render_to_file(template_fn, path, params) diff --git a/cloudinit/config/cc_seed_random.py b/cloudinit/config/cc_seed_random.py index e76b9c09..65f6e777 100644 --- a/cloudinit/config/cc_seed_random.py +++ b/cloudinit/config/cc_seed_random.py @@ -95,7 +95,8 @@ def handle_random_seed_command(command, required, env=None): cmd = command[0] if not util.which(cmd): if required: - raise ValueError("command '%s' not found but required=true", cmd) + raise ValueError( + "command '{cmd}' not found but required=true".format(cmd=cmd)) else: LOG.debug("command '%s' not found for seed_command", cmd) return diff --git a/cloudinit/config/cc_snap_config.py b/cloudinit/config/cc_snap_config.py index fe0cc73e..e82c0811 100644 --- a/cloudinit/config/cc_snap_config.py +++ b/cloudinit/config/cc_snap_config.py @@ -87,7 +87,9 @@ def add_assertions(assertions=None): assertions = [] if not isinstance(assertions, list): - raise ValueError('assertion parameter was not a list: %s', assertions) + raise ValueError( + 'assertion parameter was not a list: {assertions}'.format( + assertions=assertions)) snap_cmd = [SNAPPY_CMD, 'ack'] combined = "\n".join(assertions) @@ -115,7 +117,8 @@ def add_snap_user(cfg=None): cfg = {} if not isinstance(cfg, dict): - raise ValueError('configuration parameter was not a dict: %s', cfg) + raise ValueError( + 'configuration parameter was not a dict: {cfg}'.format(cfg=cfg)) snapuser = cfg.get('email', None) if not snapuser: diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py index 38b27a52..7b2cc9db 100755 --- a/cloudinit/net/cmdline.py +++ b/cloudinit/net/cmdline.py @@ -116,10 +116,11 @@ def config_from_klibc_net_cfg(files=None, mac_addrs=None): prev = names[name]['entry'] if prev.get('mac_address') != entry.get('mac_address'): raise ValueError( - "device '%s' was defined multiple times (%s)" - " but had differing mac addresses: %s -> %s.", - (name, ' '.join(names[name]['files']), - prev.get('mac_address'), entry.get('mac_address'))) + "device '{name}' was defined multiple times ({files})" + " but had differing mac addresses: {old} -> {new}.".format( + name=name, files=' '.join(names[name]['files']), + old=prev.get('mac_address'), + new=entry.get('mac_address'))) prev['subnets'].extend(entry['subnets']) names[name]['files'].append(cfg_file) else: diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index e9e2cf4e..31738c73 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -474,8 +474,9 @@ class NetworkStateInterpreter(object): elif bridge_stp in ['off', '0', 0]: bridge_stp = False else: - raise ValueError("Cannot convert bridge_stp value" - "(%s) to boolean", bridge_stp) + raise ValueError( + 'Cannot convert bridge_stp value ({stp}) to' + ' boolean'.format(stp=bridge_stp)) iface.update({'bridge_stp': bridge_stp}) interfaces.update({iface['name']: iface}) @@ -692,7 +693,8 @@ class NetworkStateInterpreter(object): elif cmd_type == "bond": self.handle_bond(v1_cmd) else: - raise ValueError('Unknown command type: %s', cmd_type) + raise ValueError('Unknown command type: {cmd_type}'.format( + cmd_type=cmd_type)) def _v2_to_v1_ipcfg(self, cfg): """Common ipconfig extraction from v2 to v1 subnets array.""" diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py index be2d6cf8..e1d0055b 100644 --- a/cloudinit/sources/DataSourceAltCloud.py +++ b/cloudinit/sources/DataSourceAltCloud.py @@ -145,7 +145,7 @@ class DataSourceAltCloud(sources.DataSource): else: cloud_type = self.get_cloud_type() - LOG.debug('cloud_type: ' + str(cloud_type)) + LOG.debug('cloud_type: %s', str(cloud_type)) if 'RHEV' in cloud_type: if self.user_data_rhevm(): diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 6978d4e5..e73b57b9 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -582,12 +582,12 @@ def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120, if os.path.exists(sempath): try: os.unlink(sempath) - LOG.debug(bmsg + " removed.") + LOG.debug('%s removed.', bmsg) except Exception as e: # python3 throws FileNotFoundError, python2 throws OSError - LOG.warning(bmsg + ": remove failed! (%s)", e) + LOG.warning('%s: remove failed! (%s)', bmsg, e) else: - LOG.debug(bmsg + " did not exist.") + LOG.debug('%s did not exist.', bmsg) return diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 5da11847..f66c95d7 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -332,8 +332,9 @@ def read_context_disk_dir(source_dir, asuser=None): try: pwd.getpwnam(asuser) except KeyError as e: - raise BrokenContextDiskDir("configured user '%s' " - "does not exist", asuser) + raise BrokenContextDiskDir( + "configured user '{user}' does not exist".format( + user=asuser)) try: path = os.path.join(source_dir, 'context.sh') content = util.load_file(path) diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index 959b1bda..6cda5721 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -274,7 +274,8 @@ class WALinuxAgentShim(object): name = os.path.basename(hook_file).replace('.json', '') dhcp_options[name] = json.loads(util.load_file((hook_file))) except ValueError: - raise ValueError("%s is not valid JSON data", hook_file) + raise ValueError( + '{_file} is not valid JSON data'.format(_file=hook_file)) return dhcp_options @staticmethod diff --git a/cloudinit/util.py b/cloudinit/util.py index 11e96a77..8a9f1ab2 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2327,7 +2327,8 @@ def pathprefix2dict(base, required=None, optional=None, delim=os.path.sep): missing.append(f) if len(missing): - raise ValueError("Missing required files: %s", ','.join(missing)) + raise ValueError( + 'Missing required files: {files}'.format(files=','.join(missing))) return ret diff --git a/tox.ini b/tox.ini index 0802072a..fdc8a665 100644 --- a/tox.ini +++ b/tox.ini @@ -24,7 +24,7 @@ setenv = basepython = python3 deps = # requirements - pylint==1.7.5 + pylint==1.8.1 # test-requirements because unit tests are now present in cloudinit tree -r{toxinidir}/test-requirements.txt commands = {envpython} -m pylint {posargs:cloudinit tests tools} -- cgit v1.2.3 From 6299e8d0cc230b0c9b41a69a5963bcd2c252c337 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 12 Jan 2018 14:23:26 -0700 Subject: Do not log warning on config files that represent None. This issue was first identified when manual_cache_clean was set, as ds-identify would write /run/cloud-init/cloud.cfg with # manual_cache_clean that would generate a warning as cloud-init expected to load a dict. Any other "empty" config would also log such a warning. Also fix reading of di_report to allow it to be None, as ds-identify would write: di_report: # manual_cache_clean which reads as 'di_report: None' rather than di_report: {}. LP: #1742479 --- cloudinit/cmd/main.py | 8 +++++++- cloudinit/util.py | 10 +++++----- tests/unittests/test_util.py | 8 ++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index 30b37fe1..d2f1b778 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -421,7 +421,13 @@ def di_report_warn(datasource, cfg): LOG.debug("no di_report found in config.") return - dicfg = cfg.get('di_report', {}) + dicfg = cfg['di_report'] + if dicfg is None: + # ds-identify may write 'di_report:\n #comment\n' + # which reads as {'di_report': None} + LOG.debug("di_report was None.") + return + if not isinstance(dicfg, dict): LOG.warning("di_report config not a dictionary: %s", dicfg) return diff --git a/cloudinit/util.py b/cloudinit/util.py index 8a9f1ab2..e42498d9 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -891,17 +891,17 @@ def load_yaml(blob, default=None, allowed=(dict,)): "of length %s with allowed root types %s", len(blob), allowed) converted = safeyaml.load(blob) - if not isinstance(converted, allowed): + if converted is None: + LOG.debug("loaded blob returned None, returning default.") + converted = default + elif not isinstance(converted, allowed): # Yes this will just be caught, but thats ok for now... raise TypeError(("Yaml load allows %s root types," " but got %s instead") % (allowed, type_utils.obj_name(converted))) loaded = converted except (yaml.YAMLError, TypeError, ValueError): - if len(blob) == 0: - LOG.debug("load_yaml given empty string, returning default") - else: - logexc(LOG, "Failed loading yaml blob") + logexc(LOG, "Failed loading yaml blob") return loaded diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 787ca208..d63b760e 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -299,6 +299,14 @@ class TestLoadYaml(helpers.TestCase): default=self.mydefault), myobj) + def test_none_returns_default(self): + """If yaml.load returns None, then default should be returned.""" + blobs = ("", " ", "# foo\n", "#") + mdef = self.mydefault + self.assertEqual( + [(b, self.mydefault) for b in blobs], + [(b, util.load_yaml(blob=b, default=mdef)) for b in blobs]) + class TestMountinfoParsing(helpers.ResourceUsingTestCase): def test_invalid_mountinfo(self): -- cgit v1.2.3 From 183d5785954af3a1e7603798d4a91ab126eb7bb9 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 23 Jan 2018 18:08:14 -0700 Subject: subp: make ProcessExecutionError have expected types in stderr, stdout. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When subp raised a ProcessExecutionError, that exception's stderr and stdout might end up being the string '-' rather than bytes. This mean that:    try:        subp(mycommand, decode=False)    except ProcessExecutionError as e:        pass Would have 'e.stdout' set to '-' while the caller would expect bytes. Also reduce the try/except block in subp to a specifically the two lines that may raise an OSError. --- cloudinit/util.py | 94 ++++++++++++++++++++++++-------------------- tests/unittests/test_util.py | 15 +++++++ 2 files changed, 66 insertions(+), 43 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index e42498d9..df0aa5db 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -253,12 +253,18 @@ class ProcessExecutionError(IOError): self.exit_code = exit_code if not stderr: - self.stderr = self.empty_attr + if stderr is None: + self.stderr = self.empty_attr + else: + self.stderr = stderr else: self.stderr = self._indent_text(stderr) if not stdout: - self.stdout = self.empty_attr + if stdout is None: + self.stdout = self.empty_attr + else: + self.stdout = stdout else: self.stdout = self._indent_text(stdout) @@ -1829,58 +1835,60 @@ def subp(args, data=None, rcs=None, env=None, capture=True, shell=False, env = env.copy() env.update(update_env) - try: - if target_path(target) != "/": - args = ['chroot', target] + list(args) + if target_path(target) != "/": + args = ['chroot', target] + list(args) - if not logstring: - LOG.debug(("Running command %s with allowed return codes %s" - " (shell=%s, capture=%s)"), args, rcs, shell, capture) - else: - LOG.debug(("Running hidden command to protect sensitive " - "input/output logstring: %s"), logstring) - - stdin = None - stdout = None - stderr = None - if capture: - stdout = subprocess.PIPE - stderr = subprocess.PIPE - if data is None: - # using devnull assures any reads get null, rather - # than possibly waiting on input. - devnull_fp = open(os.devnull) - stdin = devnull_fp - else: - stdin = subprocess.PIPE - if not isinstance(data, bytes): - data = data.encode() + if not logstring: + LOG.debug(("Running command %s with allowed return codes %s" + " (shell=%s, capture=%s)"), args, rcs, shell, capture) + else: + LOG.debug(("Running hidden command to protect sensitive " + "input/output logstring: %s"), logstring) + + stdin = None + stdout = None + stderr = None + if capture: + stdout = subprocess.PIPE + stderr = subprocess.PIPE + if data is None: + # using devnull assures any reads get null, rather + # than possibly waiting on input. + devnull_fp = open(os.devnull) + stdin = devnull_fp + else: + stdin = subprocess.PIPE + if not isinstance(data, bytes): + data = data.encode() + try: sp = subprocess.Popen(args, stdout=stdout, stderr=stderr, stdin=stdin, env=env, shell=shell) (out, err) = sp.communicate(data) - - # Just ensure blank instead of none. - if not out and capture: - out = b'' - if not err and capture: - err = b'' - if decode: - def ldecode(data, m='utf-8'): - if not isinstance(data, bytes): - return data - return data.decode(m, decode) - - out = ldecode(out) - err = ldecode(err) except OSError as e: - raise ProcessExecutionError(cmd=args, reason=e, - errno=e.errno) + raise ProcessExecutionError( + cmd=args, reason=e, errno=e.errno, + stdout="-" if decode else b"-", + stderr="-" if decode else b"-") finally: if devnull_fp: devnull_fp.close() + # Just ensure blank instead of none. + if not out and capture: + out = b'' + if not err and capture: + err = b'' + if decode: + def ldecode(data, m='utf-8'): + if not isinstance(data, bytes): + return data + return data.decode(m, decode) + + out = ldecode(out) + err = ldecode(err) + rc = sp.returncode if rc not in rcs: raise ProcessExecutionError(stdout=out, stderr=err, diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index d63b760e..4a92e741 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -623,6 +623,7 @@ class TestSubp(helpers.CiTestCase): utf8_valid = b'start \xc3\xa9 end' utf8_valid_2 = b'd\xc3\xa9j\xc8\xa7' printenv = [BASH, '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--'] + bogus_command = 'this-is-not-expected-to-be-a-program-name' def printf_cmd(self, *args): # bash's printf supports \xaa. So does /usr/bin/printf @@ -712,6 +713,20 @@ class TestSubp(helpers.CiTestCase): self.assertIsNone(err) self.assertIsNone(out) + def test_exception_has_out_err_are_bytes_if_decode_false(self): + """Raised exc should have stderr, stdout as bytes if no decode.""" + with self.assertRaises(util.ProcessExecutionError) as cm: + util.subp([self.bogus_command], decode=False) + self.assertTrue(isinstance(cm.exception.stdout, bytes)) + self.assertTrue(isinstance(cm.exception.stderr, bytes)) + + def test_exception_has_out_err_are_bytes_if_decode_true(self): + """Raised exc should have stderr, stdout as string if no decode.""" + with self.assertRaises(util.ProcessExecutionError) as cm: + util.subp([self.bogus_command], decode=True) + self.assertTrue(isinstance(cm.exception.stdout, six.string_types)) + self.assertTrue(isinstance(cm.exception.stderr, six.string_types)) + def test_bunch_of_slashes_in_path(self): self.assertEqual("/target/my/path/", util.target_path("/target/", "//my/path/")) -- cgit v1.2.3 From b28ab78089d362c5c6cab985feee0f5f84c9db44 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Mon, 27 Nov 2017 19:05:52 -0500 Subject: btrfs: support resizing if root is mounted ro. Resize of btrfs fails if the mount point for the file system we are trying to resize, i.e. the root of the filesystem is read only. With this change we use a known (currently snapper specific) rw location to work around a flaw that blocks resizing of the ro filesystem. LP: #1734787 --- cloudinit/config/cc_resizefs.py | 12 +++++- cloudinit/tests/test_util.py | 46 ++++++++++++++++++++++ cloudinit/util.py | 23 ++++++++--- .../test_handler/test_handler_resizefs.py | 22 ++++++++++- 4 files changed, 95 insertions(+), 8 deletions(-) create mode 100644 cloudinit/tests/test_util.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index 0d282e63..cec22bb7 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -59,7 +59,17 @@ __doc__ = get_schema_doc(schema) # Supplement python help() def _resize_btrfs(mount_point, devpth): - return ('btrfs', 'filesystem', 'resize', 'max', mount_point) + # If "/" is ro resize will fail. However it should be allowed since resize + # makes everything bigger and subvolumes that are not ro will benefit. + # Use a subvolume that is not ro to trick the resize operation to do the + # "right" thing. The use of ".snapshot" is specific to "snapper" a generic + # solution would be walk the subvolumes and find a rw mounted subvolume. + if (not util.mount_is_read_write(mount_point) and + os.path.isdir("%s/.snapshots" % mount_point)): + return ('btrfs', 'filesystem', 'resize', 'max', + '%s/.snapshots' % mount_point) + else: + return ('btrfs', 'filesystem', 'resize', 'max', mount_point) def _resize_ext(mount_point, devpth): diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py new file mode 100644 index 00000000..ba6bf699 --- /dev/null +++ b/cloudinit/tests/test_util.py @@ -0,0 +1,46 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +"""Tests for cloudinit.util""" + +import logging + +import cloudinit.util as util + +from cloudinit.tests.helpers import CiTestCase, mock + +LOG = logging.getLogger(__name__) + +MOUNT_INFO = [ + '68 0 8:3 / / ro,relatime shared:1 - btrfs /dev/sda1 ro,attr2,inode64', + '153 68 254:0 / /home rw,relatime shared:101 - xfs /dev/sda2 rw,attr2' +] + + +class TestUtil(CiTestCase): + + def test_parse_mount_info_no_opts_no_arg(self): + result = util.parse_mount_info('/home', MOUNT_INFO, LOG) + self.assertEqual(('/dev/sda2', 'xfs', '/home'), result) + + def test_parse_mount_info_no_opts_arg(self): + result = util.parse_mount_info('/home', MOUNT_INFO, LOG, False) + self.assertEqual(('/dev/sda2', 'xfs', '/home'), result) + + def test_parse_mount_info_with_opts(self): + result = util.parse_mount_info('/', MOUNT_INFO, LOG, True) + self.assertEqual( + ('/dev/sda1', 'btrfs', '/', 'ro,relatime'), + result + ) + + @mock.patch('cloudinit.util.get_mount_info') + def test_mount_is_rw(self, m_mount_info): + m_mount_info.return_value = ('/dev/sda1', 'btrfs', '/', 'rw,relatime') + is_rw = util.mount_is_read_write('/') + self.assertEqual(is_rw, True) + + @mock.patch('cloudinit.util.get_mount_info') + def test_mount_is_ro(self, m_mount_info): + m_mount_info.return_value = ('/dev/sda1', 'btrfs', '/', 'ro,relatime') + is_rw = util.mount_is_read_write('/') + self.assertEqual(is_rw, False) diff --git a/cloudinit/util.py b/cloudinit/util.py index df0aa5db..9976400f 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2059,7 +2059,7 @@ def expand_package_list(version_fmt, pkgs): return pkglist -def parse_mount_info(path, mountinfo_lines, log=LOG): +def parse_mount_info(path, mountinfo_lines, log=LOG, get_mnt_opts=False): """Return the mount information for PATH given the lines from /proc/$$/mountinfo.""" @@ -2121,11 +2121,16 @@ def parse_mount_info(path, mountinfo_lines, log=LOG): match_mount_point = mount_point match_mount_point_elements = mount_point_elements + mount_options = parts[5] - if devpth and fs_type and match_mount_point: - return (devpth, fs_type, match_mount_point) + if get_mnt_opts: + if devpth and fs_type and match_mount_point and mount_options: + return (devpth, fs_type, match_mount_point, mount_options) else: - return None + if devpth and fs_type and match_mount_point: + return (devpth, fs_type, match_mount_point) + + return None def parse_mtab(path): @@ -2195,7 +2200,7 @@ def parse_mount(path): return None -def get_mount_info(path, log=LOG): +def get_mount_info(path, log=LOG, get_mnt_opts=False): # Use /proc/$$/mountinfo to find the device where path is mounted. # This is done because with a btrfs filesystem using os.stat(path) # does not return the ID of the device. @@ -2227,7 +2232,7 @@ def get_mount_info(path, log=LOG): mountinfo_path = '/proc/%s/mountinfo' % os.getpid() if os.path.exists(mountinfo_path): lines = load_file(mountinfo_path).splitlines() - return parse_mount_info(path, lines, log) + return parse_mount_info(path, lines, log, get_mnt_opts) elif os.path.exists("/etc/mtab"): return parse_mtab(path) else: @@ -2613,4 +2618,10 @@ def wait_for_files(flist, maxwait, naplen=.5, log_pre=""): return need +def mount_is_read_write(mount_point): + """Check whether the given mount point is mounted rw""" + result = get_mount_info(mount_point, get_mnt_opts=True) + mount_opts = result[-1].split(',') + return mount_opts[0] == 'rw' + # vi: ts=4 expandtab diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py index 29d5574d..5aa3c498 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit.config.cc_resizefs import ( - can_skip_resize, handle, maybe_get_writable_device_path) + can_skip_resize, handle, maybe_get_writable_device_path, _resize_btrfs) from collections import namedtuple import logging @@ -293,5 +293,25 @@ class TestMaybeGetDevicePathAsWritableBlock(CiTestCase): " per kernel cmdline", self.logs.getvalue()) + @mock.patch('cloudinit.util.mount_is_read_write') + @mock.patch('cloudinit.config.cc_resizefs.os.path.isdir') + def test_resize_btrfs_mount_is_ro(self, m_is_dir, m_is_rw): + """Do not resize / directly if it is read-only. (LP: #1734787).""" + m_is_rw.return_value = False + m_is_dir.return_value = True + self.assertEqual( + ('btrfs', 'filesystem', 'resize', 'max', '//.snapshots'), + _resize_btrfs("/", "/dev/sda1")) + + @mock.patch('cloudinit.util.mount_is_read_write') + @mock.patch('cloudinit.config.cc_resizefs.os.path.isdir') + def test_resize_btrfs_mount_is_rw(self, m_is_dir, m_is_rw): + """Do not resize / directly if it is read-only. (LP: #1734787).""" + m_is_rw.return_value = True + m_is_dir.return_value = True + self.assertEqual( + ('btrfs', 'filesystem', 'resize', 'max', '/'), + _resize_btrfs("/", "/dev/sda1")) + # vi: ts=4 expandtab -- cgit v1.2.3 From 1d8c327139a8c291eeb244ee1a6a8badd83e9e72 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 26 Jan 2018 13:36:30 -0700 Subject: Fix potential cases of uninitialized variables. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While addressing undeclared variable in 'cloud-init status', I also fixed the errors raised by automated code reviews against cloud-init master at https://lgtm.com/projects/g/cloud-init/cloud-init/alerts The following items are addressed:  * Fix 'cloud-init status':     * Only report 'running' state when any stage in /run/cloud-init/status.json has a start time but no finished time. Default start time to 0 if null.     * undeclared variable 'reason' now reports 'Cloud-init enabled by systemd cloud-init-generator' when systemd enables cloud-init  * cc_rh_subscription.py util.subp return values aren't set during if an exception is raised, use ProcessExecution as e instead.  * distros/freebsd.py:    * Drop repetitive looping over ipv4 and ipv6 nic lists.    * Initialize bsddev to 'NOTFOUND' in the event that no devs are discovered    * declare nics_with_addresses = set() in broader scope outside check_downable conditional  * cloudinit/util.py: Raise TypeError if mtype parameter isn't string, iterable or None. LP: #1744796 --- cloudinit/cmd/status.py | 7 +++++-- cloudinit/cmd/tests/test_status.py | 21 ++++++++++++++++++--- cloudinit/config/cc_power_state_change.py | 1 + cloudinit/config/cc_rh_subscription.py | 5 ++--- cloudinit/distros/freebsd.py | 11 +++-------- cloudinit/util.py | 4 ++++ 6 files changed, 33 insertions(+), 16 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py index 3e5d0d07..d7aaee9d 100644 --- a/cloudinit/cmd/status.py +++ b/cloudinit/cmd/status.py @@ -93,6 +93,8 @@ def _is_cloudinit_disabled(disable_file, paths): elif not os.path.exists(os.path.join(paths.run_dir, 'enabled')): is_disabled = True reason = 'Cloud-init disabled by cloud-init-generator' + else: + reason = 'Cloud-init enabled by systemd cloud-init-generator' return (is_disabled, reason) @@ -127,10 +129,11 @@ def _get_status_details(paths): status_detail = value elif isinstance(value, dict): errors.extend(value.get('errors', [])) + start = value.get('start') or 0 finished = value.get('finished') or 0 - if finished == 0: + if finished == 0 and start != 0: status = STATUS_RUNNING - event_time = max(value.get('start', 0), finished) + event_time = max(start, finished) if event_time > latest_event: latest_event = event_time if errors: diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py index 6d4a11e8..a7c0a91a 100644 --- a/cloudinit/cmd/tests/test_status.py +++ b/cloudinit/cmd/tests/test_status.py @@ -93,6 +93,19 @@ class TestStatus(CiTestCase): self.assertTrue(is_disabled, 'expected disabled cloud-init') self.assertEqual('Cloud-init disabled by cloud-init-generator', reason) + def test__is_cloudinit_disabled_false_when_enabled_in_systemd(self): + '''Report enabled when systemd generator creates the enabled file.''' + enabled_file = os.path.join(self.paths.run_dir, 'enabled') + write_file(enabled_file, '') + (is_disabled, reason) = wrap_and_call( + 'cloudinit.cmd.status', + {'uses_systemd': True, + 'get_cmdline': 'something ignored'}, + status._is_cloudinit_disabled, self.disable_file, self.paths) + self.assertFalse(is_disabled, 'expected enabled cloud-init') + self.assertEqual( + 'Cloud-init enabled by systemd cloud-init-generator', reason) + def test_status_returns_not_run(self): '''When status.json does not exist yet, return 'not run'.''' self.assertFalse( @@ -137,8 +150,9 @@ class TestStatus(CiTestCase): self.assertEqual(expected, m_stdout.getvalue()) def test_status_returns_running(self): - '''Report running when status file exists but isn't finished.''' - write_json(self.status_file, {'v1': {'init': {'finished': None}}}) + '''Report running when status exists with an unfinished stage.''' + write_json(self.status_file, + {'v1': {'init': {'start': 1, 'finished': None}}}) cmdargs = myargs(long=False, wait=False) with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: retcode = wrap_and_call( @@ -338,7 +352,8 @@ class TestStatus(CiTestCase): def test_status_main(self): '''status.main can be run as a standalone script.''' - write_json(self.status_file, {'v1': {'init': {'finished': None}}}) + write_json(self.status_file, + {'v1': {'init': {'start': 1, 'finished': None}}}) with self.assertRaises(SystemExit) as context_manager: with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: wrap_and_call( diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index eba58b02..4da3a588 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -194,6 +194,7 @@ def doexit(sysexit): def execmd(exe_args, output=None, data_in=None): + ret = 1 try: proc = subprocess.Popen(exe_args, stdin=subprocess.PIPE, stdout=output, stderr=subprocess.STDOUT) diff --git a/cloudinit/config/cc_rh_subscription.py b/cloudinit/config/cc_rh_subscription.py index a9d21e78..530808ce 100644 --- a/cloudinit/config/cc_rh_subscription.py +++ b/cloudinit/config/cc_rh_subscription.py @@ -276,9 +276,8 @@ class SubscriptionManager(object): cmd = ['attach', '--auto'] try: return_out, return_err = self._sub_man_cli(cmd) - except util.ProcessExecutionError: - self.log_warn("Auto-attach failed with: " - "{0}]".format(return_err.strip())) + except util.ProcessExecutionError as e: + self.log_warn("Auto-attach failed with: {0}".format(e)) return False for line in return_out.split("\n"): if line is not "": diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index bad112fe..aa468bca 100644 --- a/cloudinit/distros/freebsd.py +++ b/cloudinit/distros/freebsd.py @@ -116,6 +116,7 @@ class Distro(distros.Distro): (out, err) = util.subp(['ifconfig', '-a']) ifconfigoutput = [x for x in (out.strip()).splitlines() if len(x.split()) > 0] + bsddev = 'NOT_FOUND' for line in ifconfigoutput: m = re.match('^\w+', line) if m: @@ -347,15 +348,9 @@ class Distro(distros.Distro): bymac[Distro.get_interface_mac(n)] = { 'name': n, 'up': self.is_up(n), 'downable': None} + nics_with_addresses = set() if check_downable: - nics_with_addresses = set() - ipv6 = self.get_ipv6() - ipv4 = self.get_ipv4() - for bytes_out in (ipv6, ipv4): - for i in ipv6: - nics_with_addresses.update(i) - for i in ipv4: - nics_with_addresses.update(i) + nics_with_addresses = set(self.get_ipv4() + self.get_ipv6()) for d in bymac.values(): d['downable'] = (d['up'] is False or diff --git a/cloudinit/util.py b/cloudinit/util.py index 9976400f..338fb971 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1587,6 +1587,10 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): mtypes = list(mtype) elif mtype is None: mtypes = None + else: + raise TypeError( + 'Unsupported type provided for mtype parameter: {_type}'.format( + _type=type(mtype))) # clean up 'mtype' input a bit based on platform. platsys = platform.system().lower() -- cgit v1.2.3