From 5c840c4941d8401ee3c2d15c9a6cfb4f6a074deb Mon Sep 17 00:00:00 2001 From: Ben Nemec Date: Tue, 23 May 2017 16:19:30 +0000 Subject: 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 | 19 ++++++++++++++++++- os_net_config/impl_eni.py | 8 ++++++++ os_net_config/impl_ifcfg.py | 7 +++++++ os_net_config/tests/test_impl_eni.py | 31 +++++++++++++++++++++++++++++++ os_net_config/tests/test_impl_ifcfg.py | 31 +++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) diff --git a/os_net_config/__init__.py b/os_net_config/__init__.py index cb56059..609d5dc 100644 --- a/os_net_config/__init__.py +++ b/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) diff --git a/os_net_config/impl_eni.py b/os_net_config/impl_eni.py index 360d8c8..944c4e9 100644 --- a/os_net_config/impl_eni.py +++ b/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.') diff --git a/os_net_config/impl_ifcfg.py b/os_net_config/impl_ifcfg.py index 6f35688..d9d362d 100644 --- a/os_net_config/impl_ifcfg.py +++ b/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 diff --git a/os_net_config/tests/test_impl_eni.py b/os_net_config/tests/test_impl_eni.py index 4911cb9..51354f8 100644 --- a/os_net_config/tests/test_impl_eni.py +++ b/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)) diff --git a/os_net_config/tests/test_impl_ifcfg.py b/os_net_config/tests/test_impl_ifcfg.py index 9ff2bd6..e400c94 100644 --- a/os_net_config/tests/test_impl_ifcfg.py +++ b/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)) -- cgit 1.2.3-korg