Fix object save after refresh failure

When we try to save() object after its refresh(), it leads to a
TypeError. Let's not keep track of changed fields in refresh(),
all fields are reloaded from database.

Closes-Bug: #1612666
Co-Authored-By: Vladyslav Drok <vdrok@mirantis.com>
Change-Id: I0f47552c424973af3704903a52f98db819b18c3f

ironic/objects/node.py

@@ -386,6 +386,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat):
         """
         current = self.get_by_uuid(self._context, self.uuid)
         self.obj_refresh(current)
+        self.obj_reset_changes()
 
     # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable
     # methods can be used in the future to replace current explicit RPC calls.

ironic/objects/port.py

@@ -290,6 +290,7 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat):
         """
         current = self.get_by_uuid(self._context, uuid=self.uuid)
         self.obj_refresh(current)
+        self.obj_reset_changes()
 
 
 @base.IronicObjectRegistry.register

ironic/objects/portgroup.py

@@ -279,3 +279,4 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat):
         """
         current = self.get_by_uuid(self._context, uuid=self.uuid)
         self.obj_refresh(current)
+        self.obj_reset_changes()

ironic/objects/volume_connector.py

@@ -237,3 +237,4 @@ class VolumeConnector(base.IronicObject,
         """
         current = self.get_by_uuid(self._context, uuid=self.uuid)
         self.obj_refresh(current)
+        self.obj_reset_changes()

ironic/objects/volume_target.py

@@ -233,3 +233,4 @@ class VolumeTarget(base.IronicObject,
         """
         current = self.get_by_uuid(self._context, uuid=self.uuid)
         self.obj_refresh(current)
+        self.obj_reset_changes()

ironic/tests/unit/objects/test_chassis.py

@@ -95,6 +95,22 @@ class TestChassisObject(base.DbTestCase):
             self.assertEqual(expected, mock_get_chassis.call_args_list)
             self.assertEqual(self.context, c._context)
 
+    # NOTE(vsaienko) current implementation of update_chassis() dbapi is
+    # differ from other object like update_port() or node_update() which
+    # allows to perform object.save() after object.refresh()
+    # This test will avoid update_chassis() regressions in future.
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        db_chassis = utils.create_test_chassis()
+        c = objects.Chassis.get_by_uuid(self.context, db_chassis.uuid)
+        c_copy = objects.Chassis.get_by_uuid(self.context, db_chassis.uuid)
+        c.description = 'b240'
+        c.save()
+        c_copy.refresh()
+        c_copy.description = 'aaff'
+        # Ensure this passes and an exception is not generated
+        c_copy.save()
+
     def test_list(self):
         with mock.patch.object(self.dbapi, 'get_chassis_list',
                                autospec=True) as mock_get_list:

ironic/tests/unit/objects/test_node.py

@@ -138,6 +138,18 @@ class TestNodeObject(base.DbTestCase):
             self.assertEqual(expected, mock_get_node.call_args_list)
             self.assertEqual(self.context, n._context)
 
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        db_node = utils.create_test_node()
+        n = objects.Node.get_by_uuid(self.context, db_node.uuid)
+        n_copy = objects.Node.get_by_uuid(self.context, db_node.uuid)
+        n.name = 'b240'
+        n.save()
+        n_copy.refresh()
+        n_copy.name = 'aaff'
+        # Ensure this passes and an exception is not generated
+        n_copy.save()
+
     def test_list(self):
         with mock.patch.object(self.dbapi, 'get_node_list',
                                autospec=True) as mock_get_list:

ironic/tests/unit/objects/test_port.py

@@ -105,6 +105,20 @@ class TestPortObject(base.DbTestCase):
             self.assertEqual(expected, mock_get_port.call_args_list)
             self.assertEqual(self.context, p._context)
 
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        address = "b2:54:00:cf:2d:40"
+        db_node = utils.create_test_node()
+        db_port = utils.create_test_port(node_id=db_node.id)
+        p = objects.Port.get_by_uuid(self.context, db_port.uuid)
+        p_copy = objects.Port.get_by_uuid(self.context, db_port.uuid)
+        p.address = address
+        p.save()
+        p_copy.refresh()
+        p_copy.address = 'aa:bb:cc:dd:ee:ff'
+        # Ensure this passes and an exception is not generated
+        p_copy.save()
+
     def test_list(self):
         with mock.patch.object(self.dbapi, 'get_port_list',
                                autospec=True) as mock_get_list:

ironic/tests/unit/objects/test_portgroup.py

@@ -114,6 +114,20 @@ class TestPortgroupObject(base.DbTestCase):
             self.assertEqual(expected, mock_get_portgroup.call_args_list)
             self.assertEqual(self.context, p._context)
 
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        address = "b2:54:00:cf:2d:40"
+        db_node = utils.create_test_node()
+        db_portgroup = utils.create_test_portgroup(node_id=db_node.id)
+        p = objects.Portgroup.get_by_uuid(self.context, db_portgroup.uuid)
+        p_copy = objects.Portgroup.get_by_uuid(self.context, db_portgroup.uuid)
+        p.address = address
+        p.save()
+        p_copy.refresh()
+        p_copy.address = 'aa:bb:cc:dd:ee:ff'
+        # Ensure this passes and an exception is not generated
+        p_copy.save()
+
     def test_list(self):
         with mock.patch.object(self.dbapi, 'get_portgroup_list',
                                autospec=True) as mock_get_list:

ironic/tests/unit/objects/test_volume_connector.py

@@ -177,3 +177,18 @@ class TestVolumeConnectorObject(base.DbTestCase):
             self.assertEqual(expected,
                              mock_get_volume_connector.call_args_list)
             self.assertEqual(self.context, c._context)
+
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        db_volume_connector = utils.create_test_volume_connector()
+
+        vc = objects.VolumeConnector.get_by_uuid(self.context,
+                                                 db_volume_connector.uuid)
+        vc_copy = objects.VolumeConnector.get_by_uuid(self.context,
+                                                      db_volume_connector.uuid)
+        vc.name = 'b240'
+        vc.save()
+        vc_copy.refresh()
+        vc_copy.name = 'aaff'
+        # Ensure this passes and an exception is not generated
+        vc_copy.save()

ironic/tests/unit/objects/test_volume_target.py

@@ -173,3 +173,18 @@ class TestVolumeTargetObject(base.DbTestCase):
             self.assertEqual(expected,
                              mock_get_volume_target.call_args_list)
             self.assertEqual(self.context, target._context)
+
+    def test_save_after_refresh(self):
+        # Ensure that it's possible to do object.save() after object.refresh()
+        db_volume_target = utils.create_test_volume_target()
+
+        vt = objects.VolumeTarget.get_by_uuid(self.context,
+                                              db_volume_target.uuid)
+        vt_copy = objects.VolumeTarget.get_by_uuid(self.context,
+                                                   db_volume_target.uuid)
+        vt.name = 'b240'
+        vt.save()
+        vt_copy.refresh()
+        vt_copy.name = 'aaff'
+        # Ensure this passes and an exception is not generated
+        vt_copy.save()