From 2af5fd889b3286dcec21e2bc89f287a0e4129d0f Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 28 Feb 2023 18:27:29 +0100 Subject: [PATCH] Add sleep before checking if ovs port is in the namespace When network device which is ovs internal port is moved to the namespace it may happend sometimes that it will have "shy port syndrome" [1]. Even though there is wait for device to be in namespace in the set_netns method it may happend that device is in namespace during this check but it dissapears for short time later and that causes failures e.g. in functional tests like described in [2]. To avoid that, this patch proposed simple (and ugly) sleep for 1 second before checking if port really exists in the namespace. If it will be "shy" port it should already flap during that 1 second. [1] https://bugs.launchpad.net/neutron/+bug/1618987 [2] https://bugs.launchpad.net/neutron/+bug/1961740 Related-Bug: #1961740 Related-Bug: #1998337 Change-Id: I442587e7ef55917f4ea873e190bf8afbc0e911e1 --- neutron/agent/linux/interface.py | 2 +- neutron/agent/linux/ip_lib.py | 11 ++++++++--- neutron/tests/unit/agent/linux/test_interface.py | 6 +++--- neutron/tests/unit/agent/linux/test_ip_lib.py | 3 ++- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 0c838e6accd..c9de1c60757 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -365,7 +365,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): namespace_obj = ip_wrapper.ensure_namespace(namespace) for i in range(9): try: - namespace_obj.add_device_to_namespace(device) + namespace_obj.add_device_to_namespace(device, is_ovs_port=True) break except ip_lib.NetworkInterfaceNotFound: # NOTE(slaweq): if the exception was NetworkInterfaceNotFound diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 18019e51ab3..4d664381f89 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -265,9 +265,9 @@ class IPWrapper(SubProcessBase): return True return False - def add_device_to_namespace(self, device): + def add_device_to_namespace(self, device, is_ovs_port=False): if self.namespace: - device.link.set_netns(self.namespace) + device.link.set_netns(self.namespace, is_ovs_port=is_ovs_port) def add_vlan(self, name, physical_interface, vlan_id): privileged.create_interface(name, @@ -457,10 +457,15 @@ class IpLinkCommand(IpDeviceCommandBase): privileged.set_link_attribute( self.name, self._parent.namespace, state='down') - def set_netns(self, namespace): + 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) diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index 0d9a7bebb45..84fe6c5af8c 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -470,11 +470,11 @@ class TestOVSInterfaceDriver(TestBase): expected.extend( [mock.call().ensure_namespace(namespace), mock.call().ensure_namespace().add_device_to_namespace( - mock.ANY), + mock.ANY, is_ovs_port=True), mock.call().ensure_namespace().add_device_to_namespace( - mock.ANY), + mock.ANY, is_ovs_port=True), mock.call().ensure_namespace().add_device_to_namespace( - mock.ANY)]) + mock.ANY, is_ovs_port=True)]) expected.extend([ mock.call(namespace=namespace), mock.call().device('tap0'), diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 9186eca5b40..da754464c35 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -507,7 +507,8 @@ class TestIpWrapper(base.BaseTestCase): def test_add_device_to_namespace(self): dev = mock.Mock() ip_lib.IPWrapper(namespace='ns').add_device_to_namespace(dev) - dev.assert_has_calls([mock.call.link.set_netns('ns')]) + dev.assert_has_calls( + [mock.call.link.set_netns('ns', is_ovs_port=False)]) def test_add_device_to_namespace_is_none(self): dev = mock.Mock()