Merge "DVR: Add static routes to FIP namespace"

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,
+                self._create_gateway_port(agent_gateway_port, interface_name)
-                                                 interface_name)
             else:
-                self._update_gateway_port(agent_gateway_port, interface_name)
+                try:
-
+                    self._update_gateway_port(
-    def _create_gateway_port_and_ns(self, agent_gateway_port, interface_name):
+                        agent_gateway_port, interface_name)
-        """Create namespace and Floating IP gateway port."""
+                except Exception:
-        self.create()
+                    # If an exception occurs at this point, then it is
-
+                    # good to clean up the namespace that has been created
-        try:
+                    # and reraise the exception in order to resync the router
-            self._create_gateway_port(agent_gateway_port, interface_name)
+                    with excutils.save_and_reraise_exception():
-        except Exception:
+                        self.unsubscribe(agent_gateway_port['network_id'])
-            # If an exception occurs at this point, then it is
+                        self.delete()
-            # good to clean up the namespace that has been created
+                        LOG.exception(_LE('DVR: Gateway update in '
-            # and reraise the exception in order to resync the router
+                                          'FIP namespace failed'))
-            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."""
+        """Create namespace, request port creationg from Plugin,
+           then configure Floating IP gateway port.
+        """
+        self.create()
+
         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

@@ -475,7 +475,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'},
+        updated_route = fg_device.route.list_routes(
-                         fg_device.route.get_gateway())
+                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)
@@ -1064,23 +1078,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
+        if router_namespace:
-        # router namespace and the snat namespace.
+            router1.router['routes'] = [{'destination': '8.8.4.0/24',
-        router1.router['routes'] = [{'destination': '8.8.4.0/24',
+                                         'nexthop': '35.4.0.20'}]
-                                     '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)
+        if router_namespace:
-        self._assert_extra_routes(router_updated)
+            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'
@@ -1094,8 +1205,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']
+        self.assertTrue(def_gw.called)
-        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)
 
     @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):
+    @mock.patch.object(dvr_fip_ns.FipNamespace, 'delete')
-        fip_sub.return_value = False
+    @mock.patch.object(dvr_fip_ns.FipNamespace, 'unsubscribe')
-        exists.return_value = False
+    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

@@ -286,8 +286,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(
+        self.assertEqual(1, mIPDevice().route.delete_gateway.call_count)
-            str(fip_to_rtr.ip), table=16)
         self.assertFalse(ri.fip_ns.unsubscribe.called)
         ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id)