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