From 3162846a7b42d860533df6dc32d9c553669df45b Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Mon, 22 Feb 2016 15:20:46 -0800 Subject: [PATCH] DVR: Create router to fip namespace connection based on gateway state In order to route traffic between the internal subnets and the external subnet that belong to the same address_scopes we need to create the gateway port and the fip namespace irrespective of the configured floatingips for the internal subnet. This will consume an additional IP from the external subnet on all nodes, but with the introduction of service_type networks, this will not be an issue any more. This patch is the first in series that creates the agent gateway port and the fip namespace on every node when the gateway is set for the router. For every router created it will connect the router namespace to the fip namespace. Partial-Bug: #1577488 DocImpact: Document the change in behavior for fip-agent-gw create Change-Id: I30c4f7fc250e486fe9a71b68540e783e90a6cf15 --- neutron/agent/l3/dvr_fip_ns.py | 73 ++++++++++-- neutron/agent/l3/dvr_local_router.py | 97 ++++----------- neutron/agent/l3/item_allocator.py | 21 +++- neutron/db/l3_dvr_db.py | 25 ++-- .../functional/agent/l3/test_dvr_router.py | 112 ++++++++++++++---- .../l3_router/test_l3_dvr_router_plugin.py | 16 ++- neutron/tests/unit/agent/l3/test_agent.py | 53 ++++----- .../tests/unit/agent/l3/test_dvr_fip_ns.py | 20 ++-- .../unit/agent/l3/test_dvr_local_router.py | 30 +---- .../unit/agent/l3/test_item_allocator.py | 6 +- neutron/tests/unit/db/test_l3_dvr_db.py | 12 +- ...mespace-on-all-nodes-c4da7ccd60ee62f5.yaml | 13 ++ 12 files changed, 277 insertions(+), 201 deletions(-) create mode 100644 releasenotes/notes/dvr-fip-namespace-on-all-nodes-c4da7ccd60ee62f5.yaml diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 80729dea1e9..b68faa690ab 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -15,6 +15,7 @@ import contextlib import os +from neutron_lib import constants as lib_constants from oslo_concurrency import lockutils from oslo_log import log as logging from oslo_utils import excutils @@ -125,6 +126,13 @@ class FipNamespace(namespaces.Namespace): with self._fip_port_lock(interface_name): is_first = self.subscribe(agent_gateway_port['network_id']) if is_first: + # Check for subnets that are populated for the agent + # gateway port that was created on the server. + if 'subnets' not in agent_gateway_port: + self.unsubscribe(agent_gateway_port['network_id']) + LOG.debug('DVR: Missing subnet in agent_gateway_port: %s', + agent_gateway_port) + return self._create_gateway_port(agent_gateway_port, interface_name) else: try: @@ -285,10 +293,8 @@ class FipNamespace(namespaces.Namespace): # We reraise the exception in order to resync the router. with excutils.save_and_reraise_exception(): self.unsubscribe(self.agent_gateway_port['network_id']) - # Reset the fip count so that the create_rtr_2_fip_link - # is called again in this context - ri.dist_fip_count = 0 - LOG.exception(_LE('DVR: Gateway update route in FIP namespace ' + self.agent_gateway_port = None + LOG.exception(_LE('DVR: Gateway setup in FIP namespace ' 'failed')) # Now add the filter match rule for the table. @@ -351,6 +357,55 @@ class FipNamespace(namespaces.Namespace): if not device.addr.list(to=ip_cidr): device.addr.add(ip_cidr, add_broadcast=False) + def delete_rtr_2_fip_link(self, ri): + """Delete the interface between router and FloatingIP namespace.""" + LOG.debug("Delete FIP link interfaces for router: %s", ri.router_id) + rtr_2_fip_name = self.get_rtr_ext_device_name(ri.router_id) + fip_2_rtr_name = self.get_int_device_name(ri.router_id) + fip_ns_name = self.get_name() + + # remove default route entry + if ri.rtr_fip_subnet is None: + # see if there is a local subnet in the cache + ri.rtr_fip_subnet = self.local_subnets.lookup(ri.router_id) + + if ri.rtr_fip_subnet: + rtr_2_fip, fip_2_rtr = ri.rtr_fip_subnet.get_pair() + device = ip_lib.IPDevice(rtr_2_fip_name, namespace=ri.ns_name) + if device.exists(): + device.route.delete_gateway(str(fip_2_rtr.ip), + table=FIP_RT_TBL) + if self.agent_gateway_port: + interface_name = self.get_ext_device_name( + self.agent_gateway_port['id']) + fg_device = ip_lib.IPDevice( + interface_name, namespace=fip_ns_name) + if fg_device.exists(): + # Remove the fip namespace rules and routes associated to + # fpr interface route table. + tbl_index = ri._get_snat_idx(fip_2_rtr) + fip_rt_rule = ip_lib.IPRule(namespace=fip_ns_name) + # Flush the table + fg_device.route.flush(lib_constants.IP_VERSION_4, + table=tbl_index) + fg_device.route.flush(lib_constants.IP_VERSION_6, + table=tbl_index) + # Remove the rule lookup + # IP is ignored in delete, but we still require it + # for getting the ip_version. + fip_rt_rule.rule.delete(ip=fip_2_rtr.ip, + iif=fip_2_rtr_name, + table=tbl_index, + priority=tbl_index) + self.local_subnets.release(ri.router_id) + ri.rtr_fip_subnet = None + + # Check for namespace before deleting the device + if not self.destroyed: + fns_ip = ip_lib.IPWrapper(namespace=fip_ns_name) + if fns_ip.device(fip_2_rtr_name).exists(): + fns_ip.del_veth(fip_2_rtr_name) + def create_rtr_2_fip_link(self, ri): """Create interface between router and Floating IP namespace.""" LOG.debug("Create FIP link interfaces for router %s", ri.router_id) @@ -386,16 +441,14 @@ class FipNamespace(namespaces.Namespace): rtr_2_fip_dev.route.add_gateway(str(fip_2_rtr.ip), table=FIP_RT_TBL) def scan_fip_ports(self, ri): - # don't scan if not dvr or count is not None - if ri.dist_fip_count is not None: - return - # scan system for any existing fip ports - ri.dist_fip_count = 0 rtr_2_fip_interface = self.get_rtr_ext_device_name(ri.router_id) device = ip_lib.IPDevice(rtr_2_fip_interface, namespace=ri.ns_name) if device.exists(): - ri.dist_fip_count = len(ri.get_router_cidrs(device)) + if len(ri.get_router_cidrs(device)): + self.rtr_fip_connect = True + else: + self.rtr_fip_connect = False # On upgrade, there could be stale IP addresses configured, check # and remove them once. # TODO(haleyb): this can go away after a cycle or two diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 767b87e809d..cfa02e35e9b 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -45,7 +45,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.floating_ips_dict = {} # Linklocal subnet for router and floating IP namespace link self.rtr_fip_subnet = None - self.dist_fip_count = None + self.rtr_fip_connect = False self.fip_ns = None self._pending_arp_set = set() @@ -104,8 +104,6 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): interface_name, floating_ip, self.agent_conf.send_arp_for_ha) - # update internal structures - self.dist_fip_count = self.dist_fip_count + 1 def _add_floating_ip_rule(self, floating_ip, fixed_ip): rule_pr = self.fip_ns.allocate_rule_priority(floating_ip) @@ -128,52 +126,18 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): def floating_ip_removed_dist(self, fip_cidr): """Remove floating IP from FIP namespace.""" floating_ip = fip_cidr.split('/')[0] - rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(self.router_id) fip_2_rtr_name = self.fip_ns.get_int_device_name(self.router_id) if self.rtr_fip_subnet is None: - self.rtr_fip_subnet = self.fip_ns.local_subnets.allocate( + self.rtr_fip_subnet = self.fip_ns.local_subnets.lookup( self.router_id) + if self.rtr_fip_subnet: + rtr_2_fip, fip_2_rtr = self.rtr_fip_subnet.get_pair() + fip_ns_name = self.fip_ns.get_name() + self._remove_floating_ip_rule(floating_ip) - rtr_2_fip, fip_2_rtr = self.rtr_fip_subnet.get_pair() - fip_ns_name = self.fip_ns.get_name() - self._remove_floating_ip_rule(floating_ip) + device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name) - device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name) - - device.route.delete_route(fip_cidr, str(rtr_2_fip.ip)) - # check if this is the last FIP for this router - self.dist_fip_count = self.dist_fip_count - 1 - if self.dist_fip_count == 0: - #remove default route entry - device = ip_lib.IPDevice(rtr_2_fip_name, namespace=self.ns_name) - ns_ip = ip_lib.IPWrapper(namespace=fip_ns_name) - device.route.delete_gateway(str(fip_2_rtr.ip), - table=dvr_fip_ns.FIP_RT_TBL) - if self.fip_ns.agent_gateway_port: - interface_name = self.fip_ns.get_ext_device_name( - self.fip_ns.agent_gateway_port['id']) - fg_device = ip_lib.IPDevice( - interface_name, namespace=fip_ns_name) - if fg_device.exists(): - # Remove the fip namespace rules and routes associated to - # fpr interface route table. - tbl_index = self._get_snat_idx(fip_2_rtr) - fip_rt_rule = ip_lib.IPRule(namespace=fip_ns_name) - # Flush the table - fg_device.route.flush(lib_constants.IP_VERSION_4, - table=tbl_index) - fg_device.route.flush(lib_constants.IP_VERSION_6, - table=tbl_index) - # Remove the rule lookup - # IP is ignored in delete, but we still require it - # for getting the ip_version. - fip_rt_rule.rule.delete(ip=fip_2_rtr.ip, - iif=fip_2_rtr_name, - table=tbl_index, - priority=tbl_index) - self.fip_ns.local_subnets.release(self.router_id) - self.rtr_fip_subnet = None - ns_ip.del_veth(fip_2_rtr_name) + device.route.delete_route(fip_cidr, str(rtr_2_fip.ip)) def floating_ip_moved_dist(self, fip): """Handle floating IP move between fixed IPs.""" @@ -470,6 +434,10 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): to_fip_interface_name = ( self.get_external_device_interface_name(ex_gw_port)) self.process_floating_ip_addresses(to_fip_interface_name) + # Remove the router to fip namespace connection after the + # gateway is removed. + self.fip_ns.delete_rtr_2_fip_link(self) + self.rtr_fip_connect = False # NOTE:_snat_redirect_remove should be only called when the # gateway is cleared and should not be called when the gateway # is moved or rescheduled. @@ -530,38 +498,25 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): def process_external(self): ex_gw_port = self.get_ex_gw_port() if ex_gw_port: - self.create_dvr_fip_interfaces(ex_gw_port) + self.create_dvr_external_gateway_on_agent(ex_gw_port) + self.connect_rtr_2_fip() super(DvrLocalRouter, self).process_external() - def create_dvr_fip_interfaces(self, ex_gw_port): - floating_ips = self.get_floating_ips() + def connect_rtr_2_fip(self): + if self.fip_ns.agent_gateway_port and not self.rtr_fip_connect: + self.fip_ns.create_rtr_2_fip_link(self) + self.rtr_fip_connect = True + self.routes_updated([], self.router['routes']) + + def create_dvr_external_gateway_on_agent(self, ex_gw_port): fip_agent_port = self.get_floating_agent_gw_interface( ex_gw_port['network_id']) - if fip_agent_port: + if not fip_agent_port: + fip_agent_port = self.agent.plugin_rpc.get_agent_gateway_port( + self.agent.context, ex_gw_port['network_id']) LOG.debug("FloatingIP agent gateway port received from the " - "plugin: %s", fip_agent_port) - if floating_ips: - if not fip_agent_port: - LOG.debug("No FloatingIP agent gateway port possibly due to " - "late binding of the private port to the host, " - "requesting agent gateway port for 'network-id' :" - "%s", ex_gw_port['network_id']) - fip_agent_port = self.agent.plugin_rpc.get_agent_gateway_port( - self.agent.context, ex_gw_port['network_id']) - if not fip_agent_port: - LOG.error(_LE("No FloatingIP agent gateway port " - "returned from server for 'network-id': " - "%s"), ex_gw_port['network_id']) - if fip_agent_port: - if 'subnets' not in fip_agent_port: - LOG.error(_LE('Missing subnet/agent_gateway_port')) - else: - self.fip_ns.create_or_update_gateway_port(fip_agent_port) - - if (self.fip_ns.agent_gateway_port and - (self.dist_fip_count == 0)): - self.fip_ns.create_rtr_2_fip_link(self) - self.routes_updated([], self.router['routes']) + "plugin: %s", fip_agent_port) + self.fip_ns.create_or_update_gateway_port(fip_agent_port) def update_routing_table(self, operation, route): # TODO(Swami): The static routes should be added to the diff --git a/neutron/agent/l3/item_allocator.py b/neutron/agent/l3/item_allocator.py index b2c8d95e176..ebf228d20eb 100644 --- a/neutron/agent/l3/item_allocator.py +++ b/neutron/agent/l3/item_allocator.py @@ -64,6 +64,18 @@ class ItemAllocator(object): LOG.debug("Re-writing file %s due to read error", state_file) self._write_allocations() + def lookup(self, key): + """Try to lookup an item of ItemClass type. + + See if there are any current or remembered allocations for the key. + """ + if key in self.allocations: + return self.allocations[key] + + if key in self.remembered: + self.allocations[key] = self.remembered.pop(key) + return self.allocations[key] + def allocate(self, key): """Try to allocate an item of ItemClass type. @@ -81,12 +93,9 @@ class ItemAllocator(object): allocations to free the pool. This final desperate step will not happen often in practice. """ - if key in self.allocations: - return self.allocations[key] - - if key in self.remembered: - self.allocations[key] = self.remembered.pop(key) - return self.allocations[key] + entry = self.lookup(key) + if entry: + return entry if not self.pool: # Desperate times. Try to get more in the pool. diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 38253874907..ac327606ae0 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -491,22 +491,29 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, return routers - def _process_routers(self, context, routers): + def _process_routers(self, context, routers, agent): routers_dict = {} snat_intfs_by_router_id = self._get_snat_sync_interfaces( context, [r['id'] for r in routers]) + fip_sync_interfaces = None + LOG.debug("FIP Agent: %s ", agent.id) for router in routers: routers_dict[router['id']] = router if router['gw_port_id']: snat_router_intfs = snat_intfs_by_router_id[router['id']] LOG.debug("SNAT ports returned: %s ", snat_router_intfs) router[l3_const.SNAT_ROUTER_INTF_KEY] = snat_router_intfs + if not fip_sync_interfaces: + fip_sync_interfaces = self._get_fip_sync_interfaces( + context, agent.id) + LOG.debug("FIP Agent ports: %s", fip_sync_interfaces) + router[l3_const.FLOATINGIP_AGENT_INTF_KEY] = ( + fip_sync_interfaces) + return routers_dict def _process_floating_ips_dvr(self, context, routers_dict, - floating_ips, host, agent): - fip_sync_interfaces = None - LOG.debug("FIP Agent : %s ", agent.id) + floating_ips, host): for floating_ip in floating_ips: router = routers_dict.get(floating_ip['router_id']) if router: @@ -518,12 +525,6 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, LOG.debug("Floating IP host: %s", floating_ip['host']) router_floatingips.append(floating_ip) router[const.FLOATINGIP_KEY] = router_floatingips - if not fip_sync_interfaces: - fip_sync_interfaces = self._get_fip_sync_interfaces( - context, agent.id) - LOG.debug("FIP Agent ports: %s", fip_sync_interfaces) - router[l3_const.FLOATINGIP_AGENT_INTF_KEY] = ( - fip_sync_interfaces) def _get_fip_sync_interfaces(self, context, fip_agent_id): """Query router interfaces that relate to list of router_ids.""" @@ -569,9 +570,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, fip['dest_host'] = ( self._get_dvr_migrating_service_port_hostid( context, fip['port_id'], port=vm_port)) - routers_dict = self._process_routers(context, routers) + routers_dict = self._process_routers(context, routers, agent) self._process_floating_ips_dvr(context, routers_dict, - floating_ips, host, agent) + floating_ips, host) ports_to_populate = [] for router in routers_dict.values(): if router.get('gw_port'): diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index cb3216be40e..619072cdf66 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -87,8 +87,6 @@ class TestDvrRouter(framework.L3AgentTestFramework): """Test to validate snat redirect rules not cleared with snat move.""" self.agent.conf.agent_mode = 'dvr_snat' router_info = self.generate_dvr_router_info(enable_snat=True) - router_info[lib_constants.FLOATINGIP_KEY] = [] - router_info[n_const.FLOATINGIP_AGENT_INTF_KEY] = [] router1 = self.manage_router(self.agent, router_info) router1.router['gw_port_host'] = "" self.agent._process_updated_router(router1.router) @@ -96,19 +94,21 @@ class TestDvrRouter(framework.L3AgentTestFramework): self.assertTrue(self._namespace_exists(router_updated.ns_name)) ns_ipr = ip_lib.IPRule(namespace=router1.ns_name) ip4_rules_list = ns_ipr.rule.list_rules(lib_constants.IP_VERSION_4) - self.assertEqual(5, len(ip4_rules_list)) - # IPRule list should have 5 entries. + self.assertEqual(6, len(ip4_rules_list)) + # IPRule list should have 6 entries. # Three entries from 'default', 'main' and 'local' table. + # One rule for the floatingip. # The remaining 2 is for the two router interfaces(csnat ports). default_rules_list_count = 0 interface_rules_list_count = 0 for ip_rule in ip4_rules_list: tbl_index = ip_rule['table'] - if tbl_index in ['local', 'default', 'main']: + if tbl_index in ['local', 'default', 'main', + str(dvr_fip_ns.FIP_RT_TBL)]: default_rules_list_count = default_rules_list_count + 1 else: interface_rules_list_count = interface_rules_list_count + 1 - self.assertEqual(3, default_rules_list_count) + self.assertEqual(4, default_rules_list_count) self.assertEqual(2, interface_rules_list_count) def test_dvr_update_gateway_port_with_no_gw_port_in_namespace(self): @@ -177,13 +177,16 @@ class TestDvrRouter(framework.L3AgentTestFramework): self.mock_plugin_api.get_external_network_id.return_value = ext_net_id # Create the fip namespace up front - dvr_fip_ns.FipNamespace(ext_net_id, - self.agent.conf, - self.agent.driver, - self.agent.use_ipv6).create() + fip_ns = dvr_fip_ns.FipNamespace(ext_net_id, + self.agent.conf, + self.agent.driver, + self.agent.use_ipv6) + fip_ns.create() # Create the router with the fip, this shouldn't allow the # update_gateway_port to be called without the fg- port fip_subscribe.return_value = False + fip_ns.agent_gateway_port = ( + router_info[n_const.FLOATINGIP_AGENT_INTF_KEY]) # This will raise the exception and will also clear # subscription for the ext_net_id self.assertRaises(n_exc.FloatingIpSetupException, @@ -191,6 +194,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): self.agent, router_info) fip_subscribe.return_value = True + self.manage_router(self.agent, router_info) # Now update the router again router = self.manage_router(self.agent, router_info) fg_port = router.fip_ns.agent_gateway_port @@ -369,8 +373,10 @@ class TestDvrRouter(framework.L3AgentTestFramework): # gateway_port information before the l3_agent will create it. # The port returned needs to have the same information as # router_info['gw_port'] - self.mock_plugin_api.get_agent_gateway_port.return_value = router_info[ - 'gw_port'] + fip_agent_gw_port = self._get_fip_agent_gw_port_for_router( + router_info['gw_port']) + self.mock_plugin_api.get_agent_gateway_port.return_value = ( + fip_agent_gw_port) # We also need to mock the get_external_network_id method to # get the correct fip namespace. @@ -440,12 +446,13 @@ class TestDvrRouter(framework.L3AgentTestFramework): enable_gw=True, agent=None, extra_routes=False, + enable_floating_ip=True, **kwargs): if not agent: agent = self.agent router = l3_test_common.prepare_router_data( enable_snat=enable_snat, - enable_floating_ip=True, + enable_floating_ip=enable_floating_ip, enable_ha=enable_ha, extra_routes=extra_routes, num_internal_ports=2, @@ -454,25 +461,58 @@ class TestDvrRouter(framework.L3AgentTestFramework): internal_ports = router.get(lib_constants.INTERFACE_KEY, []) router['distributed'] = True router['gw_port_host'] = agent.conf.host - - floating_ip = router['_floatingips'][0] - floating_ip['host'] = agent.conf.host + if enable_floating_ip: + floating_ip = router['_floatingips'][0] + floating_ip['host'] = agent.conf.host if enable_gw: external_gw_port = router['gw_port'] router['gw_port'][portbindings.HOST_ID] = agent.conf.host - floating_ip['floating_network_id'] = external_gw_port['network_id'] - floating_ip['port_id'] = internal_ports[0]['id'] - floating_ip['status'] = 'ACTIVE' - self._add_snat_port_info_to_router(router, internal_ports) # FIP has a dependency on external gateway. So we need to create # the snat_port info and fip_agent_gw_port_info irrespective of # the agent type the dvr supports. The namespace creation is # dependent on the agent_type. - self._add_fip_agent_gw_port_info_to_router(router, - external_gw_port) + if enable_floating_ip: + floating_ip = router['_floatingips'][0] + floating_ip['host'] = agent.conf.host + floating_ip['floating_network_id'] = ( + external_gw_port['network_id']) + floating_ip['port_id'] = internal_ports[0]['id'] + floating_ip['status'] = 'ACTIVE' + + self._add_fip_agent_gw_port_info_to_router(router, + external_gw_port) return router + def _get_fip_agent_gw_port_for_router( + self, external_gw_port): + # Add fip agent gateway port information to the router_info + if external_gw_port: + # Get values from external gateway port + fixed_ip = external_gw_port['fixed_ips'][0] + float_subnet = external_gw_port['subnets'][0] + port_ip = fixed_ip['ip_address'] + # Pick an ip address which is not the same as port_ip + fip_gw_port_ip = str(netaddr.IPAddress(port_ip) + 5) + # Add floatingip agent gateway port info to router + prefixlen = netaddr.IPNetwork(float_subnet['cidr']).prefixlen + fip_agent_gw_port_info = { + 'subnets': [ + {'cidr': float_subnet['cidr'], + 'gateway_ip': float_subnet['gateway_ip'], + 'id': fixed_ip['subnet_id']}], + 'network_id': external_gw_port['network_id'], + 'device_owner': lib_constants.DEVICE_OWNER_AGENT_GW, + 'mac_address': 'fa:16:3e:80:8d:89', + portbindings.HOST_ID: self.agent.conf.host, + 'fixed_ips': [{'subnet_id': fixed_ip['subnet_id'], + 'ip_address': fip_gw_port_ip, + 'prefixlen': prefixlen}], + 'id': framework._uuid(), + 'device_id': framework._uuid() + } + return fip_agent_gw_port_info + def _add_fip_agent_gw_port_info_to_router(self, router, external_gw_port): # Add fip agent gateway port information to the router_info fip_gw_port_list = router.get( @@ -853,7 +893,10 @@ class TestDvrRouter(framework.L3AgentTestFramework): router1 = self.manage_router(self.agent, router_info) fip_ns = router1.fip_ns.get_name() self.assertTrue(self._namespace_exists(router1.ns_name)) - self.assertFalse(self._namespace_exists(fip_ns)) + # FIP Namespace creation does not depend on the floatingip's + # anymore and will be created on each agent when there is + # a valid gateway. + self.assertTrue(self._namespace_exists(fip_ns)) self._assert_snat_namespace_does_not_exist(router1) def test_dvr_router_fip_create_for_migrating_port(self): @@ -903,6 +946,26 @@ class TestDvrRouter(framework.L3AgentTestFramework): self.assertTrue(self._namespace_exists(fip_ns)) self._assert_snat_namespace_does_not_exist(router1) + def test_dvr_router_fip_namespace_create_without_floatingip(self): + """Test to validate the floatingip namespace creation without fip. + + This test validates the condition where floatingip namespace gets + created on the agent when the gateway is added and without floatingip + configured for the router. + """ + self.agent.conf.agent_mode = 'dvr' + router_info = self.generate_dvr_router_info(enable_floating_ip=False) + fip_agent_gw_port = self._get_fip_agent_gw_port_for_router( + router_info['gw_port']) + self.mock_plugin_api.get_agent_gateway_port.return_value = ( + fip_agent_gw_port) + router1 = self.manage_router(self.agent, router_info) + fip_ns = router1.fip_ns.get_name() + self.assertTrue(self._namespace_exists(router1.ns_name)) + self.assertTrue(self._namespace_exists(fip_ns)) + self.assertTrue(router1.rtr_fip_connect) + self._assert_snat_namespace_does_not_exist(router1) + def _assert_snat_namespace_exists(self, router): namespace = dvr_snat_ns.SnatNamespace.get_snat_ns_name( router.router_id) @@ -1111,6 +1174,9 @@ class TestDvrRouter(framework.L3AgentTestFramework): self._assert_extra_routes(router_updated, namespace=snat_ns_name) if check_fpr_int_rule_delete: router_updated.router[lib_constants.FLOATINGIP_KEY] = [] + router_updated.router['gw_port'] = "" + router_updated.router['gw_port_host'] = "" + router_updated.router['external_gateway_info'] = "" self.agent._process_updated_router(router_updated.router) new_router_info = self.agent.router_info[router_updated.router_id] self.assertTrue(self._namespace_exists(fip_ns_name)) diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py index dba4c20613d..4c62c72de02 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py @@ -21,6 +21,7 @@ from neutron.api.rpc.handlers import l3_rpc from neutron.callbacks import events from neutron.callbacks import registry from neutron.callbacks import resources +from neutron.common import constants as n_const from neutron.common import topics from neutron.extensions import external_net from neutron.extensions import l3 @@ -177,9 +178,22 @@ class L3DvrTestCase(L3DvrTestCaseBase): def test_process_routers(self): router = self._create_router() - result = self.l3_plugin._process_routers(self.context, [router]) + if not router.get('gw_port_id'): + router['gw_port_id'] = 'fake_gw_id' + self.l3_plugin._get_fip_sync_interfaces = mock.Mock( + return_value='fip_interface') + self.l3_plugin._get_snat_sync_interfaces = mock.Mock( + return_value={router['id']: 'snat_interface'}) + result = self.l3_plugin._process_routers(self.context, [router], + self.l3_agent) + self.assertEqual( router['id'], result[router['id']]['id']) + self.assertIn(n_const.FLOATINGIP_AGENT_INTF_KEY, result[router['id']]) + self.l3_plugin._get_fip_sync_interfaces.assert_called_once_with( + self.context, self.l3_agent['id']) + self.l3_plugin._get_snat_sync_interfaces.assert_called_once_with( + self.context, [router['id']]) def test_agent_gw_port_delete_when_last_gateway_for_ext_net_removed(self): kwargs = {'arg_list': (external_net.EXTERNAL,), diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 8d478aa1eaf..56fcb8d506e 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -968,7 +968,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router = ri.router agent.host = HOSTNAME fake_fip_id = 'fake_fip_id' - ri.create_dvr_fip_interfaces = mock.Mock() + ri.create_dvr_external_gateway_on_agent = mock.Mock() ri.process_floating_ip_addresses = mock.Mock() ri.process_floating_ip_nat_rules = mock.Mock() ri.process_floating_ip_addresses.return_value = { @@ -1047,7 +1047,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): with mock.patch.object(lla.LinkLocalAllocator, '_write'): if ri.router['distributed']: ri.fip_ns = agent.get_fip_ns(ex_gw_port['network_id']) - ri.create_dvr_fip_interfaces(ex_gw_port) + ri.create_dvr_external_gateway_on_agent(ex_gw_port) fip_statuses = ri.process_floating_ip_addresses( mock.sentinel.interface_name) self.assertEqual({fip_id: lib_constants.FLOATINGIP_STATUS_ACTIVE}, @@ -1089,37 +1089,34 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ri = dvr_router.DvrEdgeRouter(HOSTNAME, **self.ri_kwargs) ext_gw_port = ri.router.get('gw_port') ri.fip_ns = agent.get_fip_ns(ext_gw_port['network_id']) - ri.dist_fip_count = 0 agent.process_router_add = mock.Mock() ri.fip_ns.create_rtr_2_fip_link = mock.Mock() with mock.patch.object(ri, 'get_floating_ips') as fips, \ mock.patch.object(ri.fip_ns, - 'create') as create_fip, \ + 'create') as create_fip, \ mock.patch.object(ri, 'get_floating_agent_gw_interface' ) as fip_gw_port: fips.return_value = fake_floatingips fip_gw_port.return_value = agent_gateway_port[0] - ri.create_dvr_fip_interfaces(ext_gw_port) + ri.create_dvr_external_gateway_on_agent(ext_gw_port) + ri.connect_rtr_2_fip() self.assertTrue(fip_gw_port.called) - self.assertTrue(fips.called) self.assertTrue(create_fip.called) self.assertEqual(agent_gateway_port[0], ri.fip_ns.agent_gateway_port) + self.assertTrue(ri.rtr_fip_connect) # Now let us associate the fip to the router ri.floating_ip_added_dist(fips, "192.168.0.1/32") - self.assertEqual(1, ri.dist_fip_count) # Now let us disassociate the fip from the router ri.floating_ip_removed_dist("192.168.0.1/32") - self.assertEqual(0, ri.dist_fip_count) - # Calling create_dvr_fip_interfaces again to make sure - # that the fip namespace create is not called again. + # Calling create_dvr_external_gateway_interfaces again to make + # sure that the fip namespace create is not called again. # If the create is not called again, that would contain # the duplicate rules configuration in the fip namespace. - ri.create_dvr_fip_interfaces(ext_gw_port) + ri.create_dvr_external_gateway_on_agent(ext_gw_port) self.assertTrue(fip_gw_port.called) - self.assertTrue(fips.called) create_fip.assert_called_once_with() - self.assertEqual(2, ri.fip_ns.create_rtr_2_fip_link.call_count) + self.assertEqual(1, ri.fip_ns.create_rtr_2_fip_link.call_count) @mock.patch.object(lla.LinkLocalAllocator, '_write') def test_create_dvr_fip_interfaces_for_late_binding(self, lla_write): @@ -1156,13 +1153,14 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ext_gw_port = ri.router.get('gw_port') ri.fip_ns = agent.get_fip_ns(ext_gw_port['network_id']) - ri.dist_fip_count = 0 ri.fip_ns.subscribe = mock.Mock() with mock.patch.object(agent.plugin_rpc, 'get_agent_gateway_port') as fip_gw_port: fip_gw_port.return_value = agent_gateway_port - ri.create_dvr_fip_interfaces(ext_gw_port) + ri.create_dvr_external_gateway_on_agent(ext_gw_port) + ri.connect_rtr_2_fip() self.assertTrue(fip_gw_port.called) + self.assertTrue(ri.rtr_fip_connect) self.assertEqual(agent_gateway_port, ri.fip_ns.agent_gateway_port) @@ -1201,21 +1199,20 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ext_gw_port = ri.router.get('gw_port') ri.fip_ns = agent.get_fip_ns(ext_gw_port['network_id']) - ri.dist_fip_count = 0 ri.fip_ns.subscribe = mock.Mock() ri.fip_ns.agent_router_gateway = mock.Mock() agent.process_router_add = mock.Mock() - with mock.patch.object(ri, 'get_floating_ips') as fips, \ - mock.patch.object(ri, 'get_floating_agent_gw_interface' - ) as fip_gw_port: - fips.return_value = fake_floatingips + with mock.patch.object( + ri, + 'get_floating_agent_gw_interface') as fip_gw_port: fip_gw_port.return_value = agent_gateway_port[0] - ri.create_dvr_fip_interfaces(ext_gw_port) + ri.create_dvr_external_gateway_on_agent(ext_gw_port) + ri.connect_rtr_2_fip() self.assertTrue(fip_gw_port.called) - self.assertTrue(fips.called) self.assertEqual(agent_gateway_port[0], ri.fip_ns.agent_gateway_port) + self.assertTrue(ri.rtr_fip_connect) self.assertTrue(ri.rtr_fip_subnet) @mock.patch.object(lla.LinkLocalAllocator, '_write') @@ -1254,19 +1251,17 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ri.fip_ns.subscribe = mock.Mock(return_value=True) ri.fip_ns.agent_router_gateway = mock.Mock() ri.rtr_fip_subnet = None - ri.dist_fip_count = 0 - with mock.patch.object(ri, 'get_floating_ips') as fips,\ - mock.patch.object(ri, 'get_floating_agent_gw_interface' - ) as fip_gw_port: - fips.return_value = fake_floatingips + with mock.patch.object( + ri, 'get_floating_agent_gw_interface') as fip_gw_port: fip_gw_port.return_value = agent_gateway_port[0] - ri.create_dvr_fip_interfaces(ext_gw_port) + ri.create_dvr_external_gateway_on_agent(ext_gw_port) + ri.connect_rtr_2_fip() self.assertTrue(fip_gw_port.called) - self.assertTrue(fips.called) self.assertEqual(agent_gateway_port[0], ri.fip_ns.agent_gateway_port) self.assertTrue(ri.rtr_fip_subnet) + self.assertTrue(ri.rtr_fip_connect) def test_process_router_cent_floating_ip_add(self): fake_floatingips = {'floatingips': [ diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index 8114dd114b8..d09f70fa06c 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -115,6 +115,7 @@ class TestDvrFipNs(base.BaseTestCase): self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) agent_gw_port = self._get_agent_gw_port() interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id']) + self.fip_ns.agent_gateway_port = agent_gw_port self.fip_ns.create_or_update_gateway_port(agent_gw_port) expected = [ mock.call(self.fip_ns.get_name(), @@ -134,14 +135,13 @@ class TestDvrFipNs(base.BaseTestCase): @mock.patch.object(dvr_fip_ns.FipNamespace, 'unsubscribe') def test_update_gateway_port_raises_exception( self, fip_unsub, fip_delete, fip_sub, exists): - self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) - self.fip_ns.agent_gateway_port = None agent_gw_port = self._get_agent_gw_port() self.fip_ns._create_gateway_port = mock.Mock() self.fip_ns.create_or_update_gateway_port(agent_gw_port) exists.return_value = False fip_sub.return_value = False self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) + self.fip_ns.agent_gateway_port = agent_gw_port self.assertRaises(n_exc.FloatingIpSetupException, self.fip_ns.create_or_update_gateway_port, @@ -159,7 +159,8 @@ class TestDvrFipNs(base.BaseTestCase): self.fip_ns.agent_gateway_port = None agent_gw_port = self._get_agent_gw_port() agent_gw_port['subnets'][0]['gateway_ip'] = '20.0.1.1' - + self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) + self.fip_ns.agent_gateway_port = agent_gw_port self.fip_ns.create_or_update_gateway_port(agent_gw_port) IPDevice().route.add_route.assert_called_once_with('20.0.1.1', @@ -329,22 +330,15 @@ class TestDvrFipNs(base.BaseTestCase): def test_scan_fip_ports_restart_fips(self): ri = mock.Mock() - ri.dist_fip_count = None ri.floating_ips_dict = {} ip_list = [{'cidr': '111.2.3.4'}, {'cidr': '111.2.3.5'}] stale_list = ['111.2.3.7/32'] self._test_scan_fip_ports(ri, ip_list, stale_list) - self.assertEqual(2, ri.dist_fip_count) + self.assertTrue(ri.rtr_fip_connect) def test_scan_fip_ports_restart_none(self): ri = mock.Mock() - ri.dist_fip_count = None ri.floating_ips_dict = {} + ri.rtr_fip_connect = False self._test_scan_fip_ports(ri, [], []) - self.assertEqual(0, ri.dist_fip_count) - - def test_scan_fip_ports_restart_zero(self): - ri = mock.Mock() - ri.dist_fip_count = 0 - self._test_scan_fip_ports(ri, None, []) - self.assertEqual(0, ri.dist_fip_count) + self.assertFalse(ri.rtr_fip_connect) diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index 22b1c58aa12..c78b3729cd4 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -177,7 +177,7 @@ class TestDvrRouterOperations(base.BaseTestCase): ri.fip_ns = mock.Mock() ri.fip_ns.subscribe.return_value = False ex_gw_port = {'network_id': 'fake_net_id'} - ri.create_dvr_fip_interfaces(ex_gw_port) + ri.create_dvr_external_gateway_on_agent(ex_gw_port) ri.fip_ns.create_or_update_gateway_port.assert_called_once_with( fip_agent_port) @@ -262,6 +262,7 @@ class TestDvrRouterOperations(base.BaseTestCase): def test_floating_ip_added_dist(self, mIPRule, mIPDevice, mock_adv_notif): router = mock.MagicMock() ri = self._create_router(router) + ri.ex_gw_port = ri.router['gw_port'] ext_net_id = _uuid() subnet_id = _uuid() agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', @@ -282,19 +283,20 @@ class TestDvrRouterOperations(base.BaseTestCase): 'port_id': _uuid()} ri.fip_ns = mock.Mock() ri.fip_ns.agent_gateway_port = agent_gw_port + ri.create_dvr_external_gateway_on_agent(ri.ex_gw_port) + ri.connect_rtr_2_fip() + self.assertTrue(ri.rtr_fip_connect) ri.fip_ns.allocate_rule_priority.return_value = FIP_PRI subnet = lla.LinkLocalAddressPair('169.254.30.42/31') ri.rtr_fip_subnet = subnet ri.fip_ns.local_subnets = mock.Mock() ri.fip_ns.local_subnets.allocate.return_value = subnet - ri.dist_fip_count = 0 ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) ri.floating_ip_added_dist(fip, ip_cidr) mIPRule().rule.add.assert_called_with(ip='192.168.0.1', table=16, priority=FIP_PRI) ri.fip_ns.local_subnets.allocate.assert_not_called() - self.assertEqual(1, ri.dist_fip_count) # Validate that fip_ns.local_subnets is called when # rtr_fip_subnet is None @@ -312,7 +314,7 @@ class TestDvrRouterOperations(base.BaseTestCase): def test_floating_ip_removed_dist(self, mIPRule, mIPDevice, mIPWrapper): router = mock.MagicMock() ri = self._create_router(router) - + ri.ex_gw_port = ri.router['gw_port'] subnet_id = _uuid() agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', 'prefixlen': 24, @@ -324,8 +326,6 @@ class TestDvrRouterOperations(base.BaseTestCase): 'network_id': _uuid(), 'mac_address': 'ca:fe:de:ad:be:ef'} fip_cidr = '11.22.33.44/24' - - ri.dist_fip_count = 2 ri.fip_ns = mock.Mock() ri.fip_ns.get_name.return_value = 'fip_ns_name' ri.floating_ips_dict['11.22.33.44'] = FIP_PRI @@ -341,20 +341,6 @@ class TestDvrRouterOperations(base.BaseTestCase): mIPDevice().route.delete_route.assert_called_with(fip_cidr, str(s.ip)) ri.fip_ns.local_subnets.allocate.assert_not_called() - ri.dist_fip_count = 1 - s1 = lla.LinkLocalAddressPair('15.1.2.3/32') - ri.rtr_fip_subnet = None - ri.fip_ns.local_subnets.allocate.return_value = s1 - _, fip_to_rtr = s1.get_pair() - fip_ns = ri.fip_ns - ri.floating_ip_removed_dist(fip_cidr) - self.assertTrue(fip_ns.destroyed) - mIPWrapper().del_veth.assert_called_once_with( - fip_ns.get_int_device_name(router['id'])) - self.assertEqual(1, mIPDevice().route.delete_gateway.call_count) - self.assertFalse(ri.fip_ns.unsubscribe.called) - ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id) - @mock.patch.object(ip_lib, 'IPRule') def test_floating_ip_moved_dist(self, mIPRule): router = mock.MagicMock() @@ -622,7 +608,6 @@ class TestDvrRouterOperations(base.BaseTestCase): self._set_ri_kwargs(agent, router['id'], router) ri = dvr_router.DvrLocalRouter(HOSTNAME, **self.ri_kwargs) ri.iptables_manager.ipv4['nat'] = mock.MagicMock() - ri.dist_fip_count = 0 fip_ns = agent.get_fip_ns(mock.sentinel.ext_net_id) subnet_id = _uuid() fip_ns.agent_gateway_port = ( @@ -701,7 +686,6 @@ class TestDvrRouterOperations(base.BaseTestCase): vm_floating_ip = '19.4.4.2' ri.floating_ips_dict[vm_floating_ip] = FIP_PRI - ri.dist_fip_count = 1 ri.rtr_fip_subnet = ri.fip_ns.local_subnets.allocate(ri.router_id) _, fip_to_rtr = ri.rtr_fip_subnet.get_pair() self.mock_ip.get_devices.return_value = [ @@ -709,11 +693,9 @@ class TestDvrRouterOperations(base.BaseTestCase): ri.get_router_cidrs = mock.Mock( return_value={vm_floating_ip + '/32', '19.4.4.1/24'}) self.device_exists.return_value = True - ri.external_gateway_removed( ri.ex_gw_port, ri.get_external_device_name(ri.ex_gw_port['id'])) - ri.remove_floating_ip.assert_called_once_with(self.mock_ip_dev, '19.4.4.2/32') diff --git a/neutron/tests/unit/agent/l3/test_item_allocator.py b/neutron/tests/unit/agent/l3/test_item_allocator.py index d350e458e1c..9a843d26fb8 100644 --- a/neutron/tests/unit/agent/l3/test_item_allocator.py +++ b/neutron/tests/unit/agent/l3/test_item_allocator.py @@ -66,16 +66,20 @@ class TestItemAllocator(base.BaseTestCase): self.assertEqual({}, a.allocations) self.assertTrue(write.called) - def test_allocate(self): + def test_allocate_and_lookup(self): test_pool = set([TestObject(33000), TestObject(33001)]) a = ia.ItemAllocator('/file', TestObject, test_pool) with mock.patch.object(ia.ItemAllocator, '_write') as write: test_object = a.allocate('test') + # a lookup should find the same object + lookup_object = a.lookup('test') + self.assertIn('test', a.allocations) self.assertIn(test_object, a.allocations.values()) self.assertNotIn(test_object, a.pool) self.assertTrue(write.called) + self.assertEqual(test_object, lookup_object) def test_allocate_repeated_call_with_same_key(self): test_pool = set([TestObject(33000), TestObject(33001), diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index a7cc975db5a..75878c2bf73 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -24,7 +24,6 @@ from oslo_utils import uuidutils from neutron.callbacks import events from neutron.callbacks import registry from neutron.callbacks import resources -from neutron.common import constants as n_const from neutron.db import agents_db from neutron.db import common_db_mixin from neutron.db import l3_agentschedulers_db @@ -381,29 +380,20 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase): return_value=fipagent) self.mixin._get_fip_sync_interfaces = mock.Mock( return_value='fip_interface') - agent = mock.Mock() - agent.id = fipagent['id'] - self.mixin._process_floating_ips_dvr(self.ctx, routers, [floatingip], - hostid, agent) + hostid) return (router, floatingip) def test_floatingip_on_port_not_host(self): router, fip = self._floatingip_on_port_test_setup(None) self.assertNotIn(const.FLOATINGIP_KEY, router) - self.assertNotIn(n_const.FLOATINGIP_AGENT_INTF_KEY, router) def test_floatingip_on_port_with_host(self): router, fip = self._floatingip_on_port_test_setup(_uuid()) - self.assertTrue(self.mixin._get_fip_sync_interfaces.called) - self.assertIn(const.FLOATINGIP_KEY, router) - self.assertIn(n_const.FLOATINGIP_AGENT_INTF_KEY, router) self.assertIn(fip, router[const.FLOATINGIP_KEY]) - self.assertIn('fip_interface', - router[n_const.FLOATINGIP_AGENT_INTF_KEY]) def _setup_test_create_floatingip( self, fip, floatingip_db, router_db): diff --git a/releasenotes/notes/dvr-fip-namespace-on-all-nodes-c4da7ccd60ee62f5.yaml b/releasenotes/notes/dvr-fip-namespace-on-all-nodes-c4da7ccd60ee62f5.yaml new file mode 100644 index 00000000000..4025603a0db --- /dev/null +++ b/releasenotes/notes/dvr-fip-namespace-on-all-nodes-c4da7ccd60ee62f5.yaml @@ -0,0 +1,13 @@ +--- +prelude: > + Add DVR Floating IP (FIP) Namespace creation event + on all nodes, based on the gateway configuration. +features: + - Proactively create Floating IP Namespace on all compute nodes + when a gateway is configured. +issues: + - This might consume public IP Address, but by using + subnet service-types as explained in the docs below + https://docs.openstack.org/networking-guide/config-service-subnets.html + consumers can use the private IPs for floating IP agent gateway ports + and need not consume any public IP addresses.