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 b8a2cb8132)

Conflicts:
	neutron/tests/functional/agent/l3/test_dvr_router.py
Change-Id: I25c23bb9ad7b9a9b90f225f37417142e2304deb8

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, [])

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')

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

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()

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)