DVR: Clear SNAT namespace when agent restarts after router move

When we manually move a router from one dvr_snat node to
another dvr_snat node the snat_namespace should be removed in
the originating node by the agent and will be re-created in the
destination node by the destination agent.

But when the agent dies, the router_update message reaches the
agent after the agent restarts. At this time the agent should
remove the snat_namespace since it is no more hosted by the
current agent.

Even though we do have logic in agent to take care of cleaning
up the snat namespaces if the gw_port_host does not match with the
existing agent host, in this particular use case the self.snat_namespace
is always set to 'None' in the dvr_edge_router init call when agent
restarts.

This patch fixes the above issue by initializing the snat namespace
object during the router_init. Since we do have a valid snat
namespace object and if the gw_port_host mismatches, the agent
should clean up the namespace.

Change-Id: I30524dc77b743429ef70941479c9b6cccb21c23c
Closes-Bug: #1557909
(cherry picked from commit 9dc70ed77e)
changes/20/313020/1
Swaminathan Vasudevan 7 years ago
parent 6502e95e25
commit b910332985
  1. 16
      neutron/agent/l3/dvr_edge_router.py
  2. 19
      neutron/agent/l3/dvr_snat_ns.py
  3. 91
      neutron/tests/unit/agent/l3/test_agent.py

@ -29,7 +29,12 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
def __init__(self, agent, host, *args, **kwargs):
super(DvrEdgeRouter, self).__init__(agent, host, *args, **kwargs)
self.snat_namespace = None
# NOTE: Initialize the snat_namespace object here.
# The namespace can be created later, just to align with the
# parent init.
self.snat_namespace = dvr_snat_ns.SnatNamespace(
self.router_id, self.agent_conf,
self.driver, self.use_ipv6)
self.snat_iptables_manager = None
def external_gateway_added(self, ex_gw_port, interface_name):
@ -155,10 +160,11 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
# TODO(mlavalle): in the near future, this method should contain the
# code in the L3 agent that creates a gateway for a dvr. The first step
# is to move the creation of the snat namespace here
self.snat_namespace = dvr_snat_ns.SnatNamespace(self.router['id'],
self.agent_conf,
self.driver,
self.use_ipv6)
if not self.snat_namespace:
self.snat_namespace = dvr_snat_ns.SnatNamespace(self.router['id'],
self.agent_conf,
self.driver,
self.use_ipv6)
self.snat_namespace.create()
return self.snat_namespace

@ -35,12 +35,13 @@ class SnatNamespace(namespaces.Namespace):
def delete(self):
ns_ip = ip_lib.IPWrapper(namespace=self.name)
for d in ns_ip.get_devices(exclude_loopback=True):
if d.name.startswith(SNAT_INT_DEV_PREFIX):
LOG.debug('Unplugging DVR device %s', d.name)
self.driver.unplug(d.name, namespace=self.name,
prefix=SNAT_INT_DEV_PREFIX)
# TODO(mrsmith): delete ext-gw-port
LOG.debug('DVR: destroy snat ns: %s', self.name)
super(SnatNamespace, self).delete()
if ns_ip.netns.exists(self.name):
for d in ns_ip.get_devices(exclude_loopback=True):
if d.name.startswith(SNAT_INT_DEV_PREFIX):
LOG.debug('Unplugging DVR device %s', d.name)
self.driver.unplug(d.name, namespace=self.name,
prefix=SNAT_INT_DEV_PREFIX)
# TODO(mrsmith): delete ext-gw-port
LOG.debug('DVR: destroy snat ns: %s', self.name)
super(SnatNamespace, self).delete()

@ -609,6 +609,95 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
def test_external_gateway_updated_dual_stack(self):
self._test_external_gateway_updated(dual_stack=True)
def test_dvr_edge_router_init_for_snat_namespace_object(self):
router = l3_test_common.prepare_router_data(num_internal_ports=2)
ri = dvr_router.DvrEdgeRouter(mock.Mock(),
HOSTNAME,
router['id'],
router,
**self.ri_kwargs)
# Makesure that ri.snat_namespace object is created when the
# router is initialized
self.assertIsNotNone(ri.snat_namespace)
@mock.patch.object(dvr_snat_ns.SnatNamespace, 'delete')
def test_ext_gw_updated_calling_snat_ns_delete_if_gw_port_host_none(
self, mock_snat_ns):
"""Function to check the impact of snat_namespace object.
This function specifically checks the impact of the snat
namespace object value on external_gateway_removed for deleting
snat_namespace when the gw_port_host mismatches or none.
"""
router = l3_test_common.prepare_router_data(num_internal_ports=2)
ri = dvr_router.DvrEdgeRouter(mock.Mock(),
HOSTNAME,
router['id'],
router,
**self.ri_kwargs)
ri._create_snat_namespace()
interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(self,
ri)
router['gw_port_host'] = ''
ri.external_gateway_updated(ex_gw_port, interface_name)
if router['gw_port_host'] != ri.host:
if ri.snat_namespace:
self.assertEqual(1, mock_snat_ns.call_count)
@mock.patch.object(dvr_snat_ns.SnatNamespace, 'delete')
def test_ext_gw_updated_not_calling_snat_ns_delete_if_gw_port_host_none(
self, mock_snat_ns):
"""Function to check the impact of snat_namespace object.
This function specifically checks the impact of the snat
namespace object value on external_gateway_removed for deleting
snat_namespace when gw_port_host mismatches and when the
self.snat_namespace is None.
"""
router = l3_test_common.prepare_router_data(num_internal_ports=2)
ri = dvr_router.DvrEdgeRouter(mock.Mock(),
HOSTNAME,
router['id'],
router,
**self.ri_kwargs)
ri._create_snat_namespace()
interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(self,
ri)
router['gw_port_host'] = ''
# Initialize the snat_namespace object to the None here to emulate
# an agent restart.
ri.snat_namespace = None
ri.external_gateway_updated(ex_gw_port, interface_name)
if router['gw_port_host'] != ri.host:
if ri.snat_namespace is None:
self.assertFalse(mock_snat_ns.called)
@mock.patch.object(namespaces.Namespace, 'delete')
def test_snat_ns_delete_not_called_when_snat_namespace_does_not_exist(
self, mock_ns_del):
"""Function to check the impact of snat_namespace object.
This function specifically checks the impact of the snat
namespace object initialization without the actual creation
of snat_namespace. When deletes are issued to the snat
namespace based on the snat namespace object existence, it
should be checking for the valid namespace existence before
it tries to delete.
"""
router = l3_test_common.prepare_router_data(num_internal_ports=2)
ri = dvr_router.DvrEdgeRouter(mock.Mock(),
HOSTNAME,
router['id'],
router,
**self.ri_kwargs)
# Make sure we set a return value to emulate the non existence
# of the namespace.
self.mock_ip.netns.exists.return_value = False
self.assertTrue(ri.snat_namespace)
if ri.snat_namespace:
ri.snat_namespace.delete()
self.assertFalse(mock_ns_del.called)
def _test_ext_gw_updated_dvr_edge_router(self, host_match,
snat_hosted_before=True):
"""
@ -628,7 +717,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
ri._create_snat_namespace()
snat_ns_name = ri.snat_namespace.name
else:
self.assertIsNone(ri.snat_namespace)
self.assertIsNotNone(ri.snat_namespace)
interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(self,
ri)

Loading…
Cancel
Save