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.
_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
