From b8a2cb813225ae6178f34363cab9a2446d471472 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Tue, 5 Jul 2016 12:15:03 +0300 Subject: [PATCH] DVR: handle floating IP reassociation on the same host The case when floating IP is reassociated from one VM to another on the same host without doing disassociation, was not properly handled: l3 dvr agent only checked that the floating address was present on the device in router namespace and considered floating IP confiigured correctly. This patch adds fip_mapping to router object so fixed IP change can be detected and handled. Closes-Bug: #1599089 Change-Id: I25c23bb9ad7b9a9b90f225f37417142e2304deb8 --- neutron/agent/l3/dvr_local_router.py | 44 +++++++++++++------ neutron/agent/l3/router_info.py | 14 ++++++ .../functional/agent/l3/test_dvr_router.py | 27 ++++++++++++ .../unit/agent/l3/test_dvr_local_router.py | 22 ++++++++++ .../tests/unit/agent/l3/test_router_info.py | 19 ++++++++ 5 files changed, 112 insertions(+), 14 deletions(-) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index cbc2c4c3159..272020fa263 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -87,13 +87,8 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): """Add floating IP to FIP namespace.""" floating_ip = fip['floating_ip_address'] fixed_ip = fip['fixed_ip_address'] - rule_pr = self.fip_ns.allocate_rule_priority(floating_ip) - self.floating_ips_dict[floating_ip] = rule_pr + self._add_floating_ip_rule(floating_ip, fixed_ip) fip_2_rtr_name = self.fip_ns.get_int_device_name(self.router_id) - ip_rule = ip_lib.IPRule(namespace=self.ns_name) - ip_rule.rule.add(ip=fixed_ip, - table=dvr_fip_ns.FIP_RT_TBL, - priority=rule_pr) #Add routing rule in fip namespace fip_ns_name = self.fip_ns.get_name() if self.rtr_fip_subnet is None: @@ -112,6 +107,24 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): # 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) + self.floating_ips_dict[floating_ip] = rule_pr + ip_rule = ip_lib.IPRule(namespace=self.ns_name) + ip_rule.rule.add(ip=fixed_ip, + table=dvr_fip_ns.FIP_RT_TBL, + priority=rule_pr) + + def _remove_floating_ip_rule(self, floating_ip): + if floating_ip in self.floating_ips_dict: + rule_pr = self.floating_ips_dict[floating_ip] + ip_rule = ip_lib.IPRule(namespace=self.ns_name) + ip_rule.rule.delete(ip=floating_ip, + table=dvr_fip_ns.FIP_RT_TBL, + priority=rule_pr) + self.fip_ns.deallocate_rule_priority(floating_ip) + #TODO(rajeev): Handle else case - exception/log? + def floating_ip_removed_dist(self, fip_cidr): """Remove floating IP from FIP namespace.""" floating_ip = fip_cidr.split('/')[0] @@ -123,14 +136,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): rtr_2_fip, fip_2_rtr = self.rtr_fip_subnet.get_pair() fip_ns_name = self.fip_ns.get_name() - if floating_ip in self.floating_ips_dict: - rule_pr = self.floating_ips_dict[floating_ip] - ip_rule = ip_lib.IPRule(namespace=self.ns_name) - ip_rule.rule.delete(ip=floating_ip, - table=dvr_fip_ns.FIP_RT_TBL, - priority=rule_pr) - self.fip_ns.deallocate_rule_priority(floating_ip) - #TODO(rajeev): Handle else case - exception/log? + self._remove_floating_ip_rule(floating_ip) device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name) @@ -147,6 +153,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.rtr_fip_subnet = None ns_ip.del_veth(fip_2_rtr_name) + def floating_ip_moved_dist(self, fip): + """Handle floating IP move between fixed IPs.""" + floating_ip = fip['floating_ip_address'] + self._remove_floating_ip_rule(floating_ip) + self._add_floating_ip_rule(floating_ip, fip['fixed_ip_address']) + def add_floating_ip(self, fip, interface_name, device): # Special Handling for DVR - update FIP namespace ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) @@ -156,6 +168,10 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): def remove_floating_ip(self, device, ip_cidr): self.floating_ip_removed_dist(ip_cidr) + def move_floating_ip(self, fip): + self.floating_ip_moved_dist(fip) + return l3_constants.FLOATINGIP_STATUS_ACTIVE + def _get_internal_port(self, subnet_id): """Return internal router port based on subnet_id.""" router_ports = self.router.get(l3_constants.INTERFACE_KEY, []) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index f3a548025ed..19709fbffb0 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -50,6 +50,7 @@ class RouterInfo(object): self.router_id = router_id self.ex_gw_port = None self._snat_enabled = None + self.fip_map = {} self.internal_ports = [] self.floating_ips = set() # Invoke the setter for establishing initial SNAT action @@ -277,6 +278,9 @@ class RouterInfo(object): def remove_floating_ip(self, device, ip_cidr): device.delete_addr_and_conntrack_state(ip_cidr) + def move_floating_ip(self, fip): + return l3_constants.FLOATINGIP_STATUS_ACTIVE + def remove_external_gateway_ip(self, device, ip_cidr): device.delete_addr_and_conntrack_state(ip_cidr) @@ -313,6 +317,13 @@ class RouterInfo(object): LOG.debug('Floating ip %(id)s added, status %(status)s', {'id': fip['id'], 'status': fip_statuses.get(fip['id'])}) + elif (fip_ip in self.fip_map and + self.fip_map[fip_ip] != fip['fixed_ip_address']): + LOG.debug("Floating IP was moved from fixed IP " + "%(old)s to %(new)s", + {'old': self.fip_map[fip_ip], + 'new': fip['fixed_ip_address']}) + fip_statuses[fip['id']] = self.move_floating_ip(fip) elif fip_statuses[fip['id']] == fip['status']: # mark the status as not changed. we can't remove it because # that's how the caller determines that it was removed @@ -1021,5 +1032,8 @@ class RouterInfo(object): # Update ex_gw_port and enable_snat on the router info cache self.ex_gw_port = self.get_ex_gw_port() + self.fip_map = dict([(fip['floating_ip_address'], + fip['fixed_ip_address']) + for fip in self.get_floating_ips()]) # TODO(Carl) FWaaS uses this. Why is it set after processing is done? self.enable_snat = self.router.get('enable_snat') diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index d37484711e6..dce1a1e9e6c 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -516,6 +516,22 @@ class TestDvrRouter(framework.L3AgentTestFramework): router_ns, floating_ips[0]['fixed_ip_address']) self.assertNotEqual(fip_rule_prio_1, fip_rule_prio_2) + def test_dvr_router_floating_ip_moved(self): + self.agent.conf.agent_mode = 'dvr' + router_info = self.generate_dvr_router_info() + router = self.manage_router(self.agent, router_info) + floating_ips = router.router[l3_constants.FLOATINGIP_KEY] + router_ns = router.ns_name + fixed_ip = floating_ips[0]['fixed_ip_address'] + self.assertTrue(self._fixed_ip_rule_exists(router_ns, fixed_ip)) + # Floating IP reassigned to another fixed IP + new_fixed_ip = '10.0.0.2' + self.assertNotEqual(new_fixed_ip, fixed_ip) + floating_ips[0]['fixed_ip_address'] = new_fixed_ip + self.agent._process_updated_router(router.router) + self.assertFalse(self._fixed_ip_rule_exists(router_ns, fixed_ip)) + self.assertTrue(self._fixed_ip_rule_exists(router_ns, new_fixed_ip)) + def _assert_iptables_rules_exist( self, router_iptables_manager, table_name, expected_rules): rules = router_iptables_manager.get_rules_for_table(table_name) @@ -550,6 +566,17 @@ class TestDvrRouter(framework.L3AgentTestFramework): info = iprule.rule._parse_line(4, line) return info['priority'] + def _fixed_ip_rule_exists(self, namespace, ip): + iprule = ip_lib.IPRule(namespace) + lines = iprule.rule._as_root([4], ['show']).splitlines() + for line in lines: + if ip in line: + info = iprule.rule._parse_line(4, line) + if info['from'] == ip: + return True + + return False + def test_dvr_router_add_internal_network_set_arp_cache(self): # Check that, when the router is set up and there are # existing ports on the uplinked subnet, the ARP 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 81793ed8252..f8f67016a2e 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -281,6 +281,28 @@ class TestDvrRouterOperations(base.BaseTestCase): 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() + ri = self._create_router(router) + floating_ip_address = '15.1.2.3' + fip = {'floating_ip_address': floating_ip_address, + 'fixed_ip_address': '192.168.0.1'} + ri.floating_ips_dict['15.1.2.3'] = FIP_PRI + ri.fip_ns = mock.Mock() + ri.fip_ns.allocate_rule_priority.return_value = FIP_PRI + ri.floating_ip_moved_dist(fip) + + mIPRule().rule.delete.assert_called_once_with( + ip=floating_ip_address, table=16, priority=FIP_PRI) + ri.fip_ns.deallocate_rule_priority.assert_called_once_with( + floating_ip_address) + ri.fip_ns.allocate_rule_priority.assert_called_once_with( + floating_ip_address) + mIPRule().rule.add.assert_called_with(ip='192.168.0.1', + table=16, + priority=FIP_PRI) + def _test_add_floating_ip(self, ri, fip, is_failure): ri.floating_ip_added_dist = mock.Mock() diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index bff7fca3f25..4eaec7a60bb 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -422,3 +422,22 @@ class TestFloatingIpWithMockDevice(BasicRouterTestCaseFramework): mock.sentinel.interface_name) self.assertEqual({}, fip_statuses) ri.remove_floating_ip.assert_called_once_with(device, '15.1.2.3/32') + + def test_process_floating_ip_reassignment(self, IPDevice): + IPDevice.return_value = device = mock.Mock() + device.addr.list.return_value = [{'cidr': '15.1.2.3/32'}] + + fip_id = _uuid() + fip = { + 'id': fip_id, 'port_id': _uuid(), + 'floating_ip_address': '15.1.2.3', + 'fixed_ip_address': '192.168.0.3', + 'status': 'DOWN' + } + ri = self._create_router() + ri.get_floating_ips = mock.Mock(return_value=[fip]) + ri.move_floating_ip = mock.Mock() + ri.fip_map = {'15.1.2.3': '192.168.0.2'} + + ri.process_floating_ip_addresses(mock.sentinel.interface_name) + ri.move_floating_ip.assert_called_once_with(fip)