diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 0719b9fabfd..5cf2ecd69b8 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.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 diff --git a/neutron/agent/l3/dvr_snat_ns.py b/neutron/agent/l3/dvr_snat_ns.py index 2e360cca3ae..7941e36ec83 100644 --- a/neutron/agent/l3/dvr_snat_ns.py +++ b/neutron/agent/l3/dvr_snat_ns.py @@ -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) + 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() + # TODO(mrsmith): delete ext-gw-port + LOG.debug('DVR: destroy snat ns: %s', self.name) + super(SnatNamespace, self).delete() diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 1a1bbc77a1b..0dbec6c5e33 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -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)