cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Approved with minor nits questions inline. +1!

Validated and inspected netplan output with the following:

PYTHONPATH=. ./tools/net-convert.py --network-data=net-config.yaml --kind=yaml --output-kind=netplan --directory=out.d --mac=eth0,52:54:00:12:34:00 --mac=ens4,52:54:00:12:34:02


Prior to this branch netplan output was empty/broken:
 cat out2.d/etc/netplan/50-cloud-init.yaml 

network:
    version: 2
    ethernets:
        ens4: {}
        eth0: {}


Diff comments:

> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
> index 87a7222..71ecada 100644
> --- a/cloudinit/net/network_state.py
> +++ b/cloudinit/net/network_state.py
> @@ -616,22 +646,34 @@ class NetworkStateInterpreter(object):
>  
>      def _handle_bond_bridge(self, command, cmd_type=None):
>          """Common handler for bond and bridge types"""
> +
> +        # inverse mapping for v2 keynames to v1 keynames
> +        key_map = dict((v, k) for k, v in

bikeshed: Namewise; can this be  netv2_to_v1?

> +                       NET_CONFIG_TO_V2.get(cmd_type).items())

could use iteritems here to use an iterator instead of expanding the whole list (albeit it's small so it won't hog memory) .

> +
>          for item_name, item_cfg in command.items():
>              item_params = dict((key, value) for (key, value) in
>                                 item_cfg.items() if key not in
>                                 NETWORK_V2_KEY_FILTER)
> +
>              v1_cmd = {
>                  'type': cmd_type,
>                  'name': item_name,
>                  cmd_type + '_interfaces': item_cfg.get('interfaces'),
> -                'params': item_params,
> +                'params': dict((key_map[k], v) for k, v in
> +                               item_params.get('parameters', {}).items())
>              }
>              subnets = self._v2_to_v1_ipcfg(item_cfg)
>              if len(subnets) > 0:
>                  v1_cmd.update({'subnets': subnets})
>  
>              LOG.debug('v2(%ss) -> v1(%s):\n%s', cmd_type, cmd_type, v1_cmd)
> -            self.handle_bridge(v1_cmd)
> +            if cmd_type == "bridge":
> +                self.handle_bridge(v1_cmd)
> +            elif cmd_type == "bond":
> +                self.handle_bond(v1_cmd)
> +            else:
> +                raise ValueError('Unknown command type: %s', cmd_type)
>  
>      def _v2_to_v1_ipcfg(self, cfg):
>          """Common ipconfig extraction from v2 to v1 subnets array."""
> @@ -753,13 +798,15 @@ def _normalize_route(route):
>      # Prune None-value keys.  Specifically allow 0 (a valid metric).
>      normal_route = dict((k, v) for k, v in route.items()
>                          if v not in ("", None))
> -    if 'destination' in normal_route:
> -        normal_route['network'] = normal_route['destination']
> -        del normal_route['destination']
> +    for target_key in ['destination', 'address']:
> +        if target_key in normal_route:
> +            normal_route['network'] = normal_route[target_key]
> +            del normal_route[target_key]
> +            break

Are there cases where we might see both 'destination' and 'address' keys and specifically want to break and ignore the unparsed address?

>  
>      normal_route.update(
>          _normalize_net_keys(
> -            normal_route, address_keys=('network', 'destination')))
> +            normal_route, address_keys=('network', target_key)))
>  
>      metric = normal_route.get('metric')
>      if metric:


-- 
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/328800
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:bug-lp-1709180-v2-params into cloud-init:master.