Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:3cfbd679f89679d8d3d23cd389d31f75afb5a194 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1025/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: MAAS Compatability Testing IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1025/rebuild -- 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. Cloud-init can now run in an environment that no longer has net-tools. This affects the network and route printing emitted to cloud-config-output.log as well as the cc_disable_ec2_metadata module. Additional changes: - separate readResource and resourceLocation into standalone test functions - Fix ipv4 address rows to report scopes represented by ip addr show - Formatted route/address ouput now handles multiple ipv4 and ipv6 addresses on a single interface This branch was originally authored by James Hogarth and adopts disable_ec2_metadata content from Robert Schweikert. For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:23f0a485a413f606a3e241cb6bf0957a267368f9 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1023/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: MAAS Compatability Testing IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1023/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:c2b81cc529db7f8c70ae030a915c15dd2901b4af https://jenkins.ubuntu.com/server/job/cloud-init-ci/1022/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: MAAS Compatability Testing IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1022/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
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()):
True, but this is a programming error if we pass in the wrong type and the all
call paths of this method should already be exercised by our unit tests on
netdev_pformat(). This is not a failure path that can be introduced easily by
users because we avoid calling this method at all if subp raises a
ProcessExecutionError. Our failure mode if that did happen will be printing
!Net device info failed to cloud-init-output.log, no functional
impact otherwise. We already catch potential errors here, but I think our error
print could contain the specific error so we can triage more easily.
> +m = re.match(r'^\d+:\s(?P[^:]+):\s+<(?P\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\S+)\sscope\s(?P\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\S+)(\sbrd\s(?P\S+))?\sscope\s'
> +r'(?P\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\S+)\s(?P\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():
Same case here if we do 'blow up' the only caller of this function will print
Net device info failed. I think I'll tweak the printed message to contain the
str(e) so we can see the actual error in cloud-init-output.log to catch this
case and the netdev_info_iproute()
> 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+ devs[curdev]['up'] = True
>
> -fieldpost = ""
> -if toks[0] == "inet6":
> -fieldpost = "6"
> -
> for i in ra
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. Cloud-init can now run in an environment that no longer has net-tools. This affects the network and route printing emitted to cloud-config-output.log as well as the cc_disable_ec2_metadata module. Additional changes: - separate readResource and resourceLocation into standalone test functions - Adds a Metric column to the route information output table - Fix ipv4 address rows to report scopes represented by ip addr show - Formatted route/address ouput now handles multiple ipv4 and ipv6 addresses on a single interface This branch was originally authored by James Hogarth and adopts disable_ec2_metadata content from Robert Schweikert. For more details, see: 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. Cloud-init can now run in an environment that no longer has net-tools. This affects the network and route printing emitted to cloud-config-output.log as well as the cc_disable_ec2_metadata module. Additional changes: - separate readResource and resourceLocation into standalone test functions - Adds a Metric column to the route information output table - Formatted route/address ouput now handles multiple ipv4 and ipv6 addresses on a single interface This branch was originally authored by James Hogarth and adopts disable_ec2_metadata content from Robert Schweikert. For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
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[^:]+):\s+<(?P\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\S+)\sscope\s(?P\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\S+)(\sbrd\s(?P\S+))?\sscope\s'
> +r'(?P\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\S+)\s(?P\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+ 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",
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
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()):
if ipaddr_out is not a string, this blows up.
> +m = re.match(r'^\d+:\s(?P[^:]+):\s+<(?P\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\S+)\sscope\s(?P\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\S+)(\sbrd\s(?P\S+))?\sscope\s'
> +r'(?P\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\S+)\s(?P\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():
Same here, if ifconfig_data is not a string, this blows up.
> 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
> @@ -84,14 +163,94 @@ def netdev_info(empty=""):
Not sure if it's worth a message to say we found neither iproute nor net-tools
commands, so we cannot report network info.
> return devs
>
>
> -def route_info():
> -(route_out, _err) = util.subp(["netstat", "-rn"], rcs=[0, 1])
> +def netdev_route_info_iproute(iproute_data):
> +"""
> +Get network route dicts from ip route info.
> +
> +@param iproute_data: Output string from ip route command.
> +
> +@returns: A dict containing ipv4 and ipv6 route entries as lists. Each
> + item in the list is a route dictionary representing
> destination,
> + gateway, flags, genmask and interface information.
> +"""
>
> routes = {}
> routes['ipv4'] = []
> routes['ipv6'] = []
> +entries = iproute_data.splitlines()
This blows up on non-string
> +default_route_entry = {
> +'destination': '', 'flags': '', 'gateway': '', 'genmask': '',
> +'iface': '', 'metric': ''}
> +for line i
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:ce6206876661f3921a709c294296309051987de2 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1018/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: MAAS Compatability Testing IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1018/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:b3b37145cdd44d11e03be6b6bff724f09034e9a8 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1014/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build FAILED: Ubuntu LTS: Integration Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1014/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:079cdd6ccda1ed8d1780a334248410e4ae998e34 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1013/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1013/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
All comments addressed. Please glance over the changes if you get a chance.
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", (0x << (32 - int(cidr)) & 0x)))
> +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 = {}
We won't blowup on empty data, we return devs = {} in that case, which feels
appropriate if no routes are listed in output.
> -for line in str(ifcfg_out).splitlines():
> +for line in str(ipaddr_data).splitlines():
Yeah, I should've looked more at some of this code when I took over the branch.
The util.subp function should take care of this data decoding for us if
necessary, I'd expect it to be handling in a common layer instead of making all
call-sites type-check and decode. This str might have been inherited from when
subp didn't return the right string type.
> +details = line.lower().strip().split()
> +curdev = details[1]
> +fieldpost = ""
Good though, changed to ip_version_suffix and added a comment about its use.
> +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("/")
I used partition('/') and set the cidr to '24' if not present. We can look at
consolidating in a separate branch.
> +devs[curdev]["mask"] = netdev_cidr_to_mask(cidr)
Moved it. good thought. I missed the duplication.
> +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]
Took both of your comments. I simplified netdev_info_iproute to parse all of
'ip addr show' in one go. I also tweaked the logic to use regex matches instead
of walking through individual tokens. It's only about the same performance as
far timeit is concerned, bit the regex gives us a better view of the expected
format of the output.
> +
> +for line in str(iplink_data).splitlines():
dropped the str throughout.
> +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():
dropped the str throughout.
> 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
> @@ -39,41 +105,50 @@ def netdev_info(empty=""):
> if re.search(r"flags=\d+ devs[curdev]['up'] = True
>
> -fieldpost = ""
> -if toks[0] == "inet6":
> -
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:a33a68fb2bd5b55dc49a2729a7c476ee14d6e657 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1012/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1012/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:4dc71d8fa6daa772888abec8d931808b87c2cd78 https://jenkins.ubuntu.com/server/job/cloud-init-ci/1011/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1011/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:9ab9d255cae75a26ad314e7697e359ba93a8cdff https://jenkins.ubuntu.com/server/job/cloud-init-ci/1002/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/1002/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:02a1b886e4a1a424b80c472a2dfa33dc730e3316 https://jenkins.ubuntu.com/server/job/cloud-init-ci/993/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/993/rebuild -- 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. Cloud-init can now run in an environment that no longer has net-tools. Additional changes: - separate readResource and resourceLocation into standalone test functions - For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
> The commit is missing cc_disable_ec2_metadata.py which uses the "route" > command from net-tools command > > Too bad this had to be started over rather than beating the since last year > pending request into shape. Sorry Robert, shameful review queue mismanagement on my part. I was fixated on the branch put up by james hogarth which sat idle for so long without handling my review comments and I didn't see yours doing exactly the same thing. I'll incorporate your fixes and suggestions and you and James should get authorship of this. Thanks again for the review and pointers. -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The commit is missing cc_disable_ec2_metadata.py which uses the "route" command
from net-tools command
Too bad this had to be started over rather than beating the since last year
pending request into shape.
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", (0x << (32 - int(cidr)) & 0x)))
> +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 = {}
> -for line in str(ifcfg_out).splitlines():
> +for line in str(ipaddr_data).splitlines():
in Python 3 the data passed in the call generated with (ipaddr_out, _err) =
util.subp(["ip", "--oneline", "addr", "list"]) is, as in Python 2, of type
string. I agree the explicit conversion is confusing. If bytes should be
handled as a "just in case" implementation then it should be type tested and
decode() should be used.
> +details = line.lower().strip().split()
> +curdev = details[1]
> +fieldpost = ""
I understand the thinking for the name, as in "entry field post fix" but it
took me a while, I think protocol_version, or ip_version would be more explicit
and more obvious. Of course for IPv4 the version string is "" and that may
throw others for a loop.
> +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("/")
> +devs[curdev]["mask"] = netdev_cidr_to_mask(cidr)
> +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]
Loops over every item although we know that more than 1/2 the entries in the
list are of no interest. The data, for example:
['2:', 'eth0', 'inet6', 'fe80::c5d:2fff:fe4e:6a54/64', 'scope', 'link', '\\',
'valid_lft', 'forever', 'preferred_lft', 'forever']
so the loop hits "2:", the "eth0" eventually it hits the ip-address etc. We
know ahead of time that we don't care about those entries. At the very least
the loop should start after the device, i.e. the loop should start at index 2.
> +
> +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():
And another str()
> 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
> @@ -39,41 +105,50 @@ def netdev_info(empty=""):
> if re.search(r"flags=\d+
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
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", (0x << (32 - int(cidr)) & 0x)))
> +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 = {}
> -for line in str(ifcfg_out).splitlines():
> +for line in str(ipaddr_data).splitlines():
why is str() here ?
it would seem thats likely an attempt to support bytes i guess? but if so its
wrong.
we should either expect bytes or string and not both. and certainly not turn
bytes into "b''" and pretend it was a string all along.
> +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("/")
> +devs[curdev]["mask"] = netdev_cidr_to_mask(cidr)
> +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]
> +
> +for line in str(iplink_data).splitlines():
same weird looking use of str() here.
> +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
--
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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
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", (0x << (32 - int(cidr)) & 0x)))
> +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\S+)\sdev\s' +
r'(?P\S+)\s+' +
r'proto\s(?P\S+)\s+' +
r'scope\s(?P\S+)\s+' +
r'src\s(?P\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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. Cloud-init can now run in an environment that no longer has net-tools. For more details, see: 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master has been updated. Commit message changed to: net: Depend on iproute2's ip instead of net-tools ifconfig or route The net-tools package is deprecated and will eventually be dropped. Use "ip route", "link" or "address" instead of "ifconfig" or "route" calls. So that we run in an environment that no longer has net-tools. For more details, see: 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
some other patches that i think make this a bit nicer. http://paste.ubuntu.com/p/4Px2hrqtG6/ http://paste.ubuntu.com/p/GtrvNcYhzc/ -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
netdev_cidr_to_mask is like net_prefix_to_ipv4_mask, right? and then.. the use of 'cidr' there is at least inconsistent with other places in cloud-init. generally I think 'cidr' is / where this fucntion just takes the 'prefix' part. -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
"where possible" What does that mean? -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Validated ipv6 route information shown using this branch https://pastebin.ubuntu.com/p/PNFXM8SCCY/ -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:bb7af534033d983d741a8cadb6d1c0db498b0f9c https://jenkins.ubuntu.com/server/job/cloud-init-ci/956/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: MAAS Compatability Testing IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/956/rebuild -- 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:d7c8cce162fd7ad30100a5fd603965ea8480414d https://jenkins.ubuntu.com/server/job/cloud-init-ci/955/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/955/rebuild -- 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into cloud-init:master
Chad Smith has proposed merging
~chad.smith/cloud-init:net-tools-deprecation-plus-review-comments into
cloud-init:master.
Commit message:
net: Depend on iproute2's ip instead of net-tools ifconfig or route
The net-tools package is deprecated and will eventually be dropped.
Where possible, use "ip route", "link" or "address" instead of "ifconfig"
or "route" calls. So that we run in an environment that no longer has
net-tools.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/342428
see commit message
--
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.
diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py
index 993b26c..f67657c 100644
--- a/cloudinit/netinfo.py
+++ b/cloudinit/netinfo.py
@@ -8,7 +8,10 @@
#
# This file is part of cloud-init. See LICENSE file for license information.
+from copy import copy
import re
+import socket
+import struct
from cloudinit import log as logging
from cloudinit import util
@@ -18,18 +21,73 @@ 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", (0x << (32 - int(cidr)) & 0x)))
+return mask
+
+
+def netdev_info_iproute(ipaddr_data, iplink_data):
devs = {}
-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)
+if details[2] == "inet6":
+fieldpost = "6"
+for i in range(len(details)):
+if details[i] == 'inet':
+(addr, cidr) = details[i + 1].split("/")
+devs[curdev]["mask"] = netdev_cidr_to_mask(cidr)
+devs[curdev]["addr"] = addr
+elif details[i] == 'inet6':
+addr = details[i + 1]
+devs[curdev]["addr6"] = addr
+elif details[i] == "scope":
+devs[curdev]["scope" + fieldpost] = details[i + 1]
+elif details[i] == "brd":
+devs[curdev]["bcast" + fieldpost] = details[i + 1]
+
+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
+if 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
+fields = ("hwaddr", "up", "addr", "bcast", "mask", "scope")
+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
@@ -39,41 +97,50 @@ def netdev_info(empty=""):
if re.search(r"flags=\d+", toks[i + 1])
+if res:
+devs[curdev]['scope6'] = res.group(1)
+return devs
+
+
+def netdev_info(empty=""):
+devs = {}
+try:
+# Try iproute first of all
+(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,83 @@ def netdev_info(empty=""):
return devs
-def route_info():
-
