cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:bug/1669860-no-rename-bonds into cloud-init:master.

Commit message:
Fix bug that resulted in an attempt to rename bonds or vlans.

When cloud-init ran in the init stage (after networking had come up).
A bug could occur where cloud-init would attempt and fail to rename
network devices that had "inherited" mac addresses.

The intent of apply_network_config_names was always to rename only
the devices that were "physical" per the network config.  (This would
include veth devices in a container).  The bug was in creating
the dictionary of interfaces by mac address.  If there were multiple
interfaces with the same mac address then renames could fail.
This situation was guaranteed to occur with bonds or vlans or other
devices that inherit their mac.

The solution is to change get_interfaces_by_mac to skip interfaces
that have an inherited mac.

LP: #1669860

Requested reviews:
  cloud init development team (cloud-init-dev)
Related bugs:
  Bug #1669860 in cloud-init: "cloud-init attempts to rename bonds"
  https://bugs.launchpad.net/cloud-init/+bug/1669860

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/321578
-- 
Your team cloud init development team is requested to review the proposed merge of ~smoser/cloud-init:bug/1669860-no-rename-bonds into cloud-init:master.

diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
old mode 100755
new mode 100644
index 1cf98ef..19a6f92
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -187,7 +187,11 @@ 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."""
+    device cannot be renamed because it is currently configured.
+
+    renames are only attempted for interfaces of type 'physical'.  It is
+    expected that the network system will create other devices with the
+    correct name in place."""
     renames = []
     for ent in netcfg.get('config', {}):
         if ent.get('type') != 'physical':
@@ -201,13 +205,36 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True):
     return _rename_interfaces(renames)
 
 
+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 strict and assign_type is None:
+        raise ValueError("%s had no addr_assign_type.")
+    return assign_type in (0, 1, 3)
+
+
 def _get_current_rename_info(check_downable=True):
-    """Collect information necessary for rename_interfaces."""
-    names = get_devicelist()
+    """Collect information necessary for rename_interfaces.
+
+    returns a dictionary by mac address like:
+       {mac:
+         {'name': name
+          'up': boolean: is_up(name),
+          'downable': None or boolean indicating that the
+                      device has only automatically assigned ip addrs.}}
+    """
     bymac = {}
-    for n in names:
-        bymac[get_interface_mac(n)] = {
-            'name': n, 'up': is_up(n), 'downable': None}
+    for mac, name in get_interfaces_by_mac().items():
+        bymac[mac] = {
+            'name': name, 'up': is_up(name), 'downable': None}
 
     if check_downable:
         nmatch = re.compile(r"[0-9]+:\s+(\w+)[@:]")
@@ -347,7 +374,9 @@ def get_interface_mac(ifname):
 
 
 def get_interfaces_by_mac(devs=None):
-    """Build a dictionary of tuples {mac: name}"""
+    """Build a dictionary of tuples {mac: name}.
+
+    Any devices that have a 'stolen' mac are excluded."""
     if devs is None:
         try:
             devs = get_devicelist()
@@ -358,6 +387,8 @@ def get_interfaces_by_mac(devs=None):
                 raise
     ret = {}
     for name in devs:
+        if not interface_has_own_mac(name):
+            continue
         mac = get_interface_mac(name)
         # some devices may not have a mac (tun0)
         if mac: