cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Maybe it's just a print problem...

tox -e citest -- tree_run -v -n artful -t tests/cloud_tests/configs/modules/write_files.yaml 

2017-05-15 19:34:34,906 - tests.cloud_tests - INFO - acquiring image for os: xenial

I would've expected artful to be acquired not xenial.



Diff comments:

> diff --git a/tests/cloud_tests/config.py b/tests/cloud_tests/config.py
> index f3a13c9..9a0ad4d 100644
> --- a/tests/cloud_tests/config.py
> +++ b/tests/cloud_tests/config.py
> @@ -61,22 +75,63 @@ def merge_config(base, override):
>      return res
>  
>  
> -def load_platform_config(platform):
> +def merge_feature_groups(feature_conf, feature_groups, overrides):
> +    """
> +    combine feature groups and overrides to construct a supported feature list
> +    feature_conf: feature config from releases.yaml
> +    feature_groups: feature groups the release is a member of
> +    overrides: overrides specified by the release's config
> +    return_value: dict of {feature: true/false} settings
> +    """
> +    res = dict().fromkeys(feature_conf['all'])
> +    for group in feature_groups:
> +        res.update(feature_conf['groups'][group])
> +    res.update(overrides)
> +    return res
> +
> +
> +def load_platform_config(platform_name, require_enabled=False):
>      """
>      load configuration for platform
> +    platform_name: name of platform to retrieve config for

docstring oneline, blank line then param descriptions etc. preferably with @param <name>: <description> but if that's too heavyweight, the blankline before params should suffice.

> +    require_enabled: if true, raise error if 'enabled' not True
> +    return_value: config dict
>      """
>      main_conf = c_util.read_conf(PLATFORM_CONF)
> -    return merge_config(main_conf.get('default_platform_config'),
> -                        main_conf.get('platforms')[platform])
> +    conf = merge_config(main_conf['default_platform_config'],
> +                        main_conf['platforms'][platform_name])
> +    if require_enabled and not enabled(conf):
> +        raise ValueError('Platform is not enabled')
> +    return conf
>  
>  
> -def load_os_config(os_name):
> +def load_os_config(platform_name, os_name, require_enabled=False,
> +                   feature_overrides={}):
>      """
>      load configuration for os
> +    platform_name: platform name to load os config for
> +    os_name: name of os to retrieve config for
> +    require_enabled: if true, raise error if 'enabled' not True
> +    feature_overrides: feature flag overrides to merge with features config
> +    return_value: config dict
>      """
>      main_conf = c_util.read_conf(RELEASES_CONF)
> -    return merge_config(main_conf.get('default_release_config'),
> -                        main_conf.get('releases')[os_name])
> +    default = main_conf['default_release_config']
> +    image = main_conf['releases'][os_name]
> +    conf = merge_config(merge_config(get(default, 'default'),
> +                                     get(default, platform_name)),
> +                        merge_config(get(image, 'default'),
> +                                     get(image, platform_name)))
> +
> +    feature_conf = main_conf['features']
> +    feature_groups = conf.get('feature_groups', [])
> +    overrides = merge_config(get(conf, 'features'), feature_overrides)
> +    conf['features'] = merge_feature_groups(
> +        feature_conf, feature_groups, overrides)
> +
> +    if require_enabled and not enabled(conf):
> +        raise ValueError('OS is not enabled')
> +    return conf
>  
>  
>  def load_test_config(path):
> @@ -87,20 +142,34 @@ def load_test_config(path):
>                          c_util.read_conf(name_to_path(path)))
>  
>  
> +def list_feature_flags():
> +    """
> +    list all supported feature flags

can be one line

> +    """
> +    feature_conf = get(c_util.read_conf(RELEASES_CONF), 'features')
> +    return feature_conf.get('all', [])
> +
> +
>  def list_enabled_platforms():
>      """
>      list all platforms enabled for testing
>      """
> -    platforms = c_util.read_conf(PLATFORM_CONF).get('platforms')
> -    return [k for k, v in platforms.items() if v.get('enabled')]
> +    platforms = get(c_util.read_conf(PLATFORM_CONF), 'platforms')
> +    return [k for k, v in platforms.items() if enabled(v)]
>  
>  
> -def list_enabled_distros():
> +def list_enabled_distros(platforms):
>      """
> -    list all distros enabled for testing
> +    list all distros enabled for testing on specified platforms
>      """
> -    releases = c_util.read_conf(RELEASES_CONF).get('releases')
> -    return [k for k, v in releases.items() if v.get('enabled')]
> +
> +    def platform_has_enabled(config):
> +        return any(enabled(merge_config(get(config, 'default'),
> +                                        get(config, platform)))
> +                   for platform in platforms)
> +
> +    releases = get(c_util.read_conf(RELEASES_CONF), 'releases')
> +    return [k for k, v in releases.items() if platform_has_enabled(v)]
>  
>  
>  def list_test_configs():
> diff --git a/tests/cloud_tests/images/base.py b/tests/cloud_tests/images/base.py
> index 394b11f..1f604cf 100644
> --- a/tests/cloud_tests/images/base.py
> +++ b/tests/cloud_tests/images/base.py
> @@ -28,10 +29,24 @@ class Image(object):
>          """
>          raise NotImplementedError
>  
> -    # FIXME: instead of having execute and push_file and other instance methods
> -    #        here which pass through to a hidden instance, it might be better
> -    #        to expose an instance that the image can be modified through
> -    def execute(self, command, stdin=None, stdout=None, stderr=None, env={}):
> +    @property
> +    def features(self):
> +        """
> +        feature flags supported by this image
> +        return_value: list of feature names
> +        """
> +        return [k for k, v in self.config.get('features', {}).items() if v]
> +
> +    @property
> +    def setup_overrides(self):
> +        """
> +        setup options that need to be overridden for the image
> +        return_value: dictionary to update args with
> +        """
> +        # NOTE: more sophisticated options may be requied at some point

d/requied/required/

> +        return self.config.get('setup_overrides', {})
> +
> +    def execute(self, *args, **kwargs):
>          """
>          execute command in image, modifying image
>          """


-- 
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.