← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~james-hogarth/cloud-init:net-tools-deprecation into cloud-init:master

 

I assumed we want to have the same output regardless of the tool... at
least initially.

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.

The existing netstat and ifconfig used short options which is why I
continued that practice... switching to long is no big deal.

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.


Follow ups

References