Merge "Fix object save after refresh failure"

11 files changed, 91 insertions, 0 deletions

diff --git a/ironic/objects/node.py b/ironic/objects/node.py
index 3f6d739d4..8a690151f 100644
--- a/ironic/objects/node.py
+++ b/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.
diff --git a/ironic/objects/port.py b/ironic/objects/port.py
index a2b5686a6..a1fb0b259 100644
--- a/ironic/objects/port.py
+++ b/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
diff --git a/ironic/objects/portgroup.py b/ironic/objects/portgroup.py
index 0d2438b25..34e6f4b60 100644
--- a/ironic/objects/portgroup.py
+++ b/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()
diff --git a/ironic/objects/volume_connector.py b/ironic/objects/volume_connector.py
index b6bc026e7..f267fc110 100644
--- a/ironic/objects/volume_connector.py
+++ b/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()
diff --git a/ironic/objects/volume_target.py b/ironic/objects/volume_target.py
index 1f94b72d6..9f92f5227 100644
--- a/ironic/objects/volume_target.py
+++ b/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()
diff --git a/ironic/tests/unit/objects/test_chassis.py b/ironic/tests/unit/objects/test_chassis.py
index 8d858f9eb..199f92a63 100644
--- a/ironic/tests/unit/objects/test_chassis.py
+++ b/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:
diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py
index d6c465481..0a0dc0924 100644
--- a/ironic/tests/unit/objects/test_node.py
+++ b/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:
diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py
index ce651c813..de5adb162 100644
--- a/ironic/tests/unit/objects/test_port.py
+++ b/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:
diff --git a/ironic/tests/unit/objects/test_portgroup.py b/ironic/tests/unit/objects/test_portgroup.py
index 7337e539c..b1b65e7ab 100644
--- a/ironic/tests/unit/objects/test_portgroup.py
+++ b/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:
diff --git a/ironic/tests/unit/objects/test_volume_connector.py b/ironic/tests/unit/objects/test_volume_connector.py
index 2f5a94b72..44eaca01f 100644
--- a/ironic/tests/unit/objects/test_volume_connector.py
+++ b/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()
diff --git a/ironic/tests/unit/objects/test_volume_target.py b/ironic/tests/unit/objects/test_volume_target.py
index 9e8530a56..40f71a25e 100644
--- a/ironic/tests/unit/objects/test_volume_target.py
+++ b/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()