openstack
/
nova
Code Issues Proposed changes

Count networks to check quota 

This changes networks from a ReservableResource to a CountableResource
and replaces quota reserve/commit/rollback with check_deltas
accordingly. Note that network quota is only relevant to nova-network
and will be obsolete when nova-network is removed.

Co-Authored-By: melanie witt <melwittt@gmail.com>

Part of blueprint cells-count-resources-to-check-quota-in-api

Change-Id: If2e279728be056566829ae58d45d31801cd59b6c
This commit is contained in:
Dan Smith 2017-03-15 16:19:14 -07:00 committed by melanie witt
parent ac41395f11
commit ff145323a9
5 changed files with 113 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
nova/api/openstack/compute/tenant_networks.py
View File

@ -31,6 +31,7 @@ from nova import context as nova_context
from nova import exception
from nova.i18n import _
import nova.network
from nova import objects
from nova.policies import tenant_networks as tn_policies
from nova import quota
@ -101,35 +102,18 @@ class TenantNetworkController(wsgi.Controller):
def delete(self, req, id):
context = req.environ['nova.context']
context.can(tn_policies.BASE_POLICY_NAME)
reservation = None
try:
if CONF.enable_network_quota:
reservation = QUOTAS.reserve(context, networks=-1)
except Exception:
reservation = None
LOG.exception("Failed to update usages deallocating network.")
def _rollback_quota(reservation):
if CONF.enable_network_quota and reservation:
QUOTAS.rollback(context, reservation)
try:
self.network_api.disassociate(context, id)
self.network_api.delete(context, id)
except exception.PolicyNotAuthorized as e:
_rollback_quota(reservation)
raise exc.HTTPForbidden(explanation=six.text_type(e))
except exception.NetworkInUse as e:
_rollback_quota(reservation)
raise exc.HTTPConflict(explanation=e.format_message())
except exception.NetworkNotFound:
_rollback_quota(reservation)
msg = _("Network not found")
raise exc.HTTPNotFound(explanation=msg)
if CONF.enable_network_quota and reservation:
QUOTAS.commit(context, reservation)
@wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
@extensions.expected_errors((400, 403, 409, 503))
@validation.schema(schema.create)
@ -157,7 +141,8 @@ class TenantNetworkController(wsgi.Controller):
try:
if CONF.enable_network_quota:
reservation = QUOTAS.reserve(context, networks=1)
objects.Quotas.check_deltas(context, {'networks': 1},
context.project_id)
except exception.OverQuota:
msg = _("Quota exceeded, too many networks.")
raise exc.HTTPForbidden(explanation=msg)
@ -167,31 +152,43 @@ class TenantNetworkController(wsgi.Controller):
try:
networks = self.network_api.create(context,
label=label, **kwargs)
if CONF.enable_network_quota:
QUOTAS.commit(context, reservation)
except exception.PolicyNotAuthorized as e:
raise exc.HTTPForbidden(explanation=six.text_type(e))
except exception.CidrConflict as e:
raise exc.HTTPConflict(explanation=e.format_message())
except Exception:
if CONF.enable_network_quota:
QUOTAS.rollback(context, reservation)
msg = _("Create networks failed")
LOG.exception(msg, extra=network)
raise exc.HTTPServiceUnavailable(explanation=msg)
# NOTE(melwitt): We recheck the quota after creating the object to
# prevent users from allocating more resources than their allowed quota
# in the event of a race. This is configurable because it can be
# expensive if strict quota limits are not required in a deployment.
if CONF.quota.recheck_quota and CONF.enable_network_quota:
try:
objects.Quotas.check_deltas(context, {'networks': 0},
context.project_id)
except exception.OverQuota:
self.network_api.delete(context,
network_dict(networks[0])['id'])
msg = _("Quota exceeded, too many networks.")
raise exc.HTTPForbidden(explanation=msg)
return {"network": network_dict(networks[0])}
def _sync_networks(context, project_id, session):
def _network_count(context, project_id):
# NOTE(melwitt): This assumes a single cell.
ctx = nova_context.RequestContext(user_id=None, project_id=project_id)
ctx = ctx.elevated()
networks = nova.network.api.API().get_all(ctx)
return dict(networks=len(networks))
return {'project': {'networks': len(networks)}}
def _register_network_quota():
if CONF.enable_network_quota:
QUOTAS.register_resource(quota.ReservableResource('networks',
_sync_networks,
QUOTAS.register_resource(quota.CountableResource('networks',
_network_count,
'quota_networks'))
_register_network_quota()

14
nova/tests/fixtures.py
View File

@ -31,6 +31,7 @@ from oslo_config import cfg
import oslo_messaging as messaging
from oslo_messaging import conffixture as messaging_conffixture
from nova.api.openstack.compute import tenant_networks
from nova.api.openstack.placement import deploy as placement_deploy
from nova.compute import rpcapi as compute_rpcapi
from nova import context
@ -41,6 +42,7 @@ from nova.network import model as network_model
from nova import objects
from nova.objects import base as obj_base
from nova.objects import service as service_obj
from nova import quota as nova_quota
from nova import rpc
from nova import service
from nova.tests.functional.api import client
@ -989,6 +991,18 @@ class AllServicesCurrent(fixtures.Fixture):
return service_obj.SERVICE_VERSION
class RegisterNetworkQuota(fixtures.Fixture):
def setUp(self):
super(RegisterNetworkQuota, self).setUp()
# Quota resource registration modifies the global QUOTAS engine, so
# this fixture registers and unregisters network quota for a test.
tenant_networks._register_network_quota()
self.addCleanup(self.cleanup)
def cleanup(self):
nova_quota.QUOTAS._resources.pop('networks', None)
class NeutronFixture(fixtures.Fixture):
"""A fixture to boot instances with neutron ports"""

2
nova/tests/functional/api_sample_tests/test_tenant_networks.py
View File

@ -17,6 +17,7 @@
from oslo_serialization import jsonutils
import nova.conf
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional.api_sample_tests import api_sample_base
CONF = nova.conf.CONF
@ -29,6 +30,7 @@ class TenantNetworksJsonTests(api_sample_base.ApiSampleTestBaseV21):
def setUp(self):
super(TenantNetworksJsonTests, self).setUp()
CONF.set_override("enable_network_quota", True)
self.useFixture(nova_fixtures.RegisterNetworkQuota())
def fake(*args, **kwargs):
pass

11
nova/tests/unit/api/openstack/compute/test_quotas.py
View File

@ -18,11 +18,11 @@ import mock
import webob
from nova.api.openstack.compute import quota_sets as quotas_v21
from nova.api.openstack.compute import tenant_networks
from nova import db
from nova import exception
from nova import quota
from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.unit.api.openstack import fakes
@ -291,10 +291,9 @@ class QuotaSetsTestV21(BaseQuotaSetsTest):
def test_update_network_quota_enabled(self):
self.flags(enable_network_quota=True)
tenant_networks._register_network_quota()
self.useFixture(nova_fixtures.RegisterNetworkQuota())
self.controller.update(self._get_http_request(),
1234, body={'quota_set': {'networks': 1}})
del quota.QUOTAS._resources['networks']
class ExtendedQuotasTestV21(BaseQuotaSetsTest):
@ -546,7 +545,7 @@ class QuotaSetsTestV236(test.NoDBTestCase):
lambda ctx, project_id: True)
self.flags(enable_network_quota=True)
tenant_networks._register_network_quota()
self.useFixture(nova_fixtures.RegisterNetworkQuota())
self.old_req = fakes.HTTPRequest.blank('', version='2.1')
self.filtered_quotas = ['fixed_ips', 'floating_ips', 'networks',
'security_group_rules', 'security_groups']
@ -586,10 +585,6 @@ class QuotaSetsTestV236(test.NoDBTestCase):
}
self.controller = quotas_v21.QuotaSetsController()
self.req = fakes.HTTPRequest.blank('', version='2.36')
self.addCleanup(self._remove_network_quota)
def _remove_network_quota(self):
del quota.QUOTAS._resources['networks']
def _ensure_filtered_quotas_existed_in_old_api(self):
res_dict = self.controller.show(self.old_req, 1234)

103
nova/tests/unit/api/openstack/compute/test_tenant_networks.py
View File

@ -22,6 +22,7 @@ from nova.api.openstack.compute import tenant_networks \
as networks_v21
from nova import exception
from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.unit.api.openstack import fakes
CONF = cfg.CONF
@ -72,6 +73,7 @@ class TenantNetworksTestV21(test.NoDBTestCase):
# result in a 503 and 500 response, respectively.
self.flags(enable_network_quota=True,
use_neutron=self.use_neutron)
self.useFixture(nova_fixtures.RegisterNetworkQuota())
self.controller = self.ctrlr()
self.req = fakes.HTTPRequest.blank('')
self.original_value = CONF.api.use_neutron_default_nets
@ -85,16 +87,12 @@ class TenantNetworksTestV21(test.NoDBTestCase):
self.assertEqual(context.project_id, kwargs['project_id'])
return NETWORKS
@mock.patch('nova.quota.QUOTAS.reserve')
@mock.patch('nova.quota.QUOTAS.rollback')
@mock.patch('nova.network.api.API.disassociate')
@mock.patch('nova.network.api.API.delete')
def _test_network_delete_exception(self, delete_ex, disassociate_ex, expex,
delete_mock, disassociate_mock,
rollback_mock, reserve_mock):
delete_mock, disassociate_mock):
ctxt = self.req.environ['nova.context']
reserve_mock.return_value = 'rv'
if delete_mock:
delete_mock.side_effect = delete_ex
if disassociate_ex:
@ -108,8 +106,6 @@ class TenantNetworksTestV21(test.NoDBTestCase):
disassociate_mock.assert_called_once_with(ctxt, 1)
if not disassociate_ex:
delete_mock.assert_called_once_with(ctxt, 1)
rollback_mock.assert_called_once_with(ctxt, 'rv')
reserve_mock.assert_called_once_with(ctxt, networks=-1)
def test_network_delete_exception_network_not_found(self):
ex = exception.NetworkNotFound(network_id=1)
@ -126,16 +122,11 @@ class TenantNetworksTestV21(test.NoDBTestCase):
expex = webob.exc.HTTPConflict
self._test_network_delete_exception(ex, None, expex)
@mock.patch('nova.quota.QUOTAS.reserve')
@mock.patch('nova.quota.QUOTAS.commit')
@mock.patch('nova.network.api.API.delete')
@mock.patch('nova.network.api.API.disassociate')
def test_network_delete(self, disassociate_mock, delete_mock, commit_mock,
reserve_mock):
def test_network_delete(self, disassociate_mock, delete_mock):
ctxt = self.req.environ['nova.context']
reserve_mock.return_value = 'rv'
delete_method = self.controller.delete
res = delete_method(self.req, 1)
# NOTE: on v2.1, http status code is set as wsgi_code of API
@ -148,8 +139,6 @@ class TenantNetworksTestV21(test.NoDBTestCase):
disassociate_mock.assert_called_once_with(ctxt, 1)
delete_mock.assert_called_once_with(ctxt, 1)
commit_mock.assert_called_once_with(ctxt, 'rv')
reserve_mock.assert_called_once_with(ctxt, networks=-1)
def test_network_show(self):
with mock.patch.object(self.controller.network_api, 'get',
@ -184,13 +173,9 @@ class TenantNetworksTestV21(test.NoDBTestCase):
def test_network_index_without_default_net(self):
self._test_network_index(default_net=False)
@mock.patch('nova.quota.QUOTAS.reserve')
@mock.patch('nova.quota.QUOTAS.commit')
@mock.patch('nova.objects.Quotas.check_deltas')
@mock.patch('nova.network.api.API.create')
def test_network_create(self, create_mock, commit_mock, reserve_mock):
ctxt = self.req.environ['nova.context']
reserve_mock.return_value = 'rv'
def test_network_create(self, create_mock, check_mock):
create_mock.side_effect = self._fake_network_api_create
body = copy.deepcopy(NETWORKS[0])
@ -199,35 +184,77 @@ class TenantNetworksTestV21(test.NoDBTestCase):
res = self.controller.create(self.req, body=body)
self.assertEqual(NETWORKS[0], res['network'])
commit_mock.assert_called_once_with(ctxt, 'rv')
reserve_mock.assert_called_once_with(ctxt, networks=1)
@mock.patch('nova.quota.QUOTAS.reserve')
def test_network_create_quota_error(self, reserve_mock):
@mock.patch('nova.objects.Quotas.check_deltas')
@mock.patch('nova.network.api.API.delete')
@mock.patch('nova.network.api.API.create')
def test_network_create_quota_error_during_recheck(self, create_mock,
delete_mock,
check_mock):
create_mock.side_effect = self._fake_network_api_create
ctxt = self.req.environ['nova.context']
reserve_mock.side_effect = exception.OverQuota(overs='fake')
# Simulate a race where the first check passes and the recheck fails.
check_mock.side_effect = [None, exception.OverQuota(overs='networks')]
body = copy.deepcopy(NETWORKS[0])
del body['id']
body = {'network': body}
self.assertRaises(webob.exc.HTTPForbidden,
self.controller.create, self.req, body=body)
self.assertEqual(2, check_mock.call_count)
call1 = mock.call(ctxt, {'networks': 1}, ctxt.project_id)
call2 = mock.call(ctxt, {'networks': 0}, ctxt.project_id)
check_mock.assert_has_calls([call1, call2])
# Verify we removed the network that was added after the first quota
# check passed.
delete_mock.assert_called_once_with(ctxt, NETWORKS[0]['id'])
@mock.patch('nova.objects.Quotas.check_deltas')
@mock.patch('nova.network.api.API.create')
def test_network_create_no_quota_recheck(self, create_mock, check_mock):
create_mock.side_effect = self._fake_network_api_create
ctxt = self.req.environ['nova.context']
# Disable recheck_quota.
self.flags(recheck_quota=False, group='quota')
body = copy.deepcopy(NETWORKS[0])
del body['id']
body = {'network': body}
self.controller.create(self.req, body=body)
# check_deltas should have been called only once.
check_mock.assert_called_once_with(ctxt, {'networks': 1},
ctxt.project_id)
@mock.patch('nova.objects.Quotas.check_deltas')
def test_network_create_quota_error(self, check_mock):
ctxt = self.req.environ['nova.context']
check_mock.side_effect = exception.OverQuota(overs='networks')
body = {'network': {"cidr": "10.20.105.0/24",
"label": "new net 1"}}
self.assertRaises(webob.exc.HTTPForbidden,
self.controller.create, self.req, body=body)
reserve_mock.assert_called_once_with(ctxt, networks=1)
check_mock.assert_called_once_with(ctxt, {'networks': 1},
ctxt.project_id)
@mock.patch('nova.quota.QUOTAS.reserve')
@mock.patch('nova.quota.QUOTAS.rollback')
@mock.patch('nova.objects.Quotas.check_deltas')
@mock.patch('nova.network.api.API.create')
def _test_network_create_exception(self, ex, expex, create_mock,
rollback_mock, reserve_mock):
check_mock):
ctxt = self.req.environ['nova.context']
reserve_mock.return_value = 'rv'
create_mock.side_effect = ex
body = {'network': {"cidr": "10.20.105.0/24",
"label": "new net 1"}}
if self.use_neutron:
expex = webob.exc.HTTPServiceUnavailable
self.assertRaises(expex, self.controller.create, self.req, body=body)
reserve_mock.assert_called_once_with(ctxt, networks=1)
check_mock.assert_called_once_with(ctxt, {'networks': 1},
ctxt.project_id)
def test_network_create_exception_policy_failed(self):
ex = exception.PolicyNotAuthorized(action='dummy')
@ -278,6 +305,18 @@ class TenantNeutronNetworksTestV21(TenantNetworksTestV21):
webob.exc.HTTPServiceUnavailable,
super(TenantNeutronNetworksTestV21, self).test_network_create)
def test_network_create_quota_error_during_recheck(self):
self.assertRaises(
webob.exc.HTTPServiceUnavailable,
super(TenantNeutronNetworksTestV21, self)
.test_network_create_quota_error_during_recheck)
def test_network_create_no_quota_recheck(self):
self.assertRaises(
webob.exc.HTTPServiceUnavailable,
super(TenantNeutronNetworksTestV21, self)
.test_network_create_no_quota_recheck)
def test_network_delete(self):
self.assertRaises(
webob.exc.HTTPInternalServerError,