← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~powersj/cloud-init:integration-test-revamp into cloud-init:master

 

Thanks for the clarification josh. While it feels strange to build a deb on a different target series --build-os, than test series per --os-name,  I suppose that use case for that could be in cases where we know we can't build on the target --os-name due to missing deps or something. In those cases we'd have to specify a different --build-os than --os-name.  


Additional review comments trickling in. I'm nearly done.

Diff comments:

> diff --git a/doc/rtd/topics/tests.rst b/doc/rtd/topics/tests.rst
> index 0663811..0e470c6 100644
> --- a/doc/rtd/topics/tests.rst
> +++ b/doc/rtd/topics/tests.rst
> @@ -252,15 +316,259 @@ configuration users can run the integration tests via tox:
>  Users need to invoke the citest enviornment and then pass any additional
>  arguments.
>  
> +Setup Image
> +-----------
> +The ``run`` and ``collect`` commands have many options to setup the image
> +before running tests in addition to installing a deb in the target. Any
> +combination of the following can be used:
> +
> +* ``--deb``: install a deb into the image
> +* ``--rpm``: install a rpm into the image
> +* ``--repo``: enable a repository and update cloud-init afterwards
> +* ``--ppa``: enable a ppa and update cloud-init afterwards
> +* ``--upgrade``: upgrade cloud-init from repos
> +* ``--upgrade-full``: run a full system upgrade
> +* ``--script``: execute a script in the image. this can perform any setup
> +  required that is not covered by the other options
> +
> +Configuring the Test Suite
> +==========================
> +
> +Most of the behavior of the test suite is configurable through several yaml
> +files. These control the behavior of the test suite's platforms, images, and
> +tests. The main config files for platforms, images and testcases are
> +``platforms.yaml``, ``releases.yaml`` and ``testcases.yaml``.
> +
> +Config handling
> +---------------
> +All configurable parts of the test suite use a defaults + overrides system for
> +managing config entries. All base config items are dictionaries.
> +
> +Merging is done on a key by key basis, with all keys in the default and
> +overrides represented in the final result. If a key exists both in
> +the defaults and the overrides, then behavior depends on the type of data the
> +key refers to. If it is atomic data or a list, then the overrides will replace
> +the default. If the data is a dictionary then the value will be the result of
> +merging that dictionary from the default config and that dictionary from the
> +overrides.
> +
> +Merging is done using the function ``tests.cloud_tests.config.merge_config``,
> +which can be examined for more detail on config merging behavior.
> +
> +The following demonstrates merge behavior:
> +
> +.. code-block:: yaml
> +
> +    defaults:
> +        list_item:
> +         - list_entry_1
> +         - list_entry_2
> +        int_item_1: 123
> +        int_item_2: 234
> +        dict_item:
> +            subkey_1: 1
> +            subkey_2: 2
> +            subkey_dict:
> +                subsubkey_1: a
> +                subsubkey_2: b
> +
> +    overrides:
> +        list_item:
> +         - overridden_list_entry
> +        int_item_1: 0
> +        dict_item:
> +            subkey_2: false
> +            subkey_dict:
> +                subsubkey_2: 'new value'
> +
> +    result:
> +        list_item:
> +         - overridden_list_entry
> +        int_item_1: 0
> +        int_item_2: 234
> +        dict_item:
> +            subkey_1: 1
> +            subkey_2: false
> +            subkey_dict:
> +                subsubkey_1: a
> +                subsubkey_2: 'new value'
> +
> +
> +Image Config Structure
> +----------------------
> +Image configuration is handled in ``releases.yaml``. The image configuration
> +controls how platforms locate and acquire images, how the platforms should
> +interact with the images, how platforms should detect when an image has fully
> +booted, any options that are required to set the image up, and features that
> +the image supports.
> +
> +Since settings for locating an image and interacting with it differ from
> +platform to platform, there are 4 levels of settings available for images on
> +top of the default image settings. The structure of the image config file is:
> +
> +.. code-block:: yaml
> +
> +    default_release_config:
> +        default:
> +            ...
> +        <platform>:
> +            ...
> +        <platform>:
> +            ...
> +
> +    releases:
> +        <release name>:
> +            <default>:
> +                ...
> +            <platform>:
> +                ...
> +            <platform>:
> +                ...
> +
> +
> +The base config is created from the overall defaults and the overrides for the
> +platform. The overrides are created from the default config for the image and
> +the platform specific overrides for the image.
> +
> +Image Config for System Boot
> +----------------------------
> +The test suite must be able to test if a system has fully booted and if
> +cloud-init has finished running, so that running collect scripts does not race
> +against the target image booting. This is done using the
> +``system_ready_script`` and ``cloud_init_ready_script`` image config keys.
> +
> +Each of these keys accepts a small bash test statement as a string that must
> +return 0 or 1. Since this test statement will be added into a larger bash
> +statement it must be a single statement using the ``[`` test syntax.
> +
> +The default image config provides a system ready script that works for any
> +systemd based image. If the iamge is not systmed based, then a different test
> +statement must be provided. The default config also provides a test for whether
> +or not cloud-init has finished which checks for the file
> +``/run/cloud-init/result.json``. This should be sufficient for most systems, as
> +writing to this file is one of the last things cloud-init does.
> +
> +The setting ``boot_timeout`` controls how long, in seconds, the platform should
> +wait for an image to boot. If the system ready script has not indicated that
> +the system is fully booted within this time an error will be raised.
> +
> +Image Config Feature Flags
> +--------------------------
> +Not all testcases can work on all images due to features the testcase requires
> +not being present on that image. If a testcase requires features in an image
> +that are not likely to be present across all distros and platforms that the
> +test suite supports, then the test can be skipped everywhere it is not
> +supported.
> +
> +This is done through feature flags, which are names for features supported on
> +some images but not all that may be required by testcases. Configuration for
> +feature flags is provided in ``releases.yaml`` under the ``features`` top level
> +key. The features config includes a list of all currently defined feature flags
> +and their meanings, and a list of feature groups.
> +
> +Feature groups are groups of features that many images have in common. For
> +example, the ``ubuntu_specific`` feature group includes features that should be
> +present across most ubuntu releases, but may or may not be for other distros.
> +Feature groups are specified for an image as a list under the key
> +``feature_groups``.
> +
> +An image's feature flags are derived from the features groups that that image
> +has and any feature overrides provided. Feature overrides can be specified
> +under the ``features`` key which accepts a dictionary of
> +``{<feature_name>: true/false}`` mappings. If a feature is omitted from an
> +image's feature flags or set to false in the overrides then the test suite will
> +skip any tests that require that feature when using that image.
> +
> +Feature flags may be overridden at runtime using the ``--feature-override``
> +command line argument. It accepts a feature flag and value to set in the format
> +``<feature name>: true/false``. Multiple ``--feature-override`` flags can be

This actually needs to be ``<feature name>=true/false``  colons result is a traceback in parse_conf_list

> +used, and will all be applied to all feature flags for images used during a
> +test.
> +
> +Image Config Setup Overrides
> +----------------------------
> +If an image requires some of the options for image setup to be used, then it
> +may specify overrides for the command line arguments passed into setup image.
> +These may be specified as a dictionary under the ``setup_overrides`` key. When
> +an image is set up, the arguments that control how it is set up will be the
> +arguments from the command line, with any entries in ``setup_overrides`` used
> +to override these arguments.
> +
> +For example, images that do not come with cloud-init already installed should
> +have ``setup_overrides: {upgrade: true}`` specified so that in the event that
> +no additional setup options are given, cloud-init will be installed from the
> +image's repos before running tests. Note that if other options such as
> +``--deb`` are passed in on the command line, these will still work as expected,
> +since apt's policy for cloud-init would prefer the locally installed deb over
> +an older version from the repos.
> +
> +Image Config Platform Specific Options
> +--------------------------------------
> +There are many platform specific options in image configuration that allow
> +platforms to locate images and that control additional setup that the platform
> +may have to do to make the image useable. For information on how these work,
> +please consult the documentation for that platform in the integration testing
> +suite and the ``releases.yaml`` file for examples.
> +
> +Error Handling Behavior
> +=======================
> +
> +The test suite makes an attempt to run as many tests as possible even in the
> +event of some failing so that automated runs collect as much data as possible.
> +In the event that something goes wrong while setting up for or running a test,
> +the test suite will attempt to continue running any tests which have not been
> +effected by the error.
> +
> +For example, if the test suite was told to run tests on one platform for two
> +releases and an error occured setting up the first image, all tests for that
> +image would be skipped, and the test suite would continue to set up the second
> +image and run tests on it. Or, if the system does not start properly for one
> +testcase out of many to run on that image, that testcase will be skipped and
> +the next one will be run.
> +
> +Note that if any errors at all occur, the test suite will record the failure
> +and where it occurred in the result data and write it out to the specified
> +result file.
> +
> +Exit Codes
> +----------
> +The test suite counts how many errors occur throughout a run. The exit code
> +after a run is the number of errors that occured. If the exit code is non-zero
> +than something is wrong either with the test suite, the configuration for an
> +image, a testcase, or cloud-init itself.
> +
> +Note that the exit code does not always direclty correspond to the number
> +of failed testcases, since in some cases, a single error during image setup
> +can mean that several testcases are not run. If run is used, then the exit code
> +will be the sum of the number of errors in the collect and verify stages.
> +
> +Result Data
> +-----------
> +The test suite generates result data that includes how long each stage of the
> +test suite took and which parts were and were not successful. This data is
> +dumped to the log after the collect and verify stages, and may also be written
> +out in yaml format to a file. If part of the setup failed, the traceback for
> +the failure and the error message will be included in the result file. If a
> +test verifier finds a problem with the collected data from a test run, the
> +class, test function and test will be recorded in the result data.
> +
> +Data Dir
> +--------
> +When using run, the collected data is written into a temporary directory. In
> +the even that all tests pass, this directory is deleted. In the even that a
> +test fails or an error occurs, this data will be left in place, and a message
> +will be written to the log giving the location of the data.
>  
>  Architecture
>  ============
>  
> -The following outlines the process flow during a complete end-to-end LXD-backed test.
> +The following outlines the process flow during a complete end-to-end
> +LXD-backed test.
>  
>  1. Configuration
>      * The back end and specific OS releases are verified as supported
> -    * The test or tests that need to be run are determined either by directory or by individual yaml
> +    * The test or tests that need to be run are determined either by
> +      directory or by individual yaml
>  
>  2. Image Creation
>      * Acquire the daily LXD image
> diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py
> index 64a8667..79ed883 100644
> --- a/tests/cloud_tests/util.py
> +++ b/tests/cloud_tests/util.py
> @@ -158,6 +174,140 @@ def write_file(*args, **kwargs):
>      """
>      write a file using cloudinit.util.write_file
>      """
> -    c_util.write_file(*args, **kwargs)
> +    return c_util.write_file(*args, **kwargs)
> +
> +
> +def read_conf(*args, **kwargs):
> +    """
> +    read configuration using cloudinit.util.read_conf
> +    """
> +    return c_util.read_conf(*args, **kwargs)
> +
> +
> +def subp(*args, **kwargs):

It feels strange to create local helper functions in the test modules which only wrap something which we need from cloudinit.util.subp because it just adds a level of indirection and makes it harder to understand which tests actually use cloudinit.util.subp. I'd vote for direct imports of cloudinit.util.subp where needed instead of function wrappers. This comment would go for read_conf too.

> +    """
> +    execute a command on the system shell using cloudinit.util.subp
> +    """
> +    return c_util.subp(*args, **kwargs)
> +
> +
> +def tmpdir(prefix='cloud_test_util_'):
> +    return tempfile.mkdtemp(prefix=prefix)
> +
> +
> +def rel_files(basedir):
> +    """
> +    list of files under directory by relative path, not including directories
> +    return_value: list or relative paths
> +    """
> +    basedir = os.path.normpath(basedir)
> +    return [path[len(basedir) + 1:] for path in
> +            glob.glob(os.path.join(basedir, '**'), recursive=True)
> +            if not os.path.isdir(path)]
> +
> +
> +def flat_tar(output, basedir, owner='root', group='root'):
> +    """
> +    create a flat tar archive (no leading ./) from basedir
> +    output: output tar file to write
> +    basedir: base directory for archive
> +    owner: owner of archive files
> +    group: group archive files belong to
> +    return_value: none
> +    """
> +    c_util.subp(['tar', 'cf', output, '--owner', owner, '--group', group,
> +                 '-C', basedir] + rel_files(basedir), capture=True)
> +
> +
> +def parse_conf_list(entries, valid=None, boolean=False):
> +    """
> +    parse config in a list of strings in key=value format
> +    entries: list of key=value strings
> +    valid: list of valid keys in result, return None if invalid input
> +    boolean: if true, then interpret all values as booleans where 'true' = True
> +    return_value: dict of configuration or None if invalid
> +    """
> +    res = {key: value.lower() == 'true' if boolean else value
> +           for key, value in (i.split('=') for i in entries)}
> +    return res if not valid or all(k in valid for k in res.keys()) else None
> +
> +
> +def update_args(args, updates, preserve_old=True):
> +    """
> +    update cmdline arguments from a dictionary
> +    args: cmdline arguments
> +    updates: dictionary of {arg_name: new_value} mappings
> +    preserve_old: if true, create a deep copy of args before updating
> +    return_value: updated cmdline arguments, as new object if preserve_old=True
> +    """
> +    args = copy.deepcopy(args) if preserve_old else args
> +    if updates:
> +        vars(args).update(updates)
> +    return args
> +
> +
> +def update_user_data(user_data, updates, dump_to_yaml=True):
> +    """
> +    user_data: user data as yaml string or dict
> +    updates: dictionary to merge with user data
> +    dump_to_yaml: return as yaml dumped string if true
> +    return_value: updated user data, as yaml string if dump_to_yaml is true
> +    """
> +    user_data = (c_util.load_yaml(user_data)
> +                 if isinstance(user_data, str) else copy.deepcopy(user_data))
> +    user_data.update(updates)
> +    return (yaml_format(user_data, content_type='cloud-config')
> +            if dump_to_yaml else user_data)
> +
> +
> +class InTargetExecuteError(c_util.ProcessExecutionError):
> +    """
> +    Error type for in target commands that fail
> +    """
> +    default_desc = 'Unexpected error while running command in target instance'
> +
> +    def __init__(self, stdout, stderr, exit_code, cmd, instance,
> +                 description=None):
> +        """
> +        init error and parent error class
> +        """
> +        if isinstance(cmd, (tuple, list)):
> +            cmd = ' '.join(cmd)
> +        super(InTargetExecuteError, self).__init__(
> +            stdout=stdout, stderr=stderr, exit_code=exit_code, cmd=cmd,
> +            reason="Instance: {}".format(instance),
> +            description=description if description else self.default_desc)
> +
> +
> +class TempDir(object):
> +    """
> +    temporary directory like tempfile.TemporaryDirectory, but configurable
> +    """
> +
> +    def __init__(self, args):
> +        """
> +        setup and store args
> +        args: cmdline arguments
> +        """
> +        self.args = args
> +        self.tmpdir = None
> +
> +    def __enter__(self):
> +        """
> +        create tempdir
> +        return_value: tempdir path
> +        """
> +        self.tmpdir = tempfile.mkdtemp(prefix='cloud_test_')
> +        LOG.debug('using tmpdir: %s', self.tmpdir)
> +        return self.tmpdir
> +
> +    def __exit__(self, etype, value, trace):
> +        """
> +        destroy tempdir if no errors occurred
> +        """
> +        if etype:
> +            LOG.warn('erros occurred, leaving data in %s', self.tmpdir)
> +        else:
> +            shutil.rmtree(self.tmpdir)
>  
>  # vi: ts=4 expandtab


-- 
https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/323588
Your team cloud init development team is requested to review the proposed merge of ~powersj/cloud-init:integration-test-revamp into cloud-init:master.


References