openstack/ironic
Code Issues Proposed changes

Allocation API: minor fixes to DB and RPC

Browse Source 
This is a follow-up to commit 36b047ff5e1b0c1cf64d4b58bf8a9ce3bcc87e85:
* Do the join on the database level when filtering allocations by node UUID
* Add a check that we do not touch instance_info in destroy_allocation to
  document it as an explicit decision.
* Fix a few grammar nits and inprecise comments.

Change-Id: I1fa7815ae3b6a0190bdb3bf0257ae79fb6a36671
Story: #2004341
This commit is contained in:
Dmitry Tantsur 2019-01-03 15:01:04 +01:00
parent a4717d9958
commit fb93d4bc3c
3 changed files with 31 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
ironic/db/sqlalchemy/api.py
View File

@ -182,6 +182,15 @@ def add_node_filter_by_chassis(query, value):
return query.filter(models.Chassis.uuid == value)
def add_allocation_filter_by_node(query, value):
if strutils.is_int_like(value):
return query.filter_by(node_id=value)
else:
query = query.join(models.Node,
models.Allocation.node_id == models.Node.id)
return query.filter(models.Node.uuid == value)
def _paginate_query(model, limit=None, marker=None, sort_key=None,
sort_dir=None, query=None):
if not query:
@ -289,8 +298,7 @@ class Connection(api.Connection):
except KeyError:
pass
else:
node_obj = self.get_node_by_uuid(node_uuid)
filters['node_id'] = node_obj.id
query = add_allocation_filter_by_node(query, node_uuid)
if filters:
query = query.filter_by(**filters)
@ -1656,8 +1664,8 @@ class Connection(api.Connection):
raise exception.AllocationDuplicateName(
name=values['name'])
elif 'instance_uuid' in exc.columns:
# Case when the referenced node is associated with an
# instance already.
# Case when the allocation UUID is already used on some
# node as instance_uuid.
raise exception.InstanceAssociated(
instance_uuid=instance_uuid, node=node_uuid)
else:

28
ironic/objects/allocation.py
View File

@ -64,11 +64,11 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
# @object_base.remotable_classmethod
@classmethod
def get(cls, context, allocation_ident):
"""Find a allocation based on its id, uuid, name or address.
"""Find an allocation by its ID, UUID or name.
:param allocation_ident: The id, uuid, name or address of a allocation.
:param allocation_ident: The ID, UUID or name of an allocation.
:param context: Security context
:returns: A :class:`Allocation` object.
:returns: An :class:`Allocation` object.
:raises: InvalidIdentity
"""
@ -87,12 +87,12 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
# @object_base.remotable_classmethod
@classmethod
def get_by_id(cls, context, allocation_id):
"""Find a allocation by its integer ID and return a Allocation object.
"""Find an allocation by its integer ID.
:param cls: the :class:`Allocation`
:param context: Security context
:param allocation_id: The ID of a allocation.
:returns: A :class:`Allocation` object.
:param allocation_id: The ID of an allocation.
:returns: An :class:`Allocation` object.
:raises: AllocationNotFound
"""
@ -106,12 +106,12 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
# @object_base.remotable_classmethod
@classmethod
def get_by_uuid(cls, context, uuid):
"""Find a allocation by UUID and return a :class:`Allocation` object.
"""Find an allocation by its UUID.
:param cls: the :class:`Allocation`
:param context: Security context
:param uuid: The UUID of a allocation.
:returns: A :class:`Allocation` object.
:param uuid: The UUID of an allocation.
:returns: An :class:`Allocation` object.
:raises: AllocationNotFound
"""
@ -125,12 +125,12 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
# @object_base.remotable_classmethod
@classmethod
def get_by_name(cls, context, name):
"""Find allocation based on name and return a :class:`Allocation` object.
"""Find an allocation based by its name.
:param cls: the :class:`Allocation`
:param context: Security context
:param name: The name of a allocation.
:returns: A :class:`Allocation` object.
:param name: The name of an allocation.
:returns: An :class:`Allocation` object.
:raises: AllocationNotFound
"""
@ -234,7 +234,7 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
def refresh(self, context=None):
"""Loads updates for this Allocation.
Loads a allocation with the same uuid from the database and
Loads an allocation with the same uuid from the database and
checks for updated attributes. Updates are applied from
the loaded allocation column by column, if there are any updates.
@ -254,7 +254,7 @@ class Allocation(base.IronicObject, object_base.VersionedObjectDictCompat):
@base.IronicObjectRegistry.register
class AllocationCRUDNotification(notification.NotificationBase):
"""Notification when ironic creates, updates or deletes a allocation."""
"""Notification when ironic creates, updates or deletes an allocation."""
# Version 1.0: Initial version
VERSION = '1.0'

6
ironic/tests/unit/db/test_allocations.py
View File

@ -125,13 +125,17 @@ class AllocationsTestCase(base.DbTestCase):
def test_destroy_allocation_with_node(self):
self.dbapi.update_node(self.node.id,
{'allocation_id': self.allocation.id,
'instance_uuid': uuidutils.generate_uuid()})
'instance_uuid': uuidutils.generate_uuid(),
'instance_info': {'traits': ['foo']}})
self.dbapi.destroy_allocation(self.allocation.id)
self.assertRaises(exception.AllocationNotFound,
self.dbapi.get_allocation_by_id, self.allocation.id)
node = self.dbapi.get_node_by_id(self.node.id)
self.assertIsNone(node.allocation_id)
self.assertIsNone(node.instance_uuid)
# NOTE(dtantsur): currently we do not clean up instance_info contents
# on deallocation. It may be changed in the future.
self.assertEqual(node.instance_info, {'traits': ['foo']})
def test_destroy_allocation_that_does_not_exist(self):
self.assertRaises(exception.AllocationNotFound,