Avoid a race when associating instance_uuid

Current code can potentially overwrite the instance_uuid without
exception when multiple dbapi.update_node calls are issued. This
patch adds a check to dbapi so that the NodeAssociated exception
will be raised in this case.
The Nova Ironic driver will handle this exception. This will be
introduced in a separate patch.

Change-Id: I28ef5ce55399bde7073ffc111e9bc4400e5577ee
Partial-Bug: #1244541
This commit is contained in:
Haomeng, Wang 2013-12-14 19:48:36 +08:00
parent 76272077ac
commit 3f858cc674
3 changed files with 35 additions and 4 deletions

View File

@ -183,6 +183,8 @@ class Connection(object):
}
}
:returns: A node.
:raises: NodeAssociated
:raises: NodeNotFound
"""
@abc.abstractmethod

View File

@ -361,11 +361,16 @@ class Connection(api.Connection):
with session.begin():
query = model_query(models.Node, session=session)
query = add_identity_filter(query, node_id)
count = query.update(values, synchronize_session='fetch')
if count != 1:
try:
ref = query.with_lockmode('update').one()
except NoResultFound:
raise exception.NodeNotFound(node=node_id)
ref = query.one()
# Prevent instance_uuid overwriting
if values.get("instance_uuid") and ref.instance_uuid:
raise exception.NodeAssociated(node=node_id,
instance=values['instance_uuid'])
ref.update(values)
return ref
@objects.objectify(objects.Port)

View File

@ -216,6 +216,30 @@ class DbNodeTestCase(base.DbTestCase):
res = self.dbapi.update_node(n['id'], {'extra': new_extra})
self.assertEqual(new_extra, res['extra'])
def test_update_node_not_found(self):
node_uuid = ironic_utils.generate_uuid()
new_extra = {'foo': 'bar'}
self.assertRaises(exception.NodeNotFound, self.dbapi.update_node,
node_uuid, {'extra': new_extra})
def test_update_node_associate_and_disassociate(self):
n = self._create_test_node()
new_i_uuid = ironic_utils.generate_uuid()
res = self.dbapi.update_node(n['id'], {'instance_uuid': new_i_uuid})
self.assertEqual(new_i_uuid, res['instance_uuid'])
res = self.dbapi.update_node(n['id'], {'instance_uuid': None})
self.assertIsNone(res['instance_uuid'])
def test_update_node_already_assosicated(self):
n = self._create_test_node()
new_i_uuid_one = ironic_utils.generate_uuid()
self.dbapi.update_node(n['id'], {'instance_uuid': new_i_uuid_one})
new_i_uuid_two = ironic_utils.generate_uuid()
self.assertRaises(exception.NodeAssociated,
self.dbapi.update_node,
n['id'],
{'instance_uuid': new_i_uuid_two})
def test_reserve_one_node(self):
n = self._create_test_node()
uuid = n['uuid']