From db54a5bb512278ffb5712fcd77cd8a54d2632a45 Mon Sep 17 00:00:00 2001 From: Nachi Ueno Date: Fri, 16 Aug 2013 10:17:48 -0700 Subject: [PATCH] Revert "Refactor configuring of floating ips on a router." This patch breaks gating job. Because nat rule for metadata will be only added on the router_add. Revert it for now. Fixes bug 1211829 This reverts commit 9382ee659212285a203550cf60476dd146d27a29. Change-Id: I05925798cddc7a706e922025ef6ce27b6638ffb6 --- neutron/agent/l3_agent.py | 101 ++++++++++++++--------- neutron/tests/unit/test_l3_agent.py | 123 ++++++++++------------------ 2 files changed, 105 insertions(+), 119 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index c44bbe496c..6b21c2cd60 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -95,6 +95,7 @@ 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,47 +410,45 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): ri.iptables_manager.apply() def process_router_floating_ips(self, ri, ex_gw_port): - """Configure the router's floating IPs - Configures floating ips in iptables and on the router's gateway device. + 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]) - Cleans up floating ips that should not longer be configured. - """ - cidr_suffix = '/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()) + id_to_fip_map = {} - # Clear out all iptables rules for these chains. - for chain, rule in self.floating_forward_rules(None, None): - ri.iptables_manager.ipv4['nat'].empty_chain(chain) + 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']) - existing_cidrs = set([addr['cidr'] for addr in device.addr.list()]) - new_cidrs = set() + # store to see if floatingip was remapped + id_to_fip_map[fip['id']] = 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) + 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) - - 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(cidr_suffix): - net = netaddr.IPNetwork(ip_cidr) - device.addr.delete(net.version, ip_cidr) + 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) def _get_ex_gw_port(self, ri): return ri.router.get('gw_port') @@ -568,6 +567,34 @@ 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/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index fd229a555a..2f9cb52bbd 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -186,6 +186,47 @@ class TestBasicRouterOperations(base.BaseTestCase): def testAgentRemoveExternalGateway(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) + arping_cmd = ['arping', '-A', '-U', + '-I', interface_name, + '-c', self.conf.send_arp_for_ha, + floating_ip] + if self.conf.use_namespaces: + self.mock_ip.netns.execute.assert_any_call( + arping_cmd, check_exit_code=True) + else: + self.utils_exec.assert_any_call( + check_exit_code=True, root_helper=self.conf.root_helper) + + 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 testAgentAddFloatingIP(self): + self._test_floating_ip_action('add') + + def testAgentRemoveFloatingIP(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( @@ -340,7 +381,6 @@ class TestBasicRouterOperations(base.BaseTestCase): def testProcessRouter(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(), @@ -367,87 +407,6 @@ class TestBasicRouterOperations(base.BaseTestCase): del router['gw_port'] agent.process_router(ri) - agent.process_router_floating_ips.assert_called_with(ri, None) - - @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._send_gratuitous_arp_packet = mock.Mock() - - 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') - self.assertTrue(agent._send_gratuitous_arp_packet.called) - - nat = ri.iptables_manager.ipv4['nat'] - rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.1') - for chain, rule in rules: - nat.empty_chain.assert_any_call(chain) - nat.add_rule.assert_any_call(chain, rule) - - @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._send_gratuitous_arp_packet = mock.Mock() - - agent.process_router_floating_ips(ri, {'id': _uuid()}) - - device.addr.delete.assert_called_once_with(4, '15.1.2.3/32') - self.assertFalse(agent._send_gratuitous_arp_packet.called) - - nat = ri.iptables_manager.ipv4['nat'] - nat = ri.iptables_manager.ipv4['nat'] - for chain, rule in agent.floating_forward_rules(None, None): - nat.empty_chain.assert_any_call(chain) - self.assertFalse(nat.add_rule.called) - - @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._send_gratuitous_arp_packet = mock.Mock() - - agent.process_router_floating_ips(ri, {'id': _uuid()}) - - self.assertFalse(device.addr.add.called) - self.assertFalse(device.addr.delete.called) - self.assertFalse(agent._send_gratuitous_arp_packet.called) - - nat = ri.iptables_manager.ipv4['nat'] - rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.2') - for chain, rule in rules: - nat.empty_chain.assert_any_call(chain) - nat.add_rule.assert_any_call(chain, rule) - def test_process_router_snat_disabled(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) router = self._prepare_router_data()