← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~smoser/cloud-init:fix/1798117-allow-toplevel-network-in-network-config into cloud-init:master

 


Diff comments:

> diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py
> index 9010f06..8b4895b 100644
> --- a/cloudinit/sources/DataSourceNoCloud.py
> +++ b/cloudinit/sources/DataSourceNoCloud.py
> @@ -311,6 +311,33 @@ def parse_cmdline_data(ds_id, fill, cmdline=None):
>      return True
>  
>  
> +def _maybe_remove_top_network(cfg):
> +    """If network-config contains top level 'network' key, then remove it.
> +
> +    Some providers of network configuration may provide a top level
> +    'network' key (LP: #1798117) even though it is not necessary.
> +
> +    Be friendly and remove it if it really seems so.
> +
> +    Return the original value if no change or the updated value if changed."""
> +    nullval = object()
> +    network_val = cfg.get('network', nullval)
> +    if network_val is nullval:
> +        return cfg
> +    bmsg = 'Top level network key in network-config %s: %s'
> +    if not isinstance(network_val, dict):
> +        LOG.debug(bmsg, "was not a dict", cfg)
> +        return cfg
> +    if len(list(cfg.keys())) != 1:
> +        LOG.debug(bmsg, "had multiple top level keys", cfg)
> +        return cfg
> +    if not ('config' in network_val or 'version' in network_val):

I think this logic is wrong here maybe?

It looks from your debug  message that you 
were expecting both version and config, but your logic only checks if "not either"  as in "neither" exist.



more pythonic might be something like
if not all(['config' in network_val, 'version' in network_val]):

   or even 
expected_keys = set(['config', 'version'])
missing_keys = expected_keys.difference(network_val)
if missing_keys:
 LOG.debug....

> +        LOG.debug(bmsg, "but missing 'config' and 'version'", cfg)
> +        return cfg
> +    LOG.debug(bmsg, "fixed by removing shifting network.", cfg)
> +    return network_val
> +
> +
>  def _merge_new_seed(cur, seeded):
>      ret = cur.copy()
>  


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/356850
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix/1798117-allow-toplevel-network-in-network-config into cloud-init:master.


References