Nicira plugin: Do not expect a minimum number of SNAT rules

Bug 1192392

The code was assuming a SNAT rule for each subnet was always present
when clearing a gateway. If the SNAT rule was removed for some other
reason from the NVP backend, this would have led to the failure of
the whole operation; however, a failure won't have been necessary
in this case, as the final effect of the operation is to remove
the rules anyway.

Also, this patch ensures a quantum router is put in ERROR state if
it is not found on the NVP backend.

Change-Id: Iac1a61342b490d0d971b884e1aa8d9567b15cac0

diff --git a/quantum/plugins/nicira/QuantumPlugin.py b/quantum/plugins/nicira/QuantumPlugin.py
index 0c14193479093dd27ff2b145d3df2826903f4e39..69d31c8b8d9666653e66f5523fd3471d3d5e93ad 100644 (file)
--- a/quantum/plugins/nicira/QuantumPlugin.py
+++ b/quantum/plugins/nicira/QuantumPlugin.py
@@ -569,11 +569,12 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
                                        port_data['name'],
                                        True,
                                        ['0.0.0.0/31'])
-            # Delete the SNAT rule for each subnet
+            # Delete the SNAT rule for each subnet, keep in mind
+            # that the rule might have already been removed from NVP
             for cidr in self._find_router_subnets_cidrs(context, router_id):
                 nvplib.delete_nat_rules_by_match(
                     self.cluster, router_id, "SourceNatRule",
-                    max_num_expected=1, min_num_expected=1,
+                    max_num_expected=1, min_num_expected=0,
                     source_ip_addresses=cidr)
             # Reset attachment
             self._update_router_port_attachment(
@@ -1479,29 +1480,36 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2,
         return self._make_router_dict(router_db)
 
     def update_router(self, context, id, router):
+        # Either nexthop is updated or should be kept as it was before
+        r = router['router']
+        nexthop = None
+        if 'external_gateway_info' in r and r.get('external_gateway_info'):
+            gw_info = r['external_gateway_info']
+            # The following DB read will be performed again when updating
+            # gateway info. This is not great, but still better than
+            # creating NVP router here and updating it later
+            network_id = (gw_info.get('network_id', None) if gw_info
+                          else None)
+            if network_id:
+                ext_net = self._get_network(context, network_id)
+                if not self._network_is_external(context, network_id):
+                    msg = (_("Network '%s' is not a valid external "
+                             "network") % network_id)
+                    raise q_exc.BadRequest(resource='router', msg=msg)
+                if ext_net.subnets:
+                    ext_subnet = ext_net.subnets[0]
+                    nexthop = ext_subnet.gateway_ip
         try:
-            # Either nexthop is updated or should be kept as it was before
-            r = router['router']
-            nexthop = None
-            if 'external_gateway_info' in r and r.get('external_gateway_info'):
-                gw_info = r['external_gateway_info']
-                # The following DB read will be performed again when updating
-                # gateway info. This is not great, but still better than
-                # creating NVP router here and updating it later
-                network_id = (gw_info.get('network_id', None) if gw_info
-                              else None)
-                if network_id:
-                    ext_net = self._get_network(context, network_id)
-                    if not self._network_is_external(context, network_id):
-                        msg = (_("Network '%s' is not a valid external "
-                                 "network") % network_id)
-                        raise q_exc.BadRequest(resource='router', msg=msg)
-                    if ext_net.subnets:
-                        ext_subnet = ext_net.subnets[0]
-                        nexthop = ext_subnet.gateway_ip
             nvplib.update_lrouter(self.cluster, id,
                                   router['router'].get('name'), nexthop)
-        except NvpApiClient.ResourceNotFound:
+        # NOTE(salv-orlando): The exception handling below is not correct, but
+        # unfortunately nvplib raises a quantum notfound exception when an
+        # object is not found in the underlying backend
+        except q_exc.NotFound:
+            # Put the router in ERROR status
+            with context.session.begin(subtransactions=True):
+                router_db = self._get_router(context, id)
+                router_db['status'] = constants.NET_STATUS_ERROR
             raise nvp_exc.NvpPluginException(
                 err_msg=_("Logical router %s not found on NVP Platform") % id)
         except NvpApiClient.NvpApiException:

diff --git a/quantum/tests/unit/nicira/test_nicira_plugin.py b/quantum/tests/unit/nicira/test_nicira_plugin.py
index c5d813fc180a2023e162619c743a2792b839f8ed..56625b99e00e6b8c5c4644dec55f277621267e35 100644 (file)
--- a/quantum/tests/unit/nicira/test_nicira_plugin.py
+++ b/quantum/tests/unit/nicira/test_nicira_plugin.py
@@ -903,6 +903,65 @@ class NiciraQuantumNVPOutOfSync(test_l3_plugin.L3NatTestCaseBase,
         self.assertEqual(router['router']['status'],
                          constants.NET_STATUS_ERROR)
 
+    def _create_network_and_subnet(self, cidr, external=False):
+        net_res = self._create_network('json', 'ext_net', True)
+        net = self.deserialize('json', net_res)
+        net_id = net['network']['id']
+        if external:
+            self._update('networks', net_id,
+                         {'network': {l3.EXTERNAL: True}})
+        sub_res = self._create_subnet('json', net_id, cidr)
+        sub = self.deserialize('json', sub_res)
+        return net_id, sub['subnet']['id']
+
+    def test_clear_gateway_nat_rule_not_in_nvp(self):
+        # Create external network and subnet
+        ext_net_id = self._create_network_and_subnet('1.1.1.0/24', True)[0]
+        # Create internal network and subnet
+        int_sub_id = self._create_network_and_subnet('10.0.0.0/24')[1]
+        res = self._create_router('json', 'tenant')
+        router = self.deserialize('json', res)
+        # Add interface to router (needed to generate NAT rule)
+        req = self.new_action_request(
+            'routers',
+            {'subnet_id': int_sub_id},
+            router['router']['id'],
+            "add_router_interface")
+        res = req.get_response(self.ext_api)
+        self.assertEqual(res.status_int, 200)
+        # Set gateway for router
+        req = self.new_update_request(
+            'routers',
+            {'router': {'external_gateway_info':
+                        {'network_id': ext_net_id}}},
+            router['router']['id'])
+        res = req.get_response(self.ext_api)
+        self.assertEqual(res.status_int, 200)
+        # Delete NAT rule from NVP, clear gateway
+        # and verify operation still succeeds
+        self.fc._fake_lrouter_nat_dict.clear()
+        req = self.new_update_request(
+            'routers',
+            {'router': {'external_gateway_info': {}}},
+            router['router']['id'])
+        res = req.get_response(self.ext_api)
+        self.assertEqual(res.status_int, 200)
+
+    def test_update_router_not_in_nvp(self):
+        res = self._create_router('json', 'tenant')
+        router = self.deserialize('json', res)
+        self.fc._fake_lrouter_dict.clear()
+        req = self.new_update_request(
+            'routers',
+            {'router': {'name': 'goo'}},
+            router['router']['id'])
+        res = req.get_response(self.ext_api)
+        self.assertEqual(res.status_int, 500)
+        req = self.new_show_request('routers', router['router']['id'])
+        router = self.deserialize('json', req.get_response(self.ext_api))
+        self.assertEqual(router['router']['status'],
+                         constants.NET_STATUS_ERROR)
+
 
 class TestNiciraNetworkGateway(test_l2_gw.NetworkGatewayDbTestCase,
                                NiciraPluginV2TestCase):