]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
ScaleIO driver: update_migrated_volume
authorXing Yang <xing.yang@emc.com>
Tue, 6 Oct 2015 19:36:49 +0000 (15:36 -0400)
committerXing Yang <xing.yang@emc.com>
Fri, 9 Oct 2015 23:46:10 +0000 (19:46 -0400)
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

cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py
cinder/volume/drivers/emc/scaleio.py

index 9e82a03766da47fb91ee3e8de058321ca2b5a54b..d21f8b04e0b77e11baf864947cd9eea5c613ca11 100644 (file)
 #    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']
+        )
index 26036dc4deb0ba64407e7351579fae1ffc61e95c..8b3f68c8d01707a9a91514acb03888bb309b25ed 100644 (file)
@@ -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