From cc9762a2d737ead386ffb9f067adc5e543224560 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 22 Aug 2017 20:06:20 -0600 Subject: schema cli: Add schema subcommand to cloud-init cli and cc_runcmd schema This branch does a few things: - Add 'schema' subcommand to cloud-init CLI for validating cloud-config files against strict module jsonschema definitions - Add --annotate parameter to 'cloud-init schema' to annotate existing cloud-config file content with validation errors - Add jsonschema definition to cc_runcmd - Add unit test coverage for cc_runcmd - Update CLI capabilities documentation This branch only imports development (and analyze) subparsers when the specific subcommand is provided on the CLI to avoid adding costly unused file imports during cloud-init system boot. The schema command allows a person to quickly validate a cloud-config text file against cloud-init's known module schemas to avoid costly roundtrips deploying instances in their cloud of choice. As of this branch, only cc_ntp and cc_runcmd cloud-config modules define schemas. Schema validation will ignore all undefined config keys until all modules define a strict schema. To perform validation of runcmd and ntp sections of a cloud-config file: $ cat > cloud.cfg < Date: Sat, 2 Sep 2017 01:51:29 -0600 Subject: schema and docs: Add jsonschema to resizefs and bootcmd modules Add schema definitions to both cc_resizefs and cc_bootcmd modules. Extend schema.py to parse and document enumerated json types. Schema definitions are used to generate module documention and log warnings for schema infractions. This branch also does the following: - drops vestigial 'resize_rootfs_tmp' option from cc_resizefs. That option only created the specified directory and didn't make use of that directory for any resize operations. - Drop yaml.dumps calls from schema documentation generation to avoid yaml import costs on module load - Add __doc__ = get_schema_doc(schema) definitions it each module to supplement python help() calls for cc_runcmd, cc_bootcmd, cc_ntp and cc_resizefs - Add a SCHEMA_EXAMPLES_SPACER_TEMPLATE string to docs for modules which contain more than one example --- cloudinit/config/cc_bootcmd.py | 87 ++++--- cloudinit/config/cc_ntp.py | 48 +--- cloudinit/config/cc_resizefs.py | 149 +++++++----- cloudinit/config/cc_runcmd.py | 5 +- cloudinit/config/schema.py | 19 +- .../unittests/test_handler/test_handler_bootcmd.py | 145 ++++++++++++ .../test_handler/test_handler_resizefs.py | 257 ++++++++++++++++++++- tests/unittests/test_handler/test_schema.py | 44 ++-- 8 files changed, 586 insertions(+), 168 deletions(-) create mode 100644 tests/unittests/test_handler/test_handler_bootcmd.py (limited to 'cloudinit/config/cc_runcmd.py') diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py index 9c0476af..233da1ef 100644 --- a/cloudinit/config/cc_bootcmd.py +++ b/cloudinit/config/cc_bootcmd.py @@ -3,45 +3,73 @@ # # Author: Scott Moser # Author: Juerg Haefliger +# Author: Chad Smith # # This file is part of cloud-init. See LICENSE file for license information. -""" -Bootcmd -------- -**Summary:** run commands early in boot process - -This module runs arbitrary commands very early in the boot process, -only slightly after a boothook would run. This is very similar to a -boothook, but more user friendly. The environment variable ``INSTANCE_ID`` -will be set to the current instance id for all run commands. Commands can be -specified either as lists or strings. For invocation details, see ``runcmd``. - -.. note:: - bootcmd should only be used for things that could not be done later in the - boot process. - -**Internal name:** ``cc_bootcmd`` - -**Module frequency:** per always - -**Supported distros:** all - -**Config keys**:: - - bootcmd: - - echo 192.168.1.130 us.archive.ubuntu.com > /etc/hosts - - [ cloud-init-per, once, mymkfs, mkfs, /dev/vdb ] -""" +"""Bootcmd: run arbitrary commands early in the boot process.""" import os +from textwrap import dedent +from cloudinit.config.schema import ( + get_schema_doc, validate_cloudconfig_schema) from cloudinit.settings import PER_ALWAYS from cloudinit import temp_utils from cloudinit import util frequency = PER_ALWAYS +# The schema definition for each cloud-config module is a strict contract for +# describing supported configuration parameters for each cloud-config section. +# It allows cloud-config to validate and alert users to invalid or ignored +# configuration options before actually attempting to deploy with said +# configuration. + +distros = ['all'] + +schema = { + 'id': 'cc_bootcmd', + 'name': 'Bootcmd', + 'title': 'Run arbitrary commands early in the boot process', + 'description': dedent("""\ + This module runs arbitrary commands very early in the boot process, + only slightly after a boothook would run. This is very similar to a + boothook, but more user friendly. The environment variable + ``INSTANCE_ID`` will be set to the current instance id for all run + commands. Commands can be specified either as lists or strings. For + invocation details, see ``runcmd``. + + .. note:: + bootcmd should only be used for things that could not be done later + in the boot process."""), + 'distros': distros, + 'examples': [dedent("""\ + bootcmd: + - echo 192.168.1.130 us.archive.ubuntu.com > /etc/hosts + - [ cloud-init-per, once, mymkfs, mkfs, /dev/vdb ] + """)], + 'frequency': PER_ALWAYS, + 'type': 'object', + 'properties': { + 'bootcmd': { + 'type': 'array', + 'items': { + 'oneOf': [ + {'type': 'array', 'items': {'type': 'string'}}, + {'type': 'string'}] + }, + 'additionalItems': False, # Reject items of non-string non-list + 'additionalProperties': False, + 'minItems': 1, + 'required': [], + 'uniqueItems': True + } + } +} + +__doc__ = get_schema_doc(schema) # Supplement python help() + def handle(name, cfg, cloud, log, _args): @@ -50,13 +78,14 @@ def handle(name, cfg, cloud, log, _args): " no 'bootcmd' key in configuration"), name) return + validate_cloudconfig_schema(cfg, schema) with temp_utils.ExtendedTemporaryFile(suffix=".sh") as tmpf: try: content = util.shellify(cfg["bootcmd"]) tmpf.write(util.encode_text(content)) tmpf.flush() - except Exception: - util.logexc(log, "Failed to shellify bootcmd") + except Exception as e: + util.logexc(log, "Failed to shellify bootcmd: %s", str(e)) raise try: diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index a02b4bf1..15ae1ecd 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -4,39 +4,10 @@ # # This file is part of cloud-init. See LICENSE file for license information. -""" -NTP ---- -**Summary:** enable and configure ntp - -Handle ntp configuration. If ntp is not installed on the system and ntp -configuration is specified, ntp will be installed. If there is a default ntp -config file in the image or one is present in the distro's ntp package, it will -be copied to ``/etc/ntp.conf.dist`` before any changes are made. A list of ntp -pools and ntp servers can be provided under the ``ntp`` config key. If no ntp -servers or pools are provided, 4 pools will be used in the format -``{0-3}.{distro}.pool.ntp.org``. - -**Internal name:** ``cc_ntp`` - -**Module frequency:** per instance - -**Supported distros:** centos, debian, fedora, opensuse, ubuntu - -**Config keys**:: - - ntp: - pools: - - 0.company.pool.ntp.org - - 1.company.pool.ntp.org - - ntp.myorg.org - servers: - - my.ntp.server.local - - ntp.ubuntu.com - - 192.168.23.2 -""" +"""NTP: enable and configure ntp""" -from cloudinit.config.schema import validate_cloudconfig_schema +from cloudinit.config.schema import ( + get_schema_doc, validate_cloudconfig_schema) from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE from cloudinit import templater @@ -76,10 +47,13 @@ schema = { ``{0-3}.{distro}.pool.ntp.org``."""), 'distros': distros, 'examples': [ - {'ntp': {'pools': ['0.company.pool.ntp.org', '1.company.pool.ntp.org', - 'ntp.myorg.org'], - 'servers': ['my.ntp.server.local', 'ntp.ubuntu.com', - '192.168.23.2']}}], + dedent("""\ + ntp: + pools: [0.int.pool.ntp.org, 1.int.pool.ntp.org, ntp.myorg.org] + servers: + - ntp.server.local + - ntp.ubuntu.com + - 192.168.23.2""")], 'frequency': PER_INSTANCE, 'type': 'object', 'properties': { @@ -117,6 +91,8 @@ schema = { } } +__doc__ = get_schema_doc(schema) # Supplement python help() + def handle(name, cfg, cloud, log, _args): """Enable and configure ntp.""" diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index ceee952b..f14d3836 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -6,31 +6,8 @@ # # This file is part of cloud-init. See LICENSE file for license information. -""" -Resizefs --------- -**Summary:** resize filesystem +"""Resizefs: cloud-config module which resizes the filesystem""" -Resize a filesystem to use all avaliable space on partition. This module is -useful along with ``cc_growpart`` and will ensure that if the root partition -has been resized the root filesystem will be resized along with it. By default, -``cc_resizefs`` will resize the root partition and will block the boot process -while the resize command is running. Optionally, the resize operation can be -performed in the background while cloud-init continues running modules. This -can be enabled by setting ``resize_rootfs`` to ``true``. This module can be -disabled altogether by setting ``resize_rootfs`` to ``false``. - -**Internal name:** ``cc_resizefs`` - -**Module frequency:** per always - -**Supported distros:** all - -**Config keys**:: - - resize_rootfs: - resize_rootfs_tmp: -""" import errno import getopt @@ -38,11 +15,47 @@ import os import re import shlex import stat +from textwrap import dedent +from cloudinit.config.schema import ( + get_schema_doc, validate_cloudconfig_schema) from cloudinit.settings import PER_ALWAYS from cloudinit import util +NOBLOCK = "noblock" + frequency = PER_ALWAYS +distros = ['all'] + +schema = { + 'id': 'cc_resizefs', + 'name': 'Resizefs', + 'title': 'Resize filesystem', + 'description': dedent("""\ + Resize a filesystem to use all avaliable space on partition. This + module is useful along with ``cc_growpart`` and will ensure that if the + root partition has been resized the root filesystem will be resized + along with it. By default, ``cc_resizefs`` will resize the root + partition and will block the boot process while the resize command is + running. Optionally, the resize operation can be performed in the + background while cloud-init continues running modules. This can be + enabled by setting ``resize_rootfs`` to ``true``. This module can be + disabled altogether by setting ``resize_rootfs`` to ``false``."""), + 'distros': distros, + 'examples': [ + 'resize_rootfs: false # disable root filesystem resize operation'], + 'frequency': PER_ALWAYS, + 'type': 'object', + 'properties': { + 'resize_rootfs': { + 'enum': [True, False, NOBLOCK], + 'description': dedent("""\ + Whether to resize the root partition. Default: 'true'""") + } + } +} + +__doc__ = get_schema_doc(schema) # Supplement python help() def _resize_btrfs(mount_point, devpth): @@ -131,8 +144,6 @@ RESIZE_FS_PRECHECK_CMDS = { 'ufs': _can_skip_resize_ufs } -NOBLOCK = "noblock" - def rootdev_from_cmdline(cmdline): found = None @@ -161,71 +172,81 @@ def can_skip_resize(fs_type, resize_what, devpth): return False -def handle(name, cfg, _cloud, log, args): - if len(args) != 0: - resize_root = args[0] - else: - resize_root = util.get_cfg_option_str(cfg, "resize_rootfs", True) - - if not util.translate_bool(resize_root, addons=[NOBLOCK]): - log.debug("Skipping module named %s, resizing disabled", name) - return - - # TODO(harlowja) is the directory ok to be used?? - resize_root_d = util.get_cfg_option_str(cfg, "resize_rootfs_tmp", "/run") - util.ensure_dir(resize_root_d) +def is_device_path_writable_block(devpath, info, log): + """Return True if devpath is a writable block device. - # TODO(harlowja): allow what is to be resized to be configurable?? - resize_what = "/" - result = util.get_mount_info(resize_what, log) - if not result: - log.warn("Could not determine filesystem type of %s", resize_what) - return - - (devpth, fs_type, mount_point) = result - - info = "dev=%s mnt_point=%s path=%s" % (devpth, mount_point, resize_what) - log.debug("resize_info: %s" % info) + @param devpath: Path to the root device we want to resize. + @param info: String representing information about the requested device. + @param log: Logger to which logs will be added upon error. + @returns Boolean True if block device is writable + """ container = util.is_container() # Ensure the path is a block device. - if (devpth == "/dev/root" and not os.path.exists(devpth) and + if (devpath == "/dev/root" and not os.path.exists(devpath) and not container): - devpth = util.rootdev_from_cmdline(util.get_cmdline()) - if devpth is None: + devpath = util.rootdev_from_cmdline(util.get_cmdline()) + if devpath is None: log.warn("Unable to find device '/dev/root'") - return - log.debug("Converted /dev/root to '%s' per kernel cmdline", devpth) + return False + log.debug("Converted /dev/root to '%s' per kernel cmdline", devpath) try: - statret = os.stat(devpth) + statret = os.stat(devpath) except OSError as exc: if container and exc.errno == errno.ENOENT: log.debug("Device '%s' did not exist in container. " - "cannot resize: %s", devpth, info) + "cannot resize: %s", devpath, info) elif exc.errno == errno.ENOENT: log.warn("Device '%s' did not exist. cannot resize: %s", - devpth, info) + devpath, info) else: raise exc - return + return False - if not os.access(devpth, os.W_OK): + if not os.access(devpath, os.W_OK): if container: log.debug("'%s' not writable in container. cannot resize: %s", - devpth, info) + devpath, info) else: - log.warn("'%s' not writable. cannot resize: %s", devpth, info) + log.warn("'%s' not writable. cannot resize: %s", devpath, info) return if not stat.S_ISBLK(statret.st_mode) and not stat.S_ISCHR(statret.st_mode): if container: log.debug("device '%s' not a block device in container." - " cannot resize: %s" % (devpth, info)) + " cannot resize: %s" % (devpath, info)) else: log.warn("device '%s' not a block device. cannot resize: %s" % - (devpth, info)) + (devpath, info)) + return False + return True + + +def handle(name, cfg, _cloud, log, args): + if len(args) != 0: + resize_root = args[0] + else: + resize_root = util.get_cfg_option_str(cfg, "resize_rootfs", True) + validate_cloudconfig_schema(cfg, schema) + if not util.translate_bool(resize_root, addons=[NOBLOCK]): + log.debug("Skipping module named %s, resizing disabled", name) + return + + # TODO(harlowja): allow what is to be resized to be configurable?? + resize_what = "/" + result = util.get_mount_info(resize_what, log) + if not result: + log.warn("Could not determine filesystem type of %s", resize_what) + return + + (devpth, fs_type, mount_point) = result + + info = "dev=%s mnt_point=%s path=%s" % (devpth, mount_point, resize_what) + log.debug("resize_info: %s" % info) + + if not is_device_path_writable_block(devpth, info, log): return resizer = None diff --git a/cloudinit/config/cc_runcmd.py b/cloudinit/config/cc_runcmd.py index 7c3ccd41..7f995693 100644 --- a/cloudinit/config/cc_runcmd.py +++ b/cloudinit/config/cc_runcmd.py @@ -8,7 +8,8 @@ """Runcmd: run arbitrary commands at rc.local with output to the console""" -from cloudinit.config.schema import validate_cloudconfig_schema +from cloudinit.config.schema import ( + get_schema_doc, validate_cloudconfig_schema) from cloudinit.settings import PER_INSTANCE from cloudinit import util @@ -67,6 +68,8 @@ schema = { } } +__doc__ = get_schema_doc(schema) # Supplement python help() + def handle(name, cfg, cloud, log, _args): if "runcmd" not in cfg: diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 73dd5c2e..c17d973e 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -14,6 +14,7 @@ import re import sys import yaml +_YAML_MAP = {True: 'true', False: 'false', None: 'null'} SCHEMA_UNDEFINED = b'UNDEFINED' CLOUD_CONFIG_HEADER = b'#cloud-config' SCHEMA_DOC_TMPL = """ @@ -34,6 +35,8 @@ SCHEMA_DOC_TMPL = """ {examples} """ SCHEMA_PROPERTY_TMPL = '{prefix}**{prop_name}:** ({type}) {description}' +SCHEMA_EXAMPLES_HEADER = '\n**Examples**::\n\n' +SCHEMA_EXAMPLES_SPACER_TEMPLATE = '\n # --- Example{0} ---' class SchemaValidationError(ValueError): @@ -212,6 +215,9 @@ def _schemapath_for_cloudconfig(config, original_content): def _get_property_type(property_dict): """Return a string representing a property type from a given jsonschema.""" property_type = property_dict.get('type', SCHEMA_UNDEFINED) + if property_type == SCHEMA_UNDEFINED and property_dict.get('enum'): + property_type = [ + str(_YAML_MAP.get(k, k)) for k in property_dict['enum']] if isinstance(property_type, list): property_type = '/'.join(property_type) items = property_dict.get('items', {}) @@ -249,15 +255,14 @@ def _get_schema_examples(schema, prefix=''): examples = schema.get('examples') if not examples: return '' - rst_content = '\n**Examples**::\n\n' - for example in examples: - if isinstance(example, str): - example_content = example - else: - example_content = yaml.dump(example, default_flow_style=False) + rst_content = SCHEMA_EXAMPLES_HEADER + for count, example in enumerate(examples): # Python2.6 is missing textwrapper.indent - lines = example_content.split('\n') + lines = example.split('\n') indented_lines = [' {0}'.format(line) for line in lines] + if rst_content != SCHEMA_EXAMPLES_HEADER: + indented_lines.insert( + 0, SCHEMA_EXAMPLES_SPACER_TEMPLATE.format(count + 1)) rst_content += '\n'.join(indented_lines) return rst_content diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py new file mode 100644 index 00000000..580017ed --- /dev/null +++ b/tests/unittests/test_handler/test_handler_bootcmd.py @@ -0,0 +1,145 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit.config import cc_bootcmd +from cloudinit.sources import DataSourceNone +from cloudinit import (distros, helpers, cloud, util) +from cloudinit.tests.helpers import CiTestCase, mock, skipIf + +import logging +import tempfile + +try: + import jsonschema + assert jsonschema # avoid pyflakes error F401: import unused + _missing_jsonschema_dep = False +except ImportError: + _missing_jsonschema_dep = True + +LOG = logging.getLogger(__name__) + + +class FakeExtendedTempFile(object): + def __init__(self, suffix): + self.suffix = suffix + self.handle = tempfile.NamedTemporaryFile( + prefix="ci-%s." % self.__class__.__name__, delete=False) + + def __enter__(self): + return self.handle + + def __exit__(self, exc_type, exc_value, traceback): + self.handle.close() + + +class TestBootcmd(CiTestCase): + + with_logs = True + + _etmpfile_path = ('cloudinit.config.cc_bootcmd.temp_utils.' + 'ExtendedTemporaryFile') + + def setUp(self): + super(TestBootcmd, self).setUp() + self.subp = util.subp + self.new_root = self.tmp_dir() + + def _get_cloud(self, distro): + paths = helpers.Paths({}) + cls = distros.fetch(distro) + mydist = cls(distro, {}, paths) + myds = DataSourceNone.DataSourceNone({}, mydist, paths) + paths.datasource = myds + return cloud.Cloud(myds, paths, {}, mydist, None) + + def test_handler_skip_if_no_bootcmd(self): + """When the provided config doesn't contain bootcmd, skip it.""" + cfg = {} + mycloud = self._get_cloud('ubuntu') + cc_bootcmd.handle('notimportant', cfg, mycloud, LOG, None) + self.assertIn( + "Skipping module named notimportant, no 'bootcmd' key", + self.logs.getvalue()) + + def test_handler_invalid_command_set(self): + """Commands which can't be converted to shell will raise errors.""" + invalid_config = {'bootcmd': 1} + cc = self._get_cloud('ubuntu') + with self.assertRaises(TypeError) as context_manager: + cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, []) + self.assertIn('Failed to shellify bootcmd', self.logs.getvalue()) + self.assertEqual( + "'int' object is not iterable", + str(context_manager.exception)) + + @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency") + def test_handler_schema_validation_warns_non_array_type(self): + """Schema validation warns of non-array type for bootcmd key. + + Schema validation is not strict, so bootcmd attempts to shellify the + invalid content. + """ + invalid_config = {'bootcmd': 1} + cc = self._get_cloud('ubuntu') + with self.assertRaises(TypeError): + cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, []) + self.assertIn( + 'Invalid config:\nbootcmd: 1 is not of type \'array\'', + self.logs.getvalue()) + self.assertIn('Failed to shellify', self.logs.getvalue()) + + @skipIf(_missing_jsonschema_dep, 'No python-jsonschema dependency') + def test_handler_schema_validation_warns_non_array_item_type(self): + """Schema validation warns of non-array or string bootcmd items. + + Schema validation is not strict, so bootcmd attempts to shellify the + invalid content. + """ + invalid_config = { + 'bootcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]} + cc = self._get_cloud('ubuntu') + with self.assertRaises(RuntimeError) as context_manager: + cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, []) + expected_warnings = [ + 'bootcmd.1: 20 is not valid under any of the given schemas', + 'bootcmd.3: {\'a\': \'n\'} is not valid under any of the given' + ' schema' + ] + logs = self.logs.getvalue() + for warning in expected_warnings: + self.assertIn(warning, logs) + self.assertIn('Failed to shellify', logs) + self.assertEqual( + 'Unable to shellify type int which is not a list or string', + str(context_manager.exception)) + + def test_handler_creates_and_runs_bootcmd_script_with_instance_id(self): + """Valid schema runs a bootcmd script with INSTANCE_ID in the env.""" + cc = self._get_cloud('ubuntu') + out_file = self.tmp_path('bootcmd.out', self.new_root) + my_id = "b6ea0f59-e27d-49c6-9f87-79f19765a425" + valid_config = {'bootcmd': [ + 'echo {0} $INSTANCE_ID > {1}'.format(my_id, out_file)]} + + with mock.patch(self._etmpfile_path, FakeExtendedTempFile): + cc_bootcmd.handle('cc_bootcmd', valid_config, cc, LOG, []) + self.assertEqual(my_id + ' iid-datasource-none\n', + util.load_file(out_file)) + + def test_handler_runs_bootcmd_script_with_error(self): + """When a valid script generates an error, that error is raised.""" + cc = self._get_cloud('ubuntu') + valid_config = {'bootcmd': ['exit 1']} # Script with error + + with mock.patch(self._etmpfile_path, FakeExtendedTempFile): + with self.assertRaises(util.ProcessExecutionError) as ctxt_manager: + cc_bootcmd.handle('does-not-matter', valid_config, cc, LOG, []) + self.assertIn( + 'Unexpected error while running command.\n' + "Command: ['/bin/sh',", + str(ctxt_manager.exception)) + self.assertIn( + 'Failed to run bootcmd module does-not-matter', + self.logs.getvalue()) + + +# 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 52591b8b..76dddbf8 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -1,17 +1,30 @@ # This file is part of cloud-init. See LICENSE file for license information. -from cloudinit.config import cc_resizefs +from cloudinit.config.cc_resizefs import ( + can_skip_resize, handle, is_device_path_writable_block, + rootdev_from_cmdline) +import logging import textwrap -import unittest + +from cloudinit.tests.helpers import (CiTestCase, mock, skipIf, util, + wrap_and_call) + + +LOG = logging.getLogger(__name__) + try: - from unittest import mock + import jsonschema + assert jsonschema # avoid pyflakes error F401: import unused + _missing_jsonschema_dep = False except ImportError: - import mock + _missing_jsonschema_dep = True + +class TestResizefs(CiTestCase): + with_logs = True -class TestResizefs(unittest.TestCase): def setUp(self): super(TestResizefs, self).setUp() self.name = "resizefs" @@ -34,7 +47,7 @@ class TestResizefs(unittest.TestCase): 58720296 3145728 3 freebsd-swap (1.5G) 61866024 1048496 - free - (512M) """) - res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth) + res = can_skip_resize(fs_type, resize_what, devpth) self.assertTrue(res) @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output') @@ -52,8 +65,238 @@ class TestResizefs(unittest.TestCase): => 34 297086 da0 GPT (145M) 34 297086 1 freebsd-ufs (145M) """) - res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth) + res = can_skip_resize(fs_type, resize_what, devpth) self.assertTrue(res) + def test_handle_noops_on_disabled(self): + """The handle function logs when the configuration disables resize.""" + cfg = {'resize_rootfs': False} + handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[]) + self.assertIn( + 'DEBUG: Skipping module named cc_resizefs, resizing disabled\n', + self.logs.getvalue()) + + @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency") + def test_handle_schema_validation_logs_invalid_resize_rootfs_value(self): + """The handle reports json schema violations as a warning. + + Invalid values for resize_rootfs result in disabling the module. + """ + cfg = {'resize_rootfs': 'junk'} + handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[]) + logs = self.logs.getvalue() + self.assertIn( + "WARNING: Invalid config:\nresize_rootfs: 'junk' is not one of" + " [True, False, 'noblock']", + logs) + self.assertIn( + 'DEBUG: Skipping module named cc_resizefs, resizing disabled\n', + logs) + + @mock.patch('cloudinit.config.cc_resizefs.util.get_mount_info') + def test_handle_warns_on_unknown_mount_info(self, m_get_mount_info): + """handle warns when get_mount_info sees unknown filesystem for /.""" + m_get_mount_info.return_value = None + cfg = {'resize_rootfs': True} + handle('cc_resizefs', cfg, _cloud=None, log=LOG, args=[]) + logs = self.logs.getvalue() + self.assertNotIn("WARNING: Invalid config:\nresize_rootfs:", logs) + self.assertIn( + 'WARNING: Could not determine filesystem type of /\n', + logs) + self.assertEqual( + [mock.call('/', LOG)], + m_get_mount_info.call_args_list) + + def test_handle_warns_on_undiscoverable_root_path_in_commandline(self): + """handle noops when the root path is not found on the commandline.""" + cfg = {'resize_rootfs': True} + exists_mock_path = 'cloudinit.config.cc_resizefs.os.path.exists' + + def fake_mount_info(path, log): + self.assertEqual('/', path) + self.assertEqual(LOG, log) + return ('/dev/root', 'ext4', '/') + + with mock.patch(exists_mock_path) as m_exists: + m_exists.return_value = False + wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': False}, + 'get_mount_info': {'side_effect': fake_mount_info}, + 'get_cmdline': {'return_value': 'BOOT_IMAGE=/vmlinuz.efi'}}, + handle, 'cc_resizefs', cfg, _cloud=None, log=LOG, + args=[]) + logs = self.logs.getvalue() + self.assertIn("WARNING: Unable to find device '/dev/root'", logs) + + +class TestRootDevFromCmdline(CiTestCase): + + def test_rootdev_from_cmdline_with_no_root(self): + """Return None from rootdev_from_cmdline when root is not present.""" + invalid_cases = [ + 'BOOT_IMAGE=/adsf asdfa werasef root adf', 'BOOT_IMAGE=/adsf', ''] + for case in invalid_cases: + self.assertIsNone(rootdev_from_cmdline(case)) + + def test_rootdev_from_cmdline_with_root_startswith_dev(self): + """Return the cmdline root when the path starts with /dev.""" + self.assertEqual( + '/dev/this', rootdev_from_cmdline('asdf root=/dev/this')) + + def test_rootdev_from_cmdline_with_root_without_dev_prefix(self): + """Add /dev prefix to cmdline root when the path lacks the prefix.""" + self.assertEqual('/dev/this', rootdev_from_cmdline('asdf root=this')) + + def test_rootdev_from_cmdline_with_root_with_label(self): + """When cmdline root contains a LABEL, our root is disk/by-label.""" + self.assertEqual( + '/dev/disk/by-label/unique', + rootdev_from_cmdline('asdf root=LABEL=unique')) + + def test_rootdev_from_cmdline_with_root_with_uuid(self): + """When cmdline root contains a UUID, our root is disk/by-uuid.""" + self.assertEqual( + '/dev/disk/by-uuid/adsfdsaf-adsf', + rootdev_from_cmdline('asdf root=UUID=adsfdsaf-adsf')) + + +class TestIsDevicePathWritableBlock(CiTestCase): + + with_logs = True + + def test_is_device_path_writable_block_warns_missing_cmdline_root(self): + """When root does not exist isn't in the cmdline, log warning.""" + info = 'does not matter' + + def fake_mount_info(path, log): + self.assertEqual('/', path) + self.assertEqual(LOG, log) + return ('/dev/root', 'ext4', '/') + + exists_mock_path = 'cloudinit.config.cc_resizefs.os.path.exists' + with mock.patch(exists_mock_path) as m_exists: + m_exists.return_value = False + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': False}, + 'get_mount_info': {'side_effect': fake_mount_info}, + 'get_cmdline': {'return_value': 'BOOT_IMAGE=/vmlinuz.efi'}}, + is_device_path_writable_block, '/dev/root', info, LOG) + self.assertFalse(is_valid) + logs = self.logs.getvalue() + self.assertIn("WARNING: Unable to find device '/dev/root'", logs) + + def test_is_device_path_writable_block_does_not_exist(self): + """When devpath does not exist, a warning is logged.""" + info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': False}}, + is_device_path_writable_block, '/I/dont/exist', info, LOG) + self.assertFalse(is_valid) + self.assertIn( + "WARNING: Device '/I/dont/exist' did not exist." + ' cannot resize: %s' % info, + self.logs.getvalue()) + + def test_is_device_path_writable_block_does_not_exist_in_container(self): + """When devpath does not exist in a container, log a debug message.""" + info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': True}}, + is_device_path_writable_block, '/I/dont/exist', info, LOG) + self.assertFalse(is_valid) + self.assertIn( + "DEBUG: Device '/I/dont/exist' did not exist in container." + ' cannot resize: %s' % info, + self.logs.getvalue()) + + def test_is_device_path_writable_block_raises_oserror(self): + """When unexpected OSError is raises by os.stat it is reraised.""" + info = 'dev=/I/dont/exist mnt_point=/ path=/dev/none' + with self.assertRaises(OSError) as context_manager: + wrap_and_call( + 'cloudinit.config.cc_resizefs', + {'util.is_container': {'return_value': True}, + 'os.stat': {'side_effect': OSError('Something unexpected')}}, + is_device_path_writable_block, '/I/dont/exist', info, LOG) + self.assertEqual( + 'Something unexpected', str(context_manager.exception)) + + def test_is_device_path_writable_block_readonly(self): + """When root device is readonly, emit a warning and return False.""" + fake_devpath = self.tmp_path('dev/readonly') + util.write_file(fake_devpath, '', mode=0o400) # read-only + info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath) + + exists_mock_path = 'cloudinit.config.cc_resizefs.os.path.exists' + with mock.patch(exists_mock_path) as m_exists: + m_exists.return_value = False + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': False}, + 'rootdev_from_cmdline': {'return_value': fake_devpath}}, + is_device_path_writable_block, '/dev/root', info, LOG) + self.assertFalse(is_valid) + logs = self.logs.getvalue() + self.assertIn( + "Converted /dev/root to '{0}' per kernel cmdline".format( + fake_devpath), + logs) + self.assertIn( + "WARNING: '{0}' not writable. cannot resize".format(fake_devpath), + logs) + + def test_is_device_path_writable_block_readonly_in_container(self): + """When root device is readonly, emit debug log and return False.""" + fake_devpath = self.tmp_path('dev/readonly') + util.write_file(fake_devpath, '', mode=0o400) # read-only + info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath) + + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': True}}, + is_device_path_writable_block, fake_devpath, info, LOG) + self.assertFalse(is_valid) + self.assertIn( + "DEBUG: '{0}' not writable in container. cannot resize".format( + fake_devpath), + self.logs.getvalue()) + + def test_is_device_path_writable_block_non_block(self): + """When device is not a block device, emit warning return False.""" + fake_devpath = self.tmp_path('dev/readwrite') + util.write_file(fake_devpath, '', mode=0o600) # read-write + info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath) + + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': False}}, + is_device_path_writable_block, fake_devpath, info, LOG) + self.assertFalse(is_valid) + self.assertIn( + "WARNING: device '{0}' not a block device. cannot resize".format( + fake_devpath), + self.logs.getvalue()) + + def test_is_device_path_writable_block_non_block_on_container(self): + """When device is non-block device in container, emit debug log.""" + fake_devpath = self.tmp_path('dev/readwrite') + util.write_file(fake_devpath, '', mode=0o600) # read-write + info = 'dev=/dev/root mnt_point=/ path={0}'.format(fake_devpath) + + is_valid = wrap_and_call( + 'cloudinit.config.cc_resizefs.util', + {'is_container': {'return_value': True}}, + is_device_path_writable_block, fake_devpath, info, LOG) + self.assertFalse(is_valid) + self.assertIn( + "DEBUG: device '{0}' not a block device in container." + ' cannot resize'.format(fake_devpath), + self.logs.getvalue()) + # vi: ts=4 expandtab diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index 6137e3cf..745bb0ff 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -27,7 +27,7 @@ class GetSchemaTest(CiTestCase): """Every cloudconfig module with schema is listed in allOf keyword.""" schema = get_schema() self.assertItemsEqual( - ['cc_ntp', 'cc_runcmd'], + ['cc_bootcmd', 'cc_ntp', 'cc_resizefs', 'cc_runcmd'], [subschema['id'] for subschema in schema['allOf']]) self.assertEqual('cloud-config-schema', schema['id']) self.assertEqual( @@ -205,6 +205,17 @@ class GetSchemaDocTest(CiTestCase): '**prop1:** (string/integer) prop-description', get_schema_doc(full_schema)) + def test_get_schema_doc_handles_enum_types(self): + """get_schema_doc converts enum types to yaml and delimits with '/'.""" + full_schema = copy(self.required_schema) + full_schema.update( + {'properties': { + 'prop1': {'enum': [True, False, 'stuff'], + 'description': 'prop-description'}}}) + self.assertIn( + '**prop1:** (true/false/stuff) prop-description', + get_schema_doc(full_schema)) + def test_get_schema_doc_handles_nested_oneof_property_types(self): """get_schema_doc describes array items oneOf declarations in type.""" full_schema = copy(self.required_schema) @@ -219,29 +230,11 @@ class GetSchemaDocTest(CiTestCase): '**prop1:** (array of (string)/(integer)) prop-description', get_schema_doc(full_schema)) - def test_get_schema_doc_returns_restructured_text_with_examples(self): - """get_schema_doc returns indented examples when present in schema.""" - full_schema = copy(self.required_schema) - full_schema.update( - {'examples': [{'ex1': [1, 2, 3]}], - 'properties': { - 'prop1': {'type': 'array', 'description': 'prop-description', - 'items': {'type': 'integer'}}}}) - self.assertIn( - dedent(""" - **Config schema**: - **prop1:** (array of integer) prop-description - - **Examples**:: - - ex1"""), - get_schema_doc(full_schema)) - - def test_get_schema_doc_handles_unstructured_examples(self): - """get_schema_doc properly indented examples which as just strings.""" + def test_get_schema_doc_handles_string_examples(self): + """get_schema_doc properly indented examples as a list of strings.""" full_schema = copy(self.required_schema) full_schema.update( - {'examples': ['My example:\n [don\'t, expand, "this"]'], + {'examples': ['ex1:\n [don\'t, expand, "this"]', 'ex2: true'], 'properties': { 'prop1': {'type': 'array', 'description': 'prop-description', 'items': {'type': 'integer'}}}}) @@ -252,8 +245,11 @@ class GetSchemaDocTest(CiTestCase): **Examples**:: - My example: - [don't, expand, "this"]"""), + ex1: + [don't, expand, "this"] + # --- Example2 --- + ex2: true + """), get_schema_doc(full_schema)) def test_get_schema_doc_raises_key_errors(self): -- cgit v1.2.3 From f761f2b5f58c8cf13cfee63619f32046216cf66a Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 13 Sep 2017 22:26:03 -0600 Subject: cloud-config modules: honor distros definitions in each module Modules can optionally define a list of supported distros on which they can run by declaring a distros attribute in the cc_*py module. This branch fixes handling of cloudinit.stages.Modules.run_section. The behavior of run_section is now the following: - always run a module if the module doesn't declare a distros attribute - always run a module if the module declares distros = [ALL_DISTROS] - skip a module if the distribution on which we run isn't in module.distros - force a run of a skipped module if unverified_modules configuration contains the module name LP: #1715738 LP: #1715690 --- cloudinit/config/cc_runcmd.py | 3 +- cloudinit/distros/__init__.py | 4 + cloudinit/stages.py | 33 ++++--- tests/unittests/test_runs/test_simple_run.py | 125 ++++++++++++++++++++++----- 4 files changed, 131 insertions(+), 34 deletions(-) (limited to 'cloudinit/config/cc_runcmd.py') diff --git a/cloudinit/config/cc_runcmd.py b/cloudinit/config/cc_runcmd.py index 7f995693..449872f0 100644 --- a/cloudinit/config/cc_runcmd.py +++ b/cloudinit/config/cc_runcmd.py @@ -10,6 +10,7 @@ from cloudinit.config.schema import ( get_schema_doc, validate_cloudconfig_schema) +from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_INSTANCE from cloudinit import util @@ -23,7 +24,7 @@ from textwrap import dedent # configuration options before actually attempting to deploy with said # configuration. -distros = ['all'] +distros = [ALL_DISTROS] schema = { 'id': 'cc_runcmd', diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index b714b9ab..d5becd12 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -30,6 +30,10 @@ from cloudinit import util from cloudinit.distros.parsers import hosts +# Used when a cloud-config module can be run on all cloud-init distibutions. +# The value 'all' is surfaced in module documentation for distro support. +ALL_DISTROS = 'all' + OSFAMILIES = { 'debian': ['debian', 'ubuntu'], 'redhat': ['centos', 'fedora', 'rhel'], diff --git a/cloudinit/stages.py b/cloudinit/stages.py index a1c4a517..d0452688 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -821,28 +821,35 @@ class Modules(object): skipped = [] forced = [] overridden = self.cfg.get('unverified_modules', []) + active_mods = [] + all_distros = set([distros.ALL_DISTROS]) for (mod, name, _freq, _args) in mostly_mods: - worked_distros = set(mod.distros) + worked_distros = set(mod.distros) # Minimally [] per fixup_modules worked_distros.update( distros.Distro.expand_osfamily(mod.osfamilies)) - # module does not declare 'distros' or lists this distro - if not worked_distros or d_name in worked_distros: - continue - - if name in overridden: - forced.append(name) - else: - skipped.append(name) + # Skip only when the following conditions are all met: + # - distros are defined in the module != ALL_DISTROS + # - the current d_name isn't in distros + # - and the module is unverified and not in the unverified_modules + # override list + if worked_distros and worked_distros != all_distros: + if d_name not in worked_distros: + if name not in overridden: + skipped.append(name) + continue + forced.append(name) + active_mods.append([mod, name, _freq, _args]) if skipped: - LOG.info("Skipping modules %s because they are not verified " + LOG.info("Skipping modules '%s' because they are not verified " "on distro '%s'. To run anyway, add them to " - "'unverified_modules' in config.", skipped, d_name) + "'unverified_modules' in config.", + ','.join(skipped), d_name) if forced: - LOG.info("running unverified_modules: %s", forced) + LOG.info("running unverified_modules: '%s'", ', '.join(forced)) - return self._run_modules(mostly_mods) + return self._run_modules(active_mods) def read_runtime_config(): diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py index 5cf666fe..b8fb4794 100644 --- a/tests/unittests/test_runs/test_simple_run.py +++ b/tests/unittests/test_runs/test_simple_run.py @@ -1,8 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. import os -import shutil -import tempfile from cloudinit.tests import helpers @@ -12,16 +10,19 @@ from cloudinit import util class TestSimpleRun(helpers.FilesystemMockingTestCase): - def _patchIn(self, root): - self.patchOS(root) - self.patchUtils(root) - - def test_none_ds(self): - new_root = tempfile.mkdtemp() - self.addCleanup(shutil.rmtree, new_root) - self.replicateTestRoot('simple_ubuntu', new_root) - cfg = { + + with_logs = True + + def setUp(self): + super(TestSimpleRun, self).setUp() + self.new_root = self.tmp_dir() + self.replicateTestRoot('simple_ubuntu', self.new_root) + + # Seed cloud.cfg file for our tests + self.cfg = { 'datasource_list': ['None'], + 'runcmd': ['ls /etc'], # test ALL_DISTROS + 'spacewalk': {}, # test non-ubuntu distros module definition 'write_files': [ { 'path': '/etc/blah.ini', @@ -29,14 +30,17 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): 'permissions': 0o755, }, ], - 'cloud_init_modules': ['write-files'], + 'cloud_init_modules': ['write-files', 'spacewalk', 'runcmd'], } - cloud_cfg = util.yaml_dumps(cfg) - util.ensure_dir(os.path.join(new_root, 'etc', 'cloud')) - util.write_file(os.path.join(new_root, 'etc', + cloud_cfg = util.yaml_dumps(self.cfg) + util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) + util.write_file(os.path.join(self.new_root, 'etc', 'cloud', 'cloud.cfg'), cloud_cfg) - self._patchIn(new_root) + self.patchOS(self.new_root) + self.patchUtils(self.new_root) + def test_none_ds_populates_var_lib_cloud(self): + """Init and run_section default behavior creates appropriate dirs.""" # Now start verifying whats created initer = stages.Init() initer.read_cfg() @@ -51,10 +55,16 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): initer.update() self.assertTrue(os.path.islink("var/lib/cloud/instance")) - initer.cloudify().run('consume_data', - initer.consume_data, - args=[PER_INSTANCE], - freq=PER_INSTANCE) + def test_none_ds_runs_modules_which_do_not_define_distros(self): + """Any modules which do not define a distros attribute are run.""" + initer = stages.Init() + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.instancify() + initer.update() + initer.cloudify().run('consume_data', initer.consume_data, + args=[PER_INSTANCE], freq=PER_INSTANCE) mods = stages.Modules(initer) (which_ran, failures) = mods.run_section('cloud_init_modules') @@ -63,5 +73,80 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): self.assertIn('write-files', which_ran) contents = util.load_file('/etc/blah.ini') self.assertEqual(contents, 'blah') + self.assertNotIn( + "Skipping modules ['write-files'] because they are not verified on" + " distro 'ubuntu'", + self.logs.getvalue()) + + def test_none_ds_skips_modules_which_define_unmatched_distros(self): + """Skip modules which define distros which don't match the current.""" + initer = stages.Init() + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.instancify() + initer.update() + initer.cloudify().run('consume_data', initer.consume_data, + args=[PER_INSTANCE], freq=PER_INSTANCE) + + mods = stages.Modules(initer) + (which_ran, failures) = mods.run_section('cloud_init_modules') + self.assertTrue(len(failures) == 0) + self.assertIn( + "Skipping modules 'spacewalk' because they are not verified on" + " distro 'ubuntu'", + self.logs.getvalue()) + self.assertNotIn('spacewalk', which_ran) + + def test_none_ds_runs_modules_which_distros_all(self): + """Skip modules which define distros attribute as supporting 'all'. + + This is done in the module with the declaration: + distros = [ALL_DISTROS]. runcmd is an example. + """ + initer = stages.Init() + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.instancify() + initer.update() + initer.cloudify().run('consume_data', initer.consume_data, + args=[PER_INSTANCE], freq=PER_INSTANCE) + + mods = stages.Modules(initer) + (which_ran, failures) = mods.run_section('cloud_init_modules') + self.assertTrue(len(failures) == 0) + self.assertIn('runcmd', which_ran) + self.assertNotIn( + "Skipping modules 'runcmd' because they are not verified on" + " distro 'ubuntu'", + self.logs.getvalue()) + + def test_none_ds_forces_run_via_unverified_modules(self): + """run_section forced skipped modules by using unverified_modules.""" + + # re-write cloud.cfg with unverified_modules override + self.cfg['unverified_modules'] = ['spacewalk'] # Would have skipped + cloud_cfg = util.yaml_dumps(self.cfg) + util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) + util.write_file(os.path.join(self.new_root, 'etc', + 'cloud', 'cloud.cfg'), cloud_cfg) + + initer = stages.Init() + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.instancify() + initer.update() + initer.cloudify().run('consume_data', initer.consume_data, + args=[PER_INSTANCE], freq=PER_INSTANCE) + + mods = stages.Modules(initer) + (which_ran, failures) = mods.run_section('cloud_init_modules') + self.assertTrue(len(failures) == 0) + self.assertIn('spacewalk', which_ran) + self.assertIn( + "running unverified_modules: 'spacewalk'", + self.logs.getvalue()) # vi: ts=4 expandtab -- cgit v1.2.3