Merge "DVR: Pings to floatingip returns with fixed-ip on same network"

neutron/agent/l3/dvr_fip_ns.py

@@ -276,11 +276,4 @@ class FipNamespace(namespaces.Namespace):
         rtr_2_fip_interface = self.get_rtr_ext_device_name(ri.router_id)
         device = ip_lib.IPDevice(rtr_2_fip_interface, namespace=ri.ns_name)
         if device.exists():
-            existing_cidrs = [addr['cidr'] for addr in device.addr.list()]
+            ri.dist_fip_count = len(ri.get_router_cidrs(device))
-            fip_cidrs = [c for c in existing_cidrs if
-                         common_utils.is_cidr_host(c)]
-            for fip_cidr in fip_cidrs:
-                fip_ip = fip_cidr.split('/')[0]
-                rule_pr = self._rule_priorities.allocate(fip_ip)
-                ri.floating_ips_dict[fip_ip] = rule_pr
-            ri.dist_fip_count = len(fip_cidrs)

neutron/agent/l3/dvr_local_router.py

@@ -56,6 +56,33 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
                    (i['host'] == self.host) or
                    (i.get('dest_host') == self.host))]
 
+    def floating_forward_rules(self, floating_ip, fixed_ip):
+        """Override this function defined in router_info for dvr routers."""
+        if not self.fip_ns:
+            return []
+
+        rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(self.router_id)
+        dnat_from_floatingip_to_fixedip = (
+            'PREROUTING', '-d %s/32 -i %s -j DNAT --to-destination %s' % (
+                floating_ip, rtr_2_fip_name, fixed_ip))
+        snat_from_fixedip_to_floatingip = (
+            'float-snat', '-s %s/32 -j SNAT --to-source %s' % (
+                fixed_ip, floating_ip))
+        return [dnat_from_floatingip_to_fixedip,
+                snat_from_fixedip_to_floatingip]
+
+    def floating_mangle_rules(self, floating_ip, fixed_ip, internal_mark):
+        if not self.fip_ns:
+            return []
+
+        rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(self.router_id)
+        mark_traffic_to_floating_ip = (
+            'floatingip', '-d %s -i %s -j MARK --set-xmark %s' % (
+                floating_ip, rtr_2_fip_name, internal_mark))
+        mark_traffic_from_fixed_ip = (
+            'FORWARD', '-s %s -j $float-snat' % fixed_ip)
+        return [mark_traffic_to_floating_ip, mark_traffic_from_fixed_ip]
+
     def floating_ip_added_dist(self, fip, fip_cidr):
         """Add floating IP to FIP namespace."""
         floating_ip = fip['floating_ip_address']
@@ -121,16 +148,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
             ns_ip.del_veth(fip_2_rtr_name)
 
     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
-
         # Special Handling for DVR - update FIP namespace
         ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address'])
         self.floating_ip_added_dist(fip, ip_cidr)
         return l3_constants.FLOATINGIP_STATUS_ACTIVE
 
     def remove_floating_ip(self, device, ip_cidr):
-        super(DvrLocalRouter, self).remove_floating_ip(device, ip_cidr)
         self.floating_ip_removed_dist(ip_cidr)
 
     def _get_internal_port(self, subnet_id):
@@ -479,6 +502,28 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
                 (self.dist_fip_count == 0)):
                 self.fip_ns.create_rtr_2_fip_link(self)
 
+    def get_router_cidrs(self, device):
+        """As no floatingip will be set on the rfp device. Get floatingip from
+        the route of fip namespace.
+        """
+        if not self.fip_ns:
+            return set()
+
+        fip_ns_name = self.fip_ns.get_name()
+        fip_2_rtr_name = self.fip_ns.get_int_device_name(self.router_id)
+        device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name)
+        if not device.exists():
+            return set()
+
+        if self.rtr_fip_subnet is None:
+            self.rtr_fip_subnet = self.fip_ns.local_subnets.allocate(
+                self.router_id)
+        rtr_2_fip, _fip_2_rtr = self.rtr_fip_subnet.get_pair()
+        exist_routes = device.route.list_routes(
+            l3_constants.IP_VERSION_4, via=str(rtr_2_fip.ip))
+        return {common_utils.ip_to_cidr(route['cidr'])
+                for route in exist_routes}
+
     def process(self, agent):
         ex_gw_port = self.get_ex_gw_port()
         if ex_gw_port:

neutron/tests/functional/agent/l3/test_dvr_router.py

@@ -413,6 +413,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
             self.assertTrue(ip_lib.device_exists(
                 device_name, namespace=router.ns_name))
 
+        # In the router namespace, check the iptables rules are set correctly
+        for fip in floating_ips:
+            floatingip = fip['floating_ip_address']
+            fixedip = fip['fixed_ip_address']
+            expected_rules = router.floating_forward_rules(floatingip,
+                                                           fixedip)
+            self._assert_iptables_rules_exist(
+                router.iptables_manager, 'nat', expected_rules)
+
     def test_dvr_router_rem_fips_on_restarted_agent(self):
         self.agent.conf.agent_mode = 'dvr_snat'
         router_info = self.generate_dvr_router_info()
@@ -443,6 +452,13 @@ class TestDvrRouter(framework.L3AgentTestFramework):
             router_ns, floating_ips[0]['fixed_ip_address'])
         self.assertNotEqual(fip_rule_prio_1, fip_rule_prio_2)
 
+    def _assert_iptables_rules_exist(
+        self, router_iptables_manager, table_name, expected_rules):
+        rules = router_iptables_manager.get_rules_for_table(table_name)
+        for rule in expected_rules:
+            self.assertIn(
+                str(iptables_manager.IptablesRule(rule[0], rule[1])), rules)
+
     def test_prevent_snat_rule_exist_on_restarted_agent(self):
         self.agent.conf.agent_mode = 'dvr_snat'
         router_info = self.generate_dvr_router_info()
@@ -452,19 +468,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
         prevent_snat_rule = router._prevent_snat_for_internal_traffic_rule(
             rfp_devicename)
 
-        def is_prevent_snat_rule_exist(router_iptables_manager):
+        self._assert_iptables_rules_exist(
-            rules = router_iptables_manager.get_rules_for_table('nat')
+            router.iptables_manager, 'nat', [prevent_snat_rule])
-            return (str(iptables_manager.IptablesRule(
-                prevent_snat_rule[0], prevent_snat_rule[1])) in rules)
-
-        self.assertTrue(is_prevent_snat_rule_exist(router.iptables_manager))
 
         restarted_agent = neutron_l3_agent.L3NATAgentWithStateReport(
             self.agent.host, self.agent.conf)
         restarted_router = self.manage_router(restarted_agent, router_info)
 
-        self.assertTrue(is_prevent_snat_rule_exist(
+        self._assert_iptables_rules_exist(
-            restarted_router.iptables_manager))
+            restarted_router.iptables_manager, 'nat', [prevent_snat_rule])
 
     def _get_fixed_ip_rule_priority(self, namespace, fip):
         iprule = ip_lib.IPRule(namespace)
@@ -883,21 +895,3 @@ class TestDvrRouter(framework.L3AgentTestFramework):
         # external networks. SNAT will be used. Direct route will not work
         # here.
         src_machine.assert_no_ping(machine_diff_scope.ip)
-
-    def test_connection_from_diff_address_scope_with_fip(self):
-        (machine_same_scope, machine_diff_scope,
-            router) = self._setup_address_scope('scope1', 'scope2', 'scope1')
-        fip = '19.4.4.11'
-        self._add_fip(router, fip,
-                      fixed_address=machine_diff_scope.ip,
-                      host=self.agent.conf.host,
-                      fixed_ip_address_scope='scope2')
-        router.process(self.agent)
-
-        # For the internal networks that are in the same address scope as
-        # external network, they should be able to reach the floating ip
-        net_helpers.assert_ping(machine_same_scope.namespace, fip, 5)
-        # For the port with fip, it should be able to reach the internal
-        # networks that are in the same address scope as external network
-        net_helpers.assert_ping(machine_diff_scope.namespace,
-                                machine_same_scope.ip, 5)

neutron/tests/unit/agent/l3/test_dvr_fip_ns.py

@@ -272,24 +272,21 @@ class TestDvrFipNs(base.BaseTestCase):
     @mock.patch.object(ip_lib, 'IPDevice')
     def _test_scan_fip_ports(self, ri, ip_list, IPDevice):
         IPDevice.return_value = device = mock.Mock()
-        device.addr.list.return_value = ip_list
+        device.exists.return_value = True
+        ri.get_router_cidrs.return_value = ip_list
         self.fip_ns.get_rtr_ext_device_name = mock.Mock(
             return_value=mock.sentinel.rtr_ext_device_name)
         self.fip_ns.scan_fip_ports(ri)
 
-    @mock.patch.object(ip_lib, 'device_exists')
+    def test_scan_fip_ports_restart_fips(self):
-    def test_scan_fip_ports_restart_fips(self, device_exists):
-        device_exists.return_value = True
         ri = mock.Mock()
         ri.dist_fip_count = None
         ri.floating_ips_dict = {}
-        ip_list = [{'cidr': '111.2.3.4/32'}, {'cidr': '111.2.3.5/32'}]
+        ip_list = [{'cidr': '111.2.3.4'}, {'cidr': '111.2.3.5'}]
         self._test_scan_fip_ports(ri, ip_list)
         self.assertEqual(2, ri.dist_fip_count)
 
-    @mock.patch.object(ip_lib, 'device_exists')
+    def test_scan_fip_ports_restart_none(self):
-    def test_scan_fip_ports_restart_none(self, device_exists):
-        device_exists.return_value = True
         ri = mock.Mock()
         ri.dist_fip_count = None
         ri.floating_ips_dict = {}

neutron/tests/unit/agent/l3/test_dvr_local_router.py

@@ -283,14 +283,13 @@ class TestDvrRouterOperations(base.BaseTestCase):
         ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id)
 
     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()
 
         result = ri.add_floating_ip(fip,
                                     mock.sentinel.interface_name,
                                     mock.sentinel.device)
-        ri._add_fip_addr_to_device.assert_called_once_with(
+        ri.floating_ip_added_dist.assert_called_once_with(
-            fip, mock.sentinel.device)
+            fip, mock.ANY)
         return result
 
     def test_add_floating_ip(self):
@@ -301,13 +300,6 @@ class TestDvrRouterOperations(base.BaseTestCase):
         ri.floating_ip_added_dist.assert_called_once_with(fip, ip + '/32')
         self.assertEqual(l3_constants.FLOATINGIP_STATUS_ACTIVE, result)
 
-    def test_add_floating_ip_error(self):
-        ri = self._create_router(mock.MagicMock())
-        result = self._test_add_floating_ip(
-            ri, {'floating_ip_address': '15.1.2.3'}, False)
-        self.assertFalse(ri.floating_ip_added_dist.called)
-        self.assertEqual(l3_constants.FLOATINGIP_STATUS_ERROR, result)
-
     @mock.patch.object(router_info.RouterInfo, 'remove_floating_ip')
     def test_remove_floating_ip(self, super_remove_floating_ip):
         ri = self._create_router(mock.MagicMock())
@@ -315,8 +307,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
 
         ri.remove_floating_ip(mock.sentinel.device, mock.sentinel.ip_cidr)
 
-        super_remove_floating_ip.assert_called_once_with(
+        self.assertFalse(super_remove_floating_ip.called)
-            mock.sentinel.device, mock.sentinel.ip_cidr)
         ri.floating_ip_removed_dist.assert_called_once_with(
             mock.sentinel.ip_cidr)
 
@@ -630,9 +621,8 @@ class TestDvrRouterOperations(base.BaseTestCase):
         _, fip_to_rtr = ri.rtr_fip_subnet.get_pair()
         self.mock_ip.get_devices.return_value = [
             l3_test_common.FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))]
-        self.mock_ip_dev.addr.list.return_value = [
+        ri.get_router_cidrs = mock.Mock(
-            {'cidr': vm_floating_ip + '/32'},
+            return_value={vm_floating_ip + '/32', '19.4.4.1/24'})
-            {'cidr': '19.4.4.1/24'}]
         self.device_exists.return_value = True
 
         ri.external_gateway_removed(