From 7f2e99f5345c227d07849da68acdf8562b44c3e1 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 25 May 2016 17:05:09 -0400 Subject: commit to push for fear of loss. == background == DataSource Mode (dsmode) is present in many datasources in cloud-init. dsmode was originally added to cloud-init to specify when this datasource should be 'realized'. cloud-init has 4 stages of boot. a.) cloud-init --local . network is guaranteed not present. b.) cloud-init (--network). network is guaranteed present. c.) cloud-config d.) cloud-init final 'init_modules' [1] are run "as early as possible". And as such, are executed in either 'a' or 'b' based on the datasource. However, executing them means that user-data has been fully consumed. User-data and vendor-data may have '#include http://...' which then rely on the network being present. boothooks are an example of the things run in init_modules. The 'dsmode' was a way for a user to indicate that init_modules should run at 'a' (dsmode=local) or 'b' (dsmode=net) directly. Things were further confused when a datasource could provide networking configuration. Then, we needed to apply the networking config at 'a' but if the user had provided boothooks that expected networking, then the init_modules would need to be executed at 'b'. The config drive datasource hacked its way through this and applies networking if *it* detects it is a new instance. == Suggested Change == The plan is to 1. incorporate 'dsmode' into DataSource superclass 2. make all existing datasources default to network 3. apply any networking configuration from a datasource on first boot only apply_networking will always rename network devices when it runs. for bug 1579130. 4. run init_modules at cloud-init (network) time frame unless datasource is 'local'. 5. Datasources can provide a 'first_boot' method that will be called when a new instance_id is found. This will allow the config drive's write_files to be applied once. Over all, this will very much simplify things. We'll no longer have 2 sources like DataSourceNoCloud and DataSourceNoCloudNet, but would just have one source with a dsmode. == Concerns == Some things have odd reliance on dsmode. For example, OpenNebula's get_hostname uses it to determine if it should do a lookup of an ip address. == Bugs to fix here == http://pad.lv/1577982 ConfigDrive: cloud-init fails to configure network from network_data.json http://pad.lv/1579130 need to support systemd.link renaming of devices in container http://pad.lv/1577844 Drop unnecessary blocking of all net udev rules --- cloudinit/net/__init__.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 91e36aca..40d330b5 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -768,4 +768,49 @@ def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None): return config_from_klibc_net_cfg(files=files, mac_addrs=mac_addrs) +def convert_eni_data(eni_data): + # return a network config representation of what is in eni_data + ifaces = {} + parse_deb_config_data(ifaces, eni_data, src_dir=None, src_path=None) + return _ifaces_to_net_config_data(ifaces) + + +def _ifaces_to_net_config_data(ifaces): + """Return network config that represents the ifaces data provided. + ifaces = parse_deb_config("/etc/network/interfaces") + config = ifaces_to_net_config_data(ifaces) + state = parse_net_config_data(config).""" + devs = {} + for name, data in ifaces.items(): + # devname is 'eth0' for name='eth0:1' + devname = name.partition(":")[0] + if devname not in devs: + devs[devname] = {'type': 'physical', 'name': devname, + 'subnets': []} + # this isnt strictly correct, but some might specify + # hwaddress on a nic for matching / declaring name. + if 'hwaddress' in data: + devs[devname]['mac_address'] = data['hwaddress'] + subnet = {'_orig_eni_name': name, 'type': data['method']} + if data.get('auto'): + subnet['control'] = 'auto' + else: + subnet['control'] = 'manual' + + if data.get('method') == 'static': + subnet['address'] = data['address'] + + if 'gateway' in data: + subnet['gateway'] = data['gateway'] + + if 'dns' in data: + for n in ('nameservers', 'search'): + if n in data['dns'] and data['dns'][n]: + subnet['dns_' + n] = data['dns'][n] + devs[devname]['subnets'].append(subnet) + + return {'version': 1, + 'config': [devs[d] for d in sorted(devs)]} + + # vi: ts=4 expandtab syntax=python -- cgit v1.2.3 From 1b8a09389654a29af7e618b803bffaed0185e9e8 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 31 May 2016 17:17:39 -0400 Subject: add renaming code for renaming interfaces currently does not work in lxc https://github.com/lxc/lxd/issues/2063 --- bin/cloud-init | 6 +-- cloudinit/distros/__init__.py | 4 ++ cloudinit/net/__init__.py | 86 +++++++++++++++++++++++++++++++++++++++++++ cloudinit/stages.py | 10 +++++ 4 files changed, 101 insertions(+), 5 deletions(-) (limited to 'cloudinit/net') diff --git a/bin/cloud-init b/bin/cloud-init index 29e9b521..21c3a684 100755 --- a/bin/cloud-init +++ b/bin/cloud-init @@ -280,11 +280,7 @@ def main_init(name, args): LOG.debug("[%s] %s will now be targeting instance id: %s. new=%s", mode, name, iid, init.is_new_instance()) - if init.is_new_instance(): - # on new instance, apply network config. - # in network mode 'bring_up' must be passed in as the OS - # has already brought up networking. - init.apply_network_config(bring_up=bool(mode != sources.DSMODE_LOCAL)) + init.apply_network_config(bring_up=bool(mode != sources.DSMODE_LOCAL)) if mode == sources.DSMODE_LOCAL: if init.datasource.dsmode != mode: diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 3bfbc484..5c29c804 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -31,6 +31,7 @@ import stat from cloudinit import importer from cloudinit import log as logging +from cloudinit import net from cloudinit import ssh_util from cloudinit import type_utils from cloudinit import util @@ -145,6 +146,9 @@ class Distro(object): return self._bring_up_interfaces(dev_names) return False + def apply_network_config_names(self, netconfig): + net.apply_network_config_names(netconfig) + @abc.abstractmethod def apply_locale(self, locale, out_fn=None): raise NotImplementedError() diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 40d330b5..ec1b3835 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -813,4 +813,90 @@ def _ifaces_to_net_config_data(ifaces): 'config': [devs[d] for d in sorted(devs)]} +def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): + """read the network config and rename devices accordingly. + if strict_present is false, then do not raise exception if no devices + match. if strict_busy is false, then do not raise exception if the + device cannot be renamed because it is currently configured.""" + renames = [] + for ent in netcfg.get('config', {}): + if ent.get('type') != 'physical': + continue + mac = ent.get('mac_address') + name = ent.get('name') + if not mac: + continue + renames.append([mac, name]) + + return rename_interfaces(renames) + + +def rename_interfaces(renames, strict_present=True, strict_busy=True): + cur_bymac = {get_interface_mac(n): n for n in get_devicelist()} + expected = {mac: name for mac, name in renames} + cur_byname = {v: k for k, v in cur_bymac.items()} + + tmpname_fmt = "cirename%d" + tmpi = -1 + + moves = [] + changes = [] + errors = [] + for mac, new_name in expected.items(): + cur_name = cur_bymac.get(mac) + if cur_name == new_name: + # nothing to do + continue + + if not cur_name: + if strict_present: + errors.append( + "[nic not present] Cannot rename mac=%s to %s" + ", not available." % (mac, new_name)) + elif is_up(cur_name): + if strict_busy: + errors.append("[busy] Error renaming mac=%s from %s to %s." % + (mac, cur_name, new_name)) + elif new_name in cur_byname: + if is_up(new_name): + if strict_busy: + errors.append( + "[busy-target] Error renaming mac=%s from %s to %s." % + (mac, cur_name, new_name)) + else: + tmp_name = None + while tmp_name is None or tmp_name in cur_byname: + tmpi += 1 + tmp_name = tmpname_fmt % tmpi + moves.append((mac, cur_name, tmp_name)) + changes.append((mac, tmp_name, new_name)) + else: + changes.append((mac, cur_name, new_name)) + + def rename_dev(cur, new): + cmd = ["ip", "link", "set", cur, "name", new] + util.subp(cmd) + + for mac, cur, new in moves + changes: + try: + rename_dev(cur, new) + except util.ProcessExecutionError as e: + errors.append( + "[unknown] Error renaming mac=%s from %s to %s. (%s)" % + (mac, cur, new, e)) + + if len(errors): + raise Exception('\n'.join(errors)) + + +def get_interface_mac(ifname): + """Returns the string value of an interface's MAC Address""" + return read_sys_net(ifname, "address", enoent=False) + + +def get_ifname_mac_pairs(): + """Build a list of tuples (ifname, mac)""" + return [(ifname, get_interface_mac(ifname)) for ifname in get_devicelist()] + + # vi: ts=4 expandtab syntax=python diff --git a/cloudinit/stages.py b/cloudinit/stages.py index f164d6f6..211d2286 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -632,6 +632,16 @@ class Init(object): LOG.info("network config is disabled by %s", src) return + try: + LOG.debug("applying net config names for %s" % netcfg) + self.distro.apply_network_config_names(netcfg) + except Exception as e: + LOG.warn("Failed to rename devices: %s", e) + + if not self.is_new_instance(): + LOG.debug("not a new instance. network config is not applied.") + return + LOG.info("Applying network configuration from %s bringup=%s: %s", src, bring_up, netcfg) try: -- cgit v1.2.3 From f67e8f104b199c9402cf047637b939516526e0ac Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 1 Jun 2016 17:14:50 -0400 Subject: support renaming and fix logic in rename_interfaces The one issue i'm aware of currently is that tap devices (ip tuntap add mode tap user root mytap1) do not work correctly with 'is_up' which means the check does not bring them down and the rename fails. The LOG.debug message should be cleaned up too, as it currently references the function rather function.__name__ for nicer message. --- cloudinit/net/__init__.py | 133 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 98 insertions(+), 35 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index ec1b3835..f5ae7705 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -831,19 +831,65 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): return rename_interfaces(renames) -def rename_interfaces(renames, strict_present=True, strict_busy=True): - cur_bymac = {get_interface_mac(n): n for n in get_devicelist()} - expected = {mac: name for mac, name in renames} - cur_byname = {v: k for k, v in cur_bymac.items()} +def _get_current_rename_info(check_downable=True): + """Collect information necessary for rename_interfaces.""" + names = get_devicelist() + bymac = {} + for n in names: + bymac[get_interface_mac(n)] = { + 'name': n, 'up': is_up(n), 'downable': None} + if check_downable: + nmatch = re.compile(r"[0-9]+:\s+(\w+)[@:]") + ipv6, _err = util.subp(['ip', '-6', 'addr', 'show', 'permanent', + 'scope', 'global'], capture=True) + ipv4, _err = util.subp(['ip', '-4', 'addr', 'show'], capture=True) + + nics_with_addresses = set() + for bytes_out in (ipv6, ipv4): + nics_with_addresses.update(nmatch.findall(bytes_out)) + + for d in bymac.values(): + d['downable'] = (d['up'] is False or + d['name'] not in nics_with_addresses) + + return bymac + + +def rename_interfaces(renames, strict_present=True, strict_busy=True, + current_info=None): + if current_info is None: + current_info = _get_current_rename_info() + + cur_bymac = {} + for mac, data in current_info.items(): + cur = data.copy() + cur['mac'] = mac + cur_bymac[mac] = cur + + def update_byname(bymac): + return {data['name']: data for data in bymac.values()} + + def rename(cur, new): + util.subp(["ip", "link", "set", cur, "name", new], capture=True) + + def down(name): + util.subp(["ip", "link", "set", name, "down"], capture=True) + + def up(name): + util.subp(["ip", "link", "set", name, "up"], capture=True) + + ops = [] + errors = [] + ups = [] + cur_byname = update_byname(cur_bymac) tmpname_fmt = "cirename%d" tmpi = -1 - moves = [] - changes = [] - errors = [] - for mac, new_name in expected.items(): - cur_name = cur_bymac.get(mac) + for mac, new_name in renames: + cur = cur_bymac.get(mac, {}) + cur_name = cur.get('name') + cur_ops = [] if cur_name == new_name: # nothing to do continue @@ -853,37 +899,54 @@ def rename_interfaces(renames, strict_present=True, strict_busy=True): errors.append( "[nic not present] Cannot rename mac=%s to %s" ", not available." % (mac, new_name)) - elif is_up(cur_name): - if strict_busy: - errors.append("[busy] Error renaming mac=%s from %s to %s." % - (mac, cur_name, new_name)) - elif new_name in cur_byname: - if is_up(new_name): + continue + + if cur['up']: + msg = "[busy] Error renaming mac=%s from %s to %s" + if not cur['downable']: if strict_busy: - errors.append( - "[busy-target] Error renaming mac=%s from %s to %s." % - (mac, cur_name, new_name)) - else: - tmp_name = None - while tmp_name is None or tmp_name in cur_byname: - tmpi += 1 - tmp_name = tmpname_fmt % tmpi - moves.append((mac, cur_name, tmp_name)) - changes.append((mac, tmp_name, new_name)) - else: - changes.append((mac, cur_name, new_name)) + errors.append(msg % (mac, cur_name, new_name)) + continue + cur['up'] = False + cur_ops.append((down, mac, new_name, (cur_name,))) + ups.append((up, mac, new_name, (new_name,))) + + if new_name in cur_byname: + target = cur_byname[new_name] + if target['up']: + msg = "[busy-target] Error renaming mac=%s from %s to %s." + if not target['downable']: + if strict_busy: + errors.append(msg % (mac, cur_name, new_name)) + continue + else: + cur_ops.append((down, mac, new_name, (new_name,))) + + tmp_name = None + while tmp_name is None or tmp_name in cur_byname: + tmpi += 1 + tmp_name = tmpname_fmt % tmpi + + cur_ops.append((rename, mac, new_name, (new_name, tmp_name))) + target['name'] = tmp_name + cur_byname = update_byname(cur_bymac) + 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) + ops += cur_ops - def rename_dev(cur, new): - cmd = ["ip", "link", "set", cur, "name", new] - util.subp(cmd) + LOG.debug("achieving renaming of %s with ops %s", renames, ops + ups) - for mac, cur, new in moves + changes: + for op, mac, new_name, params in ops + ups: try: - rename_dev(cur, new) - except util.ProcessExecutionError as e: + op(*params) + except Exception as e: errors.append( - "[unknown] Error renaming mac=%s from %s to %s. (%s)" % - (mac, cur, new, e)) + "[unknown] Error performing %s%s for %s, %s: %s" % + (op.__name__, params, mac, new_name, e)) if len(errors): raise Exception('\n'.join(errors)) -- cgit v1.2.3 From 7f46de87ee543a82c9a95137478676edaba2acc1 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Wed, 1 Jun 2016 20:23:00 -0400 Subject: clean up log message a bit. --- cloudinit/net/__init__.py | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index f5ae7705..6d9ea575 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -908,8 +908,8 @@ def rename_interfaces(renames, strict_present=True, strict_busy=True, errors.append(msg % (mac, cur_name, new_name)) continue cur['up'] = False - cur_ops.append((down, mac, new_name, (cur_name,))) - ups.append((up, mac, new_name, (new_name,))) + cur_ops.append(("down", mac, new_name, (cur_name,))) + ups.append(("up", mac, new_name, (new_name,))) if new_name in cur_byname: target = cur_byname[new_name] @@ -920,33 +920,41 @@ def rename_interfaces(renames, strict_present=True, strict_busy=True, errors.append(msg % (mac, cur_name, new_name)) continue else: - cur_ops.append((down, mac, new_name, (new_name,))) + cur_ops.append(("down", mac, new_name, (new_name,))) tmp_name = None while tmp_name is None or tmp_name in cur_byname: tmpi += 1 tmp_name = tmpname_fmt % tmpi - cur_ops.append((rename, mac, new_name, (new_name, tmp_name))) + cur_ops.append(("rename", mac, new_name, (new_name, tmp_name))) target['name'] = tmp_name cur_byname = update_byname(cur_bymac) if target['up']: - ups.append((up, mac, new_name, (tmp_name,))) + ups.append(("up", mac, new_name, (tmp_name,))) - cur_ops.append((rename, mac, new_name, (cur['name'], new_name))) + cur_ops.append(("rename", mac, new_name, (cur['name'], new_name))) cur['name'] = new_name cur_byname = update_byname(cur_bymac) ops += cur_ops - LOG.debug("achieving renaming of %s with ops %s", renames, ops + ups) + opmap = {'rename': rename, 'down': down, 'up': up} - for op, mac, new_name, params in ops + ups: - try: - op(*params) - except Exception as e: - errors.append( - "[unknown] Error performing %s%s for %s, %s: %s" % - (op.__name__, params, mac, new_name, e)) + if len(ops) + len(ups) == 0: + if len(errors): + LOG.debug("unable to do any work for renaming of %s", renames) + else: + LOG.debug("no work necessary for renaming of %s", renames) + else: + LOG.debug("achieving renaming of %s with ops %s", renames, ops + ups) + + for op, mac, new_name, params in ops + ups: + try: + opmap.get(op)(*params) + except Exception as e: + errors.append( + "[unknown] Error performing %s%s for %s, %s: %s" % + (op, params, mac, new_name, e)) if len(errors): raise Exception('\n'.join(errors)) -- cgit v1.2.3 From 80648a623fe6c7ae397629da30c04e52d79759f2 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 2 Jun 2016 13:19:50 -0400 Subject: eni parsing: support 'ether' in hwaddress, netmask and broadcast this adds ability to support ENI that has: hwadress ether 36:4c:e1:3b:14:31 or hwaddress 36:4c:e1:3b:14:31 the former is written by openstack (at least on dreamhost). Also, in the conversion of eni to network config support broadcast and netmask. --- cloudinit/net/__init__.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 6d9ea575..05152ead 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -201,7 +201,11 @@ def parse_deb_config_data(ifaces, contents, src_dir, src_path): ifaces[iface]['method'] = method currif = iface elif option == "hwaddress": - ifaces[currif]['hwaddress'] = split[1] + if split[1] == "ether": + val = split[2] + else: + val = split[1] + ifaces[currif]['hwaddress'] = val elif option in NET_CONFIG_OPTIONS: ifaces[currif][option] = split[1] elif option in NET_CONFIG_COMMANDS: @@ -800,8 +804,9 @@ def _ifaces_to_net_config_data(ifaces): if data.get('method') == 'static': subnet['address'] = data['address'] - if 'gateway' in data: - subnet['gateway'] = data['gateway'] + for copy_key in ('netmask', 'gateway', 'broadcast'): + if copy_key in data: + subnet[copy_key] = data[copy_key] if 'dns' in data: for n in ('nameservers', 'search'): -- cgit v1.2.3 From 6bd7fbc35ac8726a8a0f422cd802d290c236bf3b Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 2 Jun 2016 23:03:38 -0400 Subject: ConfigDrive: do not use 'id' on a link for the device name 'id' on a link in the openstack spec should be "Generic, generated ID". current implementation was to use the host's name for the host side nic. Which provided names like 'tap-adfasdffd'. We do not want to name devices like that as its quite unexpected and non user friendly. So here we use the system name for any nic that is present, but then require that the nics found also be present at the time of rendering. The end result is that if the system boots with net.ifnames=0 then it will get 'eth0' like names. and if it boots without net.ifnames then it will get enp0s1 like names. --- cloudinit/net/__init__.py | 15 +++++--- cloudinit/sources/DataSourceConfigDrive.py | 29 ++++++++++++--- .../unittests/test_datasource/test_configdrive.py | 41 ++++++++++++++++++++-- 3 files changed, 74 insertions(+), 11 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 05152ead..f47053b2 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -970,9 +970,16 @@ def get_interface_mac(ifname): return read_sys_net(ifname, "address", enoent=False) -def get_ifname_mac_pairs(): - """Build a list of tuples (ifname, mac)""" - return [(ifname, get_interface_mac(ifname)) for ifname in get_devicelist()] - +def get_interfaces_by_mac(devs=None): + """Build a dictionary of tuples {mac: name}""" + if devs is None: + devs = get_devicelist() + ret = {} + for name in devs: + mac = get_interface_mac(name) + # some devices may not have a mac (tun0) + if mac: + ret[mac] = name + return ret # vi: ts=4 expandtab syntax=python diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index 61d967d9..3cc9155d 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -255,7 +255,7 @@ def find_candidate_devs(probe_optical=True): # Convert OpenStack ConfigDrive NetworkData json to network_config yaml -def convert_network_data(network_json=None): +def convert_network_data(network_json=None, known_macs=None): """Return a dictionary of network_config by parsing provided OpenStack ConfigDrive NetworkData json format @@ -319,9 +319,15 @@ def convert_network_data(network_json=None): subnets = [] cfg = {k: v for k, v in link.items() if k in valid_keys['physical']} - cfg.update({'name': link['id']}) - for network in [net for net in networks - if net['link'] == link['id']]: + # 'name' is not in openstack spec yet, but we will support it if it is + # present. The 'id' in the spec is currently implemented as the host + # nic's name, meaning something like 'tap-adfasdffd'. We do not want + # to name guest devices with such ugly names. + if 'name' in link: + cfg['name'] = link['name'] + + for network in [n for n in networks + if n['link'] == link['id']]: subnet = {k: v for k, v in network.items() if k in valid_keys['subnet']} if 'dhcp' in network['type']: @@ -365,6 +371,21 @@ def convert_network_data(network_json=None): config.append(cfg) + need_names = [d for d in config + if d.get('type') == 'physical' and 'name' not in d] + + if need_names: + if known_macs is None: + known_macs = net.get_interfaces_by_mac() + + for d in need_names: + mac = d.get('mac_address') + if not mac: + raise ValueError("No mac_address or name entry for %s" % d) + if mac not in known_macs: + raise ValueError("Unable to find a system nic for %s" % d) + d['name'] = known_macs[mac] + for service in services: cfg = service cfg.update({'type': 'nameserver'}) diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 195b8207..1364b39d 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -73,7 +73,7 @@ NETWORK_DATA = { 'type': 'ovs', 'mtu': None, 'id': 'tap2f88d109-5b'}, {'vif_id': '1a5382f8-04c5-4d75-ab98-d666c1ef52cc', 'ethernet_mac_address': 'fa:16:3e:05:30:fe', - 'type': 'ovs', 'mtu': None, 'id': 'tap1a5382f8-04'} + 'type': 'ovs', 'mtu': None, 'id': 'tap1a5382f8-04', 'name': 'nic0'} ], 'networks': [ {'link': 'tap2ecc7709-b3', 'type': 'ipv4_dhcp', @@ -88,6 +88,10 @@ NETWORK_DATA = { ] } +KNOWN_MACS = { + 'fa:16:3e:69:b0:58': 'enp0s1', + 'fa:16:3e:d4:57:ad': 'enp0s2'} + CFG_DRIVE_FILES_V2 = { 'ec2/2009-04-04/meta-data.json': json.dumps(EC2_META), 'ec2/2009-04-04/user-data': USER_DATA, @@ -365,10 +369,40 @@ class TestConfigDriveDataSource(TestCase): """Verify that network_data is converted and present on ds object.""" populate_dir(self.tmp, CFG_DRIVE_FILES_V2) myds = cfg_ds_from_dir(self.tmp) - network_config = ds.convert_network_data(NETWORK_DATA) + network_config = ds.convert_network_data(NETWORK_DATA, + known_macs=KNOWN_MACS) self.assertEqual(myds.network_config, network_config) +class TestConvertNetworkData(TestCase): + def _getnames_in_config(self, ncfg): + return set([n['name'] for n in ncfg['config'] + if n['type'] == 'physical']) + + def test_conversion_fills_names(self): + ncfg = ds.convert_network_data(NETWORK_DATA, known_macs=KNOWN_MACS) + expected = set(['nic0', 'enp0s1', 'enp0s2']) + found = self._getnames_in_config(ncfg) + self.assertEqual(found, expected) + + @mock.patch('cloudinit.net.get_interfaces_by_mac') + def test_convert_reads_system_prefers_name(self, get_interfaces_by_mac): + macs = KNOWN_MACS.copy() + macs.update({'fa:16:3e:05:30:fe': 'foonic1', + 'fa:16:3e:69:b0:58': 'ens1'}) + get_interfaces_by_mac.return_value = macs + + ncfg = ds.convert_network_data(NETWORK_DATA) + expected = set(['nic0', 'ens1', 'enp0s2']) + found = self._getnames_in_config(ncfg) + self.assertEqual(found, expected) + + def test_convert_raises_value_error_on_missing_name(self): + macs = {'aa:aa:aa:aa:aa:00': 'ens1'} + self.assertRaises(ValueError, ds.convert_network_data, + NETWORK_DATA, known_macs=macs) + + def cfg_ds_from_dir(seed_d): found = ds.read_config_drive(seed_d) cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, None, @@ -387,7 +421,8 @@ def populate_ds_from_read_config(cfg_ds, source, results): cfg_ds.userdata_raw = results.get('userdata') cfg_ds.version = results.get('version') cfg_ds.network_json = results.get('networkdata') - cfg_ds._network_config = ds.convert_network_data(cfg_ds.network_json) + cfg_ds._network_config = ds.convert_network_data( + cfg_ds.network_json, known_macs=KNOWN_MACS) def populate_dir(seed_dir, files): -- cgit v1.2.3 From 8a8d7eed2ed5696d58a825ec7301d8424c23ce5e Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 3 Jun 2016 14:23:34 -0400 Subject: avoid rendering 'lo' twice by not writing it in network config. --- cloudinit/net/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index f47053b2..c72b6ff8 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -788,6 +788,10 @@ def _ifaces_to_net_config_data(ifaces): for name, data in ifaces.items(): # devname is 'eth0' for name='eth0:1' devname = name.partition(":")[0] + if devname == "lo": + # currently provding 'lo' in network config results in duplicate + # entries. in rendered interfaces file. so skip it. + continue if devname not in devs: devs[devname] = {'type': 'physical', 'name': devname, 'subnets': []} -- cgit v1.2.3 From f495947a701d5629b6dbfd2ff9e01dad7bd5166b Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 3 Jun 2016 14:58:51 -0400 Subject: fix issue with routes on subnets not getting rendered --- cloudinit/net/__init__.py | 2 ++ .../unittests/test_datasource/test_configdrive.py | 41 +++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index c72b6ff8..49e9d5c2 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -574,6 +574,8 @@ def render_interfaces(network_state): content += iface_start_entry(iface, index) content += iface_add_subnet(iface, subnet) content += iface_add_attrs(iface) + for route in subnet.get('routes', []): + content += render_route(route, indent=" ") else: # ifenslave docs say to auto the slave devices if 'bond-master' in iface: diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 1364b39d..83d75f9c 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -15,6 +15,7 @@ except ImportError: from contextlib2 import ExitStack from cloudinit import helpers +from cloudinit import net from cloudinit import settings from cloudinit.sources import DataSourceConfigDrive as ds from cloudinit.sources.helpers import openstack @@ -88,9 +89,33 @@ NETWORK_DATA = { ] } +NETWORK_DATA_2 = { + "services": [ + {"type": "dns", "address": "1.1.1.191"}, + {"type": "dns", "address": "1.1.1.4"}], + "networks": [ + {"network_id": "d94bbe94-7abc-48d4-9c82-4628ea26164a", "type": "ipv4", + "netmask": "255.255.255.248", "link": "eth0", + "routes": [{"netmask": "0.0.0.0", "network": "0.0.0.0", + "gateway": "2.2.2.9"}], + "ip_address": "2.2.2.10", "id": "network0-ipv4"}, + {"network_id": "ca447c83-6409-499b-aaef-6ad1ae995348", "type": "ipv4", + "netmask": "255.255.255.224", "link": "eth1", + "routes": [], "ip_address": "3.3.3.24", "id": "network1-ipv4"}], + "links": [ + {"ethernet_mac_address": "fa:16:3e:dd:50:9a", "mtu": 1500, + "type": "vif", "id": "eth0", "vif_id": "vif-foo1"}, + {"ethernet_mac_address": "fa:16:3e:a8:14:69", "mtu": 1500, + "type": "vif", "id": "eth1", "vif_id": "vif-foo2"}] +} + + KNOWN_MACS = { 'fa:16:3e:69:b0:58': 'enp0s1', - 'fa:16:3e:d4:57:ad': 'enp0s2'} + 'fa:16:3e:d4:57:ad': 'enp0s2', + 'fa:16:3e:dd:50:9a': 'foo1', + 'fa:16:3e:a8:14:69': 'foo2', +} CFG_DRIVE_FILES_V2 = { 'ec2/2009-04-04/meta-data.json': json.dumps(EC2_META), @@ -402,6 +427,20 @@ class TestConvertNetworkData(TestCase): self.assertRaises(ValueError, ds.convert_network_data, NETWORK_DATA, known_macs=macs) + def test_conversion_with_route(self): + ncfg = ds.convert_network_data(NETWORK_DATA_2, known_macs=KNOWN_MACS) + # not the best test, but see that we get a route in the + # network config and that it gets rendered to an ENI file + routes = [] + for n in ncfg['config']: + for s in n.get('subnets', []): + routes.extend(s.get('routes', [])) + self.assertIn( + {'network': '0.0.0.0', 'netmask': '0.0.0.0', 'gateway': '2.2.2.9'}, + routes) + eni = net.render_interfaces(net.parse_net_config_data(ncfg)) + self.assertIn("route add default gw 2.2.2.9", eni) + def cfg_ds_from_dir(seed_d): found = ds.read_config_drive(seed_d) -- cgit v1.2.3