From: Xing Yang Date: Tue, 6 Oct 2015 19:36:49 +0000 (-0400) Subject: ScaleIO driver: update_migrated_volume X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=64437898cdf58d4d028975155bda3d21a014bfe1;p=openstack-build%2Fcinder-build.git ScaleIO driver: update_migrated_volume This patch implements the update_migrated_volume function. Without this change, when the migrate finishes, the original volume will be deleted but the name on the new volume will not be changed to the old id even though the Cinder database will be updated. The volume name is a base64() encoding of the Cinder volume id so any calls after the migrate fail since the lookup fails. Also changed 'SCALEIO' to connector.SCALEIO now that os-brick 0.4.0 is released. Closes-Bug: #1512387 Change-Id: I70fe4bd651b0ac87b9208efb5918a4cdaf6f06b4 --- diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py index 9e82a0376..d21f8b04e 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py @@ -12,10 +12,14 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import mock from six.moves import urllib +from cinder import context from cinder import exception +from cinder.tests.unit import fake_volume from cinder.tests.unit.volume.drivers.emc import scaleio +from cinder.tests.unit.volume.drivers.emc.scaleio import mocks class TestMisc(scaleio.TestScaleIODriver): @@ -29,9 +33,16 @@ class TestMisc(scaleio.TestScaleIODriver): Defines the mock HTTPS responses for the REST API calls. """ super(TestMisc, self).setUp() - self.domain_name_enc = urllib.parse.quote(self.DOMAIN_NAME) self.pool_name_enc = urllib.parse.quote(self.POOL_NAME) + self.ctx = context.RequestContext('fake', 'fake', auth_token=True) + + self.volume = fake_volume.fake_volume_obj( + self.ctx, **{'name': 'vol1', 'provider_id': '0123456789abcdef'} + ) + self.new_volume = fake_volume.fake_volume_obj( + self.ctx, **{'name': 'vol2', 'provider_id': 'fedcba9876543210'} + ) self.HTTPS_MOCK_RESPONSES = { self.RESPONSE_MODE.Valid: { @@ -50,6 +61,12 @@ class TestMisc(scaleio.TestScaleIODriver): 'capacityLimitInKb': 1024, }, }, + 'instances/Volume::{}/action/setVolumeName'.format( + self.volume['provider_id']): + self.new_volume['provider_id'], + 'instances/Volume::{}/action/setVolumeName'.format( + self.new_volume['provider_id']): + self.volume['provider_id'], }, self.RESPONSE_MODE.BadStatus: { 'types/Domain/instances/getByName::' + @@ -58,6 +75,13 @@ class TestMisc(scaleio.TestScaleIODriver): self.RESPONSE_MODE.Invalid: { 'types/Domain/instances/getByName::' + self.domain_name_enc: None, + 'instances/Volume::{}/action/setVolumeName'.format( + self.volume['provider_id']): mocks.MockHTTPSResponse( + { + 'message': 'Invalid volume.', + 'httpStatusCode': 400, + 'errorCode': 0 + }, 400), }, } @@ -114,3 +138,51 @@ class TestMisc(scaleio.TestScaleIODriver): def test_get_volume_stats(self): self.driver.storage_pools = self.STORAGE_POOLS self.driver.get_volume_stats(True) + + @mock.patch( + 'cinder.volume.drivers.emc.scaleio.ScaleIODriver._rename_volume', + return_value=None) + def test_update_migrated_volume(self, mock_rename): + test_vol = self.driver.update_migrated_volume( + self.ctx, self.volume, self.new_volume, 'available') + mock_rename.assert_called_with(self.new_volume, self.volume['id']) + self.assertEqual({'_name_id': None, 'provider_location': None}, + test_vol) + + @mock.patch( + 'cinder.volume.drivers.emc.scaleio.ScaleIODriver._rename_volume', + return_value=None) + def test_update_unavailable_migrated_volume(self, mock_rename): + test_vol = self.driver.update_migrated_volume( + self.ctx, self.volume, self.new_volume, 'unavailable') + self.assertFalse(mock_rename.called) + self.assertEqual({'_name_id': '1', 'provider_location': None}, + test_vol) + + @mock.patch( + 'cinder.volume.drivers.emc.scaleio.ScaleIODriver._rename_volume', + side_effect=exception.VolumeBackendAPIException(data='Error!')) + def test_fail_update_migrated_volume(self, mock_rename): + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver.update_migrated_volume, + self.ctx, + self.volume, + self.new_volume, + 'available' + ) + mock_rename.assert_called_with(self.volume, "ff" + self.volume['id']) + + def test_rename_volume(self): + rc = self.driver._rename_volume( + self.volume, self.new_volume['id']) + self.assertIsNone(rc) + + def test_fail_rename_volume(self): + self.set_https_response_mode(self.RESPONSE_MODE.Invalid) + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver._rename_volume, + self.volume, + self.new_volume['id'] + ) diff --git a/cinder/volume/drivers/emc/scaleio.py b/cinder/volume/drivers/emc/scaleio.py index 26036dc4d..8b3f68c8d 100644 --- a/cinder/volume/drivers/emc/scaleio.py +++ b/cinder/volume/drivers/emc/scaleio.py @@ -147,9 +147,7 @@ class ScaleIODriver(driver.VolumeDriver): {'domain_id': self.protection_domain_id}) self.connector = connector.InitiatorConnector.factory( - # TODO(xyang): Change 'SCALEIO' to connector.SCALEIO after - # os-brick 0.4.0 is released. - 'SCALEIO', utils.get_root_helper(), + connector.SCALEIO, utils.get_root_helper(), device_scan_attempts= self.configuration.num_volume_device_scan_tries ) @@ -908,6 +906,75 @@ class ScaleIODriver(driver.VolumeDriver): finally: self._sio_detach_volume(volume) + def update_migrated_volume(self, ctxt, volume, new_volume, + original_volume_status): + """Return the update from ScaleIO migrated volume. + + This method updates the volume name of the new ScaleIO volume to + match the updated volume ID. + The original volume is renamed first since ScaleIO does not allow + multiple volumes to have the same name. + """ + name_id = None + location = None + if original_volume_status == 'available': + # During migration, a new volume is created and will replace + # the original volume at the end of the migration. We need to + # rename the new volume. The current_name of the new volume, + # which is the id of the new volume, will be changed to the + # new_name, which is the id of the original volume. + current_name = new_volume['id'] + new_name = volume['id'] + vol_id = new_volume['provider_id'] + LOG.info(_LI("Renaming %(id)s from %(current_name)s to " + "%(new_name)s."), + {'id': vol_id, 'current_name': current_name, + 'new_name': new_name}) + + # Original volume needs to be renamed first + self._rename_volume(volume, "ff" + new_name) + self._rename_volume(new_volume, new_name) + else: + # The back-end will not be renamed. + name_id = new_volume['_name_id'] or new_volume['id'] + location = new_volume['provider_location'] + + return {'_name_id': name_id, 'provider_location': location} + + def _rename_volume(self, volume, new_id): + new_name = self._id_to_base64(new_id) + vol_id = volume['provider_id'] + + req_vars = {'server_ip': self.server_ip, + 'server_port': self.server_port, + 'id': vol_id} + request = ("https://%(server_ip)s:%(server_port)s" + "/api/instances/Volume::%(id)s/action/setVolumeName" % + req_vars) + LOG.info(_LI("ScaleIO rename volume request: %s."), request) + + params = {'newName': new_name} + r = requests.post( + request, + data=json.dumps(params), + headers=self._get_headers(), + auth=(self.server_username, + self.server_token), + verify=self._get_verify_cert() + ) + r = self._check_response(r, request, False, params) + + if r.status_code != OK_STATUS_CODE: + response = r.json() + msg = (_("Error renaming volume %(vol)s: %(err)s.") % + {'vol': vol_id, 'err': response['message']}) + LOG.error(msg) + raise exception.VolumeBackendAPIException(data=msg) + else: + LOG.info(_LI("ScaleIO volume %(vol)s was renamed to " + "%(new_name)s."), + {'vol': vol_id, 'new_name': new_name}) + def ensure_export(self, context, volume): """Driver entry point to get the export info for an existing volume.""" pass