Delete qg device during DVR-SNAT router deletion

In the DVR SNAT case, the 'qg' device was not deleted because
of patch:
https://review.openstack.org/#/c/151882/

During functional testing, the device is deleted
during the external bridge deletion. Because that happens after
the SNAT namespace is already deleted, it can cause a kernel
panic or ovs-vswitchd crash for certain OVS versions.

Also added assertions that all router interfaces were properly
cleaned up during functional testing, and enabled the unit tests
to catch this type of error.

Change-Id: Ica86a030860aa967d5b685b5bfb5546a85b2a809
Closes-Bug: #1418097
This commit is contained in:
Assaf Muller 2015-02-13 11:35:02 -05:00
parent 4f5917e9e9
commit a172e20068
3 changed files with 19 additions and 3 deletions

View File

@ -891,9 +891,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
def external_gateway_removed(self, ri, ex_gw_port, interface_name): def external_gateway_removed(self, ri, ex_gw_port, interface_name):
if ri.router['distributed']: if ri.router['distributed']:
self.process_router_floating_ip_nat_rules(ri) self.process_router_floating_ip_nat_rules(ri)
interface_name = self._get_external_device_interface_name( to_fip_interface_name = self._get_external_device_interface_name(
ri, ex_gw_port) ri, ex_gw_port)
self.process_router_floating_ip_addresses(ri, interface_name) self.process_router_floating_ip_addresses(
ri, to_fip_interface_name)
for p in ri.internal_ports: for p in ri.internal_ports:
internal_interface = self.get_internal_device_name(p['id']) internal_interface = self.get_internal_device_name(p['id'])
self._snat_redirect_remove(ri, p, internal_interface) self._snat_redirect_remove(ri, p, internal_interface)

View File

@ -30,6 +30,7 @@ from neutron.agent import l3_agent as l3_agent_main
from neutron.agent.linux import dhcp from neutron.agent.linux import dhcp
from neutron.agent.linux import external_process from neutron.agent.linux import external_process
from neutron.agent.linux import ip_lib from neutron.agent.linux import ip_lib
from neutron.agent.linux import ovs_lib
from neutron.agent.metadata import agent as metadata_agent from neutron.agent.metadata import agent as metadata_agent
from neutron.common import config as common_config from neutron.common import config as common_config
from neutron.common import constants as l3_constants from neutron.common import constants as l3_constants
@ -255,6 +256,14 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase):
for extra_route in router.router['routes']: for extra_route in router.router['routes']:
self.assertIn(extra_route, routes) self.assertIn(extra_route, routes)
def _assert_interfaces_deleted_from_ovs(self):
def assert_ovs_bridge_empty(bridge_name):
bridge = ovs_lib.OVSBridge(bridge_name, self.root_helper)
self.assertFalse(bridge.get_port_name_list())
assert_ovs_bridge_empty(self.agent.conf.ovs_integration_bridge)
assert_ovs_bridge_empty(self.agent.conf.external_network_bridge)
class L3AgentTestCase(L3AgentTestFramework): class L3AgentTestCase(L3AgentTestFramework):
def test_observer_notifications_legacy_router(self): def test_observer_notifications_legacy_router(self):
@ -412,6 +421,7 @@ class L3AgentTestCase(L3AgentTestFramework):
self._delete_router(self.agent, router.router_id) self._delete_router(self.agent, router.router_id)
self._assert_interfaces_deleted_from_ovs()
self._assert_router_does_not_exist(router) self._assert_router_does_not_exist(router)
if enable_ha: if enable_ha:
self.assertFalse(router.keepalived_manager.process.active) self.assertFalse(router.keepalived_manager.process.active)
@ -611,6 +621,7 @@ class TestDvrRouter(L3AgentTestFramework):
self._assert_extra_routes(router) self._assert_extra_routes(router)
self._delete_router(self.agent, router.router_id) self._delete_router(self.agent, router.router_id)
self._assert_interfaces_deleted_from_ovs()
self._assert_router_does_not_exist(router) self._assert_router_does_not_exist(router)
def generate_dvr_router_info(self, enable_ha=False, enable_snat=False): def generate_dvr_router_info(self, enable_ha=False, enable_snat=False):

View File

@ -475,7 +475,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
elif action == 'remove': elif action == 'remove':
self.device_exists.return_value = True self.device_exists.return_value = True
agent.external_gateway_removed(ri, ex_gw_port, interface_name) agent.external_gateway_removed(ri, ex_gw_port, interface_name)
self.assertEqual(self.mock_driver.unplug.call_count, 1) self.mock_driver.unplug.assert_called_once_with(
interface_name,
bridge=agent.conf.external_network_bridge,
namespace=mock.ANY,
prefix=mock.ANY)
else: else:
raise Exception("Invalid action %s" % action) raise Exception("Invalid action %s" % action)