diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 22cbd1d7d2..862212a5b9 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -174,7 +174,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 3d134f5bff..8cf3f83d89 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 35cc7e7f89..0ababf85db 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -514,6 +514,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()