Diff comments:
> diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py > index db3c357..9def76d 100644 > --- a/cloudinit/net/network_state.py > +++ b/cloudinit/net/network_state.py > @@ -289,19 +289,16 @@ class NetworkStateInterpreter(object): > iface.update({param: val}) > > # convert subnet ipv6 netmask to cidr as needed > - subnets = command.get('subnets') > - if subnets: > + subnets = _normalize_subnets(command.get('subnets')) > + print("normalized subnets: %s" % subnets) drop the debugging print > + > + # automatically set 'use_ipv6' if any addresses are ipv6 > + if not self.use_ipv6: > for subnet in subnets: > - if subnet['type'] == 'static': > - if ':' in subnet['address']: > - self.use_ipv6 = True > - if 'netmask' in subnet and ':' in subnet['address']: > - subnet['netmask'] = mask2cidr(subnet['netmask']) > - for route in subnet.get('routes', []): > - if 'netmask' in route: > - route['netmask'] = > mask2cidr(route['netmask']) > - elif subnet['type'].endswith('6'): > + if (subnet.get('type').endswith('6') or > + is_ipv6_addr(subnet.get('address'))): > self.use_ipv6 = True > + break > > iface.update({ > 'name': command.get('name'), > @@ -692,6 +689,122 @@ class NetworkStateInterpreter(object): > return subnets > > > +def _normalize_subnet(subnet): > + # Prune all keys with None values. > + subnet = copy.deepcopy(subnet) > + normal_subnet = dict((k, v) for k, v in subnet.items() if v) > + > + if subnet.get('type') == 'static': do we also need this for static6? > + normal_subnet.update( > + _normalize_net_keys(normal_subnet, address_keys=('address',))) > + normal_subnet['routes'] = [_normalize_route(r) > + for r in subnet.get('routes', [])] > + return normal_subnet > + > + > +def _normalize_net_keys(network, address_keys=()): > + > + """Normalize dictionary network keys returning prefix and address keys. > + > + @param network: A dict of network-related definition containing prefix, > + netmask and address_keys. > + @param address_keys: A tuple of keys to search for representing the > address > + or cidr. The first address_key discovered will be used for > + normalization. > + > + @returns: A dict containing normalized prefix and matching addr_key. > + """ > + network = copy.deepcopy(network) > + net = dict((k, v) for k, v in network.items() if v) > + addr_key = None > + for key in address_keys: > + if net.get(key): > + addr_key = key > + break > + if not addr_key: > + message = ( > + 'No config network address keys [%s] found in %s' % > + (','.join(address_keys), network)) > + LOG.error(message) > + raise ValueError(message) > + > + addr = net.get(addr_key) > + ipv6 = is_ipv6_addr(addr) > + netmask = net.get('netmask') > + if "/" in addr: > + toks = addr.split("/", 2) A comment of what the expected tokens here are would be nice. My preference would be for named variables. > + net[addr_key] = toks[0] > + # If prefix is defined by the user but addr_key is a CIDR, we > + # silently overwrite the user's original prefix here. We should > + # log a warning. Add the log.warning ? > + net['prefix'] = toks[1] > + try: > + net['prefix'] = int(toks[1]) > + except ValueError: > + # this supports input of <address>/255.255.255.0 > + net['prefix'] = mask_to_net_prefix(toks[1]) > + > + elif netmask: > + net['prefix'] = mask_to_net_prefix(netmask) > + else: > + net['prefix'] = 64 if ipv6 else 24 This looks worthy of at least a debug message? Injecting a netmask if one isn't provided? > + > + if ipv6: > + if 'netmask' in net: > + del net['netmask'] > + else: > + net['netmask'] = net_prefix_to_ipv4_mask(net['prefix']) > + > + prefix = net['prefix'] > + if prefix: > + try: > + net['prefix'] = int(prefix) > + except ValueError: > + raise TypeError( > + 'Network config prefix {} is not an integer'.format(prefix)) > + return net > + > + > +def _normalize_route(route): > + """normalize a route. > + return a dictionary with only: > + 'type': 'route' (only present if it was present in input) > + 'network': the network portion of the route as a string. > + 'prefix': the network prefix for address as an integer. > + 'metric': integer metric (only if present in input). > + 'netmask': netmask (string) equivalent to prefix if ipv4. > + """ > + route = copy.deepcopy(route) > + print("normalizing %s" % route) remove debugging > + # Prune None-value keys > + normal_route = dict((k, v) for k, v in route.items() if v) > + normal_route.update( > + _normalize_net_keys( > + normal_route, address_keys=('network', 'destination'))) > + > + metric = normal_route.get('metric') > + if metric: > + try: > + normal_route['metric'] = int(metric) > + except ValueError: > + raise TypeError( > + 'Route config metric {} is not an integer'.format(metric)) > + print("normaled to %s" % normal_route) > + return normal_route > + > + > +def _normalize_subnets(subnets): > + if not subnets: > + subnets = [] > + return [_normalize_subnet(s) for s in subnets] > + > + > +def is_ipv6_addr(address): > + if not address: > + return False > + return ":" in address > + > + > def subnet_is_ipv6(subnet): > """Common helper for checking network_state subnets for ipv6.""" > # 'static6' or 'dhcp6' > @@ -733,6 +869,28 @@ def ipv6mask2cidr(mask): > return cidr > > > +def mask_to_net_prefix(mask): > + """Return the network prefix for the netmask provided. > + > + Supports ipv4 or ipv6 netmasks.""" > + if ':' in str(mask): > + return ipv6_mask_to_net_prefix(mask) > + elif '.' in str(mask): > + return ipv4_mask_to_net_prefix(mask) > + should we raise a ValueError here in case mask has neither a ":" or "." in it? ie, it's not a mask? > + > +def ipv4mask2cidr(mask): > + return ipv4_mask_to_net_prefix(mask, strict=False) > + > + > +def cidr2mask(cidr): > + return net_prefix_to_ipv4_mask(cidr) > + > + > +def ipv6mask2cidr(mask): > + return ipv6_mask_to_net_prefix(mask, strict=False) > + > + > def mask2cidr(mask): > if ':' in mask: > return ipv6mask2cidr(mask) > @@ -741,4 +899,5 @@ def mask2cidr(mask): > else: > return mask > > + whitespace > # vi: ts=4 expandtab > diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py > index 58c5713..3f917ed 100644 > --- a/cloudinit/net/sysconfig.py > +++ b/cloudinit/net/sysconfig.py > @@ -26,11 +26,9 @@ def _make_header(sep='#'): > > > def _is_default_route(route): > - if route['network'] == '::' and route['netmask'] == 0: > - return True > - if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0': > - return True > - return False > + print("checking idr route=%s" % route) remove debugging > + default_nets = ('::', '0.0.0.0') > + return route['prefix'] == 0 and route['network'] in default_nets > > > def _quote_value(value): > @@ -323,22 +321,18 @@ class Renderer(renderer.Renderer): > " " + ipv6_cidr) > else: > ipv4_index = ipv4_index + 1 > - if ipv4_index == 0: > - iface_cfg['IPADDR'] = subnet['address'] > - if 'netmask' in subnet: > - iface_cfg['NETMASK'] = subnet['netmask'] > - else: > - iface_cfg['IPADDR' + str(ipv4_index)] = \ > - subnet['address'] > - if 'netmask' in subnet: > - iface_cfg['NETMASK' + str(ipv4_index)] = \ > - subnet['netmask'] > + suff = "" if ipv4_index == 0 else str(ipv4_index) > + iface_cfg['IPADDR' + suff] = subnet['address'] > + iface_cfg['NETMASK' + suff] = \ > + net_prefix_to_ipv4_mask(subnet['prefix']) > > @classmethod > def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets): > for i, subnet in enumerate(subnets, start=len(iface_cfg.children)): > + print("subnet=%s" % subnet) > for route in subnet.get('routes', []): > is_ipv6 = subnet.get('ipv6') > + print("route=%s" % route) remove debugging here and line 331 > > if _is_default_route(route): > if ( > diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py > index 5169821..a5a43bd 100644 > --- a/tests/unittests/test_net.py > +++ b/tests/unittests/test_net.py > @@ -890,6 +888,7 @@ USERCTL=no > macs = {'fa:16:3e:ed:9a:59': 'eth0'} > render_dir = self.tmp_dir() > network_cfg = openstack.convert_net_json(net_json, known_macs=macs) > + print("network_cfg=%s" % network_cfg) remove debugging > ns = network_state.parse_net_config_data(network_cfg, > skip_broken=False) > renderer = sysconfig.Renderer() -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324677 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/mask2cidr 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