From: Salvatore Orlando Date: Wed, 19 Jun 2013 22:51:29 +0000 (+0200) Subject: Nicira plugin: Do not expect a minimum number of SNAT rules X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0d5dccfd3223da59c446e4a8c2e34d8f250a28af;p=openstack-build%2Fneutron-build.git 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 0c1419347..69d31c8b8 100644 --- 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 c5d813fc1..56625b99e 100644 --- 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):