From 78629e0d3780af4a8a8cc1b5b0762e0bc8a48f1f Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Wed, 27 Mar 2019 08:40:31 +0800 Subject: [PATCH] Remove L3 IP QoS cache when router is down When router admin-state is down or removed, fip-qos and gateway-ip-qos extension should delete the router IPs' QoS rate limit cache. Then if the router is up again the router floating IPs QoS can be reconfigured. This patch achives these: 1. make sure floating IP or gateway IP QoS cache removed. 2. floating IP QoS can be re-configured to snat device when router doing admin_state down/up. Closes-Bug: #1826695 Change-Id: I24fcecd9686ad17fa50093bb8bccab0d6c711298 --- neutron/agent/l3/extensions/qos/fip.py | 22 ++- neutron/tests/fullstack/resources/client.py | 7 +- neutron/tests/fullstack/resources/config.py | 4 + .../tests/fullstack/resources/environment.py | 4 +- neutron/tests/fullstack/test_l3_agent.py | 140 ++++++++++++++---- .../unit/agent/l3/extensions/qos/test_fip.py | 32 +++- .../l3/extensions/qos/test_gateway_ip.py | 17 +++ 7 files changed, 187 insertions(+), 39 deletions(-) diff --git a/neutron/agent/l3/extensions/qos/fip.py b/neutron/agent/l3/extensions/qos/fip.py index 6fd1a49a286..5b5e53114f2 100644 --- a/neutron/agent/l3/extensions/qos/fip.py +++ b/neutron/agent/l3/extensions/qos/fip.py @@ -104,6 +104,17 @@ class RouterFipRateLimitMaps(qos_base.RateLimitMaps): return _get_fip_ratelimit_cache() + def remove_fip_all_cache(self, fip): + for direction in constants.VALID_DIRECTIONS: + self.remove_fip_ratelimit_cache(direction, fip) + self.clean_by_resource(fip) + + def clean_router_all_fip_cache(self, router_id): + floating_ips = self.router_floating_ips.pop( + router_id, []) + for fip in floating_ips: + self.remove_fip_all_cache(fip) + class FipQosAgentExtension(qos_base.L3QosAgentExtensionBase, l3_extension.L3AgentExtension): @@ -143,9 +154,6 @@ class FipQosAgentExtension(qos_base.L3QosAgentExtensionBase, fip, dvr_fip_device, rates, with_cache=False) self.fip_qos_map.update_policy(qos_policy) - def _process_reset_fip(self, fip): - self.fip_qos_map.clean_by_resource(fip) - @coordination.synchronized('qos-floating-ip-{ip}') def process_ip_rate_limit(self, ip, direction, device, rate, burst): @@ -197,7 +205,7 @@ class FipQosAgentExtension(qos_base.L3QosAgentExtensionBase, def get_fip_qos_rates(self, context, fip, policy_id): if policy_id is None: - self._process_reset_fip(fip) + self.fip_qos_map.clean_by_resource(fip) # process_ip_rate_limit will treat value 0 as # cleaning the tc filters if exits or no action. return {constants.INGRESS_DIRECTION: { @@ -321,7 +329,7 @@ class FipQosAgentExtension(qos_base.L3QosAgentExtensionBase, self._remove_fip_rate_limit(device, fip) if dvr_fip_device: self._remove_fip_rate_limit(dvr_fip_device, fip) - self._process_reset_fip(fip) + self.fip_qos_map.clean_by_resource(fip) def add_router(self, context, data): router_info = self._get_router_info(data['id']) @@ -334,9 +342,7 @@ class FipQosAgentExtension(qos_base.L3QosAgentExtensionBase, self.process_floating_ip_addresses(context, router_info) def delete_router(self, context, data): - # NOTE(liuyulong): to delete the router, you need to disassociate the - # floating IP first, so the update_router has done the cache clean. - pass + self.fip_qos_map.clean_router_all_fip_cache(data['id']) def ha_state_change(self, context, data): pass diff --git a/neutron/tests/fullstack/resources/client.py b/neutron/tests/fullstack/resources/client.py index 89e68a6d58b..2a9a7fa2889 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -73,6 +73,9 @@ class ClientFixture(fixtures.Fixture): return self._create_resource(resource_type, spec) + def update_router(self, router_id, **kwargs): + return self._update_resource('router', router_id, kwargs) + def create_network(self, tenant_id, name=None, external=False, network_type=None, segmentation_id=None, physical_network=None, mtu=None): @@ -141,7 +144,7 @@ class ClientFixture(fixtures.Fixture): return self._update_resource('port', port_id, kwargs) def create_floatingip(self, tenant_id, floating_network_id, - fixed_ip_address, port_id): + fixed_ip_address, port_id, qos_policy_id=None): spec = { 'floating_network_id': floating_network_id, 'tenant_id': tenant_id, @@ -149,6 +152,8 @@ class ClientFixture(fixtures.Fixture): 'port_id': port_id } + if qos_policy_id: + spec['qos_policy_id'] = qos_policy_id return self._create_resource('floatingip', spec) def add_router_interface(self, router_id, subnet_id): diff --git a/neutron/tests/fullstack/resources/config.py b/neutron/tests/fullstack/resources/config.py index e96c9f60df7..5230bb819a7 100644 --- a/neutron/tests/fullstack/resources/config.py +++ b/neutron/tests/fullstack/resources/config.py @@ -391,6 +391,10 @@ class L3ConfigFixture(ConfigFixture): self.config['agent'].update({ 'availability_zone': host_desc.availability_zone }) + if host_desc.l3_agent_extensions: + self.config['agent'].update({ + 'extensions': host_desc.l3_agent_extensions + }) def _prepare_config_with_ovs_agent(self, integration_bridge): self.config.update({ diff --git a/neutron/tests/fullstack/resources/environment.py b/neutron/tests/fullstack/resources/environment.py index e1a133749f0..0db0003e675 100644 --- a/neutron/tests/fullstack/resources/environment.py +++ b/neutron/tests/fullstack/resources/environment.py @@ -74,13 +74,15 @@ class HostDescription(object): def __init__(self, l3_agent=False, dhcp_agent=False, l2_agent_type=constants.AGENT_TYPE_OVS, firewall_driver='noop', availability_zone=None, - l3_agent_mode=None): + l3_agent_mode=None, + l3_agent_extensions=None): self.l2_agent_type = l2_agent_type self.l3_agent = l3_agent self.dhcp_agent = dhcp_agent self.firewall_driver = firewall_driver self.availability_zone = availability_zone self.l3_agent_mode = l3_agent_mode + self.l3_agent_extensions = l3_agent_extensions class Host(fixtures.Fixture): diff --git a/neutron/tests/fullstack/test_l3_agent.py b/neutron/tests/fullstack/test_l3_agent.py index d29db14d541..c061765b8e8 100644 --- a/neutron/tests/fullstack/test_l3_agent.py +++ b/neutron/tests/fullstack/test_l3_agent.py @@ -23,6 +23,7 @@ from oslo_utils import uuidutils from neutron.agent.l3 import ha_router from neutron.agent.l3 import namespaces from neutron.agent.linux import ip_lib +from neutron.agent.linux import l3_tc_lib from neutron.common import utils as common_utils from neutron.tests import base as tests_base from neutron.tests.common.exclusive_resources import ip_network @@ -115,22 +116,118 @@ class TestL3Agent(base.BaseFullStackTestCase): # ping router old gateway IP, should fail now external_vm.block_until_no_ping(old_gw_ip) + def _get_namespace(self, router_id, agent=None): + namespace = namespaces.build_ns_name(namespaces.NS_PREFIX, router_id) + if agent: + suffix = agent.get_namespace_suffix() + else: + suffix = self.environment.hosts[0].l3_agent.get_namespace_suffix() + return "%s@%s" % (namespace, suffix) + + def _get_l3_agents_with_ha_state( + self, l3_agents, router_id, ha_state=None): + found_agents = [] + agents_hosting_router = self.client.list_l3_agent_hosting_routers( + router_id)['agents'] + for agent in l3_agents: + agent_host = agent.neutron_cfg_fixture.get_host() + for agent_hosting_router in agents_hosting_router: + if (agent_hosting_router['host'] == agent_host and + ((ha_state is None) or ( + agent_hosting_router['ha_state'] == ha_state))): + found_agents.append(agent) + break + return found_agents + + def _router_fip_qos_after_admin_state_down_up(self, ha=False): + tenant_id = uuidutils.generate_uuid() + ext_net, ext_sub = self._create_external_network_and_subnet(tenant_id) + external_vm = self._create_external_vm(ext_net, ext_sub) + + router = self.safe_client.create_router(tenant_id, + ha=ha, + external_network=ext_net['id']) + + vm = self._create_net_subnet_and_vm( + tenant_id, ['20.0.0.0/24', '2001:db8:aaaa::/64'], + self.environment.hosts[1], router) + # ping external vm to test snat + vm.block_until_ping(external_vm.ip) + + qos_policy = self.safe_client.create_qos_policy( + tenant_id, 'fs_policy', 'Fullstack testing policy', + shared='False', is_default='False') + self.safe_client.create_bandwidth_limit_rule( + tenant_id, qos_policy['id'], 1111, 2222, + constants.INGRESS_DIRECTION) + self.safe_client.create_bandwidth_limit_rule( + tenant_id, qos_policy['id'], 3333, 4444, + constants.EGRESS_DIRECTION) + + fip = self.safe_client.create_floatingip( + tenant_id, ext_net['id'], vm.ip, vm.neutron_port['id'], + qos_policy_id=qos_policy['id']) + # ping floating ip from external vm + external_vm.block_until_ping(fip['floating_ip_address']) + + self.safe_client.update_router(router['id'], admin_state_up=False) + external_vm.block_until_no_ping(fip['floating_ip_address']) + + self.safe_client.update_router(router['id'], admin_state_up=True) + external_vm.block_until_ping(fip['floating_ip_address']) + + if ha: + l3_agents = [host.agents['l3'] for host in self.environment.hosts] + router_agent = self._get_l3_agents_with_ha_state( + l3_agents, router['id'])[0] + qrouter_ns = self._get_namespace( + router['id'], + router_agent) + else: + qrouter_ns = self._get_namespace(router['id']) + ip = ip_lib.IPWrapper(qrouter_ns) + common_utils.wait_until_true(lambda: ip.get_devices()) + + devices = ip.get_devices() + for dev in devices: + if dev.name.startswith("qg-"): + interface_name = dev.name + + tc_wrapper = l3_tc_lib.FloatingIPTcCommand( + interface_name, + namespace=qrouter_ns) + common_utils.wait_until_true( + functools.partial( + self._wait_until_filters_set, + tc_wrapper), + timeout=60) + + def _wait_until_filters_set(self, tc_wrapper): + + def _is_filter_set(direction): + filter_ids = tc_wrapper.get_existing_filter_ids( + direction) + if not filter_ids: + return False + return 1 == len(filter_ids) + return (_is_filter_set(constants.INGRESS_DIRECTION) and + _is_filter_set(constants.EGRESS_DIRECTION)) + class TestLegacyL3Agent(TestL3Agent): def setUp(self): host_descriptions = [ - environment.HostDescription(l3_agent=True, dhcp_agent=True), + environment.HostDescription(l3_agent=True, dhcp_agent=True, + l3_agent_extensions="fip_qos"), environment.HostDescription()] env = environment.Environment( environment.EnvironmentDescription( - network_type='vlan', l2_pop=False), + network_type='vlan', l2_pop=False, + qos=True), host_descriptions) super(TestLegacyL3Agent, self).setUp(env) - def _get_namespace(self, router_id): - return namespaces.build_ns_name(namespaces.NS_PREFIX, router_id) - def test_namespace_exists(self): tenant_id = uuidutils.generate_uuid() @@ -140,9 +237,7 @@ class TestLegacyL3Agent(TestL3Agent): tenant_id, network['id'], '20.0.0.0/24', gateway_ip='20.0.0.1') self.safe_client.add_router_interface(router['id'], subnet['id']) - namespace = "%s@%s" % ( - self._get_namespace(router['id']), - self.environment.hosts[0].l3_agent.get_namespace_suffix(), ) + namespace = self._get_namespace(router['id']) self.assert_namespace_exists(namespace) def test_mtu_update(self): @@ -154,9 +249,7 @@ class TestLegacyL3Agent(TestL3Agent): tenant_id, network['id'], '20.0.0.0/24', gateway_ip='20.0.0.1') self.safe_client.add_router_interface(router['id'], subnet['id']) - namespace = "%s@%s" % ( - self._get_namespace(router['id']), - self.environment.hosts[0].l3_agent.get_namespace_suffix(), ) + namespace = self._get_namespace(router['id']) self.assert_namespace_exists(namespace) ip = ip_lib.IPWrapper(namespace) @@ -259,16 +352,21 @@ class TestLegacyL3Agent(TestL3Agent): def test_gateway_ip_changed(self): self._test_gateway_ip_changed() + def test_router_fip_qos_after_admin_state_down_up(self): + self._router_fip_qos_after_admin_state_down_up() + class TestHAL3Agent(TestL3Agent): def setUp(self): host_descriptions = [ - environment.HostDescription(l3_agent=True, dhcp_agent=True) + environment.HostDescription(l3_agent=True, dhcp_agent=True, + l3_agent_extensions="fip_qos") for _ in range(2)] env = environment.Environment( environment.EnvironmentDescription( - network_type='vlan', l2_pop=True), + network_type='vlan', l2_pop=True, + qos=True), host_descriptions) super(TestHAL3Agent, self).setUp(env) @@ -308,19 +406,6 @@ class TestHAL3Agent(TestL3Agent): if self._get_keepalived_state(keepalived_state_file) == "master": return keepalived_state_file - def _get_l3_agents_with_ha_state(self, l3_agents, router_id, ha_state): - found_agents = [] - agents_hosting_router = self.client.list_l3_agent_hosting_routers( - router_id)['agents'] - for agent in l3_agents: - agent_host = agent.neutron_cfg_fixture.get_host() - for agent_hosting_router in agents_hosting_router: - if (agent_hosting_router['host'] == agent_host and - agent_hosting_router['ha_state'] == ha_state): - found_agents.append(agent) - break - return found_agents - def test_keepalived_multiple_sighups_does_not_forfeit_mastership(self): """Setup a complete "Neutron stack" - both an internal and an external network+subnet, and a router connected to both. @@ -413,3 +498,6 @@ class TestHAL3Agent(TestL3Agent): def test_gateway_ip_changed(self): self._test_gateway_ip_changed() + + def test_router_fip_qos_after_admin_state_down_up(self): + self._router_fip_qos_after_admin_state_down_up(ha=True) diff --git a/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py b/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py index 2def99ead6b..1a597f45041 100644 --- a/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py +++ b/neutron/tests/unit/agent/l3/extensions/qos/test_fip.py @@ -110,15 +110,16 @@ class QosExtensionBaseTestCase(test_agent.BasicRouterOperationsFramework): 'port_id': _uuid(), 'host': HOSTNAME, 'qos_policy_id': self.policy.id} - self.router = {'id': _uuid(), + self.router_id = _uuid() + self.router = {'id': self.router_id, 'gw_port': self.ex_gw_port, 'ha': False, 'distributed': False, lib_const.FLOATINGIP_KEY: [self.fip]} - self.router_info = l3router.RouterInfo(self.agent, _uuid(), + self.router_info = l3router.RouterInfo(self.agent, self.router_id, self.router, **self.ri_kwargs) self.router_info.ex_gw_port = self.ex_gw_port - self.agent.router_info[self.router['id']] = self.router_info + self.agent.router_info[self.router_id] = self.router_info def _mock_get_router_info(router_id): return self.router_info @@ -275,6 +276,31 @@ class FipQosExtensionTestCase(QosExtensionBaseTestCase): TEST_QOS_FIP)], any_order=True) + def test_delete_router(self): + tc_wrapper = mock.Mock() + with mock.patch.object(self.fip_qos_ext, '_get_tc_wrapper', + return_value=tc_wrapper): + self.fip_qos_ext.update_router(self.context, self.router) + tc_wrapper.set_ip_rate_limit.assert_has_calls( + [mock.call(lib_const.INGRESS_DIRECTION, + TEST_QOS_FIP, 1111, 2222), + mock.call(lib_const.EGRESS_DIRECTION, + TEST_QOS_FIP, 3333, 4444)], + any_order=True) + self.fip_qos_ext.delete_router(self.context, self.router) + self.assertIsNone( + self.fip_qos_ext.fip_qos_map.router_floating_ips.get( + self.router_id)) + self.assertIsNone( + self.fip_qos_ext.fip_qos_map.ingress_ratelimits.get( + TEST_QOS_FIP)) + self.assertIsNone( + self.fip_qos_ext.fip_qos_map.egress_ratelimits.get( + TEST_QOS_FIP)) + self.assertIsNone( + self.fip_qos_ext.fip_qos_map.get_resource_policy( + TEST_QOS_FIP)) + def test_update_router_fip_removed(self): self._test_qos_policy_scenarios() diff --git a/neutron/tests/unit/agent/l3/extensions/qos/test_gateway_ip.py b/neutron/tests/unit/agent/l3/extensions/qos/test_gateway_ip.py index e53f222a27e..404d4f7fa73 100644 --- a/neutron/tests/unit/agent/l3/extensions/qos/test_gateway_ip.py +++ b/neutron/tests/unit/agent/l3/extensions/qos/test_gateway_ip.py @@ -196,6 +196,23 @@ class RouterGatewayIPQosAgentExtensionTestCase( def test_update_router(self): self._test_gateway_ip_add(self.gw_ip_qos_ext.update_router) + def test_delete_router(self): + tc_wrapper = mock.Mock() + with mock.patch.object(self.gw_ip_qos_ext, '_get_tc_wrapper', + return_value=tc_wrapper): + self.gw_ip_qos_ext.add_router(self.context, self.router) + tc_wrapper.set_ip_rate_limit.assert_has_calls( + [mock.call(lib_const.INGRESS_DIRECTION, + TEST_QOS_GW_IP, 1111, 2222), + mock.call(lib_const.EGRESS_DIRECTION, + TEST_QOS_GW_IP, 3333, 4444)], + any_order=True) + + self.gw_ip_qos_ext.delete_router(self.context, self.router) + self.assertIsNone( + self.gw_ip_qos_ext.gateway_ip_qos_map.get_resource_policy( + self.router_info.router_id)) + def test__process_update_policy(self): tc_wrapper = mock.Mock() with mock.patch.object(self.gw_ip_qos_ext, '_get_tc_wrapper',