diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 16c73fa81d..28b691d215 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -50,6 +50,7 @@ NS_PREFIX = 'qrouter-' INTERNAL_DEV_PREFIX = 'qr-' EXTERNAL_DEV_PREFIX = 'qg-' RPC_LOOP_INTERVAL = 1 +FLOATING_IP_CIDR_SUFFIX = '/32' class L3PluginApi(proxy.RpcProxy): @@ -95,7 +96,6 @@ class RouterInfo(object): self._snat_enabled = None self._snat_action = None self.internal_ports = [] - self.floating_ips = [] self.root_helper = root_helper self.use_namespaces = use_namespaces # Invoke the setter for establishing initial SNAT action @@ -409,7 +409,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): internal_cidrs, interface_name) # Process DNAT rules for floating IPs - if ex_gw_port or ri.ex_gw_port: + if ex_gw_port: self.process_router_floating_ips(ri, ex_gw_port) ri.ex_gw_port = ex_gw_port @@ -440,45 +440,46 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): ri.iptables_manager.apply() def process_router_floating_ips(self, ri, ex_gw_port): - floating_ips = ri.router.get(l3_constants.FLOATINGIP_KEY, []) - existing_floating_ip_ids = set([fip['id'] for fip in ri.floating_ips]) - cur_floating_ip_ids = set([fip['id'] for fip in floating_ips]) + """Configure the router's floating IPs + Configures floating ips in iptables and on the router's gateway device. - id_to_fip_map = {} + Cleans up floating ips that should not longer be configured. + """ + interface_name = self.get_external_device_name(ex_gw_port['id']) + device = ip_lib.IPDevice(interface_name, self.root_helper, + namespace=ri.ns_name()) - for fip in floating_ips: - if fip['port_id']: - if fip['id'] not in existing_floating_ip_ids: - ri.floating_ips.append(fip) - self.floating_ip_added(ri, ex_gw_port, - fip['floating_ip_address'], - fip['fixed_ip_address']) + # Clear out all iptables rules for floating ips + ri.iptables_manager.ipv4['nat'].clear_rules_by_tag('floating_ip') - # store to see if floatingip was remapped - id_to_fip_map[fip['id']] = fip + existing_cidrs = set([addr['cidr'] for addr in device.addr.list()]) + new_cidrs = set() - floating_ip_ids_to_remove = (existing_floating_ip_ids - - cur_floating_ip_ids) - for fip in ri.floating_ips: - if fip['id'] in floating_ip_ids_to_remove: - ri.floating_ips.remove(fip) - self.floating_ip_removed(ri, ri.ex_gw_port, - fip['floating_ip_address'], - fip['fixed_ip_address']) - else: - # handle remapping of a floating IP - new_fip = id_to_fip_map[fip['id']] - new_fixed_ip = new_fip['fixed_ip_address'] - existing_fixed_ip = fip['fixed_ip_address'] - if (new_fixed_ip and existing_fixed_ip and - new_fixed_ip != existing_fixed_ip): - floating_ip = fip['floating_ip_address'] - self.floating_ip_removed(ri, ri.ex_gw_port, - floating_ip, existing_fixed_ip) - self.floating_ip_added(ri, ri.ex_gw_port, - floating_ip, new_fixed_ip) - ri.floating_ips.remove(fip) - ri.floating_ips.append(new_fip) + # Loop once to ensure that floating ips are configured. + for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []): + fip_ip = fip['floating_ip_address'] + ip_cidr = str(fip_ip) + FLOATING_IP_CIDR_SUFFIX + + new_cidrs.add(ip_cidr) + + if ip_cidr not in existing_cidrs: + net = netaddr.IPNetwork(ip_cidr) + device.addr.add(net.version, ip_cidr, str(net.broadcast)) + self._send_gratuitous_arp_packet(ri, interface_name, fip_ip) + + # Rebuild iptables rules for the floating ip. + fixed = fip['fixed_ip_address'] + for chain, rule in self.floating_forward_rules(fip_ip, fixed): + ri.iptables_manager.ipv4['nat'].add_rule(chain, rule, + tag='floating_ip') + + ri.iptables_manager.apply() + + # Clean up addresses that no longer belong on the gateway interface. + for ip_cidr in existing_cidrs - new_cidrs: + if ip_cidr.endswith(FLOATING_IP_CIDR_SUFFIX): + net = netaddr.IPNetwork(ip_cidr) + device.addr.delete(net.version, ip_cidr) def _get_ex_gw_port(self, ri): return ri.router.get('gw_port') @@ -602,34 +603,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): (internal_cidr, ex_gw_ip))] return rules - def floating_ip_added(self, ri, ex_gw_port, floating_ip, fixed_ip): - ip_cidr = str(floating_ip) + '/32' - interface_name = self.get_external_device_name(ex_gw_port['id']) - device = ip_lib.IPDevice(interface_name, self.root_helper, - namespace=ri.ns_name()) - - if ip_cidr not in [addr['cidr'] for addr in device.addr.list()]: - net = netaddr.IPNetwork(ip_cidr) - device.addr.add(net.version, ip_cidr, str(net.broadcast)) - self._send_gratuitous_arp_packet(ri, interface_name, floating_ip) - - for chain, rule in self.floating_forward_rules(floating_ip, fixed_ip): - ri.iptables_manager.ipv4['nat'].add_rule(chain, rule) - ri.iptables_manager.apply() - - def floating_ip_removed(self, ri, ex_gw_port, floating_ip, fixed_ip): - ip_cidr = str(floating_ip) + '/32' - net = netaddr.IPNetwork(ip_cidr) - interface_name = self.get_external_device_name(ex_gw_port['id']) - - device = ip_lib.IPDevice(interface_name, self.root_helper, - namespace=ri.ns_name()) - device.addr.delete(net.version, ip_cidr) - - for chain, rule in self.floating_forward_rules(floating_ip, fixed_ip): - ri.iptables_manager.ipv4['nat'].remove_rule(chain, rule) - ri.iptables_manager.apply() - def floating_forward_rules(self, floating_ip, fixed_ip): return [('PREROUTING', '-d %s -j DNAT --to %s' % (floating_ip, fixed_ip)), diff --git a/neutron/agent/linux/iptables_manager.py b/neutron/agent/linux/iptables_manager.py index acb32b8219..87c7c80848 100644 --- a/neutron/agent/linux/iptables_manager.py +++ b/neutron/agent/linux/iptables_manager.py @@ -63,12 +63,13 @@ class IptablesRule(object): """ def __init__(self, chain, rule, wrap=True, top=False, - binary_name=binary_name): + binary_name=binary_name, tag=None): self.chain = get_chain_name(chain, wrap) self.rule = rule self.wrap = wrap self.top = top self.wrap_name = binary_name[:16] + self.tag = tag def __eq__(self, other): return ((self.chain == other.chain) and @@ -177,7 +178,7 @@ class IptablesTable(object): self.rules = [r for r in self.rules if jump_snippet not in r.rule] - def add_rule(self, chain, rule, wrap=True, top=False): + def add_rule(self, chain, rule, wrap=True, top=False, tag=None): """Add a rule to the table. This is just like what you'd feed to iptables, just without @@ -195,7 +196,8 @@ class IptablesTable(object): if '$' in rule: rule = ' '.join(map(self._wrap_target_chain, rule.split(' '))) - self.rules.append(IptablesRule(chain, rule, wrap, top, self.wrap_name)) + self.rules.append(IptablesRule(chain, rule, wrap, top, self.wrap_name, + tag)) def _wrap_target_chain(self, s): if s.startswith('$'): @@ -231,6 +233,13 @@ class IptablesTable(object): for rule in chained_rules: self.rules.remove(rule) + def clear_rules_by_tag(self, tag): + if not tag: + return + rules = [rule for rule in self.rules if rule.tag == tag] + for rule in rules: + self.rules.remove(rule) + class IptablesManager(object): """Wrapper for iptables. diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index f7cfdcff3f..f0f5068a81 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -217,39 +217,6 @@ class TestBasicRouterOperations(base.BaseTestCase): def test_agent_remove_external_gateway(self): self._test_external_gateway_action('remove') - def _test_floating_ip_action(self, action): - router_id = _uuid() - ri = l3_agent.RouterInfo(router_id, self.conf.root_helper, - self.conf.use_namespaces, None) - agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - floating_ip = '20.0.0.100' - fixed_ip = '10.0.0.23' - ex_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', - 'subnet_id': _uuid()}], - 'subnet': {'gateway_ip': '20.0.0.1'}, - 'id': _uuid(), - 'mac_address': 'ca:fe:de:ad:be:ef', - 'ip_cidr': '20.0.0.30/24'} - interface_name = agent.get_external_device_name(ex_gw_port['id']) - - if action == 'add': - self.device_exists.return_value = False - agent.floating_ip_added(ri, ex_gw_port, floating_ip, fixed_ip) - self.send_arp.assert_called_once_with(ri, interface_name, - floating_ip) - - elif action == 'remove': - self.device_exists.return_value = True - agent.floating_ip_removed(ri, ex_gw_port, floating_ip, fixed_ip) - else: - raise Exception("Invalid action %s" % action) - - def test_agent_add_floating_ip(self): - self._test_floating_ip_action('add') - - def test_agent_remove_floating_ip(self): - self._test_floating_ip_action('remove') - def _check_agent_method_called(self, agent, calls, namespace): if namespace: self.mock_ip.netns.execute.assert_has_calls( @@ -405,6 +372,7 @@ class TestBasicRouterOperations(base.BaseTestCase): def test_process_router(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + agent.process_router_floating_ips = mock.Mock() router = self._prepare_router_data() fake_floatingips1 = {'floatingips': [ {'id': _uuid(), @@ -414,6 +382,9 @@ class TestBasicRouterOperations(base.BaseTestCase): ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper, self.conf.use_namespaces, router=router) agent.process_router(ri) + ex_gw_port = agent._get_ex_gw_port(ri) + agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port) + agent.process_router_floating_ips.reset_mock() # remap floating IP to a new fixed ip fake_floatingips2 = copy.deepcopy(fake_floatingips1) @@ -421,16 +392,94 @@ class TestBasicRouterOperations(base.BaseTestCase): router[l3_constants.FLOATINGIP_KEY] = fake_floatingips2['floatingips'] agent.process_router(ri) + ex_gw_port = agent._get_ex_gw_port(ri) + agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port) + agent.process_router_floating_ips.reset_mock() # remove just the floating ips del router[l3_constants.FLOATINGIP_KEY] agent.process_router(ri) + ex_gw_port = agent._get_ex_gw_port(ri) + agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port) + agent.process_router_floating_ips.reset_mock() # now no ports so state is torn down del router[l3_constants.INTERFACE_KEY] del router['gw_port'] agent.process_router(ri) self.send_arp.assert_called_once() + self.assertFalse(agent.process_router_floating_ips.called) + + @mock.patch('neutron.agent.linux.ip_lib.IPDevice') + def test_process_router_floating_ip_add(self, IPDevice): + fip = { + 'id': _uuid(), 'port_id': _uuid(), + 'floating_ip_address': '15.1.2.3', + 'fixed_ip_address': '192.168.0.1' + } + + IPDevice.return_value = device = mock.Mock() + device.addr.list.return_value = [] + + ri = mock.MagicMock() + ri.router.get.return_value = [fip] + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + + agent.process_router_floating_ips(ri, {'id': _uuid()}) + + device.addr.add.assert_called_once_with(4, '15.1.2.3/32', '15.1.2.3') + + nat = ri.iptables_manager.ipv4['nat'] + nat.clear_rules_by_tag.assert_called_once_with('floating_ip') + rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.1') + for chain, rule in rules: + nat.add_rule.assert_any_call(chain, rule, tag='floating_ip') + + @mock.patch('neutron.agent.linux.ip_lib.IPDevice') + def test_process_router_floating_ip_remove(self, IPDevice): + IPDevice.return_value = device = mock.Mock() + device.addr.list.return_value = [{'cidr': '15.1.2.3/32'}] + + ri = mock.MagicMock() + ri.router.get.return_value = [] + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + + agent.process_router_floating_ips(ri, {'id': _uuid()}) + + device.addr.delete.assert_called_once_with(4, '15.1.2.3/32') + + nat = ri.iptables_manager.ipv4['nat'] + nat = ri.iptables_manager.ipv4['nat'] + nat.clear_rules_by_tag.assert_called_once_with('floating_ip') + + @mock.patch('neutron.agent.linux.ip_lib.IPDevice') + def test_process_router_floating_ip_remap(self, IPDevice): + fip = { + 'id': _uuid(), 'port_id': _uuid(), + 'floating_ip_address': '15.1.2.3', + 'fixed_ip_address': '192.168.0.2' + } + + IPDevice.return_value = device = mock.Mock() + device.addr.list.return_value = [{'cidr': '15.1.2.3/32'}] + ri = mock.MagicMock() + + ri.router.get.return_value = [fip] + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + + agent.process_router_floating_ips(ri, {'id': _uuid()}) + + self.assertFalse(device.addr.add.called) + self.assertFalse(device.addr.delete.called) + + nat = ri.iptables_manager.ipv4['nat'] + nat.clear_rules_by_tag.assert_called_once_with('floating_ip') + rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.2') + for chain, rule in rules: + nat.add_rule.assert_any_call(chain, rule, tag='floating_ip') def test_process_router_snat_disabled(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)