DVR: Add static routes to FIP namespace

The static routes are currently only added to the router
namespace and snat namespace, but not added to the fip
namespace.

This patch adds the static routes configured for the router
to the FIP namespace, to its own table. So this will allow
the FIP namespace to be configured with different nexthop
routes for different routers.

Change-Id: Ida165d1ecf5c07af31dac11d9daed33ccaaf5605
Closes-Bug: #1571676

neutron/agent/l3/dvr_fip_ns.py

@@ -123,29 +123,27 @@ class FipNamespace(namespaces.Namespace):
         with self._fip_port_lock(interface_name):
             is_first = self.subscribe(agent_gateway_port['network_id'])
             if is_first:
-                self._create_gateway_port_and_ns(agent_gateway_port,
-                                                 interface_name)
+                self._create_gateway_port(agent_gateway_port, interface_name)
             else:
-                self._update_gateway_port(agent_gateway_port, interface_name)
+                try:
+                    self._update_gateway_port(
+                        agent_gateway_port, interface_name)
+                except Exception:
+                    # If an exception occurs at this point, then it is
+                    # good to clean up the namespace that has been created
+                    # and reraise the exception in order to resync the router
+                    with excutils.save_and_reraise_exception():
+                        self.unsubscribe(agent_gateway_port['network_id'])
+                        self.delete()
+                        LOG.exception(_LE('DVR: Gateway update in '
+                                          'FIP namespace failed'))
 
-    def _create_gateway_port_and_ns(self, agent_gateway_port, interface_name):
-        """Create namespace and Floating IP gateway port."""
+    def _create_gateway_port(self, ex_gw_port, interface_name):
+        """Create namespace, request port creationg from Plugin,
+           then configure Floating IP gateway port.
+        """
         self.create()
 
-        try:
-            self._create_gateway_port(agent_gateway_port, interface_name)
-        except Exception:
-            # If an exception occurs at this point, then it is
-            # good to clean up the namespace that has been created
-            # and reraise the exception in order to resync the router
-            with excutils.save_and_reraise_exception():
-                self.unsubscribe(agent_gateway_port['network_id'])
-                self.delete()
-                LOG.exception(_LE('DVR: Gateway setup in FIP namespace '
-                                  'failed'))
-
-    def _create_gateway_port(self, ex_gw_port, interface_name):
-        """Request port creation from Plugin then configure gateway port."""
         LOG.debug("DVR: adding gateway interface: %s", interface_name)
         ns_name = self.get_name()
         self.driver.plug(ex_gw_port['network_id'],
@@ -174,7 +172,7 @@ class FipNamespace(namespaces.Namespace):
         self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name,
                             clean_connections=True)
 
-        self._update_gateway_port(ex_gw_port, interface_name)
+        self.agent_gateway_port = ex_gw_port
 
         cmd = ['sysctl', '-w', 'net.ipv4.conf.%s.proxy_arp=1' % interface_name]
         ip_wrapper.netns.execute(cmd, check_exit_code=False)
@@ -242,11 +240,77 @@ class FipNamespace(namespaces.Namespace):
 
         return new_gw_ips != old_gw_ips
 
+    def get_fip_table_indexes(self, ip_version):
+        ns_ipr = ip_lib.IPRule(namespace=self.get_name())
+        ip_rules_list = ns_ipr.rule.list_rules(ip_version)
+        tbl_index_list = []
+        for ip_rule in ip_rules_list:
+            tbl_index = ip_rule['table']
+            if tbl_index in ['local', 'default', 'main']:
+                continue
+            tbl_index_list.append(tbl_index)
+        return tbl_index_list
+
+    def _add_default_gateway_for_fip(self, gw_ip, ip_device, tbl_index):
+        """Adds default gateway for fip based on the tbl_index passed."""
+        if tbl_index is None:
+            ip_version = ip_lib.get_ip_version(gw_ip)
+            tbl_index_list = self.get_fip_table_indexes(ip_version)
+            for tbl_index in tbl_index_list:
+                ip_device.route.add_gateway(gw_ip, table=tbl_index)
+        else:
+            ip_device.route.add_gateway(gw_ip, table=tbl_index)
+
+    def _add_rtr_ext_route_rule_to_route_table(self, ri, fip_2_rtr,
+                                               fip_2_rtr_name):
+        """Creates external route table and adds routing rules."""
+        # TODO(Swami): Rename the _get_snat_idx function to some
+        # generic name that can be used for SNAT and FIP
+        rt_tbl_index = ri._get_snat_idx(fip_2_rtr)
+        interface_name = self.get_ext_device_name(
+            self.agent_gateway_port['id'])
+        try:
+            # The lock is used to make sure another thread doesn't call to
+            # update the gateway route before we are done initializing things.
+            with self._fip_port_lock(interface_name):
+                self._update_gateway_route(self.agent_gateway_port,
+                                           interface_name,
+                                           tbl_index=rt_tbl_index)
+        except Exception:
+            # If an exception occurs at this point, then it is
+            # good to unsubscribe this external network so that
+            # the next call will trigger the interface to be plugged.
+            # We reraise the exception in order to resync the router.
+            with excutils.save_and_reraise_exception():
+                self.unsubscribe(self.agent_gateway_port['network_id'])
+                # Reset the fip count so that the create_rtr_2_fip_link
+                # is called again in this context
+                ri.dist_fip_count = 0
+                LOG.exception(_LE('DVR: Gateway update route in FIP namespace '
+                                  'failed'))
+
+        # Now add the filter match rule for the table.
+        ip_rule = ip_lib.IPRule(namespace=self.get_name())
+        ip_rule.rule.add(ip=str(fip_2_rtr.ip),
+                         iif=fip_2_rtr_name,
+                         table=rt_tbl_index,
+                         priority=rt_tbl_index)
+
     def _update_gateway_port(self, agent_gateway_port, interface_name):
         if (self.agent_gateway_port and
             not self._check_for_gateway_ip_change(agent_gateway_port)):
                 return
+        # Caller already holding lock
+        self._update_gateway_route(
+            agent_gateway_port, interface_name, tbl_index=None)
 
+        # Cache the agent gateway port after successfully updating
+        # the gateway route, so that checking on self.agent_gateway_port
+        # will be a valid check
+        self.agent_gateway_port = agent_gateway_port
+
+    def _update_gateway_route(self, agent_gateway_port,
+                             interface_name, tbl_index):
         ns_name = self.get_name()
         ipd = ip_lib.IPDevice(interface_name, namespace=ns_name)
         # If the 'fg-' device doesn't exist in the namespace then trying
@@ -254,12 +318,11 @@ class FipNamespace(namespaces.Namespace):
         # throw exceptions.  Unsubscribe this external network so that
         # the next call will trigger the interface to be plugged.
         if not ipd.exists():
-            self.unsubscribe(agent_gateway_port['network_id'])
             LOG.warning(_LW('DVR: FIP gateway port with interface '
                             'name: %(device)s does not exist in the given '
                             'namespace: %(ns)s'), {'device': interface_name,
                                                    'ns': ns_name})
-            msg = _('DVR: Gateway setup in FIP namespace failed, retry '
+            msg = _('DVR: Gateway update route in FIP namespace failed, retry '
                     'should be attempted on next call')
             raise n_exc.FloatingIpSetupException(msg)
 
@@ -276,15 +339,11 @@ class FipNamespace(namespaces.Namespace):
                                                 subnet.get('cidr'), gw_ip)
                 if is_gateway_not_in_subnet:
                     ipd.route.add_route(gw_ip, scope='link')
-                ipd.route.add_gateway(gw_ip)
+                self._add_default_gateway_for_fip(gw_ip, ipd, tbl_index)
             else:
                 current_gateway = ipd.route.get_gateway()
                 if current_gateway and current_gateway.get('gateway'):
                     ipd.route.delete_gateway(current_gateway.get('gateway'))
-        # Cache the agent gateway port after successfully configuring
-        # the gateway, so that checking on self.agent_gateway_port
-        # will be a valid check
-        self.agent_gateway_port = agent_gateway_port
 
     def _add_cidr_to_device(self, device, ip_cidr):
         if not device.addr.list(to=ip_cidr):
@@ -318,6 +377,8 @@ class FipNamespace(namespaces.Namespace):
 
         self._add_cidr_to_device(rtr_2_fip_dev, str(rtr_2_fip))
         self._add_cidr_to_device(fip_2_rtr_dev, str(fip_2_rtr))
+        self._add_rtr_ext_route_rule_to_route_table(ri, fip_2_rtr,
+                                                    fip_2_rtr_name)
 
         # add default route for the link local interface
         rtr_2_fip_dev.route.add_gateway(str(fip_2_rtr.ip), table=FIP_RT_TBL)

neutron/agent/l3/dvr_local_router.py

@@ -149,6 +149,28 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
             ns_ip = ip_lib.IPWrapper(namespace=fip_ns_name)
             device.route.delete_gateway(str(fip_2_rtr.ip),
                                         table=dvr_fip_ns.FIP_RT_TBL)
+            if self.fip_ns.agent_gateway_port:
+                interface_name = self.fip_ns.get_ext_device_name(
+                    self.fip_ns.agent_gateway_port['id'])
+                fg_device = ip_lib.IPDevice(
+                    interface_name, namespace=fip_ns_name)
+                if fg_device.exists():
+                    # Remove the fip namespace rules and routes associated to
+                    # fpr interface route table.
+                    tbl_index = self._get_snat_idx(fip_2_rtr)
+                    fip_rt_rule = ip_lib.IPRule(namespace=fip_ns_name)
+                    # Flush the table
+                    fg_device.route.flush(lib_constants.IP_VERSION_4,
+                                          table=tbl_index)
+                    fg_device.route.flush(lib_constants.IP_VERSION_6,
+                                          table=tbl_index)
+                    # Remove the rule lookup
+                    # IP is ignored in delete, but we still require it
+                    # for getting the ip_version.
+                    fip_rt_rule.rule.delete(ip=fip_2_rtr.ip,
+                                            iif=fip_2_rtr_name,
+                                            table=tbl_index,
+                                            priority=tbl_index)
             self.fip_ns.local_subnets.release(self.router_id)
             self.rtr_fip_subnet = None
             ns_ip.del_veth(fip_2_rtr_name)
@@ -539,6 +561,59 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
             if (self.fip_ns.agent_gateway_port and
                 (self.dist_fip_count == 0)):
                 self.fip_ns.create_rtr_2_fip_link(self)
+                self.routes_updated([], self.router['routes'])
+
+    def update_routing_table(self, operation, route):
+        # TODO(Swami): The static routes should be added to the
+        # specific namespace based on the availability of the
+        # network interfaces. In the case of DVR the static routes
+        # for local internal router networks can be added to the
+        # snat_namespace and router_namespace but should not be
+        # added to the fip namespace. Likewise the static routes
+        # for the external router networks should only be added to
+        # the snat_namespace and fip_namespace.
+        # The current code adds static routes to all namespaces in
+        # order to reduce the complexity. This should be revisited
+        # later.
+        if self.fip_ns and self.fip_ns.agent_gateway_port:
+            fip_ns_name = self.fip_ns.get_name()
+            agent_gw_port = self.fip_ns.agent_gateway_port
+            route_apply = self._check_if_route_applicable_to_fip_namespace(
+                route, agent_gw_port)
+            if route_apply:
+                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()
+                tbl_index = self._get_snat_idx(fip_2_rtr)
+                self._update_fip_route_table_with_next_hop_routes(
+                    operation, route, fip_ns_name, tbl_index)
+        super(DvrLocalRouter, self).update_routing_table(operation, route)
+
+    def _update_fip_route_table_with_next_hop_routes(
+        self, operation, route, fip_ns_name, tbl_index):
+        cmd = ['ip', 'route', operation, 'to', route['destination'],
+               'via', route['nexthop'], 'table', tbl_index]
+        ip_wrapper = ip_lib.IPWrapper(namespace=fip_ns_name)
+        if ip_wrapper.netns.exists(fip_ns_name):
+            ip_wrapper.netns.execute(cmd, check_exit_code=False)
+        else:
+            LOG.debug("The FIP namespace %(ns)s does not exist for "
+                      "router %(id)s",
+                      {'ns': fip_ns_name, 'id': self.router_id})
+
+    def _check_if_route_applicable_to_fip_namespace(
+        self, route, agent_gateway_port):
+        ip_cidrs = common_utils.fixed_ip_cidrs(agent_gateway_port['fixed_ips'])
+        nexthop_cidr = netaddr.IPAddress(route['nexthop'])
+        for gw_cidr in ip_cidrs:
+            gw_subnet_cidr = netaddr.IPNetwork(gw_cidr)
+            # NOTE: In the case of DVR routers apply the extra routes
+            # on the FIP namespace only if it is associated with the
+            # external agent gateway subnets.
+            if nexthop_cidr in gw_subnet_cidr:
+                return True
+        return False
 
     def get_router_cidrs(self, device):
         """As no floatingip will be set on the rfp device. Get floatingip from

neutron/agent/linux/ip_lib.py

@@ -474,7 +474,10 @@ class IpRuleCommand(IpCommandBase):
     def add(self, ip, **kwargs):
         ip_version = get_ip_version(ip)
 
-        kwargs.update({'from': ip})
+        # In case if we need to add in a rule based on incoming
+        # interface we don't need to pass in the ip.
+        if not kwargs.get('iif'):
+            kwargs.update({'from': ip})
         canonical_kwargs = self._make_canonical(ip_version, kwargs)
 
         if not self._exists(ip_version, **canonical_kwargs):

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

@@ -125,7 +125,11 @@ class TestDvrRouter(framework.L3AgentTestFramework):
         fg_device = ip_lib.IPDevice(fg_port_name,
                                     namespace=router.fip_ns.name)
         # Now validate if the gateway is properly configured.
-        self.assertIn('gateway', fg_device.route.get_gateway())
+        rtr_2_fip, fip_2_rtr = router.rtr_fip_subnet.get_pair()
+        tbl_index = router._get_snat_idx(fip_2_rtr)
+        tbl_filter = ['table', tbl_index]
+        self.assertIn('gateway', fg_device.route.get_gateway(
+            filters=tbl_filter))
         self._validate_fips_for_external_network(
             router, router.fip_ns.get_name())
         # Now delete the fg- port that was created
@@ -148,8 +152,14 @@ class TestDvrRouter(framework.L3AgentTestFramework):
                           router.router)
         router = self.manage_router(self.agent, router.router)
         self.assertTrue(fg_device.exists())
-        self.assertEqual({'gateway': u'19.4.4.2'},
-                         fg_device.route.get_gateway())
+        updated_route = fg_device.route.list_routes(
+                ip_version=lib_constants.IP_VERSION_4,
+                table=tbl_index)
+        expected_route = [{'cidr': '0.0.0.0/0',
+                           'dev': fg_port_name,
+                           'table': tbl_index,
+                           u'via': u'19.4.4.2'}]
+        self.assertEqual(expected_route, updated_route)
         self._validate_fips_for_external_network(
             router, router.fip_ns.get_name())
         self._delete_router(self.agent, router.router_id)
@@ -187,8 +197,12 @@ class TestDvrRouter(framework.L3AgentTestFramework):
         fg_port_name = router.fip_ns.get_ext_device_name(fg_port['id'])
         fg_device = ip_lib.IPDevice(fg_port_name,
                                     namespace=router.fip_ns.name)
+        rtr_2_fip, fip_2_rtr = router.rtr_fip_subnet.get_pair()
+        tbl_index = router._get_snat_idx(fip_2_rtr)
+        tbl_filter = ['table', tbl_index]
         # Now validate if the gateway is properly configured.
-        self.assertIn('gateway', fg_device.route.get_gateway())
+        self.assertIn('gateway', fg_device.route.get_gateway(
+            filters=tbl_filter))
         self._validate_fips_for_external_network(
             router, router.fip_ns.get_name())
         self._delete_router(self.agent, router.router_id)
@@ -1062,23 +1076,120 @@ class TestDvrRouter(framework.L3AgentTestFramework):
     def test_dvr_ha_router_failover_without_gw(self):
         self._test_dvr_ha_router_failover(enable_gw=False)
 
-    def test_dvr_router_static_routes(self):
+    def _setup_dvr_router_static_routes(
+        self, router_namespace=True, check_fpr_int_rule_delete=False):
         """Test to validate the extra routes on dvr routers."""
         self.agent.conf.agent_mode = 'dvr_snat'
         router_info = self.generate_dvr_router_info(enable_snat=True)
         router1 = self.manage_router(self.agent, router_info)
         self.assertTrue(self._namespace_exists(router1.ns_name))
         self._assert_snat_namespace_exists(router1)
+        fip_ns_name = router1.fip_ns.get_name()
+        self.assertTrue(self._namespace_exists(fip_ns_name))
         snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
             router1.router_id)
-        # Now try to add routes that are suitable for both the
-        # router namespace and the snat namespace.
-        router1.router['routes'] = [{'destination': '8.8.4.0/24',
-                                     'nexthop': '35.4.0.20'}]
+        if router_namespace:
+            router1.router['routes'] = [{'destination': '8.8.4.0/24',
+                                         'nexthop': '35.4.0.20'}]
+        else:
+            router1.router['routes'] = [{'destination': '8.8.4.0/24',
+                                         'nexthop': '19.4.4.10'}]
+
         self.agent._process_updated_router(router1.router)
         router_updated = self.agent.router_info[router_info['id']]
-        self._assert_extra_routes(router_updated, namespace=snat_ns_name)
-        self._assert_extra_routes(router_updated)
+        if router_namespace:
+            self._assert_extra_routes(router_updated)
+            self._assert_extra_routes(router_updated, namespace=snat_ns_name)
+        else:
+            rtr_2_fip, fip_2_rtr = router_updated.rtr_fip_subnet.get_pair()
+            # Now get the table index based on the fpr-interface ip.
+            router_fip_table_idx = router_updated._get_snat_idx(fip_2_rtr)
+            self._assert_extra_routes_for_fipns(
+                router_updated, router_fip_table_idx)
+            self._assert_extra_routes(router_updated, namespace=snat_ns_name)
+        if check_fpr_int_rule_delete:
+            router_updated.router[lib_constants.FLOATINGIP_KEY] = []
+            self.agent._process_updated_router(router_updated.router)
+            new_router_info = self.agent.router_info[router_updated.router_id]
+            self.assertTrue(self._namespace_exists(fip_ns_name))
+            self._assert_extra_routes_for_fipns(
+                new_router_info, router_fip_table_idx,
+                check_fpr_int_rule_delete=check_fpr_int_rule_delete)
+
+    def _assert_extra_routes_for_fipns(self, router, router_fip_table_idx,
+                                       check_fpr_int_rule_delete=False):
+
+        fip_ns_name = router.fip_ns.get_name()
+        self.assertTrue(self._namespace_exists(fip_ns_name))
+        fg_port = router.fip_ns.agent_gateway_port
+        fg_port_name = router.fip_ns.get_ext_device_name(fg_port['id'])
+        fip_ns_int_name = router.fip_ns.get_int_device_name(router.router_id)
+        fg_device = ip_lib.IPDevice(fg_port_name,
+                                    namespace=fip_ns_name)
+        tbl_filter = ['table', router_fip_table_idx]
+        if not check_fpr_int_rule_delete:
+            self.assertIn('gateway', fg_device.route.get_gateway(
+                filters=tbl_filter))
+        else:
+            self.assertIsNone(fg_device.route.get_gateway(filters=tbl_filter))
+
+        ip_rule = ip_lib.IPRule(namespace=fip_ns_name)
+        ext_net_fw_rules_list = ip_rule.rule.list_rules(
+            lib_constants.IP_VERSION_4)
+        if not check_fpr_int_rule_delete:
+            # When floatingip are associated, make sure that the
+            # corresponding rules and routes in route table are created
+            # for the router.
+            expected_rule = {u'from': '0.0.0.0/0',
+                             u'iif': fip_ns_int_name,
+                             'priority': str(router_fip_table_idx),
+                             'table': str(router_fip_table_idx),
+                             'type': 'unicast'}
+            for rule in ext_net_fw_rules_list:
+                rule_tbl = rule['table']
+                if rule_tbl in ['default', 'local', 'main']:
+                    continue
+                if rule_tbl == str(router_fip_table_idx):
+                    self.assertEqual(expected_rule, rule)
+            # Now check the routes in the table.
+            destination = router.router['routes'][0]['destination']
+            next_hop = router.router['routes'][0]['nexthop']
+            actual_routes = fg_device.route.list_routes(
+                ip_version=lib_constants.IP_VERSION_4,
+                table=router_fip_table_idx,
+                via=str(next_hop))
+            expected_extra_route = [{'cidr': unicode(destination),
+                                     'dev': fg_port_name,
+                                     'table': router_fip_table_idx,
+                                     'via': next_hop}]
+            self.assertEqual(expected_extra_route, actual_routes)
+        else:
+            # When floatingip are deleted or disassociated, make sure that the
+            # corresponding rules and routes are cleared from the table
+            # corresponding to the router.
+            self.assertEqual(3, len(ext_net_fw_rules_list))
+            rule_exist = False
+            for rule in ext_net_fw_rules_list:
+                rule_tbl = rule['table']
+                if rule_tbl not in ['default', 'local', 'main']:
+                    rule_exist = True
+            self.assertFalse(rule_exist)
+            tbl_routes = fg_device.route.list_routes(
+                ip_version=lib_constants.IP_VERSION_4,
+                table=router_fip_table_idx)
+            self.assertEqual([], tbl_routes)
+
+    def test_dvr_router_static_routes_in_fip_and_snat_namespace(self):
+        self._setup_dvr_router_static_routes(router_namespace=False)
+
+    def test_dvr_router_static_routes_in_snat_namespace_and_router_namespace(
+        self):
+        self._setup_dvr_router_static_routes()
+
+    def test_dvr_router_rule_and_route_table_cleared_when_fip_removed(
+        self):
+        self._setup_dvr_router_static_routes(
+            router_namespace=False, check_fpr_int_rule_delete=True)
 
     def test_dvr_router_gateway_update_to_none(self):
         self.agent.conf.agent_mode = 'dvr_snat'
@@ -1092,8 +1203,13 @@ class TestDvrRouter(framework.L3AgentTestFramework):
         fg_port_name = router.fip_ns.get_ext_device_name(fg_port['id'])
         fg_device = ip_lib.IPDevice(fg_port_name,
                                     namespace=router.fip_ns.name)
+        rtr_2_fip, fip_2_rtr = router.rtr_fip_subnet.get_pair()
+        tbl_index = router._get_snat_idx(fip_2_rtr)
+
         self.assertIn('gateway', ex_gw_device.route.get_gateway())
-        self.assertIn('gateway', fg_device.route.get_gateway())
+        tbl_filter = ['table', tbl_index]
+        self.assertIn('gateway', fg_device.route.get_gateway(
+            filters=tbl_filter))
 
         # Make this copy to make agent think gw_port changed.
         router.ex_gw_port = copy.deepcopy(router.ex_gw_port)

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

@@ -97,86 +97,63 @@ class TestDvrFipNs(base.BaseTestCase):
     @mock.patch.object(dvr_fip_ns.FipNamespace, 'create')
     def test_create_gateway_port(self, fip_create, device_exists, ip_wrapper):
         agent_gw_port = self._get_agent_gw_port()
-        interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id'])
 
         device_exists.return_value = False
-        self.fip_ns._update_gateway_port = mock.Mock()
         self.fip_ns.create_or_update_gateway_port(agent_gw_port)
         self.assertTrue(fip_create.called)
         self.assertEqual(1, self.driver.plug.call_count)
         self.assertEqual(1, self.driver.init_l3.call_count)
-        self.fip_ns._update_gateway_port.assert_called_once_with(
-            agent_gw_port, interface_name)
-
-    @mock.patch.object(ip_lib, 'IPWrapper')
-    @mock.patch.object(ip_lib, 'device_exists')
-    @mock.patch.object(dvr_fip_ns.FipNamespace, 'create')
-    @mock.patch.object(dvr_fip_ns.FipNamespace, 'delete')
-    @mock.patch.object(dvr_fip_ns.FipNamespace, 'unsubscribe')
-    def test_create_gateway_port_raises_exception(
-        self, fip_desub, fip_delete, fip_create, device_exists, ip_wrapper):
-        agent_gw_port = self._get_agent_gw_port()
-        interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id'])
-
-        device_exists.return_value = False
-        msg = 'L3 agent failed to setup fip gateway in the namespace'
-        self.fip_ns._update_gateway_port = mock.Mock(
-            side_effect=n_exc.FloatingIpSetupException(msg))
-        self.assertRaises(n_exc.FloatingIpSetupException,
-                          self.fip_ns.create_or_update_gateway_port,
-                          agent_gw_port)
-        self.assertTrue(fip_create.called)
-        self.assertEqual(1, self.driver.plug.call_count)
-        self.assertEqual(1, self.driver.init_l3.call_count)
-        self.fip_ns._update_gateway_port.assert_called_once_with(
-            agent_gw_port, interface_name)
-        self.assertTrue(fip_desub.called)
-        self.assertTrue(fip_delete.called)
-        self.assertIsNone(self.fip_ns.agent_gateway_port)
 
     @mock.patch.object(ip_lib, 'IPDevice')
     @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif')
     @mock.patch.object(dvr_fip_ns.FipNamespace, 'subscribe')
-    def test_update_gateway_port(self, fip_sub, send_adv_notif, IPDevice):
+    @mock.patch.object(dvr_fip_ns.FipNamespace, '_add_default_gateway_for_fip')
+    def test_update_gateway_port(
+        self, def_gw, fip_sub, send_adv_notif, IPDevice):
         fip_sub.return_value = False
         self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True)
-        self.fip_ns.agent_gateway_port = None
         agent_gw_port = self._get_agent_gw_port()
-
+        interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id'])
         self.fip_ns.create_or_update_gateway_port(agent_gw_port)
         expected = [
             mock.call(self.fip_ns.get_name(),
-                      self.fip_ns.get_ext_device_name(agent_gw_port['id']),
+                      interface_name,
                       agent_gw_port['fixed_ips'][0]['ip_address'],
                       mock.ANY),
             mock.call(self.fip_ns.get_name(),
-                      self.fip_ns.get_ext_device_name(agent_gw_port['id']),
+                      interface_name,
                       agent_gw_port['fixed_ips'][1]['ip_address'],
                       mock.ANY)]
         send_adv_notif.assert_has_calls(expected)
-        gw_ipv4 = agent_gw_port['subnets'][0]['gateway_ip']
-        gw_ipv6 = agent_gw_port['subnets'][1]['gateway_ip']
-        expected = [mock.call(gw_ipv4), mock.call(gw_ipv6)]
-        IPDevice().route.add_gateway.assert_has_calls(expected)
+        self.assertTrue(def_gw.called)
 
     @mock.patch.object(ip_lib.IPDevice, 'exists')
     @mock.patch.object(dvr_fip_ns.FipNamespace, 'subscribe')
-    def test_update_gateway_port_raises_exception(self, fip_sub, exists):
-        fip_sub.return_value = False
-        exists.return_value = False
+    @mock.patch.object(dvr_fip_ns.FipNamespace, 'delete')
+    @mock.patch.object(dvr_fip_ns.FipNamespace, 'unsubscribe')
+    def test_update_gateway_port_raises_exception(
+        self, fip_unsub, fip_delete, fip_sub, exists):
         self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True)
         self.fip_ns.agent_gateway_port = None
         agent_gw_port = self._get_agent_gw_port()
+        self.fip_ns._create_gateway_port = mock.Mock()
+        self.fip_ns.create_or_update_gateway_port(agent_gw_port)
+        exists.return_value = False
+        fip_sub.return_value = False
+        self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True)
 
         self.assertRaises(n_exc.FloatingIpSetupException,
                           self.fip_ns.create_or_update_gateway_port,
                           agent_gw_port)
+        self.assertTrue(fip_unsub.called)
+        self.assertTrue(fip_delete.called)
 
     @mock.patch.object(ip_lib, 'IPDevice')
     @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif')
     @mock.patch.object(dvr_fip_ns.FipNamespace, 'subscribe')
+    @mock.patch.object(dvr_fip_ns.FipNamespace, '_add_default_gateway_for_fip')
     def test_update_gateway_port_gateway_outside_subnet_added(
-            self, fip_sub, send_adv_notif, IPDevice):
+            self, def_gw, fip_sub, send_adv_notif, IPDevice):
         fip_sub.return_value = False
         self.fip_ns.agent_gateway_port = None
         agent_gw_port = self._get_agent_gw_port()
@@ -186,6 +163,7 @@ class TestDvrFipNs(base.BaseTestCase):
 
         IPDevice().route.add_route.assert_called_once_with('20.0.1.1',
                                                            scope='link')
+        self.assertTrue(def_gw.called)
 
     def test_check_gateway_ip_changed_no_change(self):
         agent_gw_port = self._get_agent_gw_port()
@@ -300,7 +278,8 @@ class TestDvrFipNs(base.BaseTestCase):
         device = IPDevice()
         device.exists.return_value = dev_exists
         device.addr.list.return_value = addr_exists
-
+        ri._get_snat_idx = mock.Mock()
+        self.fip_ns._add_rtr_ext_route_rule_to_route_table = mock.Mock()
         self.fip_ns.create_rtr_2_fip_link(ri)
 
         if not dev_exists:
@@ -320,6 +299,8 @@ class TestDvrFipNs(base.BaseTestCase):
 
         device.route.add_gateway.assert_called_once_with(
             '169.254.31.29', table=16)
+        self.assertTrue(
+            self.fip_ns._add_rtr_ext_route_rule_to_route_table.called)
 
     def test_create_rtr_2_fip_link(self):
         self._test_create_rtr_2_fip_link(False, False)

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

@@ -282,8 +282,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
         self.assertTrue(fip_ns.destroyed)
         mIPWrapper().del_veth.assert_called_once_with(
             fip_ns.get_int_device_name(router['id']))
-        mIPDevice().route.delete_gateway.assert_called_once_with(
-            str(fip_to_rtr.ip), table=16)
+        self.assertEqual(1, mIPDevice().route.delete_gateway.call_count)
         self.assertFalse(ri.fip_ns.unsubscribe.called)
         ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id)