openstack/neutron
Code Issues Proposed changes

Make Neutron gate great again

Browse Source 
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 <ralonsoh@redhat.com>

Change-Id: I7a3f20a795c89ab1ab037d046a1101cd5c0287d6
Closes-Bug: #1842659
Closes-Bug: #1842482
This commit is contained in:
Slawek Kaplonski 2019-09-04 21:50:01 +02:00
parent 78aae12a88
commit bacc7abf83
2 changed files with 52 additions and 34 deletions
neutron/tests/functional
privileged/agent/linux
test_ip_lib.py
test_server.py

42
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)

44
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,