From adf917ee165e80501380d4dd7fdb076a837007d2 Mon Sep 17 00:00:00 2001 From: Yuriy Zveryanskyy Date: Wed, 1 Feb 2017 14:37:42 +0200 Subject: [PATCH] Allow duplicate execution of update node DB api method Before this patch if _do_update_node() sets "instance_uuid" field it fails on second execution one by one with exception. This prevents normal usage of DB reconnect method which proposed with change I61db83637adfd98a5394d1f570f3de4302c93497. Check of "instance_uuid" moved to manager method. Change-Id: I2ae84c95cbd1895068dc4bce01bc122abc7345da --- ironic/conductor/manager.py | 8 +++++++- ironic/db/sqlalchemy/api.py | 5 ----- ironic/tests/unit/conductor/test_manager.py | 16 ++++++++++++++++ ironic/tests/unit/db/test_nodes.py | 10 ---------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 968f728510..f4109f14a8 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -172,7 +172,13 @@ class ConductorManager(base_manager.BaseConductorManager): driver_name = node_obj.driver if 'driver' in delta else None with task_manager.acquire(context, node_id, shared=False, driver_name=driver_name, - purpose='node update'): + purpose='node update') as task: + # Prevent instance_uuid overwriting + if ('instance_uuid' in delta and node_obj.instance_uuid and + task.node.instance_uuid): + raise exception.NodeAssociated( + node=node_id, instance=task.node.instance_uuid) + node_obj.save() return node_obj diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index f56216121f..f079ae0576 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -431,11 +431,6 @@ class Connection(api.Connection): except NoResultFound: raise exception.NodeNotFound(node=node_id) - # Prevent instance_uuid overwriting - if values.get("instance_uuid") and ref.instance_uuid: - raise exception.NodeAssociated( - node=ref.uuid, instance=ref.instance_uuid) - if 'provision_state' in values: values['provision_updated_at'] = timeutils.utcnow() if values['provision_state'] == states.INSPECTING: diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 7998721504..59d174c248 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -513,6 +513,22 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, res = objects.Node.get_by_uuid(self.context, node['uuid']) self.assertEqual({'test': 'one'}, res['extra']) + def test_update_node_already_associated(self): + old_instance = uuidutils.generate_uuid() + node = obj_utils.create_test_node(self.context, driver='fake', + instance_uuid=old_instance) + node.instance_uuid = uuidutils.generate_uuid() + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_node, + self.context, + node) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeAssociated, exc.exc_info[0]) + + # verify change did not happen + res = objects.Node.get_by_uuid(self.context, node['uuid']) + self.assertEqual(old_instance, res['instance_uuid']) + @mock.patch('ironic.drivers.modules.fake.FakePower.get_power_state') def _test_associate_node(self, power_state, mock_get_power_state): mock_get_power_state.return_value = power_state diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index 111ad34e74..2c52dbeaa5 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -429,16 +429,6 @@ class DbNodeTestCase(base.DbTestCase): res = self.dbapi.update_node(node.id, {'instance_uuid': None}) self.assertIsNone(res.instance_uuid) - def test_update_node_already_associated(self): - node = utils.create_test_node() - new_i_uuid_one = uuidutils.generate_uuid() - self.dbapi.update_node(node.id, {'instance_uuid': new_i_uuid_one}) - new_i_uuid_two = uuidutils.generate_uuid() - self.assertRaises(exception.NodeAssociated, - self.dbapi.update_node, - node.id, - {'instance_uuid': new_i_uuid_two}) - def test_update_node_instance_already_associated(self): node1 = utils.create_test_node(uuid=uuidutils.generate_uuid()) new_i_uuid = uuidutils.generate_uuid()