From: Tom Swanson Date: Tue, 29 Sep 2015 15:26:48 +0000 (-0500) Subject: Dell SC: cgsnapshot-delete doesn't actually delete X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f9e6c5f88c96bcbd181e991219d4ecbf9b8cf833;p=openstack-build%2Fcinder-build.git 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 2847bbe0f..41f0f5fc8 100644 --- 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 dbd47d38a..afb28a3d7 100644 --- 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'),