net: update net sequence, include wait on netdevs, opensuse netrules path

On systems with many interfaces, processing udev events may take a while.
Cloud-init expects devices included in a provided network-configuration
to be present when attempting to configure them.  This patch adds a step
in net configuration where it will check for devices provided in the
configuration and if not found, issue udevadm settle commands to wait
for them to appear.

Additionally, the default path for udev persistent network rules
70-persistent-net.rules may also be written to systems which include
the 75-net-generator.rules.  During boot, cloud-init and the
generator may race and interleave values causing issues.  OpenSUSE
will now use a newer file, 85-persistent-net-cloud-init.rules which
will take precedence over values created by 75-net-generator and
avoid collisions on the same file.

LP: #1817368

6 files changed, 315 insertions, 32 deletions

diff --git a/cloudinit/distros/opensuse.py b/cloudinit/distros/opensuse.py
index 1bfe0478..e41e2f7b 100644
--- a/cloudinit/distros/opensuse.py
+++ b/cloudinit/distros/opensuse.py
@@ -38,6 +38,8 @@ class Distro(distros.Distro):
         'sysconfig': {
             'control': 'etc/sysconfig/network/config',
             'iface_templates': '%(base)s/network/ifcfg-%(name)s',
+            'netrules_path': (
+                'etc/udev/rules.d/85-persistent-net-cloud-init.rules'),
             'route_templates': {
                 'ipv4': '%(base)s/network/ifroute-%(name)s',
                 'ipv6': '%(base)s/network/ifroute-%(name)s',
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index 624c9b42..f3cec794 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -9,6 +9,7 @@ import errno
 import logging
 import os
 import re
+from functools import partial
 
 from cloudinit.net.network_state import mask_to_net_prefix
 from cloudinit import util
@@ -292,18 +293,10 @@ def generate_fallback_config(blacklist_drivers=None, config_driver=None):
         return None
 
 
-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 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."""
+def extract_physdevs(netcfg):
 
     def _version_1(netcfg):
-        renames = []
+        physdevs = []
         for ent in netcfg.get('config', {}):
             if ent.get('type') != 'physical':
                 continue
@@ -317,11 +310,11 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True):
                 driver = device_driver(name)
             if not device_id:
                 device_id = device_devid(name)
-            renames.append([mac, name, driver, device_id])
-        return renames
+            physdevs.append([mac, name, driver, device_id])
+        return physdevs
 
     def _version_2(netcfg):
-        renames = []
+        physdevs = []
         for ent in netcfg.get('ethernets', {}).values():
             # only rename if configured to do so
             name = ent.get('set-name')
@@ -337,16 +330,69 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True):
                 driver = device_driver(name)
             if not device_id:
                 device_id = device_devid(name)
-            renames.append([mac, name, driver, device_id])
-        return renames
+            physdevs.append([mac, name, driver, device_id])
+        return physdevs
+
+    version = netcfg.get('version')
+    if version == 1:
+        return _version_1(netcfg)
+    elif version == 2:
+        return _version_2(netcfg)
+
+    raise RuntimeError('Unknown network config version: %s' % version)
+
 
-    if netcfg.get('version') == 1:
-        return _rename_interfaces(_version_1(netcfg))
-    elif netcfg.get('version') == 2:
-        return _rename_interfaces(_version_2(netcfg))
+def wait_for_physdevs(netcfg, strict=True):
+    physdevs = extract_physdevs(netcfg)
+
+    # set of expected iface names and mac addrs
+    expected_ifaces = dict([(iface[0], iface[1]) for iface in physdevs])
+    expected_macs = set(expected_ifaces.keys())
+
+    # set of current macs
+    present_macs = get_interfaces_by_mac().keys()
+
+    # compare the set of expected mac address values to
+    # the current macs present; we only check MAC as cloud-init
+    # has not yet renamed interfaces and the netcfg may include
+    # such renames.
+    for _ in range(0, 5):
+        if expected_macs.issubset(present_macs):
+            LOG.debug('net: all expected physical devices present')
+            return
 
-    raise RuntimeError('Failed to apply network config names. Found bad'
-                       ' network config version: %s' % netcfg.get('version'))
+        missing = expected_macs.difference(present_macs)
+        LOG.debug('net: waiting for expected net devices: %s', missing)
+        for mac in missing:
+            # trigger a settle, unless this interface exists
+            syspath = sys_dev_path(expected_ifaces[mac])
+            settle = partial(util.udevadm_settle, exists=syspath)
+            msg = 'Waiting for udev events to settle or %s exists' % syspath
+            util.log_time(LOG.debug, msg, func=settle)
+
+        # update present_macs after settles
+        present_macs = get_interfaces_by_mac().keys()
+
+    msg = 'Not all expected physical devices present: %s' % missing
+    LOG.warning(msg)
+    if strict:
+        raise RuntimeError(msg)
+
+
+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 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."""
+
+    try:
+        _rename_interfaces(extract_physdevs(netcfg))
+    except RuntimeError as e:
+        raise RuntimeError('Failed to apply network config names: %s' % e)
 
 
 def interface_has_own_mac(ifname, strict=False):
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index d393e6ad..e6e77d7a 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -708,3 +708,216 @@ class TestHasURLConnectivity(HttprettyTestCase):
         httpretty.register_uri(httpretty.GET, self.url, body={}, status=404)
         self.assertFalse(
             net.has_url_connectivity(self.url), 'Expected False on url fail')
+
+
+def _mk_v1_phys(mac, name, driver, device_id):
+    v1_cfg = {'type': 'physical', 'name': name, 'mac_address': mac}
+    params = {}
+    if driver:
+        params.update({'driver': driver})
+    if device_id:
+        params.update({'device_id': device_id})
+
+    if params:
+        v1_cfg.update({'params': params})
+
+    return v1_cfg
+
+
+def _mk_v2_phys(mac, name, driver=None, device_id=None):
+    v2_cfg = {'set-name': name, 'match': {'macaddress': mac}}
+    if driver:
+        v2_cfg['match'].update({'driver': driver})
+    if device_id:
+        v2_cfg['match'].update({'device_id': device_id})
+
+    return v2_cfg
+
+
+class TestExtractPhysdevs(CiTestCase):
+
+    def setUp(self):
+        super(TestExtractPhysdevs, self).setUp()
+        self.add_patch('cloudinit.net.device_driver', 'm_driver')
+        self.add_patch('cloudinit.net.device_devid', 'm_devid')
+
+    def test_extract_physdevs_looks_up_driver_v1(self):
+        driver = 'virtio'
+        self.m_driver.return_value = driver
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', None, '0x1000'],
+        ]
+        netcfg = {
+            'version': 1,
+            'config': [_mk_v1_phys(*args) for args in physdevs],
+        }
+        # insert the driver value for verification
+        physdevs[0][2] = driver
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+        self.m_driver.assert_called_with('eth0')
+
+    def test_extract_physdevs_looks_up_driver_v2(self):
+        driver = 'virtio'
+        self.m_driver.return_value = driver
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', None, '0x1000'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs},
+        }
+        # insert the driver value for verification
+        physdevs[0][2] = driver
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+        self.m_driver.assert_called_with('eth0')
+
+    def test_extract_physdevs_looks_up_devid_v1(self):
+        devid = '0x1000'
+        self.m_devid.return_value = devid
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', None],
+        ]
+        netcfg = {
+            'version': 1,
+            'config': [_mk_v1_phys(*args) for args in physdevs],
+        }
+        # insert the driver value for verification
+        physdevs[0][3] = devid
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+        self.m_devid.assert_called_with('eth0')
+
+    def test_extract_physdevs_looks_up_devid_v2(self):
+        devid = '0x1000'
+        self.m_devid.return_value = devid
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', None],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs},
+        }
+        # insert the driver value for verification
+        physdevs[0][3] = devid
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+        self.m_devid.assert_called_with('eth0')
+
+    def test_get_v1_type_physical(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+            ['09:87:65:43:21:10', 'ens0p1', 'mlx4_core', '0:0:1000'],
+        ]
+        netcfg = {
+            'version': 1,
+            'config': [_mk_v1_phys(*args) for args in physdevs],
+        }
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+
+    def test_get_v2_type_physical(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+            ['09:87:65:43:21:10', 'ens0p1', 'mlx4_core', '0:0:1000'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs},
+        }
+        self.assertEqual(sorted(physdevs),
+                         sorted(net.extract_physdevs(netcfg)))
+
+    def test_get_v2_type_physical_skips_if_no_set_name(self):
+        netcfg = {
+            'version': 2,
+            'ethernets': {
+                'ens3': {
+                    'match': {'macaddress': '00:11:22:33:44:55'},
+                }
+            }
+        }
+        self.assertEqual([], net.extract_physdevs(netcfg))
+
+    def test_runtime_error_on_unknown_netcfg_version(self):
+        with self.assertRaises(RuntimeError):
+            net.extract_physdevs({'version': 3, 'awesome_config': []})
+
+
+class TestWaitForPhysdevs(CiTestCase):
+
+    with_logs = True
+
+    def setUp(self):
+        super(TestWaitForPhysdevs, self).setUp()
+        self.add_patch('cloudinit.net.get_interfaces_by_mac',
+                       'm_get_iface_mac')
+        self.add_patch('cloudinit.util.udevadm_settle', 'm_udev_settle')
+
+    def test_wait_for_physdevs_skips_settle_if_all_present(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args)
+                          for args in physdevs},
+        }
+        self.m_get_iface_mac.side_effect = iter([
+            {'aa:bb:cc:dd:ee:ff': 'eth0',
+             '00:11:22:33:44:55': 'ens3'},
+        ])
+        net.wait_for_physdevs(netcfg)
+        self.assertEqual(0, self.m_udev_settle.call_count)
+
+    def test_wait_for_physdevs_calls_udev_settle_on_missing(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args)
+                          for args in physdevs},
+        }
+        self.m_get_iface_mac.side_effect = iter([
+            {'aa:bb:cc:dd:ee:ff': 'eth0'},   # first call ens3 is missing
+            {'aa:bb:cc:dd:ee:ff': 'eth0',
+             '00:11:22:33:44:55': 'ens3'},   # second call has both
+        ])
+        net.wait_for_physdevs(netcfg)
+        self.m_udev_settle.assert_called_with(exists=net.sys_dev_path('ens3'))
+
+    def test_wait_for_physdevs_raise_runtime_error_if_missing_and_strict(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args)
+                          for args in physdevs},
+        }
+        self.m_get_iface_mac.return_value = {}
+        with self.assertRaises(RuntimeError):
+            net.wait_for_physdevs(netcfg)
+
+        self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count)
+
+    def test_wait_for_physdevs_no_raise_if_not_strict(self):
+        physdevs = [
+            ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'],
+            ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'],
+        ]
+        netcfg = {
+            'version': 2,
+            'ethernets': {args[1]: _mk_v2_phys(*args)
+                          for args in physdevs},
+        }
+        self.m_get_iface_mac.return_value = {}
+        net.wait_for_physdevs(netcfg, strict=False)
+        self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count)
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index da7d349a..5f9d47b9 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -644,18 +644,21 @@ class Init(object):
                 return (ncfg, loc)
         return (self.distro.generate_fallback_config(), "fallback")
 
-    def apply_network_config(self, bring_up):
-        netcfg, src = self._find_networking_config()
-        if netcfg is None:
-            LOG.info("network config is disabled by %s", src)
-            return
-
+    def _apply_netcfg_names(self, netcfg):
         try:
             LOG.debug("applying net config names for %s", netcfg)
             self.distro.apply_network_config_names(netcfg)
         except Exception as e:
             LOG.warning("Failed to rename devices: %s", e)
 
+    def apply_network_config(self, bring_up):
+        # get a network config
+        netcfg, src = self._find_networking_config()
+        if netcfg is None:
+            LOG.info("network config is disabled by %s", src)
+            return
+
+        # request an update if needed/available
         if self.datasource is not NULL_DATA_SOURCE:
             if not self.is_new_instance():
                 if not self.datasource.update_metadata([EventType.BOOT]):
@@ -663,8 +666,20 @@ class Init(object):
                         "No network config applied. Neither a new instance"
                         " nor datasource network update on '%s' event",
                         EventType.BOOT)
+                    # nothing new, but ensure proper names
+                    self._apply_netcfg_names(netcfg)
                     return
+                else:
+                    # refresh netcfg after update
+                    netcfg, src = self._find_networking_config()
+
+        # ensure all physical devices in config are present
+        net.wait_for_physdevs(netcfg)
+
+        # apply renames from config
+        self._apply_netcfg_names(netcfg)
 
+        # rendering config
         LOG.info("Applying network configuration from %s bringup=%s: %s",
                  src, bring_up, netcfg)
         try:
diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py
index 94b6b255..9b483121 100644
--- a/cloudinit/tests/test_stages.py
+++ b/cloudinit/tests/test_stages.py
@@ -37,6 +37,7 @@ class FakeDataSource(sources.DataSource):
 
 class TestInit(CiTestCase):
     with_logs = True
+    allowed_subp = False
 
     def setUp(self):
         super(TestInit, self).setUp()
@@ -166,8 +167,9 @@ class TestInit(CiTestCase):
             'INFO: network config is disabled by %s' % disable_file,
             self.logs.getvalue())
 
+    @mock.patch('cloudinit.net.get_interfaces_by_mac')
     @mock.patch('cloudinit.distros.ubuntu.Distro')
-    def test_apply_network_on_new_instance(self, m_ubuntu):
+    def test_apply_network_on_new_instance(self, m_ubuntu, m_macs):
         """Call distro apply_network_config methods on is_new_instance."""
         net_cfg = {
             'version': 1, 'config': [
@@ -177,6 +179,8 @@ class TestInit(CiTestCase):
         def fake_network_config():
             return net_cfg, 'fallback'
 
+        m_macs.return_value = {'42:42:42:42:42:42': 'eth9'}
+
         self.init._find_networking_config = fake_network_config
         self.init.apply_network_config(True)
         self.init.distro.apply_network_config_names.assert_called_with(net_cfg)
@@ -206,8 +210,9 @@ class TestInit(CiTestCase):
             " nor datasource network update on '%s' event" % EventType.BOOT,
             self.logs.getvalue())
 
+    @mock.patch('cloudinit.net.get_interfaces_by_mac')
     @mock.patch('cloudinit.distros.ubuntu.Distro')
-    def test_apply_network_on_datasource_allowed_event(self, m_ubuntu):
+    def test_apply_network_on_datasource_allowed_event(self, m_ubuntu, m_macs):
         """Apply network if datasource.update_metadata permits BOOT event."""
         old_instance_id = os.path.join(
             self.init.paths.get_cpath('data'), 'instance-id')
@@ -220,6 +225,8 @@ class TestInit(CiTestCase):
         def fake_network_config():
             return net_cfg, 'fallback'
 
+        m_macs.return_value = {'42:42:42:42:42:42': 'eth9'}
+
         self.init._find_networking_config = fake_network_config
         self.init.datasource = FakeDataSource(paths=self.init.paths)
         self.init.datasource.update_events = {'network': [EventType.BOOT]}
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 18efce98..de4e7f4f 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -515,7 +515,7 @@ nameserver 172.19.0.12
 [main]
 dns = none
 """.lstrip()),
-            ('etc/udev/rules.d/70-persistent-net.rules',
+            ('etc/udev/rules.d/85-persistent-net-cloud-init.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))],
         'out_sysconfig_rhel': [
@@ -619,7 +619,7 @@ nameserver 172.19.0.12
 [main]
 dns = none
 """.lstrip()),
-            ('etc/udev/rules.d/70-persistent-net.rules',
+            ('etc/udev/rules.d/85-persistent-net-cloud-init.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))],
         'out_sysconfig_rhel': [
@@ -750,7 +750,7 @@ nameserver 172.19.0.12
 [main]
 dns = none
 """.lstrip()),
-            ('etc/udev/rules.d/70-persistent-net.rules',
+            ('etc/udev/rules.d/85-persistent-net-cloud-init.rules',
              "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ',
                       'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))],
         'out_sysconfig_rhel': [