From 8771277b0fd0dbbbc2331fce6fa4b4c45734924f Mon Sep 17 00:00:00 2001
From: TommyLike <tommylikehu@gmail.com>
Date: Tue, 28 Mar 2017 16:50:35 +0800
Subject: [PATCH] [BugFix]Cinder forgets to update 'deleted_at' when deleting

Usually we would update the 'deleted_at' column when
soft-deleting records, otherwise the 'db purge' command
would fail because it depends on the 'deleted_at' column.

Closes-Bug: #1671354

Change-Id: Ib302488d3a007df09a2e7ece40488c00bf732119
(cherry picked from commit f9012466db95a4e693291a7b5af21842a85843c2)
(cherry picked from commit 1695e7c0cb49008032762c7116484b60a9cc9e82)
---
 cinder/db/sqlalchemy/api.py      |  7 ++++--
 cinder/tests/unit/test_db_api.py | 41 ++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 15eeb31bb82..12854092f2e 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2354,7 +2354,9 @@ def _volume_x_metadata_update(context, volume_id, metadata, delete, model,
             # We don't want to delete keys we are going to update
             if metadata:
                 expected_values['key'] = db.Not(metadata.keys())
-            conditional_update(context, model, {'deleted': True},
+            conditional_update(context, model,
+                               {'deleted': True,
+                                'deleted_at': timeutils.utcnow()},
                                expected_values)
 
         # Get existing metadata
@@ -2871,7 +2873,8 @@ def snapshot_metadata_update(context, snapshot_id, metadata, delete):
                     meta_ref = _snapshot_metadata_get_item(context,
                                                            snapshot_id,
                                                            meta_key, session)
-                    meta_ref.update({'deleted': True})
+                    meta_ref.update({'deleted': True,
+                                     'deleted_at': timeutils.utcnow()})
                     meta_ref.save(session=session)
 
         meta_ref = None
diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index 592fbcb8784..468c46177c7 100644
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -1066,6 +1066,27 @@ class DBAPIVolumeTestCase(BaseTest):
                           False,
                           FAKE_METADATA_TYPE.fake_type)
 
+    @ddt.data(common.METADATA_TYPES.user, common.METADATA_TYPES.image)
+    @mock.patch.object(timeutils, 'utcnow')
+    @mock.patch.object(sqlalchemy_api, 'resource_exists')
+    @mock.patch.object(sqlalchemy_api, 'conditional_update')
+    @mock.patch.object(sqlalchemy_api, '_volume_x_metadata_get_query')
+    def test_volume_metadata_delete_deleted_at_updated(self,
+                                                       meta_type,
+                                                       mock_query,
+                                                       mock_update,
+                                                       mock_resource,
+                                                       mock_utc):
+        mock_query.all.return_value = {}
+        mock_utc.return_value = 'fake_time'
+
+        db.volume_metadata_update(self.ctxt, 1, {}, True, meta_type=meta_type)
+
+        mock_update.assert_called_once_with(mock.ANY, mock.ANY,
+                                            {'deleted': True,
+                                             'deleted_at': 'fake_time'},
+                                            mock.ANY)
+
     def test_volume_metadata_update_delete(self):
         metadata1 = {'a': '1', 'c': '2'}
         metadata2 = {'a': '3', 'd': '4'}
@@ -1497,6 +1518,26 @@ class DBAPISnapshotTestCase(BaseTest):
 
         self.assertEqual(should_be, db_meta)
 
+    @mock.patch.object(timeutils, 'utcnow')
+    @mock.patch.object(sqlalchemy_api, 'resource_exists')
+    @mock.patch.object(sqlalchemy_api, '_snapshot_metadata_get')
+    @mock.patch.object(sqlalchemy_api, '_snapshot_metadata_get_item')
+    def test_snapshot_metadata_delete_deleted_at_updated(self,
+                                                         mock_metadata_item,
+                                                         mock_metadata,
+                                                         mock_resource,
+                                                         mock_utc):
+        fake_metadata = {'fake_key1': 'fake_value1'}
+        mock_item = mock.Mock()
+        mock_metadata.return_value = fake_metadata
+        mock_utc.return_value = 'fake_time'
+        mock_metadata_item.side_effect = [mock_item]
+
+        db.snapshot_metadata_update(self.ctxt, 1, {}, True)
+
+        mock_item.update.assert_called_once_with({'deleted': True,
+                                                  'deleted_at': 'fake_time'})
+
     def test_snapshot_metadata_delete(self):
         metadata = {'a': '1', 'c': '2'}
         should_be = {'a': '1'}