Merge "Allow duplicate execution of update node DB api method"

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

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:

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

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()