Continue bringing up interfaces even if one fails

Currently os-net-config exits immediately if an interface fails to
come up.  This causes problems when we call it with a failsafe
configuration because it can result in working interfaces not
being configured if there are also non-functional interfaces, which
can leave a system unreachable over the network even though it may
have a working connection.

This change stores errors and handles them after all interfaces have
been processed to allow os-net-config to do what it can even with an
invalid configuration.  If any failures are detected it will still
cause os-net-config to report failure at the end.

Change-Id: I3bc75e217d0b7c5ae62900f4253ad57ee3720685
Closes-Bug: 1692725

os_net_config/__init__.py

@@ -30,6 +30,10 @@ class NotImplemented(Exception):
     pass
 
 
+class ConfigurationError(Exception):
+    pass
+
+
 class NetConfig(object):
     """Common network config methods class."""
 
@@ -37,6 +41,7 @@ class NetConfig(object):
         self.noop = noop
         self.log_prefix = "NOOP: " if noop else ""
         self.root_dir = root_dir
+        self.errors = []
 
     def add_object(self, obj):
         """Convenience method to add any type of object to the network config.
@@ -249,8 +254,20 @@ class NetConfig(object):
         self.execute(msg, '/sbin/ifdown', interface, check_exit_code=False)
 
     def ifup(self, interface, iftype='interface'):
+        """Run 'ifup' on the specified interface
+
+        If a failure occurs when bringing up the interface it will be saved
+        to self.errors for later handling.  This allows callers to continue
+        trying to bring up interfaces even if one fails.
+
+        :param interface: The name of the interface to be started.
+        :param iftype: The type of the interface.
+        """
         msg = 'running ifup on %s: %s' % (iftype, interface)
-        self.execute(msg, '/sbin/ifup', interface)
+        try:
+            self.execute(msg, '/sbin/ifup', interface)
+        except processutils.ProcessExecutionError as e:
+            self.errors.append(e)
 
     def ifrename(self, oldname, newname):
         msg = 'renaming %s to %s: ' % (oldname, newname)

os_net_config/impl_eni.py

@@ -241,6 +241,14 @@ class ENINetConfig(os_net_config.NetConfig):
 
                 for interface in self.interfaces.keys():
                     self.ifup(interface)
+
+                if self.errors:
+                    message = 'Failure(s) occurred when applying configuration'
+                    logger.error(message)
+                    for e in self.errors:
+                        logger.error('stdout: %s, stderr: %s', e.stdout,
+                                     e.stderr)
+                    raise os_net_config.ConfigurationError(message)
         else:
             logger.info('No interface changes are required.')
 

os_net_config/impl_ifcfg.py

@@ -1054,4 +1054,11 @@ class IfcfgNetConfig(os_net_config.NetConfig):
                     logger.info('Updating VPP mapping')
                     utils.update_vpp_mapping(vpp_interfaces)
 
+            if self.errors:
+                message = 'Failure(s) occurred when applying configuration'
+                logger.error(message)
+                for e in self.errors:
+                    logger.error('stdout: %s, stderr: %s', e.stdout, e.stderr)
+                raise os_net_config.ConfigurationError(message)
+
         return update_files

os_net_config/tests/test_impl_eni.py

@@ -18,6 +18,7 @@ import tempfile
 
 from oslo_concurrency import processutils
 
+import os_net_config
 from os_net_config import impl_eni
 from os_net_config import objects
 from os_net_config.tests import base
@@ -360,3 +361,33 @@ class TestENINetConfigApply(base.TestCase):
         self.assertEqual((_OVS_BRIDGE_DHCP + _OVS_PORT_IFACE), iface_data)
         self.assertIn('eth0', self.ifup_interface_names)
         self.assertIn('br0', self.ifup_interface_names)
+
+    def _failed_execute(*args, **kwargs):
+        if kwargs.get('check_exit_code', True):
+            raise processutils.ProcessExecutionError('Test stderr',
+                                                     'Test stdout',
+                                                     str(kwargs))
+
+    def test_interface_failure(self):
+        self.stubs.Set(processutils, 'execute', self._failed_execute)
+        v4_addr = objects.Address('192.168.1.2/24')
+        interface = objects.Interface('em1', addresses=[v4_addr])
+        self.provider.add_interface(interface)
+
+        self.assertRaises(os_net_config.ConfigurationError,
+                          self.provider.apply)
+        self.assertEqual(1, len(self.provider.errors))
+
+    def test_interface_failure_multiple(self):
+        self.stubs.Set(processutils, 'execute', self._failed_execute)
+        v4_addr = objects.Address('192.168.1.2/24')
+        interface = objects.Interface('em1', addresses=[v4_addr])
+        v4_addr2 = objects.Address('192.168.2.2/24')
+        interface2 = objects.Interface('em2', addresses=[v4_addr2])
+        self.provider.add_interface(interface)
+        self.provider.add_interface(interface2)
+
+        self.assertRaises(os_net_config.ConfigurationError,
+                          self.provider.apply)
+        # Even though the first one failed, we should have attempted both
+        self.assertEqual(2, len(self.provider.errors))

os_net_config/tests/test_impl_ifcfg.py

@@ -19,6 +19,7 @@ import tempfile
 
 from oslo_concurrency import processutils
 
+import os_net_config
 from os_net_config import impl_ifcfg
 from os_net_config import NetConfig
 from os_net_config import objects
@@ -1231,3 +1232,33 @@ class TestIfcfgNetConfigApply(base.TestCase):
         self.provider.add_interface(interface)
         self.provider.apply()
         self.assertNotIn('Restart openvswitch', execute_strings)
+
+    def _failed_execute(*args, **kwargs):
+        if kwargs.get('check_exit_code', True):
+            raise processutils.ProcessExecutionError('Test stderr',
+                                                     'Test stdout',
+                                                     str(kwargs))
+
+    def test_interface_failure(self):
+        self.stubs.Set(processutils, 'execute', self._failed_execute)
+        v4_addr = objects.Address('192.168.1.2/24')
+        interface = objects.Interface('em1', addresses=[v4_addr])
+        self.provider.add_interface(interface)
+
+        self.assertRaises(os_net_config.ConfigurationError,
+                          self.provider.apply)
+        self.assertEqual(1, len(self.provider.errors))
+
+    def test_interface_failure_multiple(self):
+        self.stubs.Set(processutils, 'execute', self._failed_execute)
+        v4_addr = objects.Address('192.168.1.2/24')
+        interface = objects.Interface('em1', addresses=[v4_addr])
+        v4_addr2 = objects.Address('192.168.2.2/24')
+        interface2 = objects.Interface('em2', addresses=[v4_addr2])
+        self.provider.add_interface(interface)
+        self.provider.add_interface(interface2)
+
+        self.assertRaises(os_net_config.ConfigurationError,
+                          self.provider.apply)
+        # Even though the first one failed, we should have attempted both
+        self.assertEqual(2, len(self.provider.errors))