Dell SC: cgsnapshot-delete doesn't actually delete

dell_storagecenter_api.py was looking for the cgsnapshot for
cgsnapshot-delete via the wrong REST API call.  It was also only
looking for the first volume's snapshot rather than all snapshots
associated with a cgsnapshot.  It now locates the snapshots properly
and expires the associated replays on cgsnapshot-delete.

Tests updated appropriately.

Change-Id: I1e635db8b06d3d7a78573e6e8a2a4c25b7fb458d
Closes-Bug: #1500583
(cherry picked from commit e529e4d6cc71003c53126612dd869fa6fb23493a)

diff --git a/cinder/tests/unit/test_dellscapi.py b/cinder/tests/unit/test_dellscapi.py
index 2847bbe0fa80c78120bb659a5e6e0a2cc72b0a39..41f0f5fc86d0cc37944b4b40ee35dcdeeb9ed115 100644 (file)
--- a/cinder/tests/unit/test_dellscapi.py
+++ b/cinder/tests/unit/test_dellscapi.py
@@ -1452,6 +1452,42 @@ class DellSCSanAPITestCase(test.TestCase):
          u'userCreated': False,
          u'volumeCount': 0}]
 
+    CGS = [{u'profile':
+            {u'instanceId': u'65690.4',
+             u'instanceName': u'0869559e-6881-454e-ba18-15c6726d33c1',
+             u'objectType': u'ScReplayProfile'},
+            u'scSerialNumber': 65690,
+            u'globalIndex': u'65690-4-2',
+            u'description': u'GUID1-0869559e-6881-454e-ba18-15c6726d33c1',
+            u'instanceId': u'65690.65690.4.2',
+            u'scName': u'Storage Center 65690',
+            u'expires': False,
+            u'freezeTime': u'2015-09-28T14:00:59-05:00',
+            u'expireTime': u'1969-12-31T18:00:00-06:00',
+            u'expectedReplayCount': 2,
+            u'writesHeldDuration': 19809,
+            u'replayCount': 2,
+            u'instanceName': u'Name1',
+            u'objectType': u'ScReplayConsistencyGroup'},
+           {u'profile':
+            {u'instanceId': u'65690.4',
+             u'instanceName': u'0869559e-6881-454e-ba18-15c6726d33c1',
+             u'objectType': u'ScReplayProfile'},
+            u'scSerialNumber': 65690,
+            u'globalIndex': u'65690-4-3',
+            u'description': u'GUID2-0869559e-6881-454e-ba18-15c6726d33c1',
+            u'instanceId': u'65690.65690.4.3',
+            u'scName': u'Storage Center 65690',
+            u'expires': False,
+            u'freezeTime': u'2015-09-28T14:00:59-05:00',
+            u'expireTime': u'1969-12-31T18:00:00-06:00',
+            u'expectedReplayCount': 2,
+            u'writesHeldDuration': 19809,
+            u'replayCount': 2,
+            u'instanceName': u'Name2',
+            u'objectType': u'ScReplayConsistencyGroup'}
+           ]
+
     ISCSI_CONFIG = {
         u'initialReadyToTransfer': True,
         u'scSerialNumber': 64065,
@@ -5147,119 +5183,171 @@ class DellSCSanAPITestCase(test.TestCase):
         self.assertTrue(mock_init_cg_volumes.called)
         self.assertFalse(res)
 
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       '_get_json',
+                       return_value=CGS)
+    @mock.patch.object(dell_storagecenter_api.HttpClient,
+                       'get',
+                       return_value=RESPONSE_200)
+    def test_find_sc_cg(self,
+                        mock_get,
+                        mock_get_json,
+                        mock_close_connection,
+                        mock_open_connection,
+                        mock_init):
+        res = self.scapi._find_sc_cg(
+            {},
+            'GUID1-0869559e-6881-454e-ba18-15c6726d33c1')
+        self.assertEqual(self.CGS[0], res)
+
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       '_get_json',
+                       return_value=CGS)
+    @mock.patch.object(dell_storagecenter_api.HttpClient,
+                       'get',
+                       return_value=RESPONSE_200)
+    def test_find_sc_cg_not_found(self,
+                                  mock_get,
+                                  mock_get_json,
+                                  mock_close_connection,
+                                  mock_open_connection,
+                                  mock_init):
+        res = self.scapi._find_sc_cg(
+            {},
+            'GUID3-0869559e-6881-454e-ba18-15c6726d33c1')
+        self.assertIsNone(res)
+
+    @mock.patch.object(dell_storagecenter_api.HttpClient,
+                       'get',
+                       return_value=RESPONSE_400)
+    def test_find_sc_cg_fail(self,
+                             mock_get,
+                             mock_close_connection,
+                             mock_open_connection,
+                             mock_init):
+        res = self.scapi._find_sc_cg(
+            {},
+            'GUID1-0869559e-6881-454e-ba18-15c6726d33c1')
+        self.assertIsNone(res)
+
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       '_find_sc_cg',
+                       return_value={'instanceId': 101})
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        '_get_json',
                        return_value=RPLAYS)
     @mock.patch.object(dell_storagecenter_api.HttpClient,
                        'get')
-    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       '_get_id',
-                       return_value='100')
-    def test_find_cg_replay(self,
-                            mock_get_id,
-                            mock_get,
-                            mock_get_json,
-                            mock_close_connection,
-                            mock_open_connection,
-                            mock_init):
+    def test_find_cg_replays(self,
+                             mock_get,
+                             mock_get_json,
+                             mock_find_sc_cg,
+                             mock_close_connection,
+                             mock_open_connection,
+                             mock_init):
         profile = {'instanceId': '100'}
         replayid = 'Cinder Test Replay012345678910'
-        res = self.scapi.find_cg_replay(profile, replayid)
-        expected_url = 'StorageCenter/ScReplayProfile/100/ReplayList'
+        res = self.scapi._find_cg_replays(profile, replayid)
+        expected_url = 'StorageCenter/ScReplayConsistencyGroup/101/ReplayList'
         mock_get.assert_called_once_with(expected_url)
-        self.assertTrue(mock_get_id.called)
+        self.assertTrue(mock_find_sc_cg.called)
         self.assertTrue(mock_get_json.called)
-        # We should fine RPLAYS[1]
-        self.assertEqual(self.RPLAYS[1], res, 'Wrong replay returned')
+        # We should fine RPLAYS
+        self.assertEqual(self.RPLAYS, res)
 
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       '_find_sc_cg',
+                       return_value=None)
+    def test_find_cg_replays_no_cg(self,
+                                   mock_find_sc_cg,
+                                   mock_close_connection,
+                                   mock_open_connection,
+                                   mock_init):
+        profile = {'instanceId': '100'}
+        replayid = 'Cinder Test Replay012345678910'
+        res = self.scapi._find_cg_replays(profile, replayid)
+        self.assertTrue(mock_find_sc_cg.called)
+        # We should return an empty list.
+        self.assertEqual([], res)
+
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       '_find_sc_cg',
+                       return_value={'instanceId': 101})
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        '_get_json',
                        return_value=None)
     @mock.patch.object(dell_storagecenter_api.HttpClient,
                        'get')
-    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       '_get_id',
-                       return_value='100')
-    def test_find_cg_replay_bad_json(self,
-                                     mock_get_id,
-                                     mock_get,
-                                     mock_get_json,
-                                     mock_close_connection,
-                                     mock_open_connection,
-                                     mock_init):
+    def test_find_cg_replays_bad_json(self,
+                                      mock_get,
+                                      mock_get_json,
+                                      mock_find_sc_cg,
+                                      mock_close_connection,
+                                      mock_open_connection,
+                                      mock_init):
         profile = {'instanceId': '100'}
         replayid = 'Cinder Test Replay012345678910'
-        res = self.scapi.find_cg_replay(profile, replayid)
-        expected_url = 'StorageCenter/ScReplayProfile/100/ReplayList'
+        res = self.scapi._find_cg_replays(profile, replayid)
+        expected_url = 'StorageCenter/ScReplayConsistencyGroup/101/ReplayList'
         mock_get.assert_called_once_with(expected_url)
-        self.assertTrue(mock_get_id.called)
+        self.assertTrue(mock_find_sc_cg.called)
         self.assertTrue(mock_get_json.called)
         self.assertIsNone(res)
 
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       'find_replay',
-                       return_value=RPLAY)
+                       '_find_cg_replays',
+                       return_value=RPLAYS)
     @mock.patch.object(dell_storagecenter_api.HttpClient,
                        'post',
                        return_value=RESPONSE_204)
-    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       '_get_id',
-                       return_value='100')
     def test_delete_cg_replay(self,
-                              mock_get_id,
                               mock_post,
-                              mock_find_replay,
+                              mock_find_cg_replays,
                               mock_close_connection,
                               mock_open_connection,
                               mock_init):
-        profile = {'instanceId': '100'}
-        replayid = 'guid'
-        expected_url = 'StorageCenter/ScReplay/100/Expire'
-        expected_payload = {}
-        res = self.scapi.delete_cg_replay(profile, replayid)
-        mock_post.assert_called_once_with(expected_url, expected_payload)
-        self.assertTrue(mock_find_replay.called)
-        self.assertTrue(mock_get_id.called)
+        res = self.scapi.delete_cg_replay({}, '')
+        expected_url = ('StorageCenter/ScReplay/' +
+                        self.RPLAYS[0]['instanceId'] +
+                        '/Expire')
+        mock_post.assert_any_call(expected_url, {})
+        expected_url = ('StorageCenter/ScReplay/' +
+                        self.RPLAYS[1]['instanceId'] +
+                        '/Expire')
+        mock_post.assert_any_call(expected_url, {})
+        self.assertTrue(mock_find_cg_replays.called)
         self.assertTrue(res)
 
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       'find_replay',
-                       return_value=RPLAY)
+                       '_find_cg_replays',
+                       return_value=RPLAYS)
     @mock.patch.object(dell_storagecenter_api.HttpClient,
                        'post',
                        return_value=RESPONSE_400)
-    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       '_get_id',
-                       return_value='100')
     def test_delete_cg_replay_error(self,
-                                    mock_get_id,
                                     mock_post,
-                                    mock_find_replay,
+                                    mock_find_cg_replays,
                                     mock_close_connection,
                                     mock_open_connection,
                                     mock_init):
-        profile = {'instanceId': '100'}
-        replayid = 'guid'
-        expected_url = 'StorageCenter/ScReplay/100/Expire'
-        expected_payload = {}
-        res = self.scapi.delete_cg_replay(profile, replayid)
-        mock_post.assert_called_once_with(expected_url, expected_payload)
-        self.assertTrue(mock_get_id.called)
-        self.assertTrue(mock_find_replay.called)
+        expected_url = ('StorageCenter/ScReplay/' +
+                        self.RPLAYS[0]['instanceId'] +
+                        '/Expire')
+        res = self.scapi.delete_cg_replay({}, '')
+        mock_post.assert_called_once_with(expected_url, {})
+        self.assertTrue(mock_find_cg_replays.called)
         self.assertFalse(res)
 
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
-                       'find_replay',
-                       return_value=None)
+                       '_find_cg_replays',
+                       return_value=[])
     def test_delete_cg_replay_cant_find(self,
-                                        mock_find_replay,
+                                        mock_find_cg_replays,
                                         mock_close_connection,
                                         mock_open_connection,
                                         mock_init):
-        profile = {'instanceId': '100'}
-        replayid = 'guid'
-        res = self.scapi.delete_cg_replay(profile, replayid)
-        self.assertTrue(mock_find_replay.called)
+        res = self.scapi.delete_cg_replay({}, '')
+        self.assertTrue(mock_find_cg_replays.called)
         self.assertTrue(res)
 
     def test_size_to_gb(self,

diff --git a/cinder/volume/drivers/dell/dell_storagecenter_api.py b/cinder/volume/drivers/dell/dell_storagecenter_api.py
index dbd47d38af4357cfcf459e119b95d7949277fd3f..afb28a3d76ad589a1959aa0b7a7370fa5ec56511 100644 (file)
--- a/cinder/volume/drivers/dell/dell_storagecenter_api.py
+++ b/cinder/volume/drivers/dell/dell_storagecenter_api.py
@@ -2038,43 +2038,57 @@ class StorageCenterApi(object):
                        'reason': r.reason})
         return False
 
-    def find_cg_replay(self, profile, replayid):
-        """Searches for the replay by replayid.
+    def _find_sc_cg(self, profile, replayid):
+        """Finds the sc consistency group that matches replayid
+
+        :param profile: Dell profile object.
+        :param replayid: Name to search for.  This is a portion of the
+                         snapshot ID as we do not have space for the entire
+                         GUID in the replay description.
+        :return: Consistency group object or None.
+        """
+        self.cg_except_on_no_support()
+        r = self.client.get(
+            'StorageCenter/ScReplayProfile/%s/ConsistencyGroupList'
+            % self._get_id(profile))
+        if r.status_code == 200:
+            cglist = self._get_json(r)
+            if cglist and isinstance(cglist, list):
+                for cg in cglist:
+                    desc = cg.get('description')
+                    if (len(desc) >= 30 and
+                            replayid.startswith(desc) is True):
+                        # We found our cg so return it.
+                        return cg
+        return None
+
+    def _find_cg_replays(self, profile, replayid):
+        """Searches for the replays that match replayid for a given profile.
 
         replayid is stored in the replay's description attribute.
 
-        :param scvolume: Dell volume object.
+        :param profile: Dell profile object.
         :param replayid: Name to search for.  This is a portion of the
                          snapshot ID as we do not have space for the entire
                          GUID in the replay description.
-        :returns: Dell replay object or None.
+        :returns: Dell replay object array.
         """
         self.cg_except_on_no_support()
-        r = self.client.get('StorageCenter/ScReplayProfile/%s/ReplayList'
-                            % self._get_id(profile))
-        replays = self._get_json(r)
-        # This will be a list.  If it isn't bail
-        if isinstance(replays, list):
-            for replay in replays:
-                # The only place to save our information with the public
-                # api is the description field which isn't quite long
-                # enough.  So we check that our description is pretty much
-                # the max length and we compare that to the start of
-                # the snapshot id.
-                description = replay.get('description')
-                if (len(description) >= 30 and
-                        replayid.startswith(description) is True and
-                        replay.get('markedForExpiration') is not True):
-                    # We found our replay so return it.
-                    return replay
-        # If we are here then we didn't find the replay so warn and leave.
-        LOG.warning(_LW('Unable to find snapshot %s'),
-                    replayid)
+        replays = []
+        sccg = self._find_sc_cg(profile, replayid)
+        if sccg:
+            r = self.client.get(
+                'StorageCenter/ScReplayConsistencyGroup/%s/ReplayList'
+                % self._get_id(sccg))
 
-        return None
+            replays = self._get_json(r)
+        else:
+            LOG.error(_LE('Unable to locate snapshot %s'), replayid)
+
+        return replays
 
     def delete_cg_replay(self, profile, replayid):
-        """Finds a Dell replay by replayid string and expires it.
+        """Finds a Dell cg replay by replayid string and expires it.
 
         Once marked for expiration we do not return the replay as a snapshot
         even though it might still exist.  (Backend requirements.)
@@ -2087,11 +2101,13 @@ class StorageCenterApi(object):
         """
         self.cg_except_on_no_support()
         LOG.debug('Expiring consistency group replay %s', replayid)
-        replay = self.find_replay(profile,
-                                  replayid)
-        if replay is not None:
+        replays = self._find_cg_replays(profile,
+                                        replayid)
+        for replay in replays:
+            instanceid = self._get_id(replay)
+            LOG.debug('Expiring replay %s', instanceid)
             r = self.client.post('StorageCenter/ScReplay/%s/Expire'
-                                 % self._get_id(replay),
+                                 % instanceid,
                                  {})
             if r.status_code != 204:
                 LOG.error(_LE('ScReplay Expire error: %(code)d %(reason)s'),