From: Oleg Bondarev Date: Tue, 7 Jul 2015 10:36:00 +0000 (+0300) Subject: DVR: fix router rescheduling on agent side X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a264b329df82f3c391f75bd9ad73a1327616a43d;p=openstack-build%2Fneutron-build.git DVR: fix router rescheduling on agent side The patch makes L3 agent aware of possible SNAT role rescheduling to/from it. The gist is to compare gw_port host change. If it was changed and agent is not on target host then it needs to clear snat namespace if one exists. If agent is on target host it needs to create snat namespace from scratch if it doesn't exist. Host field was excluded from gw_port comparison on agent side as part of HA Router feature implementation. This code was moved to corresponding module. Closes-Bug: #1472205 Change-Id: I840bded9eb547df014c6fb2b4cbfe4a876b9b878 --- diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index b68af5cde..ea8c71309 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -40,17 +40,27 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): if not self._is_this_snat_host(): # no centralized SNAT gateway for this node/agent LOG.debug("not hosting snat for router: %s", self.router['id']) + if self.snat_namespace: + LOG.debug("SNAT was rescheduled to host %s. Clearing snat " + "namespace.", self.router.get('gw_port_host')) + return self.external_gateway_removed( + ex_gw_port, interface_name) return - self._external_gateway_added(ex_gw_port, - interface_name, - self.snat_namespace.name, - preserve_ips=[]) + if not self.snat_namespace: + # SNAT might be rescheduled to this agent; need to process like + # newly created gateway + return self.external_gateway_added(ex_gw_port, interface_name) + else: + self._external_gateway_added(ex_gw_port, + interface_name, + self.snat_namespace.name, + preserve_ips=[]) def external_gateway_removed(self, ex_gw_port, interface_name): super(DvrEdgeRouter, self).external_gateway_removed(ex_gw_port, interface_name) - if not self._is_this_snat_host(): + if not self._is_this_snat_host() and not self.snat_namespace: # no centralized SNAT gateway for this node/agent LOG.debug("not hosting snat for router: %s", self.router['id']) return diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index e7b7b5020..33d750d30 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -333,6 +333,16 @@ class HaRouter(router.RouterInfo): self.ha_state = state callback(self.router_id, state) + @staticmethod + def _gateway_ports_equal(port1, port2): + def _get_filtered_dict(d, ignore): + return {k: v for k, v in d.items() if k not in ignore} + + keys_to_ignore = set(['binding:host_id']) + port1_filtered = _get_filtered_dict(port1, keys_to_ignore) + port2_filtered = _get_filtered_dict(port2, keys_to_ignore) + return port1_filtered == port2_filtered + def external_gateway_added(self, ex_gw_port, interface_name): self._plug_external_gateway(ex_gw_port, interface_name, self.ns_name) self._add_gateway_vip(ex_gw_port, interface_name) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 8b25f0a6a..81bca3877 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -15,7 +15,6 @@ import netaddr from oslo_log import log as logging -import six from neutron.agent.l3 import namespaces from neutron.agent.linux import ip_lib @@ -479,6 +478,10 @@ class RouterInfo(object): namespace=self.ns_name, prefix=EXTERNAL_DEV_PREFIX) + @staticmethod + def _gateway_ports_equal(port1, port2): + return port1 == port2 + def _process_external_gateway(self, ex_gw_port): # TODO(Carl) Refactor to clarify roles of ex_gw_port vs self.ex_gw_port ex_gw_port_id = (ex_gw_port and ex_gw_port['id'] or @@ -488,19 +491,9 @@ class RouterInfo(object): if ex_gw_port_id: interface_name = self.get_external_device_name(ex_gw_port_id) if ex_gw_port: - def _gateway_ports_equal(port1, port2): - def _get_filtered_dict(d, ignore): - return dict((k, v) for k, v in six.iteritems(d) - if k not in ignore) - - keys_to_ignore = set(['binding:host_id']) - port1_filtered = _get_filtered_dict(port1, keys_to_ignore) - port2_filtered = _get_filtered_dict(port2, keys_to_ignore) - return port1_filtered == port2_filtered - if not self.ex_gw_port: self.external_gateway_added(ex_gw_port, interface_name) - elif not _gateway_ports_equal(ex_gw_port, self.ex_gw_port): + elif not self._gateway_ports_equal(ex_gw_port, self.ex_gw_port): self.external_gateway_updated(ex_gw_port, interface_name) elif not ex_gw_port and self.ex_gw_port: self.external_gateway_removed(self.ex_gw_port, interface_name) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index b59c9cc63..b4921692d 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -578,41 +578,60 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): def test_external_gateway_updated_dual_stack(self): self._test_external_gateway_updated(dual_stack=True) - def _test_ext_gw_updated_dvr_agent_mode(self, host, - agent_mode, expected_call_count): + def _test_ext_gw_updated_dvr_edge_router(self, host_match, + snat_hosted_before=True): + """ + Helper to test external gw update for edge router on dvr_snat agent + + :param host_match: True if new gw host should be the same as agent host + :param snat_hosted_before: True if agent has already been hosting + snat for the router + """ router = l3_test_common.prepare_router_data(num_internal_ports=2) - agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - ri = dvr_router.DvrEdgeRouter(agent, + ri = dvr_router.DvrEdgeRouter(mock.Mock(), HOSTNAME, router['id'], router, **self.ri_kwargs) - ri.create_snat_namespace() + if snat_hosted_before: + ri.create_snat_namespace() + snat_ns_name = ri.snat_namespace.name + else: + self.assertIsNone(ri.snat_namespace) + interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(self, ri) ri._external_gateway_added = mock.Mock() - # test agent mode = dvr (compute node) - router['gw_port_host'] = host - agent.conf.agent_mode = agent_mode + router['gw_port_host'] = ri.host if host_match else (ri.host + 'foo') ri.external_gateway_updated(ex_gw_port, interface_name) - # no gateway should be added on dvr node - self.assertEqual(expected_call_count, - ri._external_gateway_added.call_count) - - def test_ext_gw_updated_dvr_agent_mode(self): - # no gateway should be added on dvr node - self._test_ext_gw_updated_dvr_agent_mode('any-foo', 'dvr', 0) - - def test_ext_gw_updated_dvr_snat_agent_mode_no_host(self): - # no gateway should be added on dvr_snat node without host match - self._test_ext_gw_updated_dvr_agent_mode('any-foo', 'dvr_snat', 0) - - def test_ext_gw_updated_dvr_snat_agent_mode_host(self): - # gateway should be added on dvr_snat node - self._test_ext_gw_updated_dvr_agent_mode(HOSTNAME, - 'dvr_snat', 1) + if not host_match: + self.assertFalse(ri._external_gateway_added.called) + if snat_hosted_before: + # host mismatch means that snat was rescheduled to another + # agent, hence need to verify that gw port was unplugged and + # snat namespace was deleted + self.mock_driver.unplug.assert_called_with( + interface_name, + bridge=self.conf.external_network_bridge, + namespace=snat_ns_name, + prefix=l3_agent.EXTERNAL_DEV_PREFIX) + self.assertIsNone(ri.snat_namespace) + else: + if not snat_hosted_before: + self.assertIsNotNone(ri.snat_namespace) + self.assertTrue(ri._external_gateway_added.called) + + def test_ext_gw_updated_dvr_edge_router(self): + self._test_ext_gw_updated_dvr_edge_router(host_match=True) + + def test_ext_gw_updated_dvr_edge_router_host_mismatch(self): + self._test_ext_gw_updated_dvr_edge_router(host_match=False) + + def test_ext_gw_updated_dvr_dvr_edge_router_snat_rescheduled(self): + self._test_ext_gw_updated_dvr_edge_router(host_match=True, + snat_hosted_before=False) def test_agent_add_external_gateway(self): router = l3_test_common.prepare_router_data(num_internal_ports=2)