cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py
> index 993b26c..a8c7f7a 100644
> --- a/cloudinit/netinfo.py
> +++ b/cloudinit/netinfo.py
> @@ -18,18 +20,89 @@ from cloudinit.simpletable import SimpleTable
>  LOG = logging.getLogger()
>  
>  
> -def netdev_info(empty=""):
> -    fields = ("hwaddr", "addr", "bcast", "mask")
> -    (ifcfg_out, _err) = util.subp(["ifconfig", "-a"], rcs=[0, 1])
> +DEFAULT_NETDEV_INFO = {
> +    "ipv4": [],
> +    "ipv6": [],
> +    "hwaddr": "",
> +    "up": False
> +}
> +
> +
> +def netdev_info_iproute(ipaddr_out):
> +    """
> +    Get network device dicts from ip route and ip link info.
> +
> +    @param ipaddr_out: Output string from 'ip addr show' command.
> +
> +    @returns: A dict of device info keyed by network device name containing
> +              device configuration values.
> +    """
>      devs = {}
> -    for line in str(ifcfg_out).splitlines():
> +    dev_name = None
> +    for num, line in enumerate(ipaddr_out.splitlines()):

if ipaddr_out is not a string, this blows up.

> +        m = re.match(r'^\d+:\s(?P<dev>[^:]+):\s+<(?P<flags>\S+)>\s+.*', line)
> +        if m:
> +            dev_name = m.group('dev').lower().split('@')[0]
> +            flags = m.group('flags').split(',')
> +            devs[dev_name] = {
> +                'ipv4': [], 'ipv6': [], 'hwaddr': '',
> +                'up': bool('UP' in flags and 'LOWER_UP' in flags),
> +            }
> +        elif 'inet6' in line:
> +            m = re.match(
> +                r'\s+inet6\s(?P<ip>\S+)\sscope\s(?P<scope6>\S+).*', line)
> +            if not m:
> +                LOG.warning(
> +                    'Could not parse ip addr show: (line:%d) %s', num, line)
> +                continue
> +            devs[dev_name]['ipv6'].append(m.groupdict())
> +        elif 'inet' in line:
> +            m = re.match(
> +                r'\s+inet\s(?P<cidr4>\S+)(\sbrd\s(?P<bcast>\S+))?\sscope\s'
> +                r'(?P<scope>\S+).*', line)
> +            if not m:
> +                LOG.warning(
> +                    'Could not parse ip addr show: (line:%d) %s', num, line)
> +                continue
> +            match = m.groupdict()
> +            cidr4 = match.pop('cidr4')
> +            addr, _, prefix = cidr4.partition('/')
> +            if not prefix:
> +                prefix = '32'
> +            devs[dev_name]['ipv4'].append({
> +                'ip': addr,
> +                'bcast': match['bcast'] if match['bcast'] else '',
> +                'mask': net_prefix_to_ipv4_mask(prefix),
> +                'scope': match['scope']})
> +        elif 'link' in line:
> +            m = re.match(
> +                r'\s+link/(?P<link_type>\S+)\s(?P<hwaddr>\S+).*', line)
> +            if not m:
> +                LOG.warning(
> +                    'Could not parse ip addr show: (line:%d) %s', num, line)
> +                continue
> +            if m.group('link_type') == 'ether':
> +                devs[dev_name]['hwaddr'] = m.group('hwaddr')
> +            else:
> +                devs[dev_name]['hwaddr'] = ''
> +        else:
> +            continue
> +    return devs
> +
> +
> +def netdev_info_ifconfig(ifconfig_data):
> +    # fields that need to be returned in devs for each dev
> +    devs = {}
> +    for line in ifconfig_data.splitlines():

Same here, if ifconfig_data is not a string, this blows up.

>          if len(line) == 0:
>              continue
>          if line[0] not in ("\t", " "):
>              curdev = line.split()[0]
> -            devs[curdev] = {"up": False}
> -            for field in fields:
> -                devs[curdev][field] = ""
> +            # current ifconfig pops a ':' on the end of the device
> +            if curdev.endswith(':'):
> +                curdev = curdev[:-1]
> +            if curdev not in devs:
> +                devs[curdev] = deepcopy(DEFAULT_NETDEV_INFO)
>          toks = line.lower().strip().split()
>          if toks[0] == "up":
>              devs[curdev]['up'] = True
> @@ -84,14 +163,94 @@ def netdev_info(empty=""):

Not sure if it's worth a message to say we found neither iproute nor net-tools commands, so we cannot report network info.

>      return devs
>  
>  
> -def route_info():
> -    (route_out, _err) = util.subp(["netstat", "-rn"], rcs=[0, 1])
> +def netdev_route_info_iproute(iproute_data):
> +    """
> +    Get network route dicts from ip route info.
> +
> +    @param iproute_data: Output string from ip route command.
> +
> +    @returns: A dict containing ipv4 and ipv6 route entries as lists. Each
> +              item in the list is a route dictionary representing destination,
> +              gateway, flags, genmask and interface information.
> +    """
>  
>      routes = {}
>      routes['ipv4'] = []
>      routes['ipv6'] = []
> +    entries = iproute_data.splitlines()

This blows up on non-string

> +    default_route_entry = {
> +        'destination': '', 'flags': '', 'gateway': '', 'genmask': '',
> +        'iface': '', 'metric': ''}
> +    for line in entries:
> +        entry = copy(default_route_entry)
> +        if not line:
> +            continue
> +        toks = line.split()
> +        flags = ['U']
> +        if toks[0] == "default":
> +            entry['destination'] = "0.0.0.0"
> +            entry['genmask'] = "0.0.0.0"
> +        else:
> +            if '/' in toks[0]:
> +                (addr, cidr) = toks[0].split("/")
> +            else:
> +                addr = toks[0]
> +                cidr = '32'
> +                flags.append("H")
> +                entry['genmask'] = net_prefix_to_ipv4_mask(cidr)
> +            entry['destination'] = addr
> +            entry['genmask'] = net_prefix_to_ipv4_mask(cidr)
> +            entry['gateway'] = "0.0.0.0"
> +        for i in range(len(toks)):
> +            if toks[i] == "via":
> +                entry['gateway'] = toks[i + 1]
> +                flags.insert(1, "G")
> +            if toks[i] == "dev":
> +                entry["iface"] = toks[i + 1]
> +            if toks[i] == "metric":
> +                entry['metric'] = toks[i + 1]
> +        entry['flags'] = ''.join(flags)
> +        routes['ipv4'].append(entry)
> +    try:
> +        (iproute_data6, _err6) = util.subp(
> +            ["ip", "--oneline", "-6", "route", "list", "table", "all"],
> +            rcs=[0, 1])
> +    except util.ProcessExecutionError:
> +        pass
> +    else:
> +        entries6 = iproute_data6.splitlines()
> +        for line in entries6:
> +            entry = {}
> +            if not line:
> +                continue
> +            toks = line.split()
> +            if toks[0] == "default":
> +                entry['destination'] = "::/0"
> +                entry['flags'] = "UG"
> +            else:
> +                entry['destination'] = toks[0]
> +                entry['gateway'] = "::"
> +                entry['flags'] = "U"
> +            for i in range(len(toks)):
> +                if toks[i] == "via":
> +                    entry['gateway'] = toks[i + 1]
> +                    entry['flags'] = "UG"
> +                if toks[i] == "dev":
> +                    entry["iface"] = toks[i + 1]
> +                if toks[i] == "metric":
> +                    entry['metric'] = toks[i + 1]
> +                if toks[i] == "expires":
> +                    entry['flags'] = entry['flags'] + 'e'
> +            routes['ipv6'].append(entry)
> +    return routes
>  
> -    entries = route_out.splitlines()[1:]
> +
> +def netdev_route_info_netstat(route_data):
> +    routes = {}
> +    routes['ipv4'] = []
> +    routes['ipv6'] = []
> +
> +    entries = route_data.splitlines()

This breaks with route_data being non-string.

>      for line in entries:
>          if not line:
>              continue
> @@ -166,21 +348,25 @@ def netdev_pformat():
>      lines = []
>      try:
>          netdev = netdev_info(empty=".")
> -    except Exception:
> +    except Exception as e:
>          lines.append(util.center("Net device info failed", '!', 80))

What are we doing with 'e' here?  logging a warning?

>      else:
>          fields = ['Device', 'Up', 'Address', 'Mask', 'Scope', 'Hw-Address']
>          tbl = SimpleTable(fields)
> -        for (dev, d) in sorted(netdev.items()):
> -            tbl.add_row([dev, d["up"], d["addr"], d["mask"], ".", d["hwaddr"]])
> -            if d.get('addr6'):
> -                tbl.add_row([dev, d["up"],
> -                             d["addr6"], ".", d.get("scope6"), d["hwaddr"]])
> +        for (dev, data) in sorted(netdev.items()):
> +            for addr in data.get('ipv4'):
> +                tbl.add_row(
> +                    [dev, data["up"], addr["ip"], addr["mask"], ".",
> +                     data["hwaddr"]])
> +            for addr in data.get('ipv6'):
> +                tbl.add_row(
> +                    [dev, data["up"], addr["ip"], ".", addr["scope6"],
> +                     data["hwaddr"]])
>          netdev_s = tbl.get_string()
>          max_len = len(max(netdev_s.splitlines(), key=len))
>          header = util.center("Net device info", "+", max_len)
>          lines.extend([header, netdev_s])
> -    return "\n".join(lines)
> +    return "\n".join(lines) + "\n"
>  
>  
>  def route_pformat():
> @@ -193,32 +379,31 @@ def route_pformat():
>      else:
>          if routes.get('ipv4'):
>              fields_v4 = ['Route', 'Destination', 'Gateway',
> -                         'Genmask', 'Interface', 'Flags']
> +                         'Genmask', 'Interface', 'Flags', 'Metric']

this sort of looks like changing output, are we adding the Metric field ?

>              tbl_v4 = SimpleTable(fields_v4)
>              for (n, r) in enumerate(routes.get('ipv4')):
>                  route_id = str(n)
>                  tbl_v4.add_row([route_id, r['destination'],
>                                  r['gateway'], r['genmask'],
> -                                r['iface'], r['flags']])
> +                                r['iface'], r['flags'], r['metric']])
>              route_s = tbl_v4.get_string()
>              max_len = len(max(route_s.splitlines(), key=len))
>              header = util.center("Route IPv4 info", "+", max_len)
>              lines.extend([header, route_s])
>          if routes.get('ipv6'):
> -            fields_v6 = ['Route', 'Proto', 'Recv-Q', 'Send-Q',
> -                         'Local Address', 'Foreign Address', 'State']
> +            fields_v6 = ['Route', 'Destination', 'Gateway', 'Interface',
> +                         'Flags', 'Metric']
>              tbl_v6 = SimpleTable(fields_v6)
>              for (n, r) in enumerate(routes.get('ipv6')):
>                  route_id = str(n)
> -                tbl_v6.add_row([route_id, r['proto'],
> -                                r['recv-q'], r['send-q'],
> -                                r['local address'], r['foreign address'],
> -                                r['state']])
> +                tbl_v6.add_row([route_id, r['destination'],
> +                                r['gateway'], r['iface'],
> +                                r['flags'], r['metric']])
>              route_s = tbl_v6.get_string()
>              max_len = len(max(route_s.splitlines(), key=len))
>              header = util.center("Route IPv6 info", "+", max_len)
>              lines.extend([header, route_s])
> -    return "\n".join(lines)
> +    return "\n".join(lines) + "\n"
>  
>  
>  def debug_info(prefix='ci-info: '):


-- 
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/342428
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master.