openstack
/
nova
Code Issues Proposed changes

Make floating_ip_bulk_destroy deallocate quota if not auto_assigned 

The expected use case for floating_ip_bulk_destroy is to destroy
auto_assigned floating IPs, for which no quota is used so no quota
needs to be recovered.  But it can also be used to destroy other
(not auto_assigned) floating IPs, in which case the previously used
quota was not recovered.  This fixes the quota leak in that unusual
case.

Closes-Bug: #1254174

Change-Id: Ibff80d9ebac8d4422e401909033da12d9ec0b593
This commit is contained in:
David Ripton 2014-01-05 23:15:14 -05:00
parent 80efcae3a3
commit 384cce84fd
2 changed files with 52 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
nova/db/sqlalchemy/api.py
View File

@ -63,6 +63,7 @@ from nova.openstack.common.gettextutils import _
from nova.openstack.common import log as logging
from nova.openstack.common import timeutils
from nova.openstack.common import uuidutils
from nova import quota
db_opts = [
cfg.StrOpt('osapi_compute_unique_server_name_scope',
@ -855,10 +856,35 @@ def _ip_range_splitter(ips, block_size=256):
def floating_ip_bulk_destroy(context, ips):
session = get_session()
with session.begin():
project_id_to_quota_count = collections.defaultdict(int)
for ip_block in _ip_range_splitter(ips):
# Find any floating IPs that were not auto_assigned and
# thus need quota released.
query = model_query(context, models.FloatingIp).\
filter(models.FloatingIp.address.in_(ip_block)).\
filter_by(auto_assigned=False)
rows = query.all()
for row in rows:
# The count is negative since we release quota by
# reserving negative quota.
project_id_to_quota_count[row['project_id']] -= 1
# Delete the floating IPs.
model_query(context, models.FloatingIp).\
filter(models.FloatingIp.address.in_(ip_block)).\
soft_delete(synchronize_session='fetch')
# Delete the quotas, if needed.
for project_id, count in project_id_to_quota_count.iteritems():
try:
reservations = quota.QUOTAS.reserve(context,
project_id=project_id,
floating_ips=count)
quota.QUOTAS.commit(context,
reservations,
project_id=project_id)
except Exception:
LOG.exception(_("Failed to update usages bulk "
"deallocating floating IP"))
raise
@require_context

30
nova/tests/db/test_db_api.py
View File

@ -3730,22 +3730,44 @@ class FloatingIpTestCase(test.TestCase, ModelsObjectComparatorMixin):
ips_for_delete = []
ips_for_non_delete = []
def create_ips(i):
return [{'address': '1.1.%s.%s' % (i, k)} for k in range(1, 256)]
def create_ips(i, j):
return [{'address': '1.1.%s.%s' % (i, k)} for k in range(1, j + 1)]
# NOTE(boris-42): Create more then 256 ip to check that
# _ip_range_splitter works properly.
for i in range(1, 3):
ips_for_delete.extend(create_ips(i))
ips_for_non_delete.extend(create_ips(3))
ips_for_delete.extend(create_ips(i, 255))
ips_for_non_delete.extend(create_ips(3, 255))
db.floating_ip_bulk_create(self.ctxt,
ips_for_delete + ips_for_non_delete)
non_bulk_ips_for_delete = create_ips(4, 3)
non_bulk_ips_for_non_delete = create_ips(5, 3)
non_bulk_ips = non_bulk_ips_for_delete + non_bulk_ips_for_non_delete
project_id = 'fake_project'
reservations = quota.QUOTAS.reserve(self.ctxt,
floating_ips=len(non_bulk_ips),
project_id=project_id)
for dct in non_bulk_ips:
self._create_floating_ip(dct)
quota.QUOTAS.commit(self.ctxt, reservations, project_id=project_id)
self.assertEqual(db.quota_usage_get_all_by_project(
self.ctxt, project_id),
{'project_id': project_id,
'floating_ips': {'in_use': 6, 'reserved': 0}})
ips_for_delete.extend(non_bulk_ips_for_delete)
ips_for_non_delete.extend(non_bulk_ips_for_non_delete)
db.floating_ip_bulk_destroy(self.ctxt, ips_for_delete)
expected_addresses = map(lambda x: x['address'], ips_for_non_delete)
self._assertEqualListsOfPrimitivesAsSets(self._get_existing_ips(),
expected_addresses)
self.assertEqual(db.quota_usage_get_all_by_project(
self.ctxt, project_id),
{'project_id': project_id,
'floating_ips': {'in_use': 3, 'reserved': 0}})
def test_floating_ip_create(self):
floating_ip = self._create_floating_ip({})