From 251d6803eb65b54d75f6eedaabc83321e194cbc3 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 (cherry picked from commit b8a2cb813225ae6178f34363cab9a2446d471472) Conflicts: neutron/tests/functional/agent/l3/test_dvr_router.py 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 4e2d8228a27..3f8c940ac71 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -76,13 +76,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: @@ -101,6 +96,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] @@ -112,14 +125,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) @@ -136,6 +142,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): if not self._add_fip_addr_to_device(fip, device): return l3_constants.FLOATINGIP_STATUS_ERROR @@ -149,6 +161,10 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): super(DvrLocalRouter, self).remove_floating_ip(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 d3f68d318cc..22fb44a1d13 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -49,6 +49,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 @@ -275,6 +276,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) @@ -311,6 +315,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 @@ -968,5 +979,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 96d530237d7..254876fdc74 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -505,6 +505,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 _get_fixed_ip_rule_priority(self, namespace, fip): iprule = ip_lib.IPRule(namespace) lines = iprule.rule._as_root([4], ['show']).splitlines() @@ -513,6 +529,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 dc7d4785ce3..22d750a0938 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._add_fip_addr_to_device = mock.Mock(return_value=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 35c1f247ac5..e0249a60446 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)