cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Looks pretty good, I'd like to see other unit tests specifically looking at return vals from util.system_info() but that can come in a later branch. Minor nits but code looks good.
I have no bsd system available otherwise I'd check it out with a couple of runs.

Diff comments:

> diff --git a/cloudinit/util.py b/cloudinit/util.py
> index 415ca37..817d347 100644
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -598,37 +598,29 @@ def get_cfg_option_int(yobj, key, default=0):
>  def system_info():
>      info = {
>          'platform': platform.platform(),
> +        'system': platform.system(),
>          'release': platform.release(),
>          'python': platform.python_version(),
>          'uname': platform.uname(),
> -        'dist': platform.linux_distribution(),  # pylint: disable=W1505
> +        'dist': platform.dist(),  # pylint: disable=W1505
>      }
> -    plat = info['platform'].lower()
> -    # Try to get more info about what it actually is, in a format
> -    # that we can easily use across linux and variants...
> -    if plat.startswith('darwin'):
> -        info['variant'] = 'darwin'
> -    elif plat.endswith("bsd"):
> -        info['variant'] = 'bsd'
> -    elif plat.startswith('win'):
> -        info['variant'] = 'windows'
> -    elif 'linux' in plat:
> -        # Try to get a single string out of these...
> -        linux_dist, _version, _id = info['dist']
> -        linux_dist = linux_dist.lower()
> -        if linux_dist in ('ubuntu', 'linuxmint', 'mint'):
> -            info['variant'] = 'ubuntu'
> +    system = info['system'].lower()
> +    var = 'unknown'
> +    if system == "linux":
> +        linux_dist = info['dist'][0].lower()
> +        if linux_dist in ('centos', 'fedora', 'debian'):

do we want a suse case here too?

> +            var = linux_dist
> +        elif linux_dist in ('ubuntu', 'linuxmint', 'mint'):
> +            var = 'ubuntu'
> +        elif linux_dist == 'redhat':
> +            var = 'rhel'
>          else:
> -            for prefix, variant in [('redhat', 'rhel'),
> -                                    ('centos', 'centos'),
> -                                    ('fedora', 'fedora'),
> -                                    ('debian', 'debian')]:
> -                if linux_dist.startswith(prefix):
> -                    info['variant'] = variant
> -        if 'variant' not in info:
> -            info['variant'] = 'linux'
> -    if 'variant' not in info:
> -        info['variant'] = 'unknown'
> +            var = 'linux'
> +    elif system in ('windows', 'darwin', "freebsd"):
> +        var = system
> +
> +    info['variant'] = var
> +
>      return info
>  
>  
> diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
> index 5af2a88..f4b9069 100644
> --- a/config/cloud.cfg.tmpl
> +++ b/config/cloud.cfg.tmpl
> @@ -130,10 +130,8 @@ cloud_final_modules:
>  # (not accessible to handlers/transforms)
>  system_info:
>     # This will affect which distro class gets used
> -{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu"] %}
> +{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu", "freebsd"] %}

suse in here too?

>     distro: {{ variant }}
> -{% elif variant in ["bsd"] %}
> -   distro: freebsd
>  {% else %}
>     # Unknown/fallback distro.
>     distro: ubuntu


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/325760
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/freebsd-variant into cloud-init:master.