From 003c6678e9c873b3b787a814016872b6592f5069 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 25 May 2017 15:37:15 -0500 Subject: net: remove systemd link file writing from eni renderer During the network v2 merge, we inadvertently re-enabled rendering systemd .link files. This files are not required as cloud-init already has to do interface renaming due to issues with udevd which may refuse to rename certain interfaces (such as veth devices in a LXD container). As such, removing the code altogether. --- cloudinit/net/eni.py | 25 ------------------------- 1 file changed, 25 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 9819d4f5..0cc4e3b2 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -304,8 +304,6 @@ class Renderer(renderer.Renderer): config = {} self.eni_path = config.get('eni_path', 'etc/network/interfaces') self.eni_header = config.get('eni_header', None) - self.links_path_prefix = config.get( - 'links_path_prefix', 'etc/systemd/network/50-cloud-init-') self.netrules_path = config.get( 'netrules_path', 'etc/udev/rules.d/70-persistent-net.rules') @@ -451,28 +449,6 @@ class Renderer(renderer.Renderer): util.write_file(netrules, self._render_persistent_net(network_state)) - if self.links_path_prefix: - self._render_systemd_links(target, network_state, - links_prefix=self.links_path_prefix) - - def _render_systemd_links(self, target, network_state, links_prefix): - fp_prefix = util.target_path(target, links_prefix) - for f in glob.glob(fp_prefix + "*"): - os.unlink(f) - for iface in network_state.iter_interfaces(): - if (iface['type'] == 'physical' and 'name' in iface and - iface.get('mac_address')): - fname = fp_prefix + iface['name'] + ".link" - content = "\n".join([ - "[Match]", - "MACAddress=" + iface['mac_address'], - "", - "[Link]", - "Name=" + iface['name'], - "" - ]) - util.write_file(fname, content) - def network_state_to_eni(network_state, header=None, render_hwaddress=False): # render the provided network state, return a string of equivalent eni @@ -480,7 +456,6 @@ def network_state_to_eni(network_state, header=None, render_hwaddress=False): renderer = Renderer(config={ 'eni_path': eni_path, 'eni_header': header, - 'links_path_prefix': None, 'netrules_path': None, }) if not header: -- cgit v1.2.3 From 00b678c61a54f176625d3f937971215faf6af2cd Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Fri, 26 May 2017 14:29:24 -0500 Subject: Fix eni rendering for bridge params that require repeated key for values. There are a few bridge parameters which require repeating the key with each value in the list when rendering eni. Extend the network unittests to cover all of the known bridge parameters and check we render eni and netplan correctly. --- cloudinit/net/eni.py | 13 +++++++++++++ tests/unittests/test_net.py | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 0cc4e3b2..20e19f5b 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -75,6 +75,15 @@ def _iface_add_attrs(iface, index): 'subnets', 'type', ] + + # The following parameters require repetitive entries of the key for + # each of the values + multiline_keys = [ + 'bridge_pathcost', + 'bridge_portprio', + 'bridge_waitport', + ] + renames = {'mac_address': 'hwaddress'} if iface['type'] not in ['bond', 'bridge', 'vlan']: ignore_map.append('mac_address') @@ -82,6 +91,10 @@ def _iface_add_attrs(iface, index): for key, value in iface.items(): if not value or key in ignore_map: continue + if key in multiline_keys: + for v in value: + content.append(" {0} {1}".format(renames.get(key, key), v)) + continue if type(value) == list: value = " ".join(value) content.append(" {0} {1}".format(renames.get(key, key), value)) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 5a0fcbcf..5f7e902a 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -511,8 +511,20 @@ iface bond0 inet6 dhcp auto br0 iface br0 inet static address 192.168.14.2/24 + bridge_ageing 250 + bridge_bridgeprio 22 + bridge_fd 1 + bridge_gcint 2 + bridge_hello 1 + bridge_maxage 10 + bridge_pathcost eth3 50 + bridge_pathcost eth4 75 + bridge_portprio eth3 28 + bridge_portprio eth4 14 bridge_ports eth3 eth4 bridge_stp off + bridge_waitport 1 eth3 + bridge_waitport 2 eth4 # control-alias br0 iface br0 inet6 static @@ -642,6 +654,15 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true interfaces: - eth3 - eth4 + parameters: + ageing-time: 250 + forward-delay: 1 + hello-time: 1 + max-age: 10 + path-cost: + eth3: 50 + eth4: 75 + priority: 22 vlans: bond0.200: dhcp4: true @@ -752,9 +773,23 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true forwarding: 1 # basically anything in /proc/sys/net/ipv6/conf/.../ params: - bridge_stp: 'off' - bridge_fd: 0 + bridge_ageing: 250 + bridge_bridgeprio: 22 + bridge_fd: 1 + bridge_gcint: 2 + bridge_hello: 1 + bridge_maxage: 10 bridge_maxwait: 0 + bridge_pathcost: + - eth3 50 + - eth4 75 + bridge_portprio: + - eth3 28 + - eth4 14 + bridge_stp: 'off' + bridge_waitport: + - 1 eth3 + - 2 eth4 subnets: - type: static address: 192.168.14.2/24 -- cgit v1.2.3 From 543e25cda6235d18adb6485e4266944d59e1979d Mon Sep 17 00:00:00 2001 From: Marc-Aurèle Brothier Date: Wed, 24 May 2017 14:45:25 +0200 Subject: net: when selecting a network device, use natural sort order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code deciding which interface to choose as the default to request the IP address through DHCP does not sort the interfaces correctly. On Ubuntu Xenial images for example, the interfaces are named ens1, ens2, ens3..., ens11, ... depending on the pci bus address. The python sorting will list 'ens11' before 'ens3' for example despite the fact that 'ens3' should be before 'ens11'. This patch address this issue and sort the interface names according to a human sorting. Signed-off-by: Marc-Aurèle Brothier --- cloudinit/net/__init__.py | 13 ++++++++++++- tests/unittests/test_net.py | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 8c6cd057..65accbb0 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -17,6 +17,17 @@ SYS_CLASS_NET = "/sys/class/net/" DEFAULT_PRIMARY_INTERFACE = 'eth0' +def _natural_sort_key(s, _nsre=re.compile('([0-9]+)')): + """Sorting for Humans: natural sort order. Can be use as the key to sort + functions. + This will sort ['eth0', 'ens3', 'ens10', 'ens12', 'ens8', 'ens0'] as + ['ens0', 'ens3', 'ens8', 'ens10', 'ens12', 'eth0'] instead of the simple + python way which will produce ['ens0', 'ens10', 'ens12', 'ens3', 'ens8', + 'eth0'].""" + return [int(text) if text.isdigit() else text.lower() + for text in re.split(_nsre, s)] + + def sys_dev_path(devname, path=""): return SYS_CLASS_NET + devname + "/" + path @@ -169,7 +180,7 @@ def generate_fallback_config(): # if eth0 exists use it above anything else, otherwise get the interface # that we can read 'first' (using the sorted defintion of first). - names = list(sorted(potential_interfaces)) + names = list(sorted(potential_interfaces, key=_natural_sort_key)) if DEFAULT_PRIMARY_INTERFACE in names: names.remove(DEFAULT_PRIMARY_INTERFACE) names.insert(0, DEFAULT_PRIMARY_INTERFACE) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 5f7e902a..0000b075 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -1,6 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit import net +from cloudinit.net import _natural_sort_key from cloudinit.net import cmdline from cloudinit.net import eni from cloudinit.net import netplan @@ -1659,6 +1660,19 @@ class TestGetInterfacesByMac(CiTestCase): self.assertEqual('lo', ret[empty_mac]) +class TestInterfacesSorting(CiTestCase): + + def test_natural_order(self): + data = ['ens5', 'ens6', 'ens3', 'ens20', 'ens13', 'ens2'] + self.assertEqual( + sorted(data, key=_natural_sort_key), + ['ens2', 'ens3', 'ens5', 'ens6', 'ens13', 'ens20']) + data2 = ['enp2s0', 'enp2s3', 'enp0s3', 'enp0s13', 'enp0s8', 'enp1s2'] + self.assertEqual( + sorted(data2, key=_natural_sort_key), + ['enp0s3', 'enp0s8', 'enp0s13', 'enp1s2', 'enp2s0', 'enp2s3']) + + def _gzip_data(data): with io.BytesIO() as iobuf: gzfp = gzip.GzipFile(mode="wb", fileobj=iobuf) -- cgit v1.2.3 From f99745cf916e707eaa1ded6f12e8b69837b7242d Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Tue, 6 Jun 2017 12:55:50 -0400 Subject: RHEL/CentOS: Fix default routes for IPv4/IPv6 configuration. Since f38fa413176, default routes get added to both ifcfg-* and route-* and route6-* files. Default routes should only go to ifcfg-* files, otherwise the information is redundant. LP: #1696176 --- cloudinit/net/sysconfig.py | 12 +++++++----- tests/unittests/test_net.py | 8 -------- 2 files changed, 7 insertions(+), 13 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 58c5713f..f7d45482 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -372,11 +372,13 @@ class Renderer(renderer.Renderer): nm_key = 'NETMASK%s' % route_cfg.last_idx addr_key = 'ADDRESS%s' % route_cfg.last_idx route_cfg.last_idx += 1 - for (old_key, new_key) in [('gateway', gw_key), - ('netmask', nm_key), - ('network', addr_key)]: - if old_key in route: - route_cfg[new_key] = route[old_key] + # add default routes only to ifcfg files, not + # to route-* or route6-* + for (old_key, new_key) in [('gateway', gw_key), + ('netmask', nm_key), + ('network', addr_key)]: + if old_key in route: + route_cfg[new_key] = route[old_key] @classmethod def _render_bonding_opts(cls, iface_cfg, iface): diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 0000b075..0a88caf1 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -149,14 +149,6 @@ NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet USERCTL=no -""".lstrip()), - ('etc/sysconfig/network-scripts/route-eth0', - """ -# Created by cloud-init on instance boot automatically, do not edit. -# -ADDRESS0=0.0.0.0 -GATEWAY0=172.19.3.254 -NETMASK0=0.0.0.0 """.lstrip()), ('etc/resolv.conf', """ -- cgit v1.2.3 From d00da2d5b0d45db5670622a66d833d2abb907388 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 24 May 2017 21:10:50 -0400 Subject: net: normalize data in network_state object The network_state object's network and route keys would have different information depending upon how the network_state object was populated. This change cleans that up. Now: * address will always contain an IP address. * prefix will always include an integer value that is the network_prefix for the address. * netmask will be present only if the address is ipv4, and its value will always correlate to the 'prefix'. --- cloudinit/net/eni.py | 4 + cloudinit/net/netplan.py | 14 +- cloudinit/net/network_state.py | 244 ++++++++++++++++++++----- cloudinit/net/sysconfig.py | 23 +-- tests/unittests/test_distros/test_netconfig.py | 5 +- tests/unittests/test_net.py | 6 +- 6 files changed, 215 insertions(+), 81 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 20e19f5b..98ce01e4 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -46,6 +46,10 @@ def _iface_add_subnet(iface, subnet): 'dns_nameservers', ] for key, value in subnet.items(): + if key == 'netmask': + continue + if key == 'address': + value = "%s/%s" % (subnet['address'], subnet['prefix']) if value and key in valid_map: if type(value) == list: value = " ".join(value) diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index a715f3b0..67543305 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -4,7 +4,7 @@ import copy import os from . import renderer -from .network_state import mask2cidr, subnet_is_ipv6 +from .network_state import subnet_is_ipv6 from cloudinit import log as logging from cloudinit import util @@ -118,10 +118,9 @@ def _extract_addresses(config, entry): sn_type += '4' entry.update({sn_type: True}) elif sn_type in ['static']: - addr = '%s' % subnet.get('address') - netmask = subnet.get('netmask') - if netmask and '/' not in addr: - addr += '/%s' % mask2cidr(netmask) + addr = "%s" % subnet.get('address') + if 'prefix' in subnet: + addr += "/%d" % subnet.get('prefix') if 'gateway' in subnet and subnet.get('gateway'): gateway = subnet.get('gateway') if ":" in gateway: @@ -138,9 +137,8 @@ def _extract_addresses(config, entry): mtukey += '6' entry.update({mtukey: subnet.get('mtu')}) for route in subnet.get('routes', []): - network = route.get('network') - netmask = route.get('netmask') - to_net = '%s/%s' % (network, mask2cidr(netmask)) + to_net = "%s/%s" % (route.get('network'), + route.get('prefix')) route = { 'via': route.get('gateway'), 'to': to_net, diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 9e9c05a0..87a7222d 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -289,19 +289,15 @@ class NetworkStateInterpreter(object): iface.update({param: val}) # convert subnet ipv6 netmask to cidr as needed - subnets = command.get('subnets') - if subnets: + subnets = _normalize_subnets(command.get('subnets')) + + # automatically set 'use_ipv6' if any addresses are ipv6 + if not self.use_ipv6: for subnet in subnets: - if subnet['type'] == 'static': - if ':' in subnet['address']: - self.use_ipv6 = True - if 'netmask' in subnet and ':' in subnet['address']: - subnet['netmask'] = mask2cidr(subnet['netmask']) - for route in subnet.get('routes', []): - if 'netmask' in route: - route['netmask'] = mask2cidr(route['netmask']) - elif subnet['type'].endswith('6'): + if (subnet.get('type').endswith('6') or + is_ipv6_addr(subnet.get('address'))): self.use_ipv6 = True + break iface.update({ 'name': command.get('name'), @@ -456,16 +452,7 @@ class NetworkStateInterpreter(object): @ensure_command_keys(['destination']) def handle_route(self, command): - routes = self._network_state.get('routes', []) - network, cidr = command['destination'].split("/") - netmask = cidr2mask(int(cidr)) - route = { - 'network': network, - 'netmask': netmask, - 'gateway': command.get('gateway'), - 'metric': command.get('metric'), - } - routes.append(route) + self._network_state['routes'].append(_normalize_route(command)) # V2 handlers def handle_bonds(self, command): @@ -666,18 +653,9 @@ class NetworkStateInterpreter(object): routes = [] for route in cfg.get('routes', []): - route_addr = route.get('to') - if "/" in route_addr: - route_addr, route_cidr = route_addr.split("/") - route_netmask = cidr2mask(route_cidr) - subnet_route = { - 'address': route_addr, - 'netmask': route_netmask, - 'gateway': route.get('via') - } - routes.append(subnet_route) - if len(routes) > 0: - subnet.update({'routes': routes}) + routes.append(_normalize_route( + {'address': route.get('to'), 'gateway': route.get('via')})) + subnet['routes'] = routes if ":" in address: if 'gateway6' in cfg and gateway6 is None: @@ -692,53 +670,219 @@ class NetworkStateInterpreter(object): return subnets +def _normalize_subnet(subnet): + # Prune all keys with None values. + subnet = copy.deepcopy(subnet) + normal_subnet = dict((k, v) for k, v in subnet.items() if v) + + if subnet.get('type') in ('static', 'static6'): + normal_subnet.update( + _normalize_net_keys(normal_subnet, address_keys=('address',))) + normal_subnet['routes'] = [_normalize_route(r) + for r in subnet.get('routes', [])] + return normal_subnet + + +def _normalize_net_keys(network, address_keys=()): + """Normalize dictionary network keys returning prefix and address keys. + + @param network: A dict of network-related definition containing prefix, + netmask and address_keys. + @param address_keys: A tuple of keys to search for representing the address + or cidr. The first address_key discovered will be used for + normalization. + + @returns: A dict containing normalized prefix and matching addr_key. + """ + net = dict((k, v) for k, v in network.items() if v) + addr_key = None + for key in address_keys: + if net.get(key): + addr_key = key + break + if not addr_key: + message = ( + 'No config network address keys [%s] found in %s' % + (','.join(address_keys), network)) + LOG.error(message) + raise ValueError(message) + + addr = net.get(addr_key) + ipv6 = is_ipv6_addr(addr) + netmask = net.get('netmask') + if "/" in addr: + addr_part, _, maybe_prefix = addr.partition("/") + net[addr_key] = addr_part + try: + prefix = int(maybe_prefix) + except ValueError: + # this supports input of
/255.255.255.0 + prefix = mask_to_net_prefix(maybe_prefix) + elif netmask: + prefix = mask_to_net_prefix(netmask) + elif 'prefix' in net: + prefix = int(prefix) + else: + prefix = 64 if ipv6 else 24 + + if 'prefix' in net and str(net['prefix']) != str(prefix): + LOG.warning("Overwriting existing 'prefix' with '%s' in " + "network info: %s", prefix, net) + net['prefix'] = prefix + + if ipv6: + # TODO: we could/maybe should add this back with the very uncommon + # 'netmask' for ipv6. We need a 'net_prefix_to_ipv6_mask' for that. + if 'netmask' in net: + del net['netmask'] + else: + net['netmask'] = net_prefix_to_ipv4_mask(net['prefix']) + + return net + + +def _normalize_route(route): + """normalize a route. + return a dictionary with only: + 'type': 'route' (only present if it was present in input) + 'network': the network portion of the route as a string. + 'prefix': the network prefix for address as an integer. + 'metric': integer metric (only if present in input). + 'netmask': netmask (string) equivalent to prefix iff network is ipv4. + """ + # Prune None-value keys. Specifically allow 0 (a valid metric). + normal_route = dict((k, v) for k, v in route.items() + if v not in ("", None)) + if 'destination' in normal_route: + normal_route['network'] = normal_route['destination'] + del normal_route['destination'] + + normal_route.update( + _normalize_net_keys( + normal_route, address_keys=('network', 'destination'))) + + metric = normal_route.get('metric') + if metric: + try: + normal_route['metric'] = int(metric) + except ValueError: + raise TypeError( + 'Route config metric {} is not an integer'.format(metric)) + return normal_route + + +def _normalize_subnets(subnets): + if not subnets: + subnets = [] + return [_normalize_subnet(s) for s in subnets] + + +def is_ipv6_addr(address): + if not address: + return False + return ":" in str(address) + + def subnet_is_ipv6(subnet): """Common helper for checking network_state subnets for ipv6.""" # 'static6' or 'dhcp6' if subnet['type'].endswith('6'): # This is a request for DHCPv6. return True - elif subnet['type'] == 'static' and ":" in subnet['address']: + elif subnet['type'] == 'static' and is_ipv6_addr(subnet.get('address')): return True return False -def cidr2mask(cidr): +def net_prefix_to_ipv4_mask(prefix): + """Convert a network prefix to an ipv4 netmask. + + This is the inverse of ipv4_mask_to_net_prefix. + 24 -> "255.255.255.0" + Also supports input as a string.""" + mask = [0, 0, 0, 0] - for i in list(range(0, cidr)): + for i in list(range(0, int(prefix))): idx = int(i / 8) mask[idx] = mask[idx] + (1 << (7 - i % 8)) return ".".join([str(x) for x in mask]) -def ipv4mask2cidr(mask): - if '.' not in mask: +def ipv4_mask_to_net_prefix(mask): + """Convert an ipv4 netmask into a network prefix length. + + If the input is already an integer or a string representation of + an integer, then int(mask) will be returned. + "255.255.255.0" => 24 + str(24) => 24 + "24" => 24 + """ + if isinstance(mask, int): return mask - return sum([bin(int(x)).count('1') for x in mask.split('.')]) + if isinstance(mask, six.string_types): + try: + return int(mask) + except ValueError: + pass + else: + raise TypeError("mask '%s' is not a string or int") + if '.' not in mask: + raise ValueError("netmask '%s' does not contain a '.'" % mask) -def ipv6mask2cidr(mask): - if ':' not in mask: + toks = mask.split(".") + if len(toks) != 4: + raise ValueError("netmask '%s' had only %d parts" % (mask, len(toks))) + + return sum([bin(int(x)).count('1') for x in toks]) + + +def ipv6_mask_to_net_prefix(mask): + """Convert an ipv6 netmask (very uncommon) or prefix (64) to prefix. + + If 'mask' is an integer or string representation of one then + int(mask) will be returned. + """ + + if isinstance(mask, int): return mask + if isinstance(mask, six.string_types): + try: + return int(mask) + except ValueError: + pass + else: + raise TypeError("mask '%s' is not a string or int") + + if ':' not in mask: + raise ValueError("mask '%s' does not have a ':'") bitCount = [0, 0x8000, 0xc000, 0xe000, 0xf000, 0xf800, 0xfc00, 0xfe00, 0xff00, 0xff80, 0xffc0, 0xffe0, 0xfff0, 0xfff8, 0xfffc, 0xfffe, 0xffff] - cidr = 0 + prefix = 0 for word in mask.split(':'): if not word or int(word, 16) == 0: break - cidr += bitCount.index(int(word, 16)) + prefix += bitCount.index(int(word, 16)) + + return prefix - return cidr +def mask_to_net_prefix(mask): + """Return the network prefix for the netmask provided. -def mask2cidr(mask): - if ':' in str(mask): - return ipv6mask2cidr(mask) - elif '.' in str(mask): - return ipv4mask2cidr(mask) + Supports ipv4 or ipv6 netmasks.""" + try: + # if 'mask' is a prefix that is an integer. + # then just return it. + return int(mask) + except ValueError: + pass + if is_ipv6_addr(mask): + return ipv6_mask_to_net_prefix(mask) else: - return mask + return ipv4_mask_to_net_prefix(mask) + # vi: ts=4 expandtab diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index f7d45482..5d9b3d10 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -9,7 +9,7 @@ from cloudinit.distros.parsers import resolv_conf from cloudinit import util from . import renderer -from .network_state import subnet_is_ipv6 +from .network_state import subnet_is_ipv6, net_prefix_to_ipv4_mask def _make_header(sep='#'): @@ -26,11 +26,8 @@ def _make_header(sep='#'): def _is_default_route(route): - if route['network'] == '::' and route['netmask'] == 0: - return True - if route['network'] == '0.0.0.0' and route['netmask'] == '0.0.0.0': - return True - return False + default_nets = ('::', '0.0.0.0') + return route['prefix'] == 0 and route['network'] in default_nets def _quote_value(value): @@ -323,16 +320,10 @@ class Renderer(renderer.Renderer): " " + ipv6_cidr) else: ipv4_index = ipv4_index + 1 - if ipv4_index == 0: - iface_cfg['IPADDR'] = subnet['address'] - if 'netmask' in subnet: - iface_cfg['NETMASK'] = subnet['netmask'] - else: - iface_cfg['IPADDR' + str(ipv4_index)] = \ - subnet['address'] - if 'netmask' in subnet: - iface_cfg['NETMASK' + str(ipv4_index)] = \ - subnet['netmask'] + suff = "" if ipv4_index == 0 else str(ipv4_index) + iface_cfg['IPADDR' + suff] = subnet['address'] + iface_cfg['NETMASK' + suff] = \ + net_prefix_to_ipv4_mask(subnet['prefix']) @classmethod def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets): diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index be9a8318..83580cc0 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -92,10 +92,9 @@ iface lo inet loopback auto eth0 iface eth0 inet static - address 192.168.1.5 + address 192.168.1.5/24 broadcast 192.168.1.0 gateway 192.168.1.254 - netmask 255.255.255.0 auto eth1 iface eth1 inet dhcp @@ -156,7 +155,7 @@ network: ethernets: eth7: addresses: - - 192.168.1.5/255.255.255.0 + - 192.168.1.5/24 gateway4: 192.168.1.254 eth9: dhcp4: true diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 0a88caf1..91e5fb59 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -334,17 +334,15 @@ iface lo inet loopback auto eth0 iface eth0 inet static - address 1.2.3.12 + address 1.2.3.12/29 broadcast 1.2.3.15 dns-nameservers 69.9.160.191 69.9.191.4 gateway 1.2.3.9 - netmask 255.255.255.248 auto eth1 iface eth1 inet static - address 10.248.2.4 + address 10.248.2.4/29 broadcast 10.248.2.7 - netmask 255.255.255.248 """.lstrip() NETWORK_CONFIGS = { -- cgit v1.2.3 From 67bab5bb804e2346673430868935f6bbcdb88f13 Mon Sep 17 00:00:00 2001 From: Ryan McCabe Date: Thu, 8 Jun 2017 13:24:23 -0400 Subject: net: Allow for NetworkManager configuration In cases where the config json specifies nameserver entries, if there are interfaces configured to use dhcp, NetworkManager, if enabled, will clobber the /etc/resolv.conf that cloud-init has produced, which can break dns. If there are no interfaces configured to use dhcp, NetworkManager could clobber /etc/resolv.conf with an empty file. This patch adds a mechanism for dropping additional configuration into /etc/NetworkManager/conf.d/ and disables management of /etc/resolv.conf by NetworkManager when nameserver information is provided in the config. LP: #1693251 Signed-off-by: Ryan McCabe --- cloudinit/distros/parsers/networkmanager_conf.py | 23 ++++++++++++++++++++++ cloudinit/net/sysconfig.py | 25 ++++++++++++++++++++++++ tests/unittests/test_net.py | 21 ++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 cloudinit/distros/parsers/networkmanager_conf.py (limited to 'cloudinit/net') diff --git a/cloudinit/distros/parsers/networkmanager_conf.py b/cloudinit/distros/parsers/networkmanager_conf.py new file mode 100644 index 00000000..ac51f122 --- /dev/null +++ b/cloudinit/distros/parsers/networkmanager_conf.py @@ -0,0 +1,23 @@ +# Copyright (C) 2017 Red Hat, Inc. +# +# Author: Ryan McCabe +# +# This file is part of cloud-init. See LICENSE file for license information. + +import configobj + +# This module is used to set additional NetworkManager configuration +# in /etc/NetworkManager/conf.d +# + + +class NetworkManagerConf(configobj.ConfigObj): + def __init__(self, contents): + configobj.ConfigObj.__init__(self, contents, + interpolation=False, + write_empty_values=False) + + def set_section_keypair(self, section_name, key, value): + if section_name not in self.sections: + self.main[section_name] = {} + self.main[section_name] = {key: value} diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 5d9b3d10..7ed11d1e 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -5,6 +5,7 @@ import re import six +from cloudinit.distros.parsers import networkmanager_conf from cloudinit.distros.parsers import resolv_conf from cloudinit import util @@ -249,6 +250,9 @@ class Renderer(renderer.Renderer): self.netrules_path = config.get( 'netrules_path', 'etc/udev/rules.d/70-persistent-net.rules') self.dns_path = config.get('dns_path', 'etc/resolv.conf') + nm_conf_path = 'etc/NetworkManager/conf.d/99-cloud-init.conf' + self.networkmanager_conf_path = config.get('networkmanager_conf_path', + nm_conf_path) @classmethod def _render_iface_shared(cls, iface, iface_cfg): @@ -438,6 +442,21 @@ class Renderer(renderer.Renderer): content.add_search_domain(searchdomain) return "\n".join([_make_header(';'), str(content)]) + @staticmethod + def _render_networkmanager_conf(network_state): + content = networkmanager_conf.NetworkManagerConf("") + + # If DNS server information is provided, configure + # NetworkManager to not manage dns, so that /etc/resolv.conf + # does not get clobbered. + if network_state.dns_nameservers: + content.set_section_keypair('main', 'dns', 'none') + + if len(content) == 0: + return None + out = "".join([_make_header(), "\n", "\n".join(content.write()), "\n"]) + return out + @classmethod def _render_bridge_interfaces(cls, network_state, iface_contents): bridge_filter = renderer.filter_by_type('bridge') @@ -498,6 +517,12 @@ class Renderer(renderer.Renderer): resolv_content = self._render_dns(network_state, existing_dns_path=dns_path) util.write_file(dns_path, resolv_content, file_mode) + if self.networkmanager_conf_path: + nm_conf_path = util.target_path(target, + self.networkmanager_conf_path) + nm_conf_content = self._render_networkmanager_conf(network_state) + if nm_conf_content: + util.write_file(nm_conf_path, nm_conf_content, file_mode) if self.netrules_path: netrules_content = self._render_persistent_net(network_state) netrules_path = util.target_path(target, self.netrules_path) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 91e5fb59..8edc0b89 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -155,6 +155,13 @@ USERCTL=no ; Created by cloud-init on instance boot automatically, do not edit. ; nameserver 172.19.0.12 +""".lstrip()), + ('etc/NetworkManager/conf.d/99-cloud-init.conf', + """ +# Created by cloud-init on instance boot automatically, do not edit. +# +[main] +dns = none """.lstrip()), ('etc/udev/rules.d/70-persistent-net.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', @@ -216,6 +223,13 @@ USERCTL=no ; Created by cloud-init on instance boot automatically, do not edit. ; nameserver 172.19.0.12 +""".lstrip()), + ('etc/NetworkManager/conf.d/99-cloud-init.conf', + """ +# Created by cloud-init on instance boot automatically, do not edit. +# +[main] +dns = none """.lstrip()), ('etc/udev/rules.d/70-persistent-net.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', @@ -299,6 +313,13 @@ USERCTL=no ; Created by cloud-init on instance boot automatically, do not edit. ; nameserver 172.19.0.12 +""".lstrip()), + ('etc/NetworkManager/conf.d/99-cloud-init.conf', + """ +# Created by cloud-init on instance boot automatically, do not edit. +# +[main] +dns = none """.lstrip()), ('etc/udev/rules.d/70-persistent-net.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', -- cgit v1.2.3 From ebc9ecbc8a76bdf511a456fb72339a7eb4c20568 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Tue, 20 Jun 2017 17:06:43 -0500 Subject: Azure: Add network-config, Refactor net layer to handle duplicate macs. On systems with network devices with duplicate mac addresses, cloud-init will fail to rename the devices according to the specified network configuration. Refactor net layer to search by device driver and device id if available. Azure systems may have duplicate mac addresses by design. Update Azure datasource to run at init-local time and let Azure datasource generate a fallback networking config to handle advanced networking configurations. Lastly, add a 'setup' method to the datasources that is called before userdata/vendordata is processed but after networking is up. That is used here on Azure to interact with the 'fabric'. --- cloudinit/cmd/main.py | 3 + cloudinit/net/__init__.py | 181 ++++++++-- cloudinit/net/eni.py | 2 + cloudinit/net/renderer.py | 4 +- cloudinit/net/udev.py | 7 +- cloudinit/sources/DataSourceAzure.py | 114 +++++- cloudinit/sources/__init__.py | 15 +- cloudinit/stages.py | 5 + tests/unittests/test_datasource/test_azure.py | 174 +++++++-- tests/unittests/test_datasource/test_common.py | 2 +- tests/unittests/test_net.py | 478 ++++++++++++++++++++++++- 11 files changed, 887 insertions(+), 98 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index ce3c10dd..139e03b3 100644 --- a/cloudinit/cmd/main.py +++ b/cloudinit/cmd/main.py @@ -372,6 +372,9 @@ def main_init(name, args): LOG.debug("[%s] %s is in local mode, will apply init modules now.", mode, init.datasource) + # Give the datasource a chance to use network resources. + # This is used on Azure to communicate with the fabric over network. + init.setup_datasource() # update fully realizes user-data (pulling in #include if necessary) init.update() # Stage 7 diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 65accbb0..cba991a5 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -97,6 +97,10 @@ def is_bridge(devname): return os.path.exists(sys_dev_path(devname, "bridge")) +def is_bond(devname): + return os.path.exists(sys_dev_path(devname, "bonding")) + + def is_vlan(devname): uevent = str(read_sys_net_safe(devname, "uevent")) return 'DEVTYPE=vlan' in uevent.splitlines() @@ -124,6 +128,26 @@ def is_present(devname): return os.path.exists(sys_dev_path(devname)) +def device_driver(devname): + """Return the device driver for net device named 'devname'.""" + driver = None + driver_path = sys_dev_path(devname, "device/driver") + # driver is a symlink to the driver *dir* + if os.path.islink(driver_path): + driver = os.path.basename(os.readlink(driver_path)) + + return driver + + +def device_devid(devname): + """Return the device id string for net device named 'devname'.""" + dev_id = read_sys_net_safe(devname, "device/device") + if dev_id is False: + return None + + return dev_id + + def get_devicelist(): return os.listdir(SYS_CLASS_NET) @@ -138,12 +162,21 @@ def is_disabled_cfg(cfg): return cfg.get('config') == "disabled" -def generate_fallback_config(): +def generate_fallback_config(blacklist_drivers=None, config_driver=None): """Determine which attached net dev is most likely to have a connection and generate network state to run dhcp on that interface""" + + if not config_driver: + config_driver = False + + if not blacklist_drivers: + blacklist_drivers = [] + # get list of interfaces that could have connections invalid_interfaces = set(['lo']) - potential_interfaces = set(get_devicelist()) + potential_interfaces = set([device for device in get_devicelist() + if device_driver(device) not in + blacklist_drivers]) potential_interfaces = potential_interfaces.difference(invalid_interfaces) # sort into interfaces with carrier, interfaces which could have carrier, # and ignore interfaces that are definitely disconnected @@ -155,6 +188,9 @@ def generate_fallback_config(): if is_bridge(interface): # skip any bridges continue + if is_bond(interface): + # skip any bonds + continue carrier = read_sys_net_int(interface, 'carrier') if carrier: connected.append(interface) @@ -194,9 +230,18 @@ def generate_fallback_config(): break if target_mac and target_name: nconf = {'config': [], 'version': 1} - nconf['config'].append( - {'type': 'physical', 'name': target_name, - 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]}) + cfg = {'type': 'physical', 'name': target_name, + 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]} + # inject the device driver name, dev_id into config if enabled and + # device has a valid device driver value + if config_driver: + driver = device_driver(target_name) + if driver: + cfg['params'] = { + 'driver': driver, + 'device_id': device_devid(target_name), + } + nconf['config'].append(cfg) return nconf else: # can't read any interfaces addresses (or there are none); give up @@ -217,10 +262,16 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): if ent.get('type') != 'physical': continue mac = ent.get('mac_address') - name = ent.get('name') if not mac: continue - renames.append([mac, name]) + name = ent.get('name') + driver = ent.get('params', {}).get('driver') + device_id = ent.get('params', {}).get('device_id') + if not driver: + driver = device_driver(name) + if not device_id: + device_id = device_devid(name) + renames.append([mac, name, driver, device_id]) return _rename_interfaces(renames) @@ -245,15 +296,27 @@ def _get_current_rename_info(check_downable=True): """Collect information necessary for rename_interfaces. returns a dictionary by mac address like: - {mac: - {'name': name - 'up': boolean: is_up(name), + {name: + { 'downable': None or boolean indicating that the - device has only automatically assigned ip addrs.}} + device has only automatically assigned ip addrs. + 'device_id': Device id value (if it has one) + 'driver': Device driver (if it has one) + 'mac': mac address + 'name': name + 'up': boolean: is_up(name) + }} """ - bymac = {} - for mac, name in get_interfaces_by_mac().items(): - bymac[mac] = {'name': name, 'up': is_up(name), 'downable': None} + cur_info = {} + for (name, mac, driver, device_id) in get_interfaces(): + cur_info[name] = { + 'downable': None, + 'device_id': device_id, + 'driver': driver, + 'mac': mac, + 'name': name, + 'up': is_up(name), + } if check_downable: nmatch = re.compile(r"[0-9]+:\s+(\w+)[@:]") @@ -265,11 +328,11 @@ def _get_current_rename_info(check_downable=True): for bytes_out in (ipv6, ipv4): nics_with_addresses.update(nmatch.findall(bytes_out)) - for d in bymac.values(): + for d in cur_info.values(): d['downable'] = (d['up'] is False or d['name'] not in nics_with_addresses) - return bymac + return cur_info def _rename_interfaces(renames, strict_present=True, strict_busy=True, @@ -282,15 +345,15 @@ def _rename_interfaces(renames, strict_present=True, strict_busy=True, if current_info is None: current_info = _get_current_rename_info() - cur_bymac = {} - for mac, data in current_info.items(): + cur_info = {} + for name, data in current_info.items(): cur = data.copy() - cur['mac'] = mac - cur_bymac[mac] = cur + cur['name'] = name + cur_info[name] = cur def update_byname(bymac): return dict((data['name'], data) - for data in bymac.values()) + for data in cur_info.values()) def rename(cur, new): util.subp(["ip", "link", "set", cur, "name", new], capture=True) @@ -304,14 +367,48 @@ def _rename_interfaces(renames, strict_present=True, strict_busy=True, ops = [] errors = [] ups = [] - cur_byname = update_byname(cur_bymac) + cur_byname = update_byname(cur_info) tmpname_fmt = "cirename%d" tmpi = -1 - for mac, new_name in renames: - cur = cur_bymac.get(mac, {}) - cur_name = cur.get('name') + def entry_match(data, mac, driver, device_id): + """match if set and in data""" + if mac and driver and device_id: + return (data['mac'] == mac and + data['driver'] == driver and + data['device_id'] == device_id) + elif mac and driver: + return (data['mac'] == mac and + data['driver'] == driver) + elif mac: + return (data['mac'] == mac) + + return False + + def find_entry(mac, driver, device_id): + match = [data for data in cur_info.values() + if entry_match(data, mac, driver, device_id)] + if len(match): + if len(match) > 1: + msg = ('Failed to match a single device. Matched devices "%s"' + ' with search values "(mac:%s driver:%s device_id:%s)"' + % (match, mac, driver, device_id)) + raise ValueError(msg) + return match[0] + + return None + + for mac, new_name, driver, device_id in renames: cur_ops = [] + cur = find_entry(mac, driver, device_id) + if not cur: + if strict_present: + errors.append( + "[nic not present] Cannot rename mac=%s to %s" + ", not available." % (mac, new_name)) + continue + + cur_name = cur.get('name') if cur_name == new_name: # nothing to do continue @@ -351,13 +448,13 @@ def _rename_interfaces(renames, strict_present=True, strict_busy=True, cur_ops.append(("rename", mac, new_name, (new_name, tmp_name))) target['name'] = tmp_name - cur_byname = update_byname(cur_bymac) + cur_byname = update_byname(cur_info) if target['up']: ups.append(("up", mac, new_name, (tmp_name,))) cur_ops.append(("rename", mac, new_name, (cur['name'], new_name))) cur['name'] = new_name - cur_byname = update_byname(cur_bymac) + cur_byname = update_byname(cur_info) ops += cur_ops opmap = {'rename': rename, 'down': down, 'up': up} @@ -426,6 +523,36 @@ def get_interfaces_by_mac(): return ret +def get_interfaces(): + """Return list of interface tuples (name, mac, driver, device_id) + + Bridges and any devices that have a 'stolen' mac are excluded.""" + try: + devs = get_devicelist() + except OSError as e: + if e.errno == errno.ENOENT: + devs = [] + else: + raise + ret = [] + empty_mac = '00:00:00:00:00:00' + for name in devs: + if not interface_has_own_mac(name): + continue + if is_bridge(name): + continue + if is_vlan(name): + continue + mac = get_interface_mac(name) + # some devices may not have a mac (tun0) + if not mac: + continue + if mac == empty_mac and name != 'lo': + continue + ret.append((name, mac, device_driver(name), device_devid(name))) + return ret + + class RendererNotFoundError(RuntimeError): pass diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 98ce01e4..b707146c 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -72,6 +72,8 @@ def _iface_add_attrs(iface, index): content = [] ignore_map = [ 'control', + 'device_id', + 'driver', 'index', 'inet', 'mode', diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index c68658dc..bba139e5 100644 --- a/cloudinit/net/renderer.py +++ b/cloudinit/net/renderer.py @@ -34,8 +34,10 @@ class Renderer(object): for iface in network_state.iter_interfaces(filter_by_physical): # for physical interfaces write out a persist net udev rule if 'name' in iface and iface.get('mac_address'): + driver = iface.get('driver', None) content.write(generate_udev_rule(iface['name'], - iface['mac_address'])) + iface['mac_address'], + driver=driver)) return content.getvalue() @abc.abstractmethod diff --git a/cloudinit/net/udev.py b/cloudinit/net/udev.py index fd2fd8c7..58c0a708 100644 --- a/cloudinit/net/udev.py +++ b/cloudinit/net/udev.py @@ -23,7 +23,7 @@ def compose_udev_setting(key, value): return '%s="%s"' % (key, value) -def generate_udev_rule(interface, mac): +def generate_udev_rule(interface, mac, driver=None): """Return a udev rule to set the name of network interface with `mac`. The rule ends up as a single line looking something like: @@ -31,10 +31,13 @@ def generate_udev_rule(interface, mac): SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}="ff:ee:dd:cc:bb:aa", NAME="eth0" """ + if not driver: + driver = '?*' + rule = ', '.join([ compose_udev_equality('SUBSYSTEM', 'net'), compose_udev_equality('ACTION', 'add'), - compose_udev_equality('DRIVERS', '?*'), + compose_udev_equality('DRIVERS', driver), compose_udev_attr_equality('address', mac), compose_udev_setting('NAME', interface), ]) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 4fe0d635..b5a95a1f 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -16,6 +16,7 @@ from xml.dom import minidom import xml.etree.ElementTree as ET from cloudinit import log as logging +from cloudinit import net from cloudinit import sources from cloudinit.sources.helpers.azure import get_metadata_from_fabric from cloudinit import util @@ -245,7 +246,9 @@ def temporary_hostname(temp_hostname, cfg, hostname_command='hostname'): set_hostname(previous_hostname, hostname_command) -class DataSourceAzureNet(sources.DataSource): +class DataSourceAzure(sources.DataSource): + _negotiated = False + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.seed_dir = os.path.join(paths.seed_dir, 'azure') @@ -255,6 +258,7 @@ class DataSourceAzureNet(sources.DataSource): util.get_cfg_by_path(sys_cfg, DS_CFG_PATH, {}), BUILTIN_DS_CONFIG]) self.dhclient_lease_file = self.ds_cfg.get('dhclient_lease_file') + self._network_config = None def __str__(self): root = sources.DataSource.__str__(self) @@ -331,6 +335,7 @@ class DataSourceAzureNet(sources.DataSource): if asset_tag != AZURE_CHASSIS_ASSET_TAG: LOG.debug("Non-Azure DMI asset tag '%s' discovered.", asset_tag) return False + ddir = self.ds_cfg['data_dir'] candidates = [self.seed_dir] @@ -375,13 +380,14 @@ class DataSourceAzureNet(sources.DataSource): LOG.debug("using files cached in %s", ddir) # azure / hyper-v provides random data here + # TODO. find the seed on FreeBSD platform + # now update ds_cfg to reflect contents pass in config if not util.is_FreeBSD(): seed = util.load_file("/sys/firmware/acpi/tables/OEM0", quiet=True, decode=False) if seed: self.metadata['random_seed'] = seed - # TODO. find the seed on FreeBSD platform - # now update ds_cfg to reflect contents pass in config + user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {}) self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg]) @@ -389,6 +395,40 @@ class DataSourceAzureNet(sources.DataSource): # the directory to be protected. write_files(ddir, files, dirmode=0o700) + self.metadata['instance-id'] = util.read_dmi_data('system-uuid') + + return True + + def device_name_to_device(self, name): + return self.ds_cfg['disk_aliases'].get(name) + + def get_config_obj(self): + return self.cfg + + def check_instance_id(self, sys_cfg): + # quickly (local check only) if self.instance_id is still valid + return sources.instance_id_matches_system_uuid(self.get_instance_id()) + + def setup(self, is_new_instance): + if self._negotiated is False: + LOG.debug("negotiating for %s (new_instance=%s)", + self.get_instance_id(), is_new_instance) + fabric_data = self._negotiate() + LOG.debug("negotiating returned %s", fabric_data) + if fabric_data: + self.metadata.update(fabric_data) + self._negotiated = True + else: + LOG.debug("negotiating already done for %s", + self.get_instance_id()) + + def _negotiate(self): + """Negotiate with fabric and return data from it. + + On success, returns a dictionary including 'public_keys'. + On failure, returns False. + """ + if self.ds_cfg['agent_command'] == AGENT_START_BUILTIN: self.bounce_network_with_azure_hostname() @@ -398,31 +438,64 @@ class DataSourceAzureNet(sources.DataSource): else: metadata_func = self.get_metadata_from_agent + LOG.debug("negotiating with fabric via agent command %s", + self.ds_cfg['agent_command']) try: fabric_data = metadata_func() except Exception as exc: - LOG.info("Error communicating with Azure fabric; assume we aren't" - " on Azure.", exc_info=True) + LOG.warning( + "Error communicating with Azure fabric; You may experience." + "connectivity issues.", exc_info=True) return False - self.metadata['instance-id'] = util.read_dmi_data('system-uuid') - self.metadata.update(fabric_data) - - return True - def device_name_to_device(self, name): - return self.ds_cfg['disk_aliases'].get(name) - - def get_config_obj(self): - return self.cfg - - def check_instance_id(self, sys_cfg): - # quickly (local check only) if self.instance_id is still valid - return sources.instance_id_matches_system_uuid(self.get_instance_id()) + return fabric_data def activate(self, cfg, is_new_instance): address_ephemeral_resize(is_new_instance=is_new_instance) return + @property + def network_config(self): + """Generate a network config like net.generate_fallback_network() with + the following execptions. + + 1. Probe the drivers of the net-devices present and inject them in + the network configuration under params: driver: value + 2. If the driver value is 'mlx4_core', the control mode should be + set to manual. The device will be later used to build a bond, + for now we want to ensure the device gets named but does not + break any network configuration + """ + blacklist = ['mlx4_core'] + if not self._network_config: + LOG.debug('Azure: generating fallback configuration') + # generate a network config, blacklist picking any mlx4_core devs + netconfig = net.generate_fallback_config( + blacklist_drivers=blacklist, config_driver=True) + + # if we have any blacklisted devices, update the network_config to + # include the device, mac, and driver values, but with no ip + # config; this ensures udev rules are generated but won't affect + # ip configuration + bl_found = 0 + for bl_dev in [dev for dev in net.get_devicelist() + if net.device_driver(dev) in blacklist]: + bl_found += 1 + cfg = { + 'type': 'physical', + 'name': 'vf%d' % bl_found, + 'mac_address': net.get_interface_mac(bl_dev), + 'params': { + 'driver': net.device_driver(bl_dev), + 'device_id': net.device_devid(bl_dev), + }, + } + netconfig['config'].append(cfg) + + self._network_config = netconfig + + return self._network_config + def _partitions_on_device(devpath, maxnum=16): # return a list of tuples (ptnum, path) for each part on devpath @@ -849,9 +922,12 @@ class NonAzureDataSource(Exception): pass +# Legacy: Must be present in case we load an old pkl object +DataSourceAzureNet = DataSourceAzure + # Used to match classes to dependencies datasources = [ - (DataSourceAzureNet, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)), + (DataSourceAzure, (sources.DEP_FILESYSTEM, )), ] diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index c3ce36d6..952caf35 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -251,10 +251,23 @@ class DataSource(object): def first_instance_boot(self): return + def setup(self, is_new_instance): + """setup(is_new_instance) + + This is called before user-data and vendor-data have been processed. + + Unless the datasource has set mode to 'local', then networking + per 'fallback' or per 'network_config' will have been written and + brought up the OS at this point. + """ + return + def activate(self, cfg, is_new_instance): """activate(cfg, is_new_instance) - This is called before the init_modules will be called. + This is called before the init_modules will be called but after + the user-data and vendor-data have been fully processed. + The cfg is fully up to date config, it contains a merged view of system config, datasource config, user config, vendor config. It should be used rather than the sys_cfg passed to __init__. diff --git a/cloudinit/stages.py b/cloudinit/stages.py index ad557827..a1c4a517 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -362,6 +362,11 @@ class Init(object): self._store_userdata() self._store_vendordata() + def setup_datasource(self): + if self.datasource is None: + raise RuntimeError("Datasource is None, cannot setup.") + self.datasource.setup(is_new_instance=self.is_new_instance()) + def activate_datasource(self): if self.datasource is None: raise RuntimeError("Datasource is None, cannot activate.") diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 7d33daf7..20e70fb7 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -181,13 +181,19 @@ scbus-1 on xpt0 bus 0 side_effect=_dmi_mocks)), ]) - dsrc = dsaz.DataSourceAzureNet( + dsrc = dsaz.DataSourceAzure( data.get('sys_cfg', {}), distro=None, paths=self.paths) if agent_command is not None: dsrc.ds_cfg['agent_command'] = agent_command return dsrc + def _get_and_setup(self, dsrc): + ret = dsrc.get_data() + if ret: + dsrc.setup(True) + return ret + def xml_equals(self, oxml, nxml): """Compare two sets of XML to make sure they are equal""" @@ -259,7 +265,7 @@ fdescfs /dev/fd fdescfs rw 0 0 # Return a non-matching asset tag value nonazure_tag = dsaz.AZURE_CHASSIS_ASSET_TAG + 'X' m_read_dmi_data.return_value = nonazure_tag - dsrc = dsaz.DataSourceAzureNet( + dsrc = dsaz.DataSourceAzure( {}, distro=None, paths=self.paths) self.assertFalse(dsrc.get_data()) self.assertEqual( @@ -299,7 +305,7 @@ fdescfs /dev/fd fdescfs rw 0 0 data = {'ovfcontent': construct_valid_ovf_env(data=odata)} dsrc = self._get_ds(data) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) self.assertEqual(data['agent_invoked'], cfg['agent_command']) @@ -312,7 +318,7 @@ fdescfs /dev/fd fdescfs rw 0 0 data = {'ovfcontent': construct_valid_ovf_env(data=odata)} dsrc = self._get_ds(data) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) self.assertEqual(data['agent_invoked'], cfg['agent_command']) @@ -322,7 +328,7 @@ fdescfs /dev/fd fdescfs rw 0 0 'sys_cfg': sys_cfg} dsrc = self._get_ds(data) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) self.assertEqual(data['agent_invoked'], '_COMMAND') @@ -394,7 +400,7 @@ fdescfs /dev/fd fdescfs rw 0 0 pubkeys=pubkeys)} dsrc = self._get_ds(data, agent_command=['not', '__builtin__']) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) for mypk in mypklist: self.assertIn(mypk, dsrc.cfg['_pubkeys']) @@ -409,7 +415,7 @@ fdescfs /dev/fd fdescfs rw 0 0 pubkeys=pubkeys)} dsrc = self._get_ds(data, agent_command=['not', '__builtin__']) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) for mypk in mypklist: @@ -425,7 +431,7 @@ fdescfs /dev/fd fdescfs rw 0 0 pubkeys=pubkeys)} dsrc = self._get_ds(data, agent_command=['not', '__builtin__']) - ret = dsrc.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) for mypk in mypklist: @@ -519,18 +525,20 @@ fdescfs /dev/fd fdescfs rw 0 0 dsrc.get_data() def test_exception_fetching_fabric_data_doesnt_propagate(self): - ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) - ds.ds_cfg['agent_command'] = '__builtin__' + """Errors communicating with fabric should warn, but return True.""" + dsrc = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) + dsrc.ds_cfg['agent_command'] = '__builtin__' self.get_metadata_from_fabric.side_effect = Exception - self.assertFalse(ds.get_data()) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) def test_fabric_data_included_in_metadata(self): - ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) - ds.ds_cfg['agent_command'] = '__builtin__' + dsrc = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) + dsrc.ds_cfg['agent_command'] = '__builtin__' self.get_metadata_from_fabric.return_value = {'test': 'value'} - ret = ds.get_data() + ret = self._get_and_setup(dsrc) self.assertTrue(ret) - self.assertEqual('value', ds.metadata['test']) + self.assertEqual('value', dsrc.metadata['test']) def test_instance_id_from_dmidecode_used(self): ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) @@ -554,6 +562,84 @@ fdescfs /dev/fd fdescfs rw 0 0 self.assertEqual( [mock.call("/dev/cd0")], m_check_fbsd_cdrom.call_args_list) + @mock.patch('cloudinit.net.get_interface_mac') + @mock.patch('cloudinit.net.get_devicelist') + @mock.patch('cloudinit.net.device_driver') + @mock.patch('cloudinit.net.generate_fallback_config') + def test_network_config(self, mock_fallback, mock_dd, + mock_devlist, mock_get_mac): + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': {}} + + fallback_config = { + 'version': 1, + 'config': [{ + 'type': 'physical', 'name': 'eth0', + 'mac_address': '00:11:22:33:44:55', + 'params': {'driver': 'hv_netsvc'}, + 'subnets': [{'type': 'dhcp'}], + }] + } + mock_fallback.return_value = fallback_config + + mock_devlist.return_value = ['eth0'] + mock_dd.return_value = ['hv_netsvc'] + mock_get_mac.return_value = '00:11:22:33:44:55' + + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + + netconfig = dsrc.network_config + self.assertEqual(netconfig, fallback_config) + mock_fallback.assert_called_with(blacklist_drivers=['mlx4_core'], + config_driver=True) + + @mock.patch('cloudinit.net.get_interface_mac') + @mock.patch('cloudinit.net.get_devicelist') + @mock.patch('cloudinit.net.device_driver') + @mock.patch('cloudinit.net.generate_fallback_config') + def test_network_config_blacklist(self, mock_fallback, mock_dd, + mock_devlist, mock_get_mac): + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': {}} + + fallback_config = { + 'version': 1, + 'config': [{ + 'type': 'physical', 'name': 'eth0', + 'mac_address': '00:11:22:33:44:55', + 'params': {'driver': 'hv_netsvc'}, + 'subnets': [{'type': 'dhcp'}], + }] + } + blacklist_config = { + 'type': 'physical', + 'name': 'eth1', + 'mac_address': '00:11:22:33:44:55', + 'params': {'driver': 'mlx4_core'} + } + mock_fallback.return_value = fallback_config + + mock_devlist.return_value = ['eth0', 'eth1'] + mock_dd.side_effect = [ + 'hv_netsvc', # list composition, skipped + 'mlx4_core', # list composition, match + 'mlx4_core', # config get driver name + ] + mock_get_mac.return_value = '00:11:22:33:44:55' + + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + + netconfig = dsrc.network_config + expected_config = fallback_config + expected_config['config'].append(blacklist_config) + self.assertEqual(netconfig, expected_config) + class TestAzureBounce(TestCase): @@ -603,12 +689,18 @@ class TestAzureBounce(TestCase): if ovfcontent is not None: populate_dir(os.path.join(self.paths.seed_dir, "azure"), {'ovf-env.xml': ovfcontent}) - dsrc = dsaz.DataSourceAzureNet( + dsrc = dsaz.DataSourceAzure( {}, distro=None, paths=self.paths) if agent_command is not None: dsrc.ds_cfg['agent_command'] = agent_command return dsrc + def _get_and_setup(self, dsrc): + ret = dsrc.get_data() + if ret: + dsrc.setup(True) + return ret + def get_ovf_env_with_dscfg(self, hostname, cfg): odata = { 'HostName': hostname, @@ -652,17 +744,20 @@ class TestAzureBounce(TestCase): host_name = 'unchanged-host-name' self.get_hostname.return_value = host_name cfg = {'hostname_bounce': {'policy': 'force'}} - self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg), - agent_command=['not', '__builtin__']).get_data() + dsrc = self._get_ds(self.get_ovf_env_with_dscfg(host_name, cfg), + agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(1, perform_hostname_bounce.call_count) def test_different_hostnames_sets_hostname(self): expected_hostname = 'azure-expected-host-name' self.get_hostname.return_value = 'default-host-name' - self._get_ds( + dsrc = self._get_ds( self.get_ovf_env_with_dscfg(expected_hostname, {}), - agent_command=['not', '__builtin__'], - ).get_data() + agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(expected_hostname, self.set_hostname.call_args_list[0][0][0]) @@ -671,19 +766,21 @@ class TestAzureBounce(TestCase): self, perform_hostname_bounce): expected_hostname = 'azure-expected-host-name' self.get_hostname.return_value = 'default-host-name' - self._get_ds( + dsrc = self._get_ds( self.get_ovf_env_with_dscfg(expected_hostname, {}), - agent_command=['not', '__builtin__'], - ).get_data() + agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(1, perform_hostname_bounce.call_count) def test_different_hostnames_sets_hostname_back(self): initial_host_name = 'default-host-name' self.get_hostname.return_value = initial_host_name - self._get_ds( + dsrc = self._get_ds( self.get_ovf_env_with_dscfg('some-host-name', {}), - agent_command=['not', '__builtin__'], - ).get_data() + agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(initial_host_name, self.set_hostname.call_args_list[-1][0][0]) @@ -693,10 +790,11 @@ class TestAzureBounce(TestCase): perform_hostname_bounce.side_effect = Exception initial_host_name = 'default-host-name' self.get_hostname.return_value = initial_host_name - self._get_ds( + dsrc = self._get_ds( self.get_ovf_env_with_dscfg('some-host-name', {}), - agent_command=['not', '__builtin__'], - ).get_data() + agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(initial_host_name, self.set_hostname.call_args_list[-1][0][0]) @@ -707,7 +805,9 @@ class TestAzureBounce(TestCase): self.get_hostname.return_value = old_hostname cfg = {'hostname_bounce': {'interface': interface, 'policy': 'force'}} data = self.get_ovf_env_with_dscfg(hostname, cfg) - self._get_ds(data, agent_command=['not', '__builtin__']).get_data() + dsrc = self._get_ds(data, agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(1, self.subp.call_count) bounce_env = self.subp.call_args[1]['env'] self.assertEqual(interface, bounce_env['interface']) @@ -719,7 +819,9 @@ class TestAzureBounce(TestCase): dsaz.BUILTIN_DS_CONFIG['hostname_bounce']['command'] = cmd cfg = {'hostname_bounce': {'policy': 'force'}} data = self.get_ovf_env_with_dscfg('some-hostname', cfg) - self._get_ds(data, agent_command=['not', '__builtin__']).get_data() + dsrc = self._get_ds(data, agent_command=['not', '__builtin__']) + ret = self._get_and_setup(dsrc) + self.assertTrue(ret) self.assertEqual(1, self.subp.call_count) bounce_args = self.subp.call_args[1]['args'] self.assertEqual(cmd, bounce_args) @@ -975,4 +1077,12 @@ class TestCanDevBeReformatted(CiTestCase): self.assertEqual(False, value) self.assertIn("3 or more", msg.lower()) + +class TestAzureNetExists(CiTestCase): + def test_azure_net_must_exist_for_legacy_objpkl(self): + """DataSourceAzureNet must exist for old obj.pkl files + that reference it.""" + self.assertTrue(hasattr(dsaz, "DataSourceAzureNet")) + + # vi: ts=4 expandtab diff --git a/tests/unittests/test_datasource/test_common.py b/tests/unittests/test_datasource/test_common.py index 7649b9ae..2ff1d9df 100644 --- a/tests/unittests/test_datasource/test_common.py +++ b/tests/unittests/test_datasource/test_common.py @@ -26,6 +26,7 @@ from cloudinit.sources import DataSourceNone as DSNone from .. import helpers as test_helpers DEFAULT_LOCAL = [ + Azure.DataSourceAzure, CloudSigma.DataSourceCloudSigma, ConfigDrive.DataSourceConfigDrive, DigitalOcean.DataSourceDigitalOcean, @@ -38,7 +39,6 @@ DEFAULT_LOCAL = [ DEFAULT_NETWORK = [ AliYun.DataSourceAliYun, AltCloud.DataSourceAltCloud, - Azure.DataSourceAzureNet, Bigstep.DataSourceBigstep, CloudStack.DataSourceCloudStack, DSNone.DataSourceNone, diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 8edc0b89..06e8f094 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -836,38 +836,176 @@ CONFIG_V1_EXPLICIT_LOOPBACK = { 'subnets': [{'control': 'auto', 'type': 'loopback'}]}, ]} +DEFAULT_DEV_ATTRS = { + 'eth1000': { + "bridge": False, + "carrier": False, + "dormant": False, + "operstate": "down", + "address": "07-1C-C6-75-A4-BE", + "device/driver": None, + "device/device": None, + } +} + def _setup_test(tmp_dir, mock_get_devicelist, mock_read_sys_net, - mock_sys_dev_path): - mock_get_devicelist.return_value = ['eth1000'] - dev_characteristics = { - 'eth1000': { - "bridge": False, - "carrier": False, - "dormant": False, - "operstate": "down", - "address": "07-1C-C6-75-A4-BE", - } - } + mock_sys_dev_path, dev_attrs=None): + if not dev_attrs: + dev_attrs = DEFAULT_DEV_ATTRS + + mock_get_devicelist.return_value = dev_attrs.keys() def fake_read(devname, path, translate=None, on_enoent=None, on_keyerror=None, on_einval=None): - return dev_characteristics[devname][path] + return dev_attrs[devname][path] mock_read_sys_net.side_effect = fake_read def sys_dev_path(devname, path=""): - return tmp_dir + devname + "/" + path + return tmp_dir + "/" + devname + "/" + path - for dev in dev_characteristics: + for dev in dev_attrs: os.makedirs(os.path.join(tmp_dir, dev)) with open(os.path.join(tmp_dir, dev, 'operstate'), 'w') as fh: - fh.write("down") + fh.write(dev_attrs[dev]['operstate']) + os.makedirs(os.path.join(tmp_dir, dev, "device")) + for key in ['device/driver']: + if key in dev_attrs[dev] and dev_attrs[dev][key]: + target = dev_attrs[dev][key] + link = os.path.join(tmp_dir, dev, key) + print('symlink %s -> %s' % (link, target)) + os.symlink(target, link) mock_sys_dev_path.side_effect = sys_dev_path +class TestGenerateFallbackConfig(CiTestCase): + + @mock.patch("cloudinit.net.sys_dev_path") + @mock.patch("cloudinit.net.read_sys_net") + @mock.patch("cloudinit.net.get_devicelist") + def test_device_driver(self, mock_get_devicelist, mock_read_sys_net, + mock_sys_dev_path): + devices = { + 'eth0': { + 'bridge': False, 'carrier': False, 'dormant': False, + 'operstate': 'down', 'address': '00:11:22:33:44:55', + 'device/driver': 'hv_netsvc', 'device/device': '0x3'}, + 'eth1': { + 'bridge': False, 'carrier': False, 'dormant': False, + 'operstate': 'down', 'address': '00:11:22:33:44:55', + 'device/driver': 'mlx4_core', 'device/device': '0x7'}, + } + + tmp_dir = self.tmp_dir() + _setup_test(tmp_dir, mock_get_devicelist, + mock_read_sys_net, mock_sys_dev_path, + dev_attrs=devices) + + network_cfg = net.generate_fallback_config(config_driver=True) + ns = network_state.parse_net_config_data(network_cfg, + skip_broken=False) + + render_dir = os.path.join(tmp_dir, "render") + os.makedirs(render_dir) + + # don't set rulepath so eni writes them + renderer = eni.Renderer( + {'eni_path': 'interfaces', 'netrules_path': 'netrules'}) + renderer.render_network_state(ns, render_dir) + + self.assertTrue(os.path.exists(os.path.join(render_dir, + 'interfaces'))) + with open(os.path.join(render_dir, 'interfaces')) as fh: + contents = fh.read() + print(contents) + expected = """ +auto lo +iface lo inet loopback + +auto eth0 +iface eth0 inet dhcp +""" + self.assertEqual(expected.lstrip(), contents.lstrip()) + + self.assertTrue(os.path.exists(os.path.join(render_dir, 'netrules'))) + with open(os.path.join(render_dir, 'netrules')) as fh: + contents = fh.read() + print(contents) + expected_rule = [ + 'SUBSYSTEM=="net"', + 'ACTION=="add"', + 'DRIVERS=="hv_netsvc"', + 'ATTR{address}=="00:11:22:33:44:55"', + 'NAME="eth0"', + ] + self.assertEqual(", ".join(expected_rule) + '\n', contents.lstrip()) + + @mock.patch("cloudinit.net.sys_dev_path") + @mock.patch("cloudinit.net.read_sys_net") + @mock.patch("cloudinit.net.get_devicelist") + def test_device_driver_blacklist(self, mock_get_devicelist, + mock_read_sys_net, mock_sys_dev_path): + devices = { + 'eth1': { + 'bridge': False, 'carrier': False, 'dormant': False, + 'operstate': 'down', 'address': '00:11:22:33:44:55', + 'device/driver': 'hv_netsvc', 'device/device': '0x3'}, + 'eth0': { + 'bridge': False, 'carrier': False, 'dormant': False, + 'operstate': 'down', 'address': '00:11:22:33:44:55', + 'device/driver': 'mlx4_core', 'device/device': '0x7'}, + } + + tmp_dir = self.tmp_dir() + _setup_test(tmp_dir, mock_get_devicelist, + mock_read_sys_net, mock_sys_dev_path, + dev_attrs=devices) + + blacklist = ['mlx4_core'] + network_cfg = net.generate_fallback_config(blacklist_drivers=blacklist, + config_driver=True) + ns = network_state.parse_net_config_data(network_cfg, + skip_broken=False) + + render_dir = os.path.join(tmp_dir, "render") + os.makedirs(render_dir) + + # don't set rulepath so eni writes them + renderer = eni.Renderer( + {'eni_path': 'interfaces', 'netrules_path': 'netrules'}) + renderer.render_network_state(ns, render_dir) + + self.assertTrue(os.path.exists(os.path.join(render_dir, + 'interfaces'))) + with open(os.path.join(render_dir, 'interfaces')) as fh: + contents = fh.read() + print(contents) + expected = """ +auto lo +iface lo inet loopback + +auto eth1 +iface eth1 inet dhcp +""" + self.assertEqual(expected.lstrip(), contents.lstrip()) + + self.assertTrue(os.path.exists(os.path.join(render_dir, 'netrules'))) + with open(os.path.join(render_dir, 'netrules')) as fh: + contents = fh.read() + print(contents) + expected_rule = [ + 'SUBSYSTEM=="net"', + 'ACTION=="add"', + 'DRIVERS=="hv_netsvc"', + 'ATTR{address}=="00:11:22:33:44:55"', + 'NAME="eth1"', + ] + self.assertEqual(", ".join(expected_rule) + '\n', contents.lstrip()) + + class TestSysConfigRendering(CiTestCase): @mock.patch("cloudinit.net.sys_dev_path") @@ -1560,6 +1698,118 @@ class TestNetRenderers(CiTestCase): priority=['sysconfig', 'eni']) +class TestGetInterfaces(CiTestCase): + _data = {'bonds': ['bond1'], + 'bridges': ['bridge1'], + 'vlans': ['bond1.101'], + 'own_macs': ['enp0s1', 'enp0s2', 'bridge1-nic', 'bridge1', + 'bond1.101', 'lo', 'eth1'], + 'macs': {'enp0s1': 'aa:aa:aa:aa:aa:01', + 'enp0s2': 'aa:aa:aa:aa:aa:02', + 'bond1': 'aa:aa:aa:aa:aa:01', + 'bond1.101': 'aa:aa:aa:aa:aa:01', + 'bridge1': 'aa:aa:aa:aa:aa:03', + 'bridge1-nic': 'aa:aa:aa:aa:aa:03', + 'lo': '00:00:00:00:00:00', + 'greptap0': '00:00:00:00:00:00', + 'eth1': 'aa:aa:aa:aa:aa:01', + 'tun0': None}, + 'drivers': {'enp0s1': 'virtio_net', + 'enp0s2': 'e1000', + 'bond1': None, + 'bond1.101': None, + 'bridge1': None, + 'bridge1-nic': None, + 'lo': None, + 'greptap0': None, + 'eth1': 'mlx4_core', + 'tun0': None}} + data = {} + + def _se_get_devicelist(self): + return list(self.data['devices']) + + def _se_device_driver(self, name): + return self.data['drivers'][name] + + def _se_device_devid(self, name): + return '0x%s' % sorted(list(self.data['drivers'].keys())).index(name) + + def _se_get_interface_mac(self, name): + return self.data['macs'][name] + + def _se_is_bridge(self, name): + return name in self.data['bridges'] + + def _se_is_vlan(self, name): + return name in self.data['vlans'] + + def _se_interface_has_own_mac(self, name): + return name in self.data['own_macs'] + + def _mock_setup(self): + self.data = copy.deepcopy(self._data) + self.data['devices'] = set(list(self.data['macs'].keys())) + mocks = ('get_devicelist', 'get_interface_mac', 'is_bridge', + 'interface_has_own_mac', 'is_vlan', 'device_driver', + 'device_devid') + self.mocks = {} + for n in mocks: + m = mock.patch('cloudinit.net.' + n, + side_effect=getattr(self, '_se_' + n)) + self.addCleanup(m.stop) + self.mocks[n] = m.start() + + def test_gi_includes_duplicate_macs(self): + self._mock_setup() + ret = net.get_interfaces() + + self.assertIn('enp0s1', self._se_get_devicelist()) + self.assertIn('eth1', self._se_get_devicelist()) + found = [ent for ent in ret if 'aa:aa:aa:aa:aa:01' in ent] + self.assertEqual(len(found), 2) + + def test_gi_excludes_any_without_mac_address(self): + self._mock_setup() + ret = net.get_interfaces() + + self.assertIn('tun0', self._se_get_devicelist()) + found = [ent for ent in ret if 'tun0' in ent] + self.assertEqual(len(found), 0) + + def test_gi_excludes_stolen_macs(self): + self._mock_setup() + ret = net.get_interfaces() + self.mocks['interface_has_own_mac'].assert_has_calls( + [mock.call('enp0s1'), mock.call('bond1')], any_order=True) + expected = [ + ('enp0s2', 'aa:aa:aa:aa:aa:02', 'e1000', '0x5'), + ('enp0s1', 'aa:aa:aa:aa:aa:01', 'virtio_net', '0x4'), + ('eth1', 'aa:aa:aa:aa:aa:01', 'mlx4_core', '0x6'), + ('lo', '00:00:00:00:00:00', None, '0x8'), + ('bridge1-nic', 'aa:aa:aa:aa:aa:03', None, '0x3'), + ] + self.assertEqual(sorted(expected), sorted(ret)) + + def test_gi_excludes_bridges(self): + self._mock_setup() + # add a device 'b1', make all return they have their "own mac", + # set everything other than 'b1' to be a bridge. + # then expect b1 is the only thing left. + self.data['macs']['b1'] = 'aa:aa:aa:aa:aa:b1' + self.data['drivers']['b1'] = None + self.data['devices'].add('b1') + self.data['bonds'] = [] + self.data['own_macs'] = self.data['devices'] + self.data['bridges'] = [f for f in self.data['devices'] if f != "b1"] + ret = net.get_interfaces() + self.assertEqual([('b1', 'aa:aa:aa:aa:aa:b1', None, '0x0')], ret) + self.mocks['is_bridge'].assert_has_calls( + [mock.call('bridge1'), mock.call('enp0s1'), mock.call('bond1'), + mock.call('b1')], + any_order=True) + + class TestGetInterfacesByMac(CiTestCase): _data = {'bonds': ['bond1'], 'bridges': ['bridge1'], @@ -1691,4 +1941,202 @@ def _gzip_data(data): gzfp.close() return iobuf.getvalue() + +class TestRenameInterfaces(CiTestCase): + + @mock.patch('cloudinit.util.subp') + def test_rename_all(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'interface0', 'virtio_net', '0x3'), + ('00:11:22:33:44:aa', 'interface2', 'virtio_net', '0x5'), + ] + current_info = { + 'ens3': { + 'downable': True, + 'device_id': '0x3', + 'driver': 'virtio_net', + 'mac': '00:11:22:33:44:55', + 'name': 'ens3', + 'up': False}, + 'ens5': { + 'downable': True, + 'device_id': '0x5', + 'driver': 'virtio_net', + 'mac': '00:11:22:33:44:aa', + 'name': 'ens5', + 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'ens3', 'name', 'interface0'], + capture=True), + mock.call(['ip', 'link', 'set', 'ens5', 'name', 'interface2'], + capture=True), + ]) + + @mock.patch('cloudinit.util.subp') + def test_rename_no_driver_no_device_id(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'interface0', None, None), + ('00:11:22:33:44:aa', 'interface1', None, None), + ] + current_info = { + 'eth0': { + 'downable': True, + 'device_id': None, + 'driver': None, + 'mac': '00:11:22:33:44:55', + 'name': 'eth0', + 'up': False}, + 'eth1': { + 'downable': True, + 'device_id': None, + 'driver': None, + 'mac': '00:11:22:33:44:aa', + 'name': 'eth1', + 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'eth0', 'name', 'interface0'], + capture=True), + mock.call(['ip', 'link', 'set', 'eth1', 'name', 'interface1'], + capture=True), + ]) + + @mock.patch('cloudinit.util.subp') + def test_rename_all_bounce(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'interface0', 'virtio_net', '0x3'), + ('00:11:22:33:44:aa', 'interface2', 'virtio_net', '0x5'), + ] + current_info = { + 'ens3': { + 'downable': True, + 'device_id': '0x3', + 'driver': 'virtio_net', + 'mac': '00:11:22:33:44:55', + 'name': 'ens3', + 'up': True}, + 'ens5': { + 'downable': True, + 'device_id': '0x5', + 'driver': 'virtio_net', + 'mac': '00:11:22:33:44:aa', + 'name': 'ens5', + 'up': True}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'ens3', 'down'], capture=True), + mock.call(['ip', 'link', 'set', 'ens3', 'name', 'interface0'], + capture=True), + mock.call(['ip', 'link', 'set', 'ens5', 'down'], capture=True), + mock.call(['ip', 'link', 'set', 'ens5', 'name', 'interface2'], + capture=True), + mock.call(['ip', 'link', 'set', 'interface0', 'up'], capture=True), + mock.call(['ip', 'link', 'set', 'interface2', 'up'], capture=True) + ]) + + @mock.patch('cloudinit.util.subp') + def test_rename_duplicate_macs(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'eth0', 'hv_netsvc', '0x3'), + ('00:11:22:33:44:55', 'vf1', 'mlx4_core', '0x5'), + ] + current_info = { + 'eth0': { + 'downable': True, + 'device_id': '0x3', + 'driver': 'hv_netsvc', + 'mac': '00:11:22:33:44:55', + 'name': 'eth0', + 'up': False}, + 'eth1': { + 'downable': True, + 'device_id': '0x5', + 'driver': 'mlx4_core', + 'mac': '00:11:22:33:44:55', + 'name': 'eth1', + 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'eth1', 'name', 'vf1'], + capture=True), + ]) + + @mock.patch('cloudinit.util.subp') + def test_rename_duplicate_macs_driver_no_devid(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'eth0', 'hv_netsvc', None), + ('00:11:22:33:44:55', 'vf1', 'mlx4_core', None), + ] + current_info = { + 'eth0': { + 'downable': True, + 'device_id': '0x3', + 'driver': 'hv_netsvc', + 'mac': '00:11:22:33:44:55', + 'name': 'eth0', + 'up': False}, + 'eth1': { + 'downable': True, + 'device_id': '0x5', + 'driver': 'mlx4_core', + 'mac': '00:11:22:33:44:55', + 'name': 'eth1', + 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'eth1', 'name', 'vf1'], + capture=True), + ]) + + @mock.patch('cloudinit.util.subp') + def test_rename_multi_mac_dups(self, mock_subp): + renames = [ + ('00:11:22:33:44:55', 'eth0', 'hv_netsvc', '0x3'), + ('00:11:22:33:44:55', 'vf1', 'mlx4_core', '0x5'), + ('00:11:22:33:44:55', 'vf2', 'mlx4_core', '0x7'), + ] + current_info = { + 'eth0': { + 'downable': True, + 'device_id': '0x3', + 'driver': 'hv_netsvc', + 'mac': '00:11:22:33:44:55', + 'name': 'eth0', + 'up': False}, + 'eth1': { + 'downable': True, + 'device_id': '0x5', + 'driver': 'mlx4_core', + 'mac': '00:11:22:33:44:55', + 'name': 'eth1', + 'up': False}, + 'eth2': { + 'downable': True, + 'device_id': '0x7', + 'driver': 'mlx4_core', + 'mac': '00:11:22:33:44:55', + 'name': 'eth2', + 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + print(mock_subp.call_args_list) + mock_subp.assert_has_calls([ + mock.call(['ip', 'link', 'set', 'eth1', 'name', 'vf1'], + capture=True), + mock.call(['ip', 'link', 'set', 'eth2', 'name', 'vf2'], + capture=True), + ]) + + # vi: ts=4 expandtab -- cgit v1.2.3 From d1e8eb73aca6a3f5cee415774dcf540e934ec250 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Fri, 9 Jun 2017 12:35:11 -0500 Subject: sysconfig: include GATEWAY value if set in subnet Render the GATEWAY= value in interface files which have a gateway in the subnet configuration. LP: #1686856 --- cloudinit/net/sysconfig.py | 3 ++ tests/unittests/test_distros/test_netconfig.py | 2 ++ tests/unittests/test_net.py | 38 ++++++++++++++++++++++++++ 3 files changed, 43 insertions(+) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 7ed11d1e..ad8c268e 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -329,6 +329,9 @@ class Renderer(renderer.Renderer): iface_cfg['NETMASK' + suff] = \ net_prefix_to_ipv4_mask(subnet['prefix']) + if 'gateway' in subnet: + iface_cfg['GATEWAY'] = subnet['gateway'] + @classmethod def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets): for i, subnet in enumerate(subnets, start=len(iface_cfg.children)): diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index 83580cc0..dffe1781 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -479,6 +479,7 @@ BOOTPROTO=none DEVICE=eth0 IPADDR=192.168.1.5 NETMASK=255.255.255.0 +GATEWAY=192.168.1.254 NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet @@ -627,6 +628,7 @@ IPV6_AUTOCONF=no BOOTPROTO=none DEVICE=eth0 IPV6ADDR=2607:f0d0:1002:0011::2/64 +GATEWAY=2607:f0d0:1002:0011::1 IPV6INIT=yes NM_CONTROLLED=no ONBOOT=yes diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 06e8f094..71c9c457 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -828,6 +828,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true } } + CONFIG_V1_EXPLICIT_LOOPBACK = { 'version': 1, 'config': [{'name': 'eth0', 'type': 'physical', @@ -836,6 +837,18 @@ CONFIG_V1_EXPLICIT_LOOPBACK = { 'subnets': [{'control': 'auto', 'type': 'loopback'}]}, ]} + +CONFIG_V1_SIMPLE_SUBNET = { + 'version': 1, + 'config': [{'mac_address': '52:54:00:12:34:00', + 'name': 'interface0', + 'subnets': [{'address': '10.0.2.15', + 'gateway': '10.0.2.2', + 'netmask': '255.255.255.0', + 'type': 'static'}], + 'type': 'physical'}]} + + DEFAULT_DEV_ATTRS = { 'eth1000': { "bridge": False, @@ -1135,6 +1148,31 @@ USERCTL=no with open(os.path.join(render_dir, fn)) as fh: self.assertEqual(expected_content, fh.read()) + def test_network_config_v1_samples(self): + ns = network_state.parse_net_config_data(CONFIG_V1_SIMPLE_SUBNET) + render_dir = self.tmp_path("render") + os.makedirs(render_dir) + renderer = sysconfig.Renderer() + renderer.render_network_state(ns, render_dir) + found = dir2dict(render_dir) + nspath = '/etc/sysconfig/network-scripts/' + self.assertNotIn(nspath + 'ifcfg-lo', found.keys()) + expected = """\ +# Created by cloud-init on instance boot automatically, do not edit. +# +BOOTPROTO=none +DEVICE=interface0 +GATEWAY=10.0.2.2 +HWADDR=52:54:00:12:34:00 +IPADDR=10.0.2.15 +NETMASK=255.255.255.0 +NM_CONTROLLED=no +ONBOOT=yes +TYPE=Ethernet +USERCTL=no +""" + self.assertEqual(expected, found[nspath + 'ifcfg-interface0']) + def test_config_with_explicit_loopback(self): ns = network_state.parse_net_config_data(CONFIG_V1_EXPLICIT_LOOPBACK) render_dir = self.tmp_path("render") -- cgit v1.2.3 From 865e941f3f88c7daeafbf1eab856e02ce2b6a5f7 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 12 Jul 2017 17:16:35 -0700 Subject: tests: fixes for issues uncovered when moving to python 3.6. This includes a few fixes found when testing with python 3.6. - fix eni renderer when target is None This just uses the util.target_path() in the event that target is None. - change test cases to not rely on the cached result of util.get_cmdline() and other cached globals. Update the base TestCase to unset that cache. - mock calls to system_is_snappy from the create_users test cases. - drop unused _pp_root in test_simple_run.py LP: #1703697 --- cloudinit/net/netplan.py | 3 ++- tests/unittests/helpers.py | 21 +++++++++++++++- tests/unittests/test_distros/test_create_users.py | 30 +++++++++-------------- tests/unittests/test_runs/test_simple_run.py | 18 -------------- 4 files changed, 34 insertions(+), 38 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index 67543305..9f35b72b 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -209,7 +209,8 @@ class Renderer(renderer.Renderer): # check network state for version # if v2, then extract network_state.config # else render_v2_from_state - fpnplan = os.path.join(target, self.netplan_path) + fpnplan = os.path.join(util.target_path(target), self.netplan_path) + util.ensure_dir(os.path.dirname(fpnplan)) header = self.netplan_header if self.netplan_header else "" diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py index 569f1aef..6691cf82 100644 --- a/tests/unittests/helpers.py +++ b/tests/unittests/helpers.py @@ -82,7 +82,26 @@ def retarget_many_wrapper(new_base, am, old_func): class TestCase(unittest2.TestCase): - pass + def reset_global_state(self): + """Reset any global state to its original settings. + + cloudinit caches some values in cloudinit.util. Unit tests that + involved those cached paths were then subject to failure if the order + of invocation changed (LP: #1703697). + + This function resets any of these global state variables to their + initial state. + + In the future this should really be done with some registry that + can then be cleaned in a more obvious way. + """ + util.PROC_CMDLINE = None + util._DNS_REDIRECT_IP = None + util._LSB_RELEASE = {} + + def setUp(self): + super(unittest2.TestCase, self).setUp() + self.reset_global_state() class CiTestCase(TestCase): diff --git a/tests/unittests/test_distros/test_create_users.py b/tests/unittests/test_distros/test_create_users.py index 9ded4f6c..1d02f7bd 100644 --- a/tests/unittests/test_distros/test_create_users.py +++ b/tests/unittests/test_distros/test_create_users.py @@ -38,6 +38,8 @@ class MyBaseDistro(distros.Distro): raise NotImplementedError() +@mock.patch("cloudinit.distros.util.system_is_snappy", return_value=False) +@mock.patch("cloudinit.distros.util.subp") class TestCreateUser(TestCase): def setUp(self): super(TestCase, self).setUp() @@ -53,8 +55,7 @@ class TestCreateUser(TestCase): logcmd[i + 1] = 'REDACTED' return mock.call(args, logstring=logcmd) - @mock.patch("cloudinit.distros.util.subp") - def test_basic(self, m_subp): + def test_basic(self, m_subp, m_is_snappy): user = 'foouser' self.dist.create_user(user) self.assertEqual( @@ -62,8 +63,7 @@ class TestCreateUser(TestCase): [self._useradd2call([user, '-m']), mock.call(['passwd', '-l', user])]) - @mock.patch("cloudinit.distros.util.subp") - def test_no_home(self, m_subp): + def test_no_home(self, m_subp, m_is_snappy): user = 'foouser' self.dist.create_user(user, no_create_home=True) self.assertEqual( @@ -71,8 +71,7 @@ class TestCreateUser(TestCase): [self._useradd2call([user, '-M']), mock.call(['passwd', '-l', user])]) - @mock.patch("cloudinit.distros.util.subp") - def test_system_user(self, m_subp): + def test_system_user(self, m_subp, m_is_snappy): # system user should have no home and get --system user = 'foouser' self.dist.create_user(user, system=True) @@ -81,8 +80,7 @@ class TestCreateUser(TestCase): [self._useradd2call([user, '--system', '-M']), mock.call(['passwd', '-l', user])]) - @mock.patch("cloudinit.distros.util.subp") - def test_explicit_no_home_false(self, m_subp): + def test_explicit_no_home_false(self, m_subp, m_is_snappy): user = 'foouser' self.dist.create_user(user, no_create_home=False) self.assertEqual( @@ -90,16 +88,14 @@ class TestCreateUser(TestCase): [self._useradd2call([user, '-m']), mock.call(['passwd', '-l', user])]) - @mock.patch("cloudinit.distros.util.subp") - def test_unlocked(self, m_subp): + def test_unlocked(self, m_subp, m_is_snappy): user = 'foouser' self.dist.create_user(user, lock_passwd=False) self.assertEqual( m_subp.call_args_list, [self._useradd2call([user, '-m'])]) - @mock.patch("cloudinit.distros.util.subp") - def test_set_password(self, m_subp): + def test_set_password(self, m_subp, m_is_snappy): user = 'foouser' password = 'passfoo' self.dist.create_user(user, passwd=password) @@ -109,8 +105,7 @@ class TestCreateUser(TestCase): mock.call(['passwd', '-l', user])]) @mock.patch("cloudinit.distros.util.is_group") - @mock.patch("cloudinit.distros.util.subp") - def test_group_added(self, m_subp, m_is_group): + def test_group_added(self, m_is_group, m_subp, m_is_snappy): m_is_group.return_value = False user = 'foouser' self.dist.create_user(user, groups=['group1']) @@ -121,8 +116,7 @@ class TestCreateUser(TestCase): self.assertEqual(m_subp.call_args_list, expected) @mock.patch("cloudinit.distros.util.is_group") - @mock.patch("cloudinit.distros.util.subp") - def test_only_new_group_added(self, m_subp, m_is_group): + def test_only_new_group_added(self, m_is_group, m_subp, m_is_snappy): ex_groups = ['existing_group'] groups = ['group1', ex_groups[0]] m_is_group.side_effect = lambda m: m in ex_groups @@ -135,8 +129,8 @@ class TestCreateUser(TestCase): self.assertEqual(m_subp.call_args_list, expected) @mock.patch("cloudinit.distros.util.is_group") - @mock.patch("cloudinit.distros.util.subp") - def test_create_groups_with_whitespace_string(self, m_subp, m_is_group): + def test_create_groups_with_whitespace_string( + self, m_is_group, m_subp, m_is_snappy): # groups supported as a comma delimeted string even with white space m_is_group.return_value = False user = 'foouser' diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py index 31324204..55f15b55 100644 --- a/tests/unittests/test_runs/test_simple_run.py +++ b/tests/unittests/test_runs/test_simple_run.py @@ -16,24 +16,6 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): self.patchOS(root) self.patchUtils(root) - def _pp_root(self, root, repatch=True): - for (dirpath, dirnames, filenames) in os.walk(root): - print(dirpath) - for f in filenames: - joined = os.path.join(dirpath, f) - if os.path.islink(joined): - print("f %s - (symlink)" % (f)) - else: - print("f %s" % (f)) - for d in dirnames: - joined = os.path.join(dirpath, d) - if os.path.islink(joined): - print("d %s - (symlink)" % (d)) - else: - print("d %s" % (d)) - if repatch: - self._patchIn(root) - def test_none_ds(self): new_root = tempfile.mkdtemp() self.addCleanup(shutil.rmtree, new_root) -- cgit v1.2.3 From c0060fe4892197179a5cfbfd3239cf3b6c3e5029 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 19 Jul 2017 09:28:52 -0400 Subject: net: fix renaming of nics to support mac addresses written in upper case. The network device renaming code previously required the case of the mac address input to match that of the data read from the system. For example, if user provided network config with mac address in upper case, then cloud-init would not rename the device correctly as /sys/class/net/address stores lower case values. The fix here is to always compare lower case mac addresses. LP: #1705147 --- cloudinit/net/__init__.py | 8 ++++++-- tests/unittests/test_net.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index cba991a5..d1740e56 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -302,7 +302,7 @@ def _get_current_rename_info(check_downable=True): device has only automatically assigned ip addrs. 'device_id': Device id value (if it has one) 'driver': Device driver (if it has one) - 'mac': mac address + 'mac': mac address (in lower case) 'name': name 'up': boolean: is_up(name) }} @@ -313,7 +313,7 @@ def _get_current_rename_info(check_downable=True): 'downable': None, 'device_id': device_id, 'driver': driver, - 'mac': mac, + 'mac': mac.lower(), 'name': name, 'up': is_up(name), } @@ -348,6 +348,8 @@ def _rename_interfaces(renames, strict_present=True, strict_busy=True, cur_info = {} for name, data in current_info.items(): cur = data.copy() + if cur.get('mac'): + cur['mac'] = cur['mac'].lower() cur['name'] = name cur_info[name] = cur @@ -399,6 +401,8 @@ def _rename_interfaces(renames, strict_present=True, strict_busy=True, return None for mac, new_name, driver, device_id in renames: + if mac: + mac = mac.lower() cur_ops = [] cur = find_entry(mac, driver, device_id) if not cur: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 71c9c457..76721bab 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -2176,5 +2176,32 @@ class TestRenameInterfaces(CiTestCase): capture=True), ]) + @mock.patch('cloudinit.util.subp') + def test_rename_macs_case_insensitive(self, mock_subp): + """_rename_interfaces must support upper or lower case macs.""" + renames = [ + ('aa:aa:aa:aa:aa:aa', 'en0', None, None), + ('BB:BB:BB:BB:BB:BB', 'en1', None, None), + ('cc:cc:cc:cc:cc:cc', 'en2', None, None), + ('DD:DD:DD:DD:DD:DD', 'en3', None, None), + ] + current_info = { + 'eth0': {'downable': True, 'mac': 'AA:AA:AA:AA:AA:AA', + 'name': 'eth0', 'up': False}, + 'eth1': {'downable': True, 'mac': 'bb:bb:bb:bb:bb:bb', + 'name': 'eth1', 'up': False}, + 'eth2': {'downable': True, 'mac': 'cc:cc:cc:cc:cc:cc', + 'name': 'eth2', 'up': False}, + 'eth3': {'downable': True, 'mac': 'DD:DD:DD:DD:DD:DD', + 'name': 'eth3', 'up': False}, + } + net._rename_interfaces(renames, current_info=current_info) + + expected = [ + mock.call(['ip', 'link', 'set', 'eth%d' % i, 'name', 'en%d' % i], + capture=True) + for i in range(len(renames))] + mock_subp.assert_has_calls(expected) + # vi: ts=4 expandtab -- cgit v1.2.3 From 97abd83513bee191b58f095f4d683b18acce0b49 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Fri, 9 Jun 2017 15:33:37 -0500 Subject: sysconfig: ipv6 and default gateway fixes. With this change, entries in IPV6ADDR and IPV6ADDR_SECONDARIES will now always be in format addr/prefix. When a subnet has a gateway will be written. If the gateway is ipv6, use the key IPV6_DEFAULTGW rather than GATEWAY. LP: #1704872 --- cloudinit/net/sysconfig.py | 20 +++++++++----------- tests/unittests/test_distros/test_netconfig.py | 6 ++++-- tests/unittests/test_net.py | 3 ++- 3 files changed, 15 insertions(+), 14 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index ad8c268e..de6601af 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -10,7 +10,8 @@ from cloudinit.distros.parsers import resolv_conf from cloudinit import util from . import renderer -from .network_state import subnet_is_ipv6, net_prefix_to_ipv4_mask +from .network_state import ( + is_ipv6_addr, net_prefix_to_ipv4_mask, subnet_is_ipv6) def _make_header(sep='#'): @@ -308,20 +309,13 @@ class Renderer(renderer.Renderer): elif subnet_type == 'static': if subnet_is_ipv6(subnet): ipv6_index = ipv6_index + 1 - if 'netmask' in subnet and str(subnet['netmask']) != "": - ipv6_cidr = (subnet['address'] + - '/' + - str(subnet['netmask'])) - else: - ipv6_cidr = subnet['address'] + ipv6_cidr = "%s/%s" % (subnet['address'], subnet['prefix']) if ipv6_index == 0: iface_cfg['IPV6ADDR'] = ipv6_cidr elif ipv6_index == 1: iface_cfg['IPV6ADDR_SECONDARIES'] = ipv6_cidr else: - iface_cfg['IPV6ADDR_SECONDARIES'] = ( - iface_cfg['IPV6ADDR_SECONDARIES'] + - " " + ipv6_cidr) + iface_cfg['IPV6ADDR_SECONDARIES'] += " " + ipv6_cidr else: ipv4_index = ipv4_index + 1 suff = "" if ipv4_index == 0 else str(ipv4_index) @@ -330,7 +324,11 @@ class Renderer(renderer.Renderer): net_prefix_to_ipv4_mask(subnet['prefix']) if 'gateway' in subnet: - iface_cfg['GATEWAY'] = subnet['gateway'] + iface_cfg['DEFROUTE'] = True + if is_ipv6_addr(subnet['gateway']): + iface_cfg['IPV6_DEFAULTGW'] = subnet['gateway'] + else: + iface_cfg['GATEWAY'] = subnet['gateway'] @classmethod def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets): diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index dffe1781..2f505d93 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -476,10 +476,11 @@ NETWORKING=yes # Created by cloud-init on instance boot automatically, do not edit. # BOOTPROTO=none +DEFROUTE=yes DEVICE=eth0 +GATEWAY=192.168.1.254 IPADDR=192.168.1.5 NETMASK=255.255.255.0 -GATEWAY=192.168.1.254 NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet @@ -626,10 +627,11 @@ IPV6_AUTOCONF=no # Created by cloud-init on instance boot automatically, do not edit. # BOOTPROTO=none +DEFROUTE=yes DEVICE=eth0 IPV6ADDR=2607:f0d0:1002:0011::2/64 -GATEWAY=2607:f0d0:1002:0011::1 IPV6INIT=yes +IPV6_DEFAULTGW=2607:f0d0:1002:0011::1 NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 76721bab..22242717 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -298,7 +298,7 @@ DEVICE=eth0 GATEWAY=172.19.3.254 HWADDR=fa:16:3e:ed:9a:59 IPADDR=172.19.1.34 -IPV6ADDR=2001:DB8::10 +IPV6ADDR=2001:DB8::10/64 IPV6ADDR_SECONDARIES="2001:DB9::10/64 2001:DB10::10/64" IPV6INIT=yes IPV6_DEFAULTGW=2001:DB8::1 @@ -1161,6 +1161,7 @@ USERCTL=no # Created by cloud-init on instance boot automatically, do not edit. # BOOTPROTO=none +DEFROUTE=yes DEVICE=interface0 GATEWAY=10.0.2.2 HWADDR=52:54:00:12:34:00 -- cgit v1.2.3 From 51febf7363692d7947fe17a4fbfcb85058168ccb Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 14 Jun 2017 12:58:40 -0500 Subject: sysconfig: fix rendering of bond, bridge and vlan types. Previously, virtual types (bond, bridge, vlan) were almost completely broken. They would not get any network configuration (ip addresses or dhcp config) and or routes rendered. This fixes those issues. For bonds we now correctly render BONDING_SLAVE entries. Also add tests for simple bond, bridge and vlan. LP: #1695092 --- cloudinit/net/renderer.py | 4 + cloudinit/net/sysconfig.py | 43 +++++-- tests/unittests/test_net.py | 266 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 304 insertions(+), 9 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index bba139e5..57652e27 100644 --- a/cloudinit/net/renderer.py +++ b/cloudinit/net/renderer.py @@ -20,6 +20,10 @@ def filter_by_name(match_name): return lambda iface: match_name == iface['name'] +def filter_by_attr(match_name): + return lambda iface: (match_name in iface and iface[match_name]) + + filter_by_physical = filter_by_type('physical') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index de6601af..eb3c91d2 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -407,24 +407,41 @@ class Renderer(renderer.Renderer): @classmethod def _render_bond_interfaces(cls, network_state, iface_contents): bond_filter = renderer.filter_by_type('bond') + slave_filter = renderer.filter_by_attr('bond-master') for iface in network_state.iter_interfaces(bond_filter): iface_name = iface['name'] iface_cfg = iface_contents[iface_name] cls._render_bonding_opts(iface_cfg, iface) - iface_master_name = iface['bond-master'] - iface_cfg['MASTER'] = iface_master_name - iface_cfg['SLAVE'] = True + # Ensure that the master interface (and any of its children) # are actually marked as being bond types... - master_cfg = iface_contents[iface_master_name] - master_cfgs = [master_cfg] - master_cfgs.extend(master_cfg.children) + master_cfgs = [iface_cfg] + master_cfgs.extend(iface_cfg.children) for master_cfg in master_cfgs: master_cfg['BONDING_MASTER'] = True master_cfg.kind = 'bond' - @staticmethod - def _render_vlan_interfaces(network_state, iface_contents): + iface_subnets = iface.get("subnets", []) + route_cfg = iface_cfg.routes + cls._render_subnets(iface_cfg, iface_subnets) + cls._render_subnet_routes(iface_cfg, route_cfg, iface_subnets) + + # iter_interfaces on network-state is not sorted to produce + # consistent numbers we need to sort. + bond_slaves = sorted( + [slave_iface['name'] for slave_iface in + network_state.iter_interfaces(slave_filter) + if slave_iface['bond-master'] == iface_name]) + for index, bond_slave in enumerate(bond_slaves): + slavestr = 'BONDING_SLAVE%s' % index + iface_cfg[slavestr] = bond_slave + + slave_cfg = iface_contents[bond_slave] + slave_cfg['MASTER'] = iface_name + slave_cfg['SLAVE'] = True + + @classmethod + def _render_vlan_interfaces(cls, network_state, iface_contents): vlan_filter = renderer.filter_by_type('vlan') for iface in network_state.iter_interfaces(vlan_filter): iface_name = iface['name'] @@ -432,6 +449,11 @@ class Renderer(renderer.Renderer): iface_cfg['VLAN'] = True iface_cfg['PHYSDEV'] = iface_name[:iface_name.rfind('.')] + iface_subnets = iface.get("subnets", []) + route_cfg = iface_cfg.routes + cls._render_subnets(iface_cfg, iface_subnets) + cls._render_subnet_routes(iface_cfg, route_cfg, iface_subnets) + @staticmethod def _render_dns(network_state, existing_dns_path=None): content = resolv_conf.ResolvConf("") @@ -478,6 +500,11 @@ class Renderer(renderer.Renderer): for bridge_cfg in bridged_cfgs: bridge_cfg['BRIDGE'] = iface_name + iface_subnets = iface.get("subnets", []) + route_cfg = iface_cfg.routes + cls._render_subnets(iface_cfg, iface_subnets) + cls._render_subnet_routes(iface_cfg, route_cfg, iface_subnets) + @classmethod def _render_sysconfig(cls, base_sysconf_dir, network_state): '''Given state, return /etc/sysconfig files + contents''' diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 22242717..f786eea0 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -825,7 +825,211 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true gateway: 11.0.0.1 metric: 3 """).lstrip(), - } + }, + 'bond': { + 'yaml': textwrap.dedent(""" + version: 1 + config: + - type: physical + name: bond0s0 + mac_address: "aa:bb:cc:dd:e8:00" + - type: physical + name: bond0s1 + mac_address: "aa:bb:cc:dd:e8:01" + - type: bond + name: bond0 + mac_address: "aa:bb:cc:dd:e8:ff" + bond_interfaces: + - bond0s0 + - bond0s1 + params: + bond-mode: active-backup + bond_miimon: 100 + bond-xmit-hash-policy: "layer3+4" + subnets: + - type: static + address: 192.168.0.2/24 + gateway: 192.168.0.1 + routes: + - gateway: 192.168.0.3 + netmask: 255.255.255.0 + network: 10.1.3.0 + - type: static + address: 192.168.1.2/24 + - type: static + address: 2001:1::1/92 + """), + 'expected_sysconfig': { + 'ifcfg-bond0': textwrap.dedent("""\ + BONDING_MASTER=yes + BONDING_OPTS="mode=active-backup xmit_hash_policy=layer3+4 miimon=100" + BONDING_SLAVE0=bond0s0 + BONDING_SLAVE1=bond0s1 + BOOTPROTO=none + DEFROUTE=yes + DEVICE=bond0 + GATEWAY=192.168.0.1 + HWADDR=aa:bb:cc:dd:e8:ff + IPADDR=192.168.0.2 + IPADDR1=192.168.1.2 + IPV6ADDR=2001:1::1/92 + IPV6INIT=yes + NETMASK=255.255.255.0 + NETMASK1=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Bond + USERCTL=no + """), + 'ifcfg-bond0s0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=bond0s0 + HWADDR=aa:bb:cc:dd:e8:00 + MASTER=bond0 + NM_CONTROLLED=no + ONBOOT=yes + SLAVE=yes + TYPE=Ethernet + USERCTL=no + """), + 'route6-bond0': textwrap.dedent("""\ + """), + 'route-bond0': textwrap.dedent("""\ + ADDRESS0=10.1.3.0 + GATEWAY0=192.168.0.3 + NETMASK0=255.255.255.0 + """), + 'ifcfg-bond0s1': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=bond0s1 + HWADDR=aa:bb:cc:dd:e8:01 + MASTER=bond0 + NM_CONTROLLED=no + ONBOOT=yes + SLAVE=yes + TYPE=Ethernet + USERCTL=no + """), + }, + }, + 'vlan': { + 'yaml': textwrap.dedent(""" + version: 1 + config: + - type: physical + name: en0 + mac_address: "aa:bb:cc:dd:e8:00" + - type: vlan + name: en0.99 + vlan_link: en0 + vlan_id: 99 + subnets: + - type: static + address: '192.168.2.2/24' + - type: static + address: '192.168.1.2/24' + gateway: 192.168.1.1 + - type: static + address: 2001:1::bbbb/96 + routes: + - gateway: 2001:1::1 + netmask: '::' + network: '::' + """), + 'expected_sysconfig': { + 'ifcfg-en0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=en0 + HWADDR=aa:bb:cc:dd:e8:00 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-en0.99': textwrap.dedent("""\ + BOOTPROTO=none + DEFROUTE=yes + DEVICE=en0.99 + GATEWAY=2001:1::1 + IPADDR=192.168.2.2 + IPADDR1=192.168.1.2 + IPV6ADDR=2001:1::bbbb/96 + IPV6INIT=yes + NETMASK=255.255.255.0 + NETMASK1=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + PHYSDEV=en0 + TYPE=Ethernet + USERCTL=no + VLAN=yes"""), + }, + }, + 'bridge': { + 'yaml': textwrap.dedent(""" + version: 1 + config: + - type: physical + name: eth0 + mac_address: "52:54:00:12:34:00" + subnets: + - type: static + address: 2001:1::100/96 + - type: physical + name: eth1 + mac_address: "52:54:00:12:34:01" + subnets: + - type: static + address: 2001:1::101/96 + - type: bridge + name: br0 + bridge_interfaces: + - eth0 + - eth1 + params: + bridge_stp: 'off' + bridge_bridgeprio: 22 + subnets: + - type: static + address: 192.168.2.2/24"""), + 'expected_sysconfig': { + 'ifcfg-br0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=br0 + IPADDR=192.168.2.2 + NETMASK=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + PRIO=22 + STP=off + TYPE=Bridge + USERCTL=no + """), + 'ifcfg-eth0': textwrap.dedent("""\ + BOOTPROTO=none + BRIDGE=br0 + DEVICE=eth0 + HWADDR=52:54:00:12:34:00 + IPV6ADDR=2001:1::100/96 + IPV6INIT=yes + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + 'ifcfg-eth1': textwrap.dedent("""\ + BOOTPROTO=none + BRIDGE=br0 + DEVICE=eth1 + HWADDR=52:54:00:12:34:01 + IPV6ADDR=2001:1::101/96 + IPV6INIT=yes + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + }, + }, } @@ -1021,6 +1225,48 @@ iface eth1 inet dhcp class TestSysConfigRendering(CiTestCase): + scripts_dir = '/etc/sysconfig/network-scripts' + header = ('# Created by cloud-init on instance boot automatically, ' + 'do not edit.\n#\n') + + def _render_and_read(self, network_config=None, state=None, dir=None): + if dir is None: + dir = self.tmp_dir() + + if network_config: + ns = network_state.parse_net_config_data(network_config) + elif state: + ns = state + else: + raise ValueError("Expected data or state, got neither") + + renderer = sysconfig.Renderer() + renderer.render_network_state(ns, dir) + return dir2dict(dir) + + def _compare_files_to_expected(self, expected, found): + orig_maxdiff = self.maxDiff + expected_d = dict( + (os.path.join(self.scripts_dir, k), util.load_shell_content(v)) + for k, v in expected.items()) + + # only compare the files in scripts_dir + scripts_found = dict( + (k, util.load_shell_content(v)) for k, v in found.items() + if k.startswith(self.scripts_dir)) + try: + self.maxDiff = None + self.assertEqual(expected_d, scripts_found) + finally: + self.maxDiff = orig_maxdiff + + def _assert_headers(self, found): + missing = [f for f in found + if (f.startswith(self.scripts_dir) and + not found[f].startswith(self.header))] + if missing: + raise AssertionError("Missing headers in: %s" % missing) + @mock.patch("cloudinit.net.sys_dev_path") @mock.patch("cloudinit.net.read_sys_net") @mock.patch("cloudinit.net.get_devicelist") @@ -1195,6 +1441,24 @@ USERCTL=no """ self.assertEqual(expected, found[nspath + 'ifcfg-eth0']) + def test_bond_config(self): + entry = NETWORK_CONFIGS['bond'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + + def test_vlan_config(self): + entry = NETWORK_CONFIGS['vlan'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + + def test_bridge_config(self): + entry = NETWORK_CONFIGS['bridge'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + class TestEniNetRendering(CiTestCase): -- cgit v1.2.3 From 31fa6f9d0f945868349c033fa049d2467ddcd478 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 15 Jun 2017 11:51:54 -0500 Subject: sysconfig: fix ipv6 gateway routes Currently only the subnet is checked for 'ipv6' setting, however, the routes array may include a mix of v4 or v6 configurations, in particular, the gateway in a route may be ipv6, and if so, should export the value via IPV6_DEFAULTGW in the ifcfg-XXXX file. Additionally, if the route is v6, it should rendering a routes6-XXXX file; this is present but missing the 'dev ' scoping. LP: #1694801 --- cloudinit/net/sysconfig.py | 11 ++++++----- tests/unittests/test_net.py | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index eb3c91d2..abdd4dee 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -152,9 +152,10 @@ class Route(ConfigMap): elif proto == "ipv6" and self.is_ipv6_route(address_value): netmask_value = str(self._conf['NETMASK' + index]) gateway_value = str(self._conf['GATEWAY' + index]) - buf.write("%s/%s via %s\n" % (address_value, - netmask_value, - gateway_value)) + buf.write("%s/%s via %s dev %s\n" % (address_value, + netmask_value, + gateway_value, + self._route_name)) return buf.getvalue() @@ -334,7 +335,7 @@ class Renderer(renderer.Renderer): def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets): for i, subnet in enumerate(subnets, start=len(iface_cfg.children)): for route in subnet.get('routes', []): - is_ipv6 = subnet.get('ipv6') + is_ipv6 = subnet.get('ipv6') or is_ipv6_addr(route['gateway']) if _is_default_route(route): if ( @@ -356,7 +357,7 @@ class Renderer(renderer.Renderer): # also provided the default route? iface_cfg['DEFROUTE'] = True if 'gateway' in route: - if is_ipv6: + if is_ipv6 or is_ipv6_addr(route['gateway']): iface_cfg['IPV6_DEFAULTGW'] = route['gateway'] route_cfg.has_set_default_ipv6 = True else: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index f786eea0..c012600f 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -949,11 +949,12 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true BOOTPROTO=none DEFROUTE=yes DEVICE=en0.99 - GATEWAY=2001:1::1 + GATEWAY=192.168.1.1 IPADDR=192.168.2.2 IPADDR1=192.168.1.2 IPV6ADDR=2001:1::bbbb/96 IPV6INIT=yes + IPV6_DEFAULTGW=2001:1::1 NETMASK=255.255.255.0 NETMASK1=255.255.255.0 NM_CONTROLLED=no -- cgit v1.2.3 From 8317bcab7cd08f1dcd96095c0cb746b57cb27234 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 15 Jun 2017 13:12:03 -0500 Subject: sysconfig: handle manual type subnets Implement manual control for sysconfig by using ONBOOT=N. This allows an interface to be configured but not brought up. Note that ONBOOT is per-interface not per address. LP: #1687725 --- cloudinit/net/sysconfig.py | 3 +++ tests/unittests/test_net.py | 46 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index abdd4dee..b0f2ccf5 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -298,6 +298,9 @@ class Renderer(renderer.Renderer): " for interface '%s'" % (subnet_type, iface_cfg.name)) + if subnet.get('control') == 'manual': + iface_cfg['ONBOOT'] = False + # set IPv4 and IPv6 static addresses ipv4_index = -1 ipv6_index = -1 diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index c012600f..e625934f 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -1031,6 +1031,39 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true """), }, }, + 'manual': { + 'yaml': textwrap.dedent(""" + version: 1 + config: + - type: physical + name: eth0 + mac_address: "52:54:00:12:34:00" + subnets: + - type: static + address: 192.168.1.2/24 + control: manual"""), + 'expected_eni': textwrap.dedent("""\ + auto lo + iface lo inet loopback + + # control-manual eth0 + iface eth0 inet static + address 192.168.1.2/24 + """), + 'expected_sysconfig': { + 'ifcfg-eth0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth0 + HWADDR=52:54:00:12:34:00 + IPADDR=192.168.1.2 + NETMASK=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=no + TYPE=Ethernet + USERCTL=no + """), + }, + }, } @@ -1460,6 +1493,12 @@ USERCTL=no self._compare_files_to_expected(entry['expected_sysconfig'], found) self._assert_headers(found) + def test_manual_config(self): + entry = NETWORK_CONFIGS['manual'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + class TestEniNetRendering(CiTestCase): @@ -1911,6 +1950,13 @@ class TestEniRoundTrip(CiTestCase): entry['expected_eni'].splitlines(), files['/etc/network/interfaces'].splitlines()) + def testsimple_render_manual(self): + entry = NETWORK_CONFIGS['manual'] + files = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self.assertEqual( + entry['expected_eni'].splitlines(), + files['/etc/network/interfaces'].splitlines()) + def test_routes_rendered(self): # as reported in bug 1649652 conf = [ -- cgit v1.2.3 From 353c6902fb94899d410eb2f8bcc0bb81916e0e9e Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 15 Jun 2017 15:09:17 -0500 Subject: sysconfig: enable mtu set per subnet, including ipv6 mtu Render MTU values if present in subnet and route configurations for v4 and v6. LP: #1702513 --- cloudinit/net/sysconfig.py | 5 +++ tests/unittests/test_net.py | 76 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index b0f2ccf5..c7df36c0 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -273,6 +273,7 @@ class Renderer(renderer.Renderer): # modifying base values according to subnets for i, subnet in enumerate(subnets, start=len(iface_cfg.children)): + mtu_key = 'MTU' subnet_type = subnet.get('type') if subnet_type == 'dhcp6': iface_cfg['IPV6INIT'] = True @@ -292,7 +293,11 @@ class Renderer(renderer.Renderer): # if iface_cfg['BOOTPROTO'] == 'none': # iface_cfg['BOOTPROTO'] = 'static' if subnet_is_ipv6(subnet): + mtu_key = 'IPV6_MTU' iface_cfg['IPV6INIT'] = True + + if 'mtu' in subnet: + iface_cfg[mtu_key] = subnet['mtu'] else: raise ValueError("Unknown subnet type '%s' found" " for interface '%s'" % (subnet_type, diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index e625934f..a9261ebd 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -482,6 +482,62 @@ NETWORK_CONFIGS = { - {'type': 'dhcp6'} """).rstrip(' '), }, + 'v4_and_v6_static': { + 'expected_eni': textwrap.dedent("""\ + auto lo + iface lo inet loopback + + auto iface0 + iface iface0 inet static + address 192.168.14.2/24 + mtu 9000 + + # control-alias iface0 + iface iface0 inet6 static + address 2001:1::1/64 + mtu 1500 + """).rstrip(' '), + 'expected_netplan': textwrap.dedent(""" + network: + version: 2 + ethernets: + iface0: + addresses: + - 192.168.14.2/24 + - 2001:1::1/64 + mtu: 9000 + mtu6: 1500 + """).rstrip(' '), + 'yaml': textwrap.dedent("""\ + version: 1 + config: + - type: 'physical' + name: 'iface0' + subnets: + - type: static + address: 192.168.14.2/24 + mtu: 9000 + - type: static + address: 2001:1::1/64 + mtu: 1500 + """).rstrip(' '), + 'expected_sysconfig': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=iface0 + IPADDR=192.168.14.2 + IPV6ADDR=2001:1::1/64 + IPV6INIT=yes + NETMASK=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + MTU=9000 + IPV6_MTU=1500 + """), + }, + }, 'all': { 'expected_eni': ("""\ auto lo @@ -1499,6 +1555,12 @@ USERCTL=no self._compare_files_to_expected(entry['expected_sysconfig'], found) self._assert_headers(found) + def test_v4_and_v6_static_config(self): + entry = NETWORK_CONFIGS['v4_and_v6_static'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + class TestEniNetRendering(CiTestCase): @@ -1892,6 +1954,13 @@ class TestNetplanRoundTrip(CiTestCase): entry['expected_netplan'].splitlines(), files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def testsimple_render_v4_and_v6_static(self): + entry = NETWORK_CONFIGS['v4_and_v6_static'] + files = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self.assertEqual( + entry['expected_netplan'].splitlines(), + files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def testsimple_render_all(self): entry = NETWORK_CONFIGS['all'] files = self._render_and_read(network_config=yaml.load(entry['yaml'])) @@ -1950,6 +2019,13 @@ class TestEniRoundTrip(CiTestCase): entry['expected_eni'].splitlines(), files['/etc/network/interfaces'].splitlines()) + def testsimple_render_v4_and_v6_static(self): + entry = NETWORK_CONFIGS['v4_and_v6_static'] + files = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self.assertEqual( + entry['expected_eni'].splitlines(), + files['/etc/network/interfaces'].splitlines()) + def testsimple_render_manual(self): entry = NETWORK_CONFIGS['manual'] files = self._render_and_read(network_config=yaml.load(entry['yaml'])) -- cgit v1.2.3 From 811ce49d74afb158b2626a201ef5c714d5c9b059 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 28 Jun 2017 16:29:18 -0500 Subject: net: eni route rendering missed ipv6 default route config In some network configurations a network value of '::' and a netmask value of '::' were used to indicate a default IPV6 gateway. Commit d00da2d5 removed ipv6 'netmask' attributes and calculate a prefix length value instead. The eni route rendering failed to update the check to use prefix value of 0 to indicate the presence of an IPV6 default route. A broken ipv6 default route rendered like: post-up route add -net :: netmask :: gw 2001:4800:78ff:1b::1 || true And with this patch, it now renders like: post-up route add -A inet6 default gw 2001:4800:78ff:1b::1 || true LP: #1701097 --- cloudinit/net/eni.py | 2 +- tests/unittests/test_net.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index b707146c..bb80ec02 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -355,7 +355,7 @@ class Renderer(renderer.Renderer): default_gw = " default gw %s" % route['gateway'] content.append(up + default_gw + or_true) content.append(down + default_gw + or_true) - elif route['network'] == '::' and route['netmask'] == 0: + elif route['network'] == '::' and route['prefix'] == 0: # ipv6! default_gw = " -A inet6 default gw %s" % route['gateway'] content.append(up + default_gw + or_true) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index a9261ebd..d15cd1f4 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -597,6 +597,8 @@ iface br0 inet static # control-alias br0 iface br0 inet6 static address 2001:1::1/64 + post-up route add -A inet6 default gw 2001:4800:78ff:1b::1 || true + pre-down route del -A inet6 default gw 2001:4800:78ff:1b::1 || true auto bond0.200 iface bond0.200 inet dhcp @@ -731,6 +733,9 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true eth3: 50 eth4: 75 priority: 22 + routes: + - to: ::/0 + via: 2001:4800:78ff:1b::1 vlans: bond0.200: dhcp4: true @@ -863,6 +868,10 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true address: 192.168.14.2/24 - type: static address: 2001:1::1/64 # default to /64 + routes: + - gateway: 2001:4800:78ff:1b::1 + netmask: '::' + network: '::' # A global nameserver. - type: nameserver address: 8.8.8.8 -- cgit v1.2.3 From 7e41b2a773b81452f14a18ec8c4f3316a66d3f5e Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 20 Jul 2017 14:46:30 -0500 Subject: sysconfig: use MACADDR on bonds/bridges to configure mac_address Previously, sysconfig rendered HWADDR for all interface types, but that value is only used to identify physical devices. Instead use MACADDR to configure the MAC on virtual devices, like bonds and bridges. - Sort bond slave list to ensure consistent ordering in sysconfig rendered files. - Add unittests for sysconfig rendering of bonds/bridges with mac_address LP: #1701417 --- cloudinit/net/sysconfig.py | 11 ++++ tests/unittests/test_net.py | 149 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index c7df36c0..9184bce6 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -264,6 +264,9 @@ class Renderer(renderer.Renderer): for (old_key, new_key) in [('mac_address', 'HWADDR'), ('mtu', 'MTU')]: old_value = iface.get(old_key) if old_value is not None: + # only set HWADDR on physical interfaces + if old_key == 'mac_address' and iface['type'] != 'physical': + continue iface_cfg[new_key] = old_value @classmethod @@ -430,6 +433,9 @@ class Renderer(renderer.Renderer): master_cfg['BONDING_MASTER'] = True master_cfg.kind = 'bond' + if iface.get('mac_address'): + iface_cfg['MACADDR'] = iface.get('mac_address') + iface_subnets = iface.get("subnets", []) route_cfg = iface_cfg.routes cls._render_subnets(iface_cfg, iface_subnets) @@ -441,6 +447,7 @@ class Renderer(renderer.Renderer): [slave_iface['name'] for slave_iface in network_state.iter_interfaces(slave_filter) if slave_iface['bond-master'] == iface_name]) + for index, bond_slave in enumerate(bond_slaves): slavestr = 'BONDING_SLAVE%s' % index iface_cfg[slavestr] = bond_slave @@ -499,6 +506,10 @@ class Renderer(renderer.Renderer): for old_key, new_key in cls.bridge_opts_keys: if old_key in iface: iface_cfg[new_key] = iface[old_key] + + if iface.get('mac_address'): + iface_cfg['MACADDR'] = iface.get('mac_address') + # Is this the right key to get all the connected interfaces? for bridged_iface_name in iface.get('bridge_ports', []): # Ensure all bridged interfaces are correctly tagged diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index d15cd1f4..caf31342 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -422,6 +422,28 @@ NETWORK_CONFIGS = { via: 65.61.151.37 set-name: eth99 """).rstrip(' '), + 'expected_sysconfig': { + 'ifcfg-eth1': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth1 + HWADDR=cf:d6:af:48:e8:80 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth99': textwrap.dedent("""\ + BOOTPROTO=dhcp + DEFROUTE=yes + DEVICE=eth99 + GATEWAY=65.61.151.37 + HWADDR=c0:d6:9f:2c:e8:80 + IPADDR=192.168.21.3 + NETMASK=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + }, 'yaml': textwrap.dedent(""" version: 1 config: @@ -758,6 +780,119 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true - sacchromyces.maas - brettanomyces.maas """).rstrip(' '), + 'expected_sysconfig': { + 'ifcfg-bond0': textwrap.dedent("""\ + BONDING_MASTER=yes + BONDING_OPTS="mode=active-backup """ + """xmit_hash_policy=layer3+4 """ + """miimon=100" + BONDING_SLAVE0=eth1 + BONDING_SLAVE1=eth2 + BOOTPROTO=dhcp + DEVICE=bond0 + DHCPV6C=yes + IPV6INIT=yes + MACADDR=aa:bb:cc:dd:ee:ff + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Bond + USERCTL=no"""), + 'ifcfg-bond0.200': textwrap.dedent("""\ + BOOTPROTO=dhcp + DEVICE=bond0.200 + NM_CONTROLLED=no + ONBOOT=yes + PHYSDEV=bond0 + TYPE=Ethernet + USERCTL=no + VLAN=yes"""), + 'ifcfg-br0': textwrap.dedent("""\ + AGEING=250 + BOOTPROTO=none + DEFROUTE=yes + DEVICE=br0 + IPADDR=192.168.14.2 + IPV6ADDR=2001:1::1/64 + IPV6INIT=yes + IPV6_DEFAULTGW=2001:4800:78ff:1b::1 + NETMASK=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + PRIO=22 + STP=off + TYPE=Bridge + USERCTL=no"""), + 'ifcfg-eth0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth0 + HWADDR=c0:d6:9f:2c:e8:80 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth0.101': textwrap.dedent("""\ + BOOTPROTO=none + DEFROUTE=yes + DEVICE=eth0.101 + GATEWAY=192.168.0.1 + IPADDR=192.168.0.2 + IPADDR1=192.168.2.10 + MTU=1500 + NETMASK=255.255.255.0 + NETMASK1=255.255.255.0 + NM_CONTROLLED=no + ONBOOT=yes + PHYSDEV=eth0 + TYPE=Ethernet + USERCTL=no + VLAN=yes"""), + 'ifcfg-eth1': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth1 + HWADDR=aa:d6:9f:2c:e8:80 + MASTER=bond0 + NM_CONTROLLED=no + ONBOOT=yes + SLAVE=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth2': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth2 + HWADDR=c0:bb:9f:2c:e8:80 + MASTER=bond0 + NM_CONTROLLED=no + ONBOOT=yes + SLAVE=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth3': textwrap.dedent("""\ + BOOTPROTO=none + BRIDGE=br0 + DEVICE=eth3 + HWADDR=66:bb:9f:2c:e8:80 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth4': textwrap.dedent("""\ + BOOTPROTO=none + BRIDGE=br0 + DEVICE=eth4 + HWADDR=98:bb:9f:2c:e8:80 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no"""), + 'ifcfg-eth5': textwrap.dedent("""\ + BOOTPROTO=dhcp + DEVICE=eth5 + HWADDR=98:bb:9f:2c:e8:8a + NM_CONTROLLED=no + ONBOOT=no + TYPE=Ethernet + USERCTL=no""") + }, 'yaml': textwrap.dedent(""" version: 1 config: @@ -934,7 +1069,7 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true DEFROUTE=yes DEVICE=bond0 GATEWAY=192.168.0.1 - HWADDR=aa:bb:cc:dd:e8:ff + MACADDR=aa:bb:cc:dd:e8:ff IPADDR=192.168.0.2 IPADDR1=192.168.1.2 IPV6ADDR=2001:1::1/92 @@ -1564,6 +1699,18 @@ USERCTL=no self._compare_files_to_expected(entry['expected_sysconfig'], found) self._assert_headers(found) + def test_all_config(self): + entry = NETWORK_CONFIGS['all'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + + def test_small_config(self): + entry = NETWORK_CONFIGS['small'] + found = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self._compare_files_to_expected(entry['expected_sysconfig'], found) + self._assert_headers(found) + def test_v4_and_v6_static_config(self): entry = NETWORK_CONFIGS['v4_and_v6_static'] found = self._render_and_read(network_config=yaml.load(entry['yaml'])) -- cgit v1.2.3 From 681baff19dc16ef2efdfb8499ad74aea0efbe467 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 21 Jul 2017 16:50:27 -0400 Subject: sysconfig: support subnet type of 'manual'. The subnet type 'manual' was used as a way to declare a device and set an MTU for it but not assign network addresses. This updates the manual example config to handle that case and provides expected rendered output for sysconfig, eni, and netplan. --- cloudinit/net/sysconfig.py | 9 ++++-- tests/unittests/test_net.py | 76 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 3 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 9184bce6..a550f97c 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -61,6 +61,9 @@ class ConfigMap(object): def __getitem__(self, key): return self._conf[key] + def __contains__(self, key): + return key in self._conf + def drop(self, key): self._conf.pop(key, None) @@ -298,14 +301,16 @@ class Renderer(renderer.Renderer): if subnet_is_ipv6(subnet): mtu_key = 'IPV6_MTU' iface_cfg['IPV6INIT'] = True - if 'mtu' in subnet: iface_cfg[mtu_key] = subnet['mtu'] + elif subnet_type == 'manual': + # If the subnet has an MTU setting, then ONBOOT=True + # to apply the setting + iface_cfg['ONBOOT'] = mtu_key in iface_cfg else: raise ValueError("Unknown subnet type '%s' found" " for interface '%s'" % (subnet_type, iface_cfg.name)) - if subnet.get('control') == 'manual': iface_cfg['ONBOOT'] = False diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index caf31342..e49abcc4 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -1241,7 +1241,20 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true subnets: - type: static address: 192.168.1.2/24 - control: manual"""), + control: manual + - type: physical + name: eth1 + mtu: 1480 + mac_address: "52:54:00:12:34:aa" + subnets: + - type: manual + - type: physical + name: eth2 + mac_address: "52:54:00:12:34:ff" + subnets: + - type: manual + control: manual + """), 'expected_eni': textwrap.dedent("""\ auto lo iface lo inet loopback @@ -1249,6 +1262,34 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true # control-manual eth0 iface eth0 inet static address 192.168.1.2/24 + + auto eth1 + iface eth1 inet manual + mtu 1480 + + # control-manual eth2 + iface eth2 inet manual + """), + 'expected_netplan': textwrap.dedent("""\ + + network: + version: 2 + ethernets: + eth0: + addresses: + - 192.168.1.2/24 + match: + macaddress: '52:54:00:12:34:00' + set-name: eth0 + eth1: + match: + macaddress: 52:54:00:12:34:aa + mtu: 1480 + set-name: eth1 + eth2: + match: + macaddress: 52:54:00:12:34:ff + set-name: eth2 """), 'expected_sysconfig': { 'ifcfg-eth0': textwrap.dedent("""\ @@ -1262,6 +1303,25 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true TYPE=Ethernet USERCTL=no """), + 'ifcfg-eth1': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth1 + HWADDR=52:54:00:12:34:aa + MTU=1480 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + 'ifcfg-eth2': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=eth2 + HWADDR=52:54:00:12:34:ff + NM_CONTROLLED=no + ONBOOT=no + TYPE=Ethernet + USERCTL=no + """), }, }, } @@ -2124,6 +2184,13 @@ class TestNetplanRoundTrip(CiTestCase): entry['expected_netplan'].splitlines(), files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def testsimple_render_manual(self): + entry = NETWORK_CONFIGS['manual'] + files = self._render_and_read(network_config=yaml.load(entry['yaml'])) + self.assertEqual( + entry['expected_netplan'].splitlines(), + files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + class TestEniRoundTrip(CiTestCase): def _render_and_read(self, network_config=None, state=None, eni_path=None, @@ -2183,6 +2250,13 @@ class TestEniRoundTrip(CiTestCase): files['/etc/network/interfaces'].splitlines()) def testsimple_render_manual(self): + """Test rendering of 'manual' for 'type' and 'control'. + + 'type: manual' in a subnet is odd, but it is the way that was used + to declare that a network device should get a mtu set on it even + if there were no addresses to configure. Also strange is the fact + that in order to apply that MTU the ifupdown device must be set + to 'auto', or the MTU would not be set.""" entry = NETWORK_CONFIGS['manual'] files = self._render_and_read(network_config=yaml.load(entry['yaml'])) self.assertEqual( -- cgit v1.2.3 From e586fe35a692b7519000005c8024ebd2bcbc82e0 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 28 Jul 2017 14:28:47 -0600 Subject: cloudinit.net: add initialize_network_device function and tests This is not yet called, but will be called in a subsequent Ec2-related branch to manually initialize a network interface with the responses using dhcp discovery without any dhcp-script side-effects. The functionality has been tested on Ec2 ubuntu and CentOS vms to ensure that network interface initialization works in both OS-types. Since there was poor unit test coverage for the cloudinit.net.__init__ module, this branch adds a bunch of coverage to the functions in cloudinit.net.__init. We can also now have unit tests local to the cloudinit modules. The benefits of having unittests under cloudinit module: - Proximity of unittest to cloudinit module makes it easier for ongoing devs to know where to augment unit tests. The tests.unittest directory is organizated such that it - Allows for 1 to 1 name mapping module -> tests/test_module.py - Improved test and module isolation, if we find unit tests have to import from a number of modules besides the module under test, it will better prompt resturcturing of the module. This also branch touches: - tox.ini to run unit tests found in cloudinit as well as include all test-requirements for pylint since we now have unit tests living within cloudinit package - setup.py to exclude any test modules under cloudinit when packaging --- cloudinit/net/__init__.py | 131 ++++++++-- cloudinit/net/tests/__init__.py | 0 cloudinit/net/tests/test_init.py | 522 +++++++++++++++++++++++++++++++++++++++ setup.py | 2 +- tox.ini | 14 +- 5 files changed, 648 insertions(+), 21 deletions(-) create mode 100644 cloudinit/net/tests/__init__.py create mode 100644 cloudinit/net/tests/test_init.py (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index d1740e56..46cb9c85 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -10,6 +10,7 @@ import logging import os import re +from cloudinit.net.network_state import mask_to_net_prefix from cloudinit import util LOG = logging.getLogger(__name__) @@ -28,8 +29,13 @@ def _natural_sort_key(s, _nsre=re.compile('([0-9]+)')): for text in re.split(_nsre, s)] +def get_sys_class_path(): + """Simple function to return the global SYS_CLASS_NET.""" + return SYS_CLASS_NET + + def sys_dev_path(devname, path=""): - return SYS_CLASS_NET + devname + "/" + path + return get_sys_class_path() + devname + "/" + path def read_sys_net(devname, path, translate=None, @@ -77,7 +83,7 @@ def read_sys_net_int(iface, field): return None try: return int(val) - except TypeError: + except ValueError: return None @@ -149,7 +155,14 @@ def device_devid(devname): def get_devicelist(): - return os.listdir(SYS_CLASS_NET) + try: + devs = os.listdir(get_sys_class_path()) + except OSError as e: + if e.errno == errno.ENOENT: + devs = [] + else: + raise + return devs class ParserError(Exception): @@ -497,14 +510,8 @@ def get_interfaces_by_mac(): """Build a dictionary of tuples {mac: name}. Bridges and any devices that have a 'stolen' mac are excluded.""" - try: - devs = get_devicelist() - except OSError as e: - if e.errno == errno.ENOENT: - devs = [] - else: - raise ret = {} + devs = get_devicelist() empty_mac = '00:00:00:00:00:00' for name in devs: if not interface_has_own_mac(name): @@ -531,14 +538,8 @@ def get_interfaces(): """Return list of interface tuples (name, mac, driver, device_id) Bridges and any devices that have a 'stolen' mac are excluded.""" - try: - devs = get_devicelist() - except OSError as e: - if e.errno == errno.ENOENT: - devs = [] - else: - raise ret = [] + devs = get_devicelist() empty_mac = '00:00:00:00:00:00' for name in devs: if not interface_has_own_mac(name): @@ -557,6 +558,102 @@ def get_interfaces(): return ret +class EphemeralIPv4Network(object): + """Context manager which sets up temporary static network configuration. + + No operations are performed if the provided interface is already connected. + If unconnected, bring up the interface with valid ip, prefix and broadcast. + If router is provided setup a default route for that interface. Upon + context exit, clean up the interface leaving no configuration behind. + """ + + def __init__(self, interface, ip, prefix_or_mask, broadcast, router=None): + """Setup context manager and validate call signature. + + @param interface: Name of the network interface to bring up. + @param ip: IP address to assign to the interface. + @param prefix_or_mask: Either netmask of the format X.X.X.X or an int + prefix. + @param broadcast: Broadcast address for the IPv4 network. + @param router: Optionally the default gateway IP. + """ + if not all([interface, ip, prefix_or_mask, broadcast]): + raise ValueError( + 'Cannot init network on {0} with {1}/{2} and bcast {3}'.format( + interface, ip, prefix_or_mask, broadcast)) + try: + self.prefix = mask_to_net_prefix(prefix_or_mask) + except ValueError as e: + raise ValueError( + 'Cannot setup network: {0}'.format(e)) + self.interface = interface + self.ip = ip + self.broadcast = broadcast + self.router = router + self.cleanup_cmds = [] # List of commands to run to cleanup state. + + def __enter__(self): + """Perform ephemeral network setup if interface is not connected.""" + self._bringup_device() + if self.router: + self._bringup_router() + + def __exit__(self, excp_type, excp_value, excp_traceback): + for cmd in self.cleanup_cmds: + util.subp(cmd, capture=True) + + def _delete_address(self, address, prefix): + """Perform the ip command to remove the specified address.""" + util.subp( + ['ip', '-family', 'inet', 'addr', 'del', + '%s/%s' % (address, prefix), 'dev', self.interface], + capture=True) + + def _bringup_device(self): + """Perform the ip comands to fully setup the device.""" + cidr = '{0}/{1}'.format(self.ip, self.prefix) + LOG.debug( + 'Attempting setup of ephemeral network on %s with %s brd %s', + self.interface, cidr, self.broadcast) + try: + util.subp( + ['ip', '-family', 'inet', 'addr', 'add', cidr, 'broadcast', + self.broadcast, 'dev', self.interface], + capture=True, update_env={'LANG': 'C'}) + except util.ProcessExecutionError as e: + if "File exists" not in e.stderr: + raise + LOG.debug( + 'Skip ephemeral network setup, %s already has address %s', + self.interface, self.ip) + else: + # Address creation success, bring up device and queue cleanup + util.subp( + ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface, + 'up'], capture=True) + self.cleanup_cmds.append( + ['ip', '-family', 'inet', 'link', 'set', 'dev', self.interface, + 'down']) + self.cleanup_cmds.append( + ['ip', '-family', 'inet', 'addr', 'del', cidr, 'dev', + self.interface]) + + def _bringup_router(self): + """Perform the ip commands to fully setup the router if needed.""" + # Check if a default route exists and exit if it does + out, _ = util.subp(['ip', 'route', 'show', '0.0.0.0/0'], capture=True) + if 'default' in out: + LOG.debug( + 'Skip ephemeral route setup. %s already has default route: %s', + self.interface, out.strip()) + return + util.subp( + ['ip', '-4', 'route', 'add', 'default', 'via', self.router, + 'dev', self.interface], capture=True) + self.cleanup_cmds.insert( + 0, ['ip', '-4', 'route', 'del', 'default', 'dev', self.interface]) + + class RendererNotFoundError(RuntimeError): pass diff --git a/cloudinit/net/tests/__init__.py b/cloudinit/net/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py new file mode 100644 index 00000000..272a6ebd --- /dev/null +++ b/cloudinit/net/tests/test_init.py @@ -0,0 +1,522 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import copy +import errno +import mock +import os + +import cloudinit.net as net +from cloudinit.util import ensure_file, write_file, ProcessExecutionError +from tests.unittests.helpers import CiTestCase + + +class TestSysDevPath(CiTestCase): + + def test_sys_dev_path(self): + """sys_dev_path returns a path under SYS_CLASS_NET for a device.""" + dev = 'something' + path = 'attribute' + expected = net.SYS_CLASS_NET + dev + '/' + path + self.assertEqual(expected, net.sys_dev_path(dev, path)) + + def test_sys_dev_path_without_path(self): + """When path param isn't provided it defaults to empty string.""" + dev = 'something' + expected = net.SYS_CLASS_NET + dev + '/' + self.assertEqual(expected, net.sys_dev_path(dev)) + + +class TestReadSysNet(CiTestCase): + with_logs = True + + def setUp(self): + super(TestReadSysNet, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_read_sys_net_strips_contents_of_sys_path(self): + """read_sys_net strips whitespace from the contents of a sys file.""" + content = 'some stuff with trailing whitespace\t\r\n' + write_file(os.path.join(self.sysdir, 'dev', 'attr'), content) + self.assertEqual(content.strip(), net.read_sys_net('dev', 'attr')) + + def test_read_sys_net_reraises_oserror(self): + """read_sys_net raises OSError/IOError when file doesn't exist.""" + # Non-specific Exception because versions of python OSError vs IOError. + with self.assertRaises(Exception) as context_manager: # noqa: H202 + net.read_sys_net('dev', 'attr') + error = context_manager.exception + self.assertIn('No such file or directory', str(error)) + + def test_read_sys_net_handles_error_with_on_enoent(self): + """read_sys_net handles OSError/IOError with on_enoent if provided.""" + handled_errors = [] + + def on_enoent(e): + handled_errors.append(e) + + net.read_sys_net('dev', 'attr', on_enoent=on_enoent) + error = handled_errors[0] + self.assertIsInstance(error, Exception) + self.assertIn('No such file or directory', str(error)) + + def test_read_sys_net_translates_content(self): + """read_sys_net translates content when translate dict is provided.""" + content = "you're welcome\n" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), content) + translate = {"you're welcome": 'de nada'} + self.assertEqual( + 'de nada', + net.read_sys_net('dev', 'attr', translate=translate)) + + def test_read_sys_net_errors_on_translation_failures(self): + """read_sys_net raises a KeyError and logs details on failure.""" + content = "you're welcome\n" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), content) + with self.assertRaises(KeyError) as context_manager: + net.read_sys_net('dev', 'attr', translate={}) + error = context_manager.exception + self.assertEqual('"you\'re welcome"', str(error)) + self.assertIn( + "Found unexpected (not translatable) value 'you're welcome' in " + "'{0}dev/attr".format(self.sysdir), + self.logs.getvalue()) + + def test_read_sys_net_handles_handles_with_onkeyerror(self): + """read_sys_net handles translation errors calling on_keyerror.""" + content = "you're welcome\n" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), content) + handled_errors = [] + + def on_keyerror(e): + handled_errors.append(e) + + net.read_sys_net('dev', 'attr', translate={}, on_keyerror=on_keyerror) + error = handled_errors[0] + self.assertIsInstance(error, KeyError) + self.assertEqual('"you\'re welcome"', str(error)) + + def test_read_sys_net_safe_false_on_translate_failure(self): + """read_sys_net_safe returns False on translation failures.""" + content = "you're welcome\n" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), content) + self.assertFalse(net.read_sys_net_safe('dev', 'attr', translate={})) + + def test_read_sys_net_safe_returns_false_on_noent_failure(self): + """read_sys_net_safe returns False on file not found failures.""" + self.assertFalse(net.read_sys_net_safe('dev', 'attr')) + + def test_read_sys_net_int_returns_none_on_error(self): + """read_sys_net_safe returns None on failures.""" + self.assertFalse(net.read_sys_net_int('dev', 'attr')) + + def test_read_sys_net_int_returns_none_on_valueerror(self): + """read_sys_net_safe returns None when content is not an int.""" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), 'NOTINT\n') + self.assertFalse(net.read_sys_net_int('dev', 'attr')) + + def test_read_sys_net_int_returns_integer_from_content(self): + """read_sys_net_safe returns None on failures.""" + write_file(os.path.join(self.sysdir, 'dev', 'attr'), '1\n') + self.assertEqual(1, net.read_sys_net_int('dev', 'attr')) + + def test_is_up_true(self): + """is_up is True if sys/net/devname/operstate is 'up' or 'unknown'.""" + for state in ['up', 'unknown']: + write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), state) + self.assertTrue(net.is_up('eth0')) + + def test_is_up_false(self): + """is_up is False if sys/net/devname/operstate is 'down' or invalid.""" + for state in ['down', 'incomprehensible']: + write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), state) + self.assertFalse(net.is_up('eth0')) + + def test_is_wireless(self): + """is_wireless is True when /sys/net/devname/wireless exists.""" + self.assertFalse(net.is_wireless('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'wireless')) + self.assertTrue(net.is_wireless('eth0')) + + def test_is_bridge(self): + """is_bridge is True when /sys/net/devname/bridge exists.""" + self.assertFalse(net.is_bridge('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'bridge')) + self.assertTrue(net.is_bridge('eth0')) + + def test_is_bond(self): + """is_bond is True when /sys/net/devname/bonding exists.""" + self.assertFalse(net.is_bond('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding')) + self.assertTrue(net.is_bond('eth0')) + + def test_is_vlan(self): + """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan.""" + ensure_file(os.path.join(self.sysdir, 'eth0', 'uevent')) + self.assertFalse(net.is_vlan('eth0')) + content = 'junk\nDEVTYPE=vlan\njunk\n' + write_file(os.path.join(self.sysdir, 'eth0', 'uevent'), content) + self.assertTrue(net.is_vlan('eth0')) + + def test_is_connected_when_physically_connected(self): + """is_connected is True when /sys/net/devname/iflink reports 2.""" + self.assertFalse(net.is_connected('eth0')) + write_file(os.path.join(self.sysdir, 'eth0', 'iflink'), "2") + self.assertTrue(net.is_connected('eth0')) + + def test_is_connected_when_wireless_and_carrier_active(self): + """is_connected is True if wireless /sys/net/devname/carrier is 1.""" + self.assertFalse(net.is_connected('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'wireless')) + self.assertFalse(net.is_connected('eth0')) + write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), "1") + self.assertTrue(net.is_connected('eth0')) + + def test_is_physical(self): + """is_physical is True when /sys/net/devname/device exists.""" + self.assertFalse(net.is_physical('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'device')) + self.assertTrue(net.is_physical('eth0')) + + def test_is_present(self): + """is_present is True when /sys/net/devname exists.""" + self.assertFalse(net.is_present('eth0')) + ensure_file(os.path.join(self.sysdir, 'eth0', 'device')) + self.assertTrue(net.is_present('eth0')) + + +class TestGenerateFallbackConfig(CiTestCase): + + def setUp(self): + super(TestGenerateFallbackConfig, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_generate_fallback_finds_connected_eth_with_mac(self): + """generate_fallback_config finds any connected device with a mac.""" + write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), '1') + write_file(os.path.join(self.sysdir, 'eth1', 'carrier'), '1') + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac) + expected = { + 'config': [{'type': 'physical', 'mac_address': mac, + 'name': 'eth1', 'subnets': [{'type': 'dhcp'}]}], + 'version': 1} + self.assertEqual(expected, net.generate_fallback_config()) + + def test_generate_fallback_finds_dormant_eth_with_mac(self): + """generate_fallback_config finds any dormant device with a mac.""" + write_file(os.path.join(self.sysdir, 'eth0', 'dormant'), '1') + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac) + expected = { + 'config': [{'type': 'physical', 'mac_address': mac, + 'name': 'eth0', 'subnets': [{'type': 'dhcp'}]}], + 'version': 1} + self.assertEqual(expected, net.generate_fallback_config()) + + def test_generate_fallback_finds_eth_by_operstate(self): + """generate_fallback_config finds any dormant device with a mac.""" + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac) + expected = { + 'config': [{'type': 'physical', 'mac_address': mac, + 'name': 'eth0', 'subnets': [{'type': 'dhcp'}]}], + 'version': 1} + valid_operstates = ['dormant', 'down', 'lowerlayerdown', 'unknown'] + for state in valid_operstates: + write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), state) + self.assertEqual(expected, net.generate_fallback_config()) + write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), 'noworky') + self.assertIsNone(net.generate_fallback_config()) + + def test_generate_fallback_config_skips_veth(self): + """generate_fallback_config will skip any veth interfaces.""" + # A connected veth which gets ignored + write_file(os.path.join(self.sysdir, 'veth0', 'carrier'), '1') + self.assertIsNone(net.generate_fallback_config()) + + def test_generate_fallback_config_skips_bridges(self): + """generate_fallback_config will skip any bridges interfaces.""" + # A connected veth which gets ignored + write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), '1') + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac) + ensure_file(os.path.join(self.sysdir, 'eth0', 'bridge')) + self.assertIsNone(net.generate_fallback_config()) + + def test_generate_fallback_config_skips_bonds(self): + """generate_fallback_config will skip any bonded interfaces.""" + # A connected veth which gets ignored + write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), '1') + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth0', 'address'), mac) + ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding')) + self.assertIsNone(net.generate_fallback_config()) + + +class TestGetDeviceList(CiTestCase): + + def setUp(self): + super(TestGetDeviceList, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_get_devicelist_raise_oserror(self): + """get_devicelist raise any non-ENOENT OSerror.""" + error = OSError('Can not do it') + error.errno = errno.EPERM # Set non-ENOENT + self.m_sys_path.side_effect = error + with self.assertRaises(OSError) as context_manager: + net.get_devicelist() + exception = context_manager.exception + self.assertEqual('Can not do it', str(exception)) + + def test_get_devicelist_empty_without_sys_net(self): + """get_devicelist returns empty list when missing SYS_CLASS_NET.""" + self.m_sys_path.return_value = 'idontexist' + self.assertEqual([], net.get_devicelist()) + + def test_get_devicelist_empty_with_no_devices_in_sys_net(self): + """get_devicelist returns empty directoty listing for SYS_CLASS_NET.""" + self.assertEqual([], net.get_devicelist()) + + def test_get_devicelist_lists_any_subdirectories_in_sys_net(self): + """get_devicelist returns a directory listing for SYS_CLASS_NET.""" + write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), 'up') + write_file(os.path.join(self.sysdir, 'eth1', 'operstate'), 'up') + self.assertItemsEqual(['eth0', 'eth1'], net.get_devicelist()) + + +class TestGetInterfaceMAC(CiTestCase): + + def setUp(self): + super(TestGetInterfaceMAC, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_get_interface_mac_false_with_no_mac(self): + """get_device_list returns False when no mac is reported.""" + ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding')) + mac_path = os.path.join(self.sysdir, 'eth0', 'address') + self.assertFalse(os.path.exists(mac_path)) + self.assertFalse(net.get_interface_mac('eth0')) + + def test_get_interface_mac(self): + """get_interfaces returns the mac from SYS_CLASS_NET/dev/address.""" + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac) + self.assertEqual(mac, net.get_interface_mac('eth1')) + + def test_get_interface_mac_grabs_bonding_address(self): + """get_interfaces returns the source device mac for bonded devices.""" + source_dev_mac = 'aa:bb:cc:aa:bb:cc' + bonded_mac = 'dd:ee:ff:dd:ee:ff' + write_file(os.path.join(self.sysdir, 'eth1', 'address'), bonded_mac) + write_file( + os.path.join(self.sysdir, 'eth1', 'bonding_slave', 'perm_hwaddr'), + source_dev_mac) + self.assertEqual(source_dev_mac, net.get_interface_mac('eth1')) + + def test_get_interfaces_empty_list_without_sys_net(self): + """get_interfaces returns an empty list when missing SYS_CLASS_NET.""" + self.m_sys_path.return_value = 'idontexist' + self.assertEqual([], net.get_interfaces()) + + def test_get_interfaces_by_mac_skips_empty_mac(self): + """Ignore 00:00:00:00:00:00 addresses from get_interfaces_by_mac.""" + empty_mac = '00:00:00:00:00:00' + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth1', 'address'), empty_mac) + write_file(os.path.join(self.sysdir, 'eth1', 'addr_assign_type'), '0') + write_file(os.path.join(self.sysdir, 'eth2', 'addr_assign_type'), '0') + write_file(os.path.join(self.sysdir, 'eth2', 'address'), mac) + expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)] + self.assertEqual(expected, net.get_interfaces()) + + def test_get_interfaces_by_mac_skips_missing_mac(self): + """Ignore interfaces without an address from get_interfaces_by_mac.""" + write_file(os.path.join(self.sysdir, 'eth1', 'addr_assign_type'), '0') + address_path = os.path.join(self.sysdir, 'eth1', 'address') + self.assertFalse(os.path.exists(address_path)) + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth2', 'addr_assign_type'), '0') + write_file(os.path.join(self.sysdir, 'eth2', 'address'), mac) + expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)] + self.assertEqual(expected, net.get_interfaces()) + + +class TestInterfaceHasOwnMAC(CiTestCase): + + def setUp(self): + super(TestInterfaceHasOwnMAC, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_interface_has_own_mac_false_when_stolen(self): + """Return False from interface_has_own_mac when address is stolen.""" + write_file(os.path.join(self.sysdir, 'eth1', 'addr_assign_type'), '2') + self.assertFalse(net.interface_has_own_mac('eth1')) + + def test_interface_has_own_mac_true_when_not_stolen(self): + """Return False from interface_has_own_mac when mac isn't stolen.""" + valid_assign_types = ['0', '1', '3'] + assign_path = os.path.join(self.sysdir, 'eth1', 'addr_assign_type') + for _type in valid_assign_types: + write_file(assign_path, _type) + self.assertTrue(net.interface_has_own_mac('eth1')) + + def test_interface_has_own_mac_strict_errors_on_absent_assign_type(self): + """When addr_assign_type is absent, interface_has_own_mac errors.""" + with self.assertRaises(ValueError): + net.interface_has_own_mac('eth1', strict=True) + + +@mock.patch('cloudinit.net.util.subp') +class TestEphemeralIPV4Network(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestEphemeralIPV4Network, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + + def test_ephemeral_ipv4_network_errors_on_missing_params(self, m_subp): + """No required params for EphemeralIPv4Network can be None.""" + required_params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255'} + for key in required_params.keys(): + params = copy.deepcopy(required_params) + params[key] = None + with self.assertRaises(ValueError) as context_manager: + net.EphemeralIPv4Network(**params) + error = context_manager.exception + self.assertIn('Cannot init network on', str(error)) + self.assertEqual(0, m_subp.call_count) + + def test_ephemeral_ipv4_network_errors_invalid_mask(self, m_subp): + """Raise an error when prefix_or_mask is not a netmask or prefix.""" + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'broadcast': '192.168.2.255'} + invalid_masks = ('invalid', 'invalid.', '123.123.123') + for error_val in invalid_masks: + params['prefix_or_mask'] = error_val + with self.assertRaises(ValueError) as context_manager: + with net.EphemeralIPv4Network(**params): + pass + error = context_manager.exception + self.assertIn('Cannot setup network: netmask', str(error)) + self.assertEqual(0, m_subp.call_count) + + def test_ephemeral_ipv4_network_performs_teardown(self, m_subp): + """EphemeralIPv4Network performs teardown on the device if setup.""" + expected_setup_calls = [ + mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'}), + mock.call( + ['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', 'up'], + capture=True)] + expected_teardown_calls = [ + mock.call( + ['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', + 'down'], capture=True), + mock.call( + ['ip', '-family', 'inet', 'addr', 'del', '192.168.2.2/24', + 'dev', 'eth0'], capture=True)] + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255'} + with net.EphemeralIPv4Network(**params): + self.assertEqual(expected_setup_calls, m_subp.call_args_list) + m_subp.assert_has_calls(expected_teardown_calls) + + def test_ephemeral_ipv4_network_noop_when_configured(self, m_subp): + """EphemeralIPv4Network handles exception when address is setup. + + It performs no cleanup as the interface was already setup. + """ + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255'} + m_subp.side_effect = ProcessExecutionError( + '', 'RTNETLINK answers: File exists', 2) + expected_calls = [ + mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'})] + with net.EphemeralIPv4Network(**params): + pass + self.assertEqual(expected_calls, m_subp.call_args_list) + self.assertIn( + 'Skip ephemeral network setup, eth0 already has address', + self.logs.getvalue()) + + def test_ephemeral_ipv4_network_with_prefix(self, m_subp): + """EphemeralIPv4Network takes a valid prefix to setup the network.""" + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '24', 'broadcast': '192.168.2.255'} + for prefix_val in ['24', 16]: # prefix can be int or string + params['prefix_or_mask'] = prefix_val + with net.EphemeralIPv4Network(**params): + pass + m_subp.assert_has_calls([mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'})]) + m_subp.assert_has_calls([mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/16', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'})]) + + def test_ephemeral_ipv4_network_with_new_default_route(self, m_subp): + """Add the route when router is set and no default route exists.""" + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255', + 'router': '192.168.2.1'} + m_subp.return_value = '', '' # Empty response from ip route gw check + expected_setup_calls = [ + mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'}), + mock.call( + ['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', 'up'], + capture=True), + mock.call( + ['ip', 'route', 'show', '0.0.0.0/0'], capture=True), + mock.call( + ['ip', '-4', 'route', 'add', 'default', 'via', + '192.168.2.1', 'dev', 'eth0'], capture=True)] + expected_teardown_calls = [mock.call( + ['ip', '-4', 'route', 'del', 'default', 'dev', 'eth0'], + capture=True)] + + with net.EphemeralIPv4Network(**params): + self.assertEqual(expected_setup_calls, m_subp.call_args_list) + m_subp.assert_has_calls(expected_teardown_calls) diff --git a/setup.py b/setup.py index b1bde43b..5c65c7fe 100755 --- a/setup.py +++ b/setup.py @@ -240,7 +240,7 @@ setuptools.setup( author='Scott Moser', author_email='scott.moser@canonical.com', url='http://launchpad.net/cloud-init/', - packages=setuptools.find_packages(exclude=['tests']), + packages=setuptools.find_packages(exclude=['tests.*', '*.tests', 'tests']), scripts=['tools/cloud-init-per'], license='Dual-licensed under GPLv3 or Apache 2.0', data_files=data_files, diff --git a/tox.ini b/tox.ini index 1140f9b2..ef768847 100644 --- a/tox.ini +++ b/tox.ini @@ -21,7 +21,11 @@ setenv = LC_ALL = en_US.utf-8 [testenv:pylint] -deps = pylint==1.7.1 +deps = + # requirements + pylint==1.7.1 + # test-requirements because unit tests are now present in cloudinit tree + -r{toxinidir}/test-requirements.txt commands = {envpython} -m pylint {posargs:cloudinit} [testenv:py3] @@ -29,7 +33,7 @@ basepython = python3 deps = -r{toxinidir}/test-requirements.txt commands = {envpython} -m nose {posargs:--with-coverage \ --cover-erase --cover-branches --cover-inclusive \ - --cover-package=cloudinit tests/unittests} + --cover-package=cloudinit tests/unittests cloudinit} [testenv:py27] basepython = python2.7 @@ -98,7 +102,11 @@ deps = pyflakes [testenv:tip-pylint] commands = {envpython} -m pylint {posargs:cloudinit} -deps = pylint +deps = + # requirements + pylint + # test-requirements + -r{toxinidir}/test-requirements.txt [testenv:citest] basepython = python3 -- cgit v1.2.3