From 2c797b107b7360e05b4f79980d4c4e433d4e669e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 16 Jan 2024 02:36:27 +0000 Subject: [PATCH] If method ``set_netns`` fails, restore previous device namespace If the ``IpLinkCommand.set_netns`` fails, the method restores the previous device namespace before raising the exception. Closes-Bug: #2049590 Change-Id: I73b36ef161441b52922d888c11a144eafe8a7ed0 (cherry picked from commit e234a7aeab72958a74e5d9540253d48aa6915816) --- neutron/agent/linux/interface.py | 5 ----- neutron/agent/linux/ip_lib.py | 21 ++++++++++--------- .../tests/unit/agent/linux/test_interface.py | 11 ++++++++-- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index c9de1c60757..d0236a03c65 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -374,11 +374,6 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): LOG.warning("Failed to set interface %s into namespace %s. " "Interface not found, attempt: %s, retrying.", device, namespace, i + 1) - # NOTE(slaweq) In such case it's required to reset device's - # namespace as it was already set to the "namespace" - # and after retry neutron will look for it in that namespace - # which is wrong - device.namespace = None time.sleep(1) except utils.WaitTimeout: # NOTE(slaweq): if the exception was WaitTimeout then it means diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 196dc17e218..c9b42d83ddd 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -25,6 +25,7 @@ from neutron_lib import constants from neutron_lib import exceptions from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import netutils from pyroute2.netlink import exceptions as netlink_exceptions from pyroute2.netlink import rtnl @@ -477,16 +478,16 @@ class IpLinkCommand(IpDeviceCommandBase): self.name, self._parent.namespace, state='down') def set_netns(self, namespace, is_ovs_port=False): - privileged.set_link_attribute( - self.name, self._parent.namespace, net_ns_fd=namespace) - self._parent.namespace = namespace - if is_ovs_port: - # NOTE(slaweq): because of the "shy port" which may dissapear for - # short time after it's moved to the namespace we need to wait - # a bit before checking if port really exists in the namespace - time.sleep(1) - common_utils.wait_until_true(lambda: self.exists, timeout=5, - sleep=0.5) + old_namespace = self._parent.namespace + try: + privileged.set_link_attribute( + self.name, self._parent.namespace, net_ns_fd=namespace) + self._parent.namespace = namespace + common_utils.wait_until_true(lambda: self.exists, timeout=3, + sleep=0.5) + except common_utils.WaitTimeout: + with excutils.save_and_reraise_exception(): + self._parent.namespace = old_namespace def set_name(self, name): privileged.set_link_attribute( diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index 84fe6c5af8c..63df74ebc73 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -13,11 +13,13 @@ # License for the specific language governing permissions and limitations # under the License. +import time from unittest import mock from neutron_lib import constants from neutron_lib.plugins.ml2 import ovs_constants as ovs_const from oslo_utils import excutils +from oslo_utils import uuidutils from pyroute2.netlink import exceptions as pyroute2_exc from neutron.agent.common import ovs_lib @@ -387,6 +389,10 @@ class TestABCDriver(TestBase): class TestOVSInterfaceDriver(TestBase): + def setUp(self): + super().setUp() + self.mock_sleep = mock.patch.object(time, 'sleep').start() + def test_get_device_name(self): br = interface.OVSInterfaceDriver(self.conf) device_name = br.get_device_name(FakePort()) @@ -525,13 +531,14 @@ class TestOVSInterfaceDriver(TestBase): self.ip.ensure_namespace.return_value = namespace_obj namespace_obj.add_device_to_namespace.side_effect = ( ip_lib.NetworkInterfaceNotFound) - device = mock.MagicMock() + namespace_name = uuidutils.generate_uuid() + device = mock.MagicMock(namespace=namespace_name) self.assertRaises( ip_lib.NetworkInterfaceNotFound, ovs._add_device_to_namespace, self.ip, device, "test-ns") self.assertEqual(10, namespace_obj.add_device_to_namespace.call_count) - self.assertIsNone(device.namespace) + self.assertEqual(namespace_name, device.namespace) class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver):