From bacc7abf83f18825a49af2c14cebbeb312615c1d Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Wed, 4 Sep 2019 21:50:01 +0200 Subject: [PATCH] Make Neutron gate great again This is combined patch to fix couple of issues which we recently had in gate. 1. [Functional tests] Fix SIGHUP handling tests Tests in neutron.functional.test_server module are testing how PluginWorker, WSGIWorker and RPCWorker are handling SIGHUP signal. Recently this was changed in Oslo.service with [1] and our tests were failing because they were still expecting that after sending SIGHUP to the process, stop() and than start() method will be called. But as our services uses "mutate" as restart method, since [1] such process don't executes stop() and start() after SIGHUP. It now executes only reset() method. This patch reflects that change in Neutron functional tests. 2. Veth pair "IFLA_LINK" populated since kernel 4.15.0-60-generic Since kernel_version=4.15.0-60-generic, "iproute2" provides the veth pair index, even if the pair interface is in other namespace. In previous versions, the parameter 'IFLA_LINK' was not present. We need to handle both cases [1] https://review.opendev.org/#/c/641907/ Co-Authored-By: Rodolfo Alonso Hernandez Change-Id: I7a3f20a795c89ab1ab037d046a1101cd5c0287d6 Closes-Bug: #1842659 Closes-Bug: #1842482 --- .../privileged/agent/linux/test_ip_lib.py | 42 ++++++++++-------- neutron/tests/functional/test_server.py | 44 ++++++++++++------- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py index 32fa9a9abff..452ec60d4e9 100644 --- a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py @@ -13,6 +13,7 @@ # under the License. import functools +import random import eventlet import netaddr @@ -191,42 +192,45 @@ class GetDevicesInfoTestCase(functional_base.BaseSudoTestCase): self.assertEqual(sorted(interfaces_tested), sorted(self.interfaces + vxlan_interfaces)) + def _retrieve_interface(self, interface_name, namespace): + for device in priv_ip_lib.get_link_devices(namespace): + if interface_name == ip_lib.get_attr(device, 'IFLA_IFNAME'): + return device + else: + self.fail('Interface "%s" not found' % interface_name) + def test_get_devices_info_veth_different_namespaces(self): namespace2 = 'ns_test-' + uuidutils.generate_uuid() priv_ip_lib.create_netns(namespace2) self.addCleanup(self._remove_ns, namespace2) + # Create a random number of dummy interfaces in namespace2, in order + # to increase the 'veth1_2' interface index in its namespace. + for idx in range(5, random.randint(15, 20)): + priv_ip_lib.create_interface('int_%s' % idx, namespace2, 'dummy') + ip_wrapper = ip_lib.IPWrapper(self.namespace) ip_wrapper.add_veth('veth1_1', 'veth1_2', namespace2) - devices = priv_ip_lib.get_link_devices(self.namespace) - for device in devices: - name = ip_lib.get_attr(device, 'IFLA_IFNAME') - if name == 'veth1_1': - veth1_1 = device - break - else: - self.fail('Interface "veth1_1" not found') + veth1_1 = self._retrieve_interface('veth1_1', self.namespace) + veth1_2 = self._retrieve_interface('veth1_2', namespace2) ifla_linkinfo = ip_lib.get_attr(veth1_1, 'IFLA_LINKINFO') self.assertEqual(ip_lib.get_attr(ifla_linkinfo, 'IFLA_INFO_KIND'), 'veth') - self.assertIsNone(ip_lib.get_attr(veth1_1, 'IFLA_LINK')) + # NOTE(ralonsoh): since kernel_version=4.15.0-60-generic, iproute2 + # provides the veth pair index, even if the pair interface is in other + # namespace. In previous versions, the parameter 'IFLA_LINK' was not + # present. We need to handle both cases. + self.assertIn(ip_lib.get_attr(veth1_1, 'IFLA_LINK'), + [None, veth1_2['index']]) def test_get_devices_info_veth_same_namespaces(self): ip_wrapper = ip_lib.IPWrapper(self.namespace) ip_wrapper.add_veth('veth1_1', 'veth1_2') - devices = priv_ip_lib.get_link_devices(self.namespace) - veth1_1 = veth1_2 = None - for device in devices: - name = ip_lib.get_attr(device, 'IFLA_IFNAME') - if name == 'veth1_1': - veth1_1 = device - elif name == 'veth1_2': - veth1_2 = device + veth1_1 = self._retrieve_interface('veth1_1', self.namespace) + veth1_2 = self._retrieve_interface('veth1_2', self.namespace) - self.assertIsNotNone(veth1_1) - self.assertIsNotNone(veth1_2) veth1_1_link = ip_lib.get_attr(veth1_1, 'IFLA_LINK') veth1_2_link = ip_lib.get_attr(veth1_2, 'IFLA_LINK') self.assertEqual(veth1_1['index'], veth1_2_link) diff --git a/neutron/tests/functional/test_server.py b/neutron/tests/functional/test_server.py index 51a1abd6a33..01377d3b73e 100644 --- a/neutron/tests/functional/test_server.py +++ b/neutron/tests/functional/test_server.py @@ -34,9 +34,10 @@ from neutron import wsgi CONF = cfg.CONF -# This message will be written to temporary file each time -# start method is called. +# Those messages will be written to temporary file each time +# start/reset methods are called. FAKE_START_MSG = b"start" +FAKE_RESET_MSG = b"reset" TARGET_PLUGIN = 'neutron.plugins.ml2.plugin.Ml2Plugin' @@ -130,6 +131,10 @@ class TestNeutronServer(base.BaseLoggingTestCase): with open(self.temp_file, 'ab') as f: f.write(FAKE_START_MSG) + def _fake_reset(self): + with open(self.temp_file, 'ab') as f: + f.write(FAKE_RESET_MSG) + def _test_restart_service_on_sighup(self, service, workers=1): """Test that a service correctly (re)starts on receiving SIGHUP. @@ -141,7 +146,11 @@ class TestNeutronServer(base.BaseLoggingTestCase): self._start_server(callback=service, workers=workers) os.kill(self.service_pid, signal.SIGHUP) - expected_msg = FAKE_START_MSG * workers * 2 + # After sending SIGHUP it is expected that there will be as many + # FAKE_RESET_MSG as number of workers + one additional for main + # process + expected_msg = ( + FAKE_START_MSG * workers + FAKE_RESET_MSG * (workers + 1)) # Wait for temp file to be created and its size reaching the expected # value @@ -208,8 +217,10 @@ class TestWsgiServer(TestNeutronServer): # Mock start method to check that children are started again on # receiving SIGHUP. - with mock.patch("neutron.wsgi.WorkerService.start") as start_method: + with mock.patch("neutron.wsgi.WorkerService.start") as start_method,\ + mock.patch("neutron.wsgi.WorkerService.reset") as reset_method: start_method.side_effect = self._fake_start + reset_method.side_effect = self._fake_reset server = wsgi.Server("Test") server.start(self.application, 0, "0.0.0.0", @@ -241,19 +252,21 @@ class TestRPCServer(TestNeutronServer): # Mock start method to check that children are started again on # receiving SIGHUP. - with mock.patch("neutron.service.RpcWorker.start") as start_method: - with mock.patch( - "neutron_lib.plugins.directory.get_plugin" - ) as get_plugin: - start_method.side_effect = self._fake_start - get_plugin.return_value = self.plugin + with mock.patch("neutron.service.RpcWorker.start") as start_method,\ + mock.patch( + "neutron.service.RpcWorker.reset") as reset_method,\ + mock.patch( + "neutron_lib.plugins.directory.get_plugin") as get_plugin: + start_method.side_effect = self._fake_start + reset_method.side_effect = self._fake_reset + get_plugin.return_value = self.plugin - CONF.set_override("rpc_workers", workers) - # not interested in state report workers specifically - CONF.set_override("rpc_state_report_workers", 0) + CONF.set_override("rpc_workers", workers) + # not interested in state report workers specifically + CONF.set_override("rpc_state_report_workers", 0) - rpc_workers_launcher = service.start_rpc_workers() - rpc_workers_launcher.wait() + rpc_workers_launcher = service.start_rpc_workers() + rpc_workers_launcher.wait() def test_restart_rpc_on_sighup_multiple_workers(self): self._test_restart_service_on_sighup(service=self._serve_rpc, @@ -292,6 +305,7 @@ class TestPluginWorker(TestNeutronServer): # Make both ABC happy and ensure 'self' is correct FakeWorker.start = self._fake_start + FakeWorker.reset = self._fake_reset workers = [FakeWorker()] self.plugin.return_value.get_workers.return_value = workers self._test_restart_service_on_sighup(service=self._start_plugin,