Ignore stale image property removal

Occasionally we fail to delete a property from an image and get a
StaleDataError from SQLAlchemy. It looks like maybe we are just racing
with another operation to delete a single property (i.e. a race
between image delete and the end of an import cleanup).

This ignores that "can't delete because it is already deleted" error
and logs to debug about the situation for later auditing and
confirmation.

Change-Id: Ida5bc659b7481fa0a7f3926a54520628a7a4c406
Closes-Bug: #1895173

glance/db/sqlalchemy/api.py

@@ -1182,12 +1182,20 @@ def _image_property_update(context, prop_ref, values, session=None):
 
 def image_property_delete(context, prop_ref, image_ref, session=None):
     """
-    Used internally by image_property_create and image_property_update.
+    Used internally by _set_properties_for_image().
     """
     session = session or get_session()
     prop = session.query(models.ImageProperty).filter_by(image_id=image_ref,
                                                          name=prop_ref).one()
-    prop.delete(session=session)
+    try:
+        prop.delete(session=session)
+    except sa_orm.exc.StaleDataError as e:
+        LOG.debug(('StaleDataError while deleting property %(prop)r '
+                   'from image %(image)r likely means we raced during delete: '
+                   '%(err)s'),
+                  {'prop': prop_ref, 'image': image_ref,
+                   'err': str(e)})
+        return
     return prop
 
 

glance/tests/unit/test_db.py

@@ -22,6 +22,7 @@ from oslo_config import cfg
 from oslo_db import exception as db_exc
 from oslo_utils import encodeutils
 from oslo_utils import timeutils
+from sqlalchemy import orm as sa_orm
 
 from glance.common import crypt
 from glance.common import exception
@@ -938,3 +939,39 @@ class RetryOnDeadlockTestCase(test_utils.BaseTestCase):
                 api.image_destroy(None, 'fake-id')
             except TestException:
                 self.assertEqual(3, sess.call_count)
+
+
+class TestImageDeleteRace(test_utils.BaseTestCase):
+
+    @mock.patch.object(api, 'LOG')
+    def test_image_property_delete_stale_data(self, mock_LOG):
+        mock_context = mock.MagicMock()
+        mock_session = mock.MagicMock()
+        mock_result = (mock_session.query.return_value.
+                       filter_by.return_value.
+                       one.return_value)
+        mock_result.delete.side_effect = sa_orm.exc.StaleDataError('myerror')
+        # StaleDataError should not be raised
+        r = api.image_property_delete(mock_context, 'myprop', 'myimage',
+                                      session=mock_session)
+        # We should not get the property back
+        self.assertIsNone(r)
+        # Make sure we logged it
+        mock_LOG.debug.assert_called_once_with(
+            'StaleDataError while deleting property %(prop)r '
+            'from image %(image)r likely means we raced during delete: '
+            '%(err)s', {'prop': 'myprop', 'image': 'myimage',
+                        'err': 'myerror'})
+
+    def test_image_property_delete_exception(self):
+        mock_context = mock.MagicMock()
+        mock_session = mock.MagicMock()
+        mock_result = (mock_session.query.return_value.
+                       filter_by.return_value.
+                       one.return_value)
+        mock_result.delete.side_effect = RuntimeError
+        # Any other exception should be raised
+        self.assertRaises(RuntimeError,
+                          api.image_property_delete,
+                          mock_context, 'myprop', 'myimage',
+                          session=mock_session)