Generally let's try to get some unittest coverage on functions defined in cloudinit.net.freebsd.
Long term (separate branch after this) our eye should be toward gutting all the net-related class methods from cloudinit.distros.freebsd in favor of using this top-level cloudinit.net utility functions. Since distros/__init__.py contains the method Distro.apply_network_config_names which calls net.apply_network_config_names which you are defining in net.freebsd. I think we can drop all net-related support methods from distros.freebsd to avoid all that duplication to support our base distros class can rely on that implementation instead of all the duplicated net-relatapply_network_config_names here's the patch I was thinking (not tested) http://paste.ubuntu.com/p/wwZgTxSY83/ Diff comments: > diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py > index 3642fb1..94fe6d9 100644 > --- a/cloudinit/net/__init__.py > +++ b/cloudinit/net/__init__.py > @@ -349,247 +211,53 @@ def apply_network_config_names(netcfg, > strict_present=True, strict_busy=True): > ' network config version: %s' % netcfg.get('version')) > > > -def interface_has_own_mac(ifname, strict=False): > - """return True if the provided interface has its own address. > - > - Based on addr_assign_type in /sys. Return true for any interface > - that does not have a 'stolen' address. Examples of such devices > - are bonds or vlans that inherit their mac from another device. > - Possible values are: > - 0: permanent address 2: stolen from another device > - 1: randomly generated 3: set using dev_set_mac_address""" > - > - assign_type = read_sys_net_int(ifname, "addr_assign_type") > - if assign_type is None: > - # None is returned if this nic had no 'addr_assign_type' entry. > - # if strict, raise an error, if not return True. > - if strict: > - raise ValueError("%s had no addr_assign_type.") > - return True > - return assign_type in (0, 1, 3) > - > - > -def _get_current_rename_info(check_downable=True): > - """Collect information necessary for rename_interfaces. > - > - returns a dictionary by mac address like: > - {name: > - { > - 'downable': None or boolean indicating that the > - 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 (in lower case) > - 'name': name > - 'up': boolean: is_up(name) > - }} > - """ > - cur_info = {} > - for (name, mac, driver, device_id) in get_interfaces(): > - cur_info[name] = { > - 'downable': None, > - 'device_id': device_id, > - 'driver': driver, > - 'mac': mac.lower(), > - 'name': name, > - 'up': is_up(name), > - } > - > - 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 cur_info.values(): > - d['downable'] = (d['up'] is False or > - d['name'] not in nics_with_addresses) > - > - return cur_info > - > - > -def _rename_interfaces(renames, strict_present=True, strict_busy=True, > - current_info=None): > - > - if not len(renames): > - LOG.debug("no interfaces to rename") > - return > - > - if current_info is None: > - current_info = _get_current_rename_info() > - > - 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 > - > - def update_byname(bymac): > - return dict((data['name'], data) > - for data in cur_info.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_info) > - tmpname_fmt = "cirename%d" > - tmpi = -1 > - > - 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 get_ib_hwaddrs_by_interface(): > + """Build a dictionary mapping Infiniband interface names to their > hardware > + address.""" > + ret = {} > + from cloudinit.net import common > + for name, _, _, _ in common.get_interfaces(): > + ib_mac = get_ib_interface_hwaddr(name, False) > + if ib_mac: > + if ib_mac in ret: > + raise RuntimeError( > + "duplicate mac found! both '%s' and '%s' have mac '%s'" % > + (name, ret[ib_mac], ib_mac)) > + ret[name] = ib_mac > + return ret > > - 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] > +def get_ib_interface_hwaddr(ifname, ethernet_format): Need two empty lines between function definitions. > + """Returns the string value of an Infiniband interface's hardware > address. > > + If ethernet_format is True, an Ethernet MAC-style 6 byte > + representation of the address will be returned. > + """ > + if not is_infiniband(ifname): > 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: > - 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 > - > - if not cur_name: > - if strict_present: > - errors.append( > - "[nic not present] Cannot rename mac=%s to %s" > - ", not available." % (mac, 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(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_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_info) > - ops += cur_ops > - > - opmap = {'rename': rename, 'down': down, 'up': up} > - > - 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)) > + mac = get_interface_mac(ifname) > + if mac and ethernet_format: > + # Use bytes 13-15 and 18-20 of the hardware address. > + mac = mac[36:-14] + mac[51:] > + return mac > > > def get_interface_mac(ifname): > - """Returns the string value of an interface's MAC Address""" > - path = "address" > - if os.path.isdir(sys_dev_path(ifname, "bonding_slave")): > - # for a bond slave, get the nic's hwaddress, not the address it > - # is using because its part of a bond. > - path = "bonding_slave/perm_hwaddr" > - return read_sys_net_safe(ifname, path) > + return netimpl.get_interface_mac(ifname) > > > -def get_ib_interface_hwaddr(ifname, ethernet_format): > - """Returns the string value of an Infiniband interface's hardware > - address. If ethernet_format is True, an Ethernet MAC-style 6 byte > - representation of the address will be returned. > - """ > - # Type 32 is Infiniband. > - if read_sys_net_safe(ifname, 'type') == '32': > - mac = get_interface_mac(ifname) > - if mac and ethernet_format: > - # Use bytes 13-15 and 18-20 of the hardware address. > - mac = mac[36:-14] + mac[51:] > - return mac > +def net_setup_link(run=False): > + return netimpl.net_setup_link(run) > > > def get_interfaces_by_mac(): > """Build a dictionary of tuples {mac: name}. > > - Bridges and any devices that have a 'stolen' mac are excluded.""" > + Bridges and any devices that have a 'stolen' mac are excluded. > + """ > ret = {} > - for name, mac, _driver, _devid in get_interfaces(): > + from cloudinit.net import common > + for name, mac, _driver, _devid in common.get_interfaces(): > if mac in ret: > raise RuntimeError( > "duplicate mac found! both '%s' and '%s' have mac '%s'" % -- https://code.launchpad.net/~i.galic/cloud-init/+git/cloud-init/+merge/358228 Your team cloud-init commiters is requested to review the proposed merge of ~i.galic/cloud-init:refactor/net-fbsd into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp