cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

> 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" <ryan.harper@xxxxxxxxxxxxx> 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.