cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py
> index 993b26c..081f7b4 100644
> --- a/cloudinit/netinfo.py
> +++ b/cloudinit/netinfo.py
> @@ -18,18 +21,81 @@ 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 = {
> +    "addr": "",
> +    "bcast": "",
> +    "hwaddr": "",
> +    "mask": "",
> +    "scope": "",
> +    "up": False
> +}
> +
> +
> +def netdev_cidr_to_mask(cidr):
> +    mask = socket.inet_ntoa(
> +        struct.pack(">I", (0xffffffff << (32 - int(cidr)) & 0xffffffff)))
> +    return mask
> +
> +
> +def netdev_info_iproute(ipaddr_data, iplink_data):
> +    """
> +    Get network device dicts from ip route and ip link info.
> +
> +    @param ipaddr_data: Output string from ip address command.
> +    @param iplink_data: Output string from ip link command.
> +
> +    @returns: A dict of device info keyed by network device name containing
> +              device configuration values.
> +    """
>      devs = {}

if not ipaddr_data:
   ipaddr_data = ""

if not iplink_data:
   iplink_data = ""

So it won't blow up on empty data?

> -    for line in str(ifcfg_out).splitlines():
> +    for line in str(ipaddr_data).splitlines():
> +        details = line.lower().strip().split()
> +        curdev = details[1]
> +        fieldpost = ""
> +        if curdev not in devs:
> +            devs[curdev] = copy(DEFAULT_NETDEV_INFO)
> +        for i in range(len(details)):
> +            if details[i] == 'inet':
> +                (addr, cidr) = details[i + 1].split("/")

Do we know if always get a inet entry with /prefix_len ?
Look at cloudinit/net/network_state.py:_normalize_net_keys()

which has some logic related to pulling out mask from cidr if present.

> +                devs[curdev]["mask"] = netdev_cidr_to_mask(cidr)

we could look to replace network_state.net_prefix_to_ipv4_mask with your
implementation, timeit says it's much faster.

> +                devs[curdev]["addr"] = addr
> +                fieldpost = ""
> +            elif details[i] == 'inet6':
> +                addr = details[i + 1]
> +                devs[curdev]["addr6"] = addr
> +                fieldpost = "6"
> +            elif details[i] == "scope":
> +                devs[curdev]["scope" + fieldpost] = details[i + 1]
> +            elif details[i] == "brd":
> +                devs[curdev]["bcast" + fieldpost] = details[i + 1]

Maybe use re.search with regex?

I've this one used in curtin parsing output from ip route show

m = re.search(r'^(?P<network>\S+)\sdev\s' +
              r'(?P<devname>\S+)\s+' +
              r'proto\s(?P<proto>\S+)\s+' +
              r'scope\s(?P<scope>\S+)\s+' +
              r'src\s(?P<src_ip>\S+)',
              line)

> +
> +    for line in str(iplink_data).splitlines():
> +        details = line.lower().strip().split()
> +        # Strip trailing ':' and truncate any interface alias '@if34'
> +        curdev = details[1].rstrip(':').split('@')[0]
> +        for i in range(len(details)):
> +            if details[i] == curdev + ":":
> +                flags = details[i + 1].strip("<>").split(",")
> +                if "lower_up" in flags and "up" in flags:
> +                    devs[curdev]["up"] = True
> +            elif details[i] == "link/ether":
> +                devs[curdev]["hwaddr"] = details[i + 1]
> +    return devs
> +
> +
> +def netdev_info_ifconfig(ifconfig_data):
> +    # fields that need to be returned in devs for each dev
> +    devs = {}
> +    for line in str(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] = copy(DEFAULT_NETDEV_INFO)
>          toks = line.lower().strip().split()
>          if toks[0] == "up":
>              devs[curdev]['up'] = True
> @@ -125,31 +279,53 @@ 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", "--route", "--numeric"], 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]

I think you can just do .replace("/128", "");  no address is going to have a /128 in it other than at the end.

>              routes['ipv6'].append(entry)
>      return routes
>  
>  
> +def route_info():
> +    routes = {}
> +    try:
> +        # Try iproute first of all
> +        (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", "--route", "--numeric", "--extend"], rcs=[0, 1])
> +        routes = netdev_route_info_netstat(route_out)
> +    return routes
> +
> +
>  def getgateway():
>      try:
>          routes = route_info()


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