cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

one useful thougt.


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()):

you could raise TypeError on the input if you wanted, but subp returns either empty string or empty bytes if capture is True.

> +        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():
>          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
> @@ -39,41 +112,47 @@ def netdev_info(empty=""):
>              if re.search(r"flags=\d+<up,", toks[1]):
>                  devs[curdev]['up'] = True
>  
> -        fieldpost = ""
> -        if toks[0] == "inet6":
> -            fieldpost = "6"
> -
>          for i in range(len(toks)):
> -            # older net-tools (ubuntu) show 'inet addr:xx.yy',
> -            # newer (freebsd and fedora) show 'inet xx.yy'
> -            # just skip this 'inet' entry. (LP: #1285185)
> -            try:
> -                if ((toks[i] in ("inet", "inet6") and
> -                     toks[i + 1].startswith("addr:"))):
> -                    continue
> -            except IndexError:
> -                pass
> -
> -            # Couple the different items we're interested in with the correct
> -            # field since FreeBSD/CentOS/Fedora differ in the output.
> -            ifconfigfields = {
> -                "addr:": "addr", "inet": "addr",
> -                "bcast:": "bcast", "broadcast": "bcast",
> -                "mask:": "mask", "netmask": "mask",
> -                "hwaddr": "hwaddr", "ether": "hwaddr",
> -                "scope": "scope",
> -            }
> -            for origfield, field in ifconfigfields.items():
> -                target = "%s%s" % (field, fieldpost)
> -                if devs[curdev].get(target, ""):
> -                    continue
> -                if toks[i] == "%s" % origfield:
> -                    try:
> -                        devs[curdev][target] = toks[i + 1]
> -                    except IndexError:
> -                        pass
> -                elif toks[i].startswith("%s" % origfield):
> -                    devs[curdev][target] = toks[i][len(field) + 1:]
> +            if toks[i] == "inet":  # Create new ipv4 addr entry
> +                devs[curdev]['ipv4'].append(
> +                    {'ip': toks[i + 1].lstrip("addr:")})
> +            elif toks[i].startswith("bcast:"):
> +                devs[curdev]['ipv4'][-1]['bcast'] = toks[i].lstrip("bcast:")
> +            elif toks[i] == "broadcast":
> +                devs[curdev]['ipv4'][-1]['bcast'] = toks[i + 1]
> +            elif toks[i].startswith("mask:"):
> +                devs[curdev]['ipv4'][-1]['mask'] = toks[i].lstrip("mask:")
> +            elif toks[i] == "netmask":
> +                devs[curdev]['ipv4'][-1]['mask'] = toks[i + 1]
> +            elif toks[i] == "hwaddr" or toks[i] == "ether":
> +                devs[curdev]['hwaddr'] = toks[i + 1]
> +            elif toks[i] == "inet6":
> +                if toks[i + 1] == "addr:":
> +                    devs[curdev]['ipv6'].append({'ip': toks[i + 2]})
> +                else:
> +                    devs[curdev]['ipv6'].append({'ip': toks[i + 1]})
> +            elif toks[i] == "prefixlen":  # Add prefix to current ipv6 value
> +                addr6 = devs[curdev]['ipv6'][-1]['ip'] + "/" + toks[i + 1]
> +                devs[curdev]['ipv6'][-1]['ip'] = addr6
> +            elif toks[i].startswith("scope:"):
> +                devs[curdev]['ipv6'][-1]['scope6'] = toks[i].lstrip("scope:")
> +            elif toks[i] == "scopeid":
> +                res = re.match(".*<(\S+)>", toks[i + 1])
> +                if res:
> +                    devs[curdev]['ipv6'][-1]['scope6'] = res.group(1)
> +    return devs
> +
> +
> +def netdev_info(empty=""):
> +    devs = {}
> +    try:
> +        # Try iproute first of all

I think i'd rather 
   if util.which("ip"):
      # go the ip route
   elif util.which("ifconfig"):
      # go that route
   else:
      LOG.warning("YIKES")

doing the try/except means that we coudl be swallowing an error and not know it.

> +        (ipaddr_out, _err) = util.subp(["ip", "addr", "show"])
> +        devs = netdev_info_iproute(ipaddr_out)
> +    except util.ProcessExecutionError:
> +        # Fall back to net-tools if iproute2 is not present
> +        (ifcfg_out, _err) = util.subp(["ifconfig", "-a"], rcs=[0, 1])
> +        devs = netdev_info_ifconfig(ifcfg_out)
>  
>      if empty != "":
>          for (_devname, dev) in devs.items():
> @@ -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))

interesting why didnt pylint find that. <back>. dug a bit, we are skipping W0612(unused-variable). Not sure why.

>      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():


-- 
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.