> I assumed we want to have the same output regardless of the tool... at > least initially.
Right; I'm just asking the question (to other cloud-init devs as well as you) if it's worth keeping the same format? While I understand there may be tools that scrape the cloud-init log for those values; I'd prefer to save execution time by dumping the iproute2 output values sort of as-is. > > It's the best way of ensuring both code paths are accurate at this time - > especially since new ifconfig syntax and route information is basically > completely broken. For sure; and I think the unittests comparing both make sense. That said, I do think we could save quite a bit of time and code by dropping it. > > The existing netstat and ifconfig used short options which is why I > continued that practice... switching to long is no big deal. Yeah, makes sense; Since we're touching it, it's worth updating for readers. > > On 14 Nov 2017 20:08, "Ryan Harper" <[email protected]> wrote: > > > There's quite a bit of work to make the tables look the same. > > Is that required or can we just dump a different format? > > > > Please make sure the subp command used long options: > > > > netstat --route --numeric --extended > > ip --oneline .. > > > > etc. > > > > > > Diff comments: > > > > > diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py > > > index 993b26c..0b76301 100644 > > > --- a/cloudinit/netinfo.py > > > +++ b/cloudinit/netinfo.py > > > @@ -74,6 +127,20 @@ def netdev_info(empty=""): > > > pass > > > elif toks[i].startswith("%s" % origfield): > > > devs[curdev][target] = toks[i][len(field) + 1:] > > > + return devs > > > + > > > + > > > +def netdev_info(empty=""): > > > + devs = {} > > > + try: > > > + # Try iproute first of all > > > > iproute2 > > > > > + (ipaddr_out, _err) = util.subp(["ip", "-o", "addr", "list"]) > > > + (iplink_out, _err) = util.subp(["ip", "-o", "link", "list"]) > > > + devs = netdev_info_iproute(ipaddr_out, iplink_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(): > > > @@ -84,14 +151,73 @@ def netdev_info(empty=""): > > > return devs > > > > > > > > > -def route_info(): > > > - (route_out, _err) = util.subp(["netstat", "-rn"], rcs=[0, 1]) > > > +def netdev_route_info_iproute(iproute_data): > > > + routes = {} > > > + routes['ipv4'] = [] > > > + routes['ipv6'] = [] > > > + entries = iproute_data.splitlines() > > > + for line in entries: > > > + entry = {} > > > + if not line: > > > + continue > > > + toks = line.split() > > > + if toks[0] == "default": > > > + entry['destination'] = "0.0.0.0" > > > + entry['genmask'] = "0.0.0.0" > > > + entry['flags'] = "UG" > > > + else: > > > + (addr, cidr) = toks[0].split("/") > > > + entry['destination'] = addr > > > + entry['genmask'] = netdev_cidr_to_mask(cidr) > > > + entry['gateway'] = "0.0.0.0" > > > + entry['flags'] = "U" > > > + for i in range(len(toks)): > > > + if toks[i] == "via": > > > + entry['gateway'] = toks[i + 1] > > > + if toks[i] == "dev": > > > + entry["iface"] = toks[i + 1] > > > + if toks[i] == "metric": > > > + entry['metric'] = toks[i + 1] > > > + routes['ipv4'].append(entry) > > > + try: > > > > it does seem odd to subp this here rather then above where you subp other > > ip commands > > > > > + (iproute_data6, _err6) = util.subp(["ip", "-o", "-6", "route", > > "list"], > > > + 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 > > > > > > + > > > +def netdev_route_info_netstat(route_data): > > > routes = {} > > > routes['ipv4'] = [] > > > routes['ipv6'] = [] > > > > > > - entries = route_out.splitlines()[1:] > > > + entries = route_data.splitlines() > > > for line in entries: > > > if not line: > > > continue > > > @@ -125,31 +251,52 @@ def route_info(): > > > routes['ipv4'].append(entry) > > > > > > try: > > > - (route_out6, _err6) = util.subp(["netstat", "-A", "inet6", > > "-n"], > > > - rcs=[0, 1]) > > > + (route_data6, _err6) = util.subp(["netstat", "-A", "inet6", > > "-rn"], > > > > wow, the -r makes a big difference. We've not been showing much for ipv6 > > here... > > > > > + rcs=[0, 1]) > > > except util.ProcessExecutionError: > > > pass > > > else: > > > - entries6 = route_out6.splitlines()[1:] > > > + entries6 = route_data6.splitlines() > > > for line in entries6: > > > if not line: > > > continue > > > toks = line.split() > > > - if (len(toks) < 6 or toks[0] == "Kernel" or > > > + if (len(toks) < 7 or toks[0] == "Kernel" or > > > + toks[0] == "Destination" or toks[0] == "Internet" or > > > toks[0] == "Proto" or toks[0] == "Active"): > > > continue > > > entry = { > > > - 'proto': toks[0], > > > - 'recv-q': toks[1], > > > - 'send-q': toks[2], > > > - 'local address': toks[3], > > > - 'foreign address': toks[4], > > > - 'state': toks[5], > > > + 'destination': toks[0], > > > + 'gateway': toks[1], > > > + 'flags': toks[2], > > > + 'metric': toks[3], > > > + 'ref': toks[4], > > > + 'use': toks[5], > > > + 'iface': toks[6], > > > } > > > + # skip lo interface on ipv6 > > > + if entry['iface'] == "lo": > > > + continue > > > + # strip /128 from address if it's included > > > + if entry['destination'].endswith('/128'): > > > + entry['destination'] = entry['destination'][:-4] > > > routes['ipv6'].append(entry) > > > return routes > > > > > > > > > +def route_info(): > > > + routes = {} > > > + try: > > > + # Try iproute first of all > > > > iproute2 > > > > > + (iproute_out, _err) = util.subp(["ip", "-o", "route", "list"]) > > > + routes = netdev_route_info_iproute(iproute_out) > > > + except util.ProcessExecutionError: > > > + # Fall back to net-tools if iproute2 is not present > > > + (route_out, _err) = util.subp(["netstat", "-rne"], rcs=[0, 1]) > > > + routes = netdev_route_info_netstat(route_out) > > > + return routes > > > + > > > + > > > def getgateway(): > > > try: > > > routes = route_info() > > > > > > -- > > https://code.launchpad.net/~james-hogarth/cloud-init/+git/ > > cloud-init/+merge/333657 > > You are the owner of ~james-hogarth/cloud-init:net-tools-deprecation. > > -- https://code.launchpad.net/~james-hogarth/cloud-init/+git/cloud-init/+merge/333657 Your team cloud-init commiters is requested to review the proposed merge of ~james-hogarth/cloud-init:net-tools-deprecation into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
