From: Xing Yang Date: Wed, 19 Aug 2015 02:16:03 +0000 (-0400) Subject: ScaleIO driver should use os-brick connector X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=73937b678a6480e72e0019e31c9446c42f26a6b9;p=openstack-build%2Fcinder-build.git ScaleIO driver should use os-brick connector Currently the ScaleIO Cinder volume driver contains the connect_volume logic inside the driver itself. This code is redundant with the code in the os-brick connector. This patch modifies the driver to use the os-brick connector and removes the redundant code. Change-Id: Ic26880f133c0ce10f9a67acd6e16b0d88cb2a242 Closes-Bug: #1486315 --- diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py b/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py index 07720b22b..ad6d43396 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py @@ -14,6 +14,7 @@ # under the License. import copy import requests +import six from cinder import test from cinder.tests.unit.volume.drivers.emc.scaleio import mocks @@ -78,6 +79,8 @@ class TestScaleIODriver(test.TestCase): }, 500 ) + VOLUME_NOT_FOUND_ERROR = 78 + HTTPS_MOCK_RESPONSES = {} __COMMON_HTTPS_MOCK_RESPONSES = { RESPONSE_MODE.Valid: { @@ -95,6 +98,12 @@ class TestScaleIODriver(test.TestCase): __https_response_mode = RESPONSE_MODE.Valid log = None + STORAGE_POOL_ID = six.text_type('1') + STORAGE_POOL_NAME = 'SP1' + + PROT_DOMAIN_ID = six.text_type('1') + PROT_DOMAIN_NAME = 'PD1' + def setUp(self): """Setup a test case environment. diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_cloned_volume.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_cloned_volume.py index 8c68368db..eb959dc72 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_cloned_volume.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_cloned_volume.py @@ -12,24 +12,19 @@ # 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 urllib -import six +import json +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 TestCreateClonedVolume(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.create_cloned_volume()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' - def setUp(self): """Setup a test case environment. @@ -38,39 +33,52 @@ class TestCreateClonedVolume(scaleio.TestScaleIODriver): super(TestCreateClonedVolume, self).setUp() ctx = context.RequestContext('fake', 'fake', auth_token=True) - self.src_volume = fake_volume.fake_volume_obj(ctx) + self.src_volume = fake_volume.fake_volume_obj( + ctx, **{'provider_id': 'pid001'}) + self.src_volume_name_2x_enc = urllib.quote( urllib.quote( - self.driver.id_to_base64(self.src_volume.id) + self.driver._id_to_base64(self.src_volume.id) ) ) + self.new_volume_extras = { + 'volumeIdList': ['cloned'], + 'snapshotGroupId': 'cloned_snapshot' + } + self.new_volume = fake_volume.fake_volume_obj( - ctx, **{'id': 'cloned', 'name': 'cloned_volume'} + ctx, **self.new_volume_extras ) self.new_volume_name_2x_enc = urllib.quote( urllib.quote( - self.driver.id_to_base64(self.new_volume.id) + self.driver._id_to_base64(self.new_volume.id) ) ) self.HTTPS_MOCK_RESPONSES = { self.RESPONSE_MODE.Valid: { 'types/Volume/instances/getByName::' + self.src_volume_name_2x_enc: self.src_volume.id, - 'instances/System/action/snapshotVolumes': '"{}"'.format( - self.new_volume.id - ), + 'instances/System/action/snapshotVolumes': '{}'.format( + json.dumps(self.new_volume_extras)), }, self.RESPONSE_MODE.BadStatus: { - 'instances/System/action/snapshotVolumes::': + 'instances/System/action/snapshotVolumes': self.BAD_STATUS_RESPONSE, 'types/Volume/instances/getByName::' + - self.src_volume_name_2x_enc: self.BAD_STATUS_RESPONSE, + self.src_volume['provider_id']: self.BAD_STATUS_RESPONSE, }, self.RESPONSE_MODE.Invalid: { 'types/Volume/instances/getByName::' + self.src_volume_name_2x_enc: None, + 'instances/System/action/snapshotVolumes': + mocks.MockHTTPSResponse( + { + 'errorCode': 400, + 'message': 'Invalid Volume Snapshot Test' + }, 400 + ), }, } diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py index 1ca9dfdc0..c3c5afd7b 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_snapshot.py @@ -12,24 +12,22 @@ # 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 json import urllib -import six - from cinder import context +from cinder import db from cinder import exception from cinder.tests.unit import fake_snapshot +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 TestCreateSnapShot(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.create_snapshot()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' + def return_fake_volume(self, ctx, id): + return self.fake_volume def setUp(self): """Setup a test case environment. @@ -40,12 +38,26 @@ class TestCreateSnapShot(scaleio.TestScaleIODriver): super(TestCreateSnapShot, self).setUp() ctx = context.RequestContext('fake', 'fake', auth_token=True) - self.snapshot = fake_snapshot.fake_snapshot_obj(ctx) + self.fake_volume = fake_volume.fake_volume_obj( + ctx, **{'provider_id': 'fake_pid'}) + + self.snapshot = fake_snapshot.fake_snapshot_obj( + ctx, **{'volume': self.fake_volume}) + + self.mock_object(db, 'volume_get', self.return_fake_volume) + self.volume_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.snapshot.volume_id)) + urllib.quote(self.driver._id_to_base64(self.snapshot.volume_id)) ) self.snapshot_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.snapshot.id)) + urllib.quote(self.driver._id_to_base64(self.snapshot.id)) + ) + + self.snapshot_reply = json.dumps( + { + 'volumeIdList': ['cloned'], + 'snapshotGroupId': 'cloned_snapshot' + } ) self.HTTPS_MOCK_RESPONSES = { @@ -54,7 +66,8 @@ class TestCreateSnapShot(scaleio.TestScaleIODriver): self.volume_name_2x_enc: '"{}"'.format( self.snapshot.volume_id ), - 'instances/System/action/snapshotVolumes': self.snapshot.id, + 'instances/System/action/snapshotVolumes': + self.snapshot_reply, 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: self.snapshot.id, }, @@ -62,12 +75,9 @@ class TestCreateSnapShot(scaleio.TestScaleIODriver): 'types/Volume/instances/getByName::' + self.volume_name_2x_enc: self.BAD_STATUS_RESPONSE, 'types/Volume/instances/getByName::' + - self.snapshot_name_2x_enc: mocks.MockHTTPSResponse( - { - 'errorCode': 401, - 'message': 'BadStatus Snapshot Test', - }, 401 - ), + self.snapshot_name_2x_enc: self.BAD_STATUS_RESPONSE, + 'instances/System/action/snapshotVolumes': + self.BAD_STATUS_RESPONSE, }, self.RESPONSE_MODE.Invalid: { 'types/Volume/instances/getByName::' + diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py index 32956dd6f..b2bbe959b 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py @@ -15,19 +15,11 @@ from cinder import context from cinder import exception from cinder.tests.unit import fake_volume - from cinder.tests.unit.volume.drivers.emc import scaleio class TestCreateVolume(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.create_volume()``""" - - PROTECTION_DOMAIN_ID = 'test_prot_id' - PROTECTION_DOMAIN_NAME = 'test_prot_name' - - STORAGE_POOL_ID = 'test_pool_id' - STORAGE_POOL_NAME = 'test_pool_name' - def setUp(self): """Setup a test case environment. @@ -42,44 +34,29 @@ class TestCreateVolume(scaleio.TestScaleIODriver): self.RESPONSE_MODE.Valid: { 'types/Volume/instances/getByName::' + self.volume.name: '"{}"'.format(self.volume.id), - 'types/Volume/instances': [ - { - 'Id': self.volume.id, - 'Name': self.volume.name, - 'volumeSizeInKb': self.volume.size, - 'isObfuscated': False, - 'creationTime': self.volume.launched_at, - 'mappingToAllSdcsEnabled': False, - 'mappingSdcInfoList': [], - 'mappingScsiInitiatorList': [], - 'ancestorVolumeId': '', - 'vtreeId': '', - 'storagePoolId': self.STORAGE_POOL_ID, - 'useRmcache': False, - } - ], + 'types/Volume/instances': {'id': self.volume.id}, 'types/Domain/instances/getByName::' + - self.PROTECTION_DOMAIN_NAME: - '"{}"'.format(self.PROTECTION_DOMAIN_ID), + self.PROT_DOMAIN_NAME: + '"{}"'.format(self.PROT_DOMAIN_ID), 'types/Pool/instances/getByName::{},{}'.format( - self.PROTECTION_DOMAIN_ID, + self.PROT_DOMAIN_ID, self.STORAGE_POOL_NAME ): '"{}"'.format(self.STORAGE_POOL_ID), }, self.RESPONSE_MODE.Invalid: { 'types/Domain/instances/getByName::' + - self.PROTECTION_DOMAIN_NAME: None, + self.PROT_DOMAIN_NAME: None, 'types/Pool/instances/getByName::{},{}'.format( - self.PROTECTION_DOMAIN_ID, + self.PROT_DOMAIN_ID, self.STORAGE_POOL_NAME ): None, }, self.RESPONSE_MODE.BadStatus: { 'types/Volume/instances': self.BAD_STATUS_RESPONSE, 'types/Domain/instances/getByName::' + - self.PROTECTION_DOMAIN_NAME: self.BAD_STATUS_RESPONSE, + self.PROT_DOMAIN_NAME: self.BAD_STATUS_RESPONSE, 'types/Pool/instances/getByName::{},{}'.format( - self.PROTECTION_DOMAIN_ID, + self.PROT_DOMAIN_ID, self.STORAGE_POOL_NAME ): self.BAD_STATUS_RESPONSE, }, @@ -95,7 +72,7 @@ class TestCreateVolume(scaleio.TestScaleIODriver): def test_no_domain_id(self): """Only protection domain name provided.""" self.driver.protection_domain_id = None - self.driver.protection_domain_name = self.PROTECTION_DOMAIN_NAME + self.driver.protection_domain_name = self.PROT_DOMAIN_NAME self.driver.storage_pool_name = None self.driver.storage_pool_id = self.STORAGE_POOL_ID self.test_create_volume() @@ -114,7 +91,7 @@ class TestCreateVolume(scaleio.TestScaleIODriver): """Only protection domain name provided.""" self.driver.storage_pool_id = None self.driver.storage_pool_name = self.STORAGE_POOL_NAME - self.driver.protection_domain_id = self.PROTECTION_DOMAIN_ID + self.driver.protection_domain_id = self.PROT_DOMAIN_ID self.driver.protection_domain_name = None self.test_create_volume() diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume_from_snapshot.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume_from_snapshot.py index 431b32d9a..acf2832d7 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume_from_snapshot.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume_from_snapshot.py @@ -12,25 +12,19 @@ # 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 json import urllib -import six - from cinder import context from cinder import exception from cinder.tests.unit import fake_snapshot 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 TestCreateVolumeFromSnapShot(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.create_volume_from_snapshot()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' - def setUp(self): """Setup a test case environment. @@ -42,27 +36,41 @@ class TestCreateVolumeFromSnapShot(scaleio.TestScaleIODriver): self.snapshot = fake_snapshot.fake_snapshot_obj(ctx) self.snapshot_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.snapshot.id)) + urllib.quote(self.driver._id_to_base64(self.snapshot.id)) ) self.volume = fake_volume.fake_volume_obj(ctx) self.volume_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.volume.id)) + urllib.quote(self.driver._id_to_base64(self.volume.id)) + ) + + self.snapshot_reply = json.dumps( + { + 'volumeIdList': [self.volume.id], + 'snapshotGroupId': 'snap_group' + } ) self.HTTPS_MOCK_RESPONSES = { self.RESPONSE_MODE.Valid: { 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: self.snapshot.id, - 'instances/System/action/snapshotVolumes': self.volume.id, + 'instances/System/action/snapshotVolumes': + self.snapshot_reply, }, self.RESPONSE_MODE.BadStatus: { - 'instances/System/action/snapshotVolumes::': + 'instances/System/action/snapshotVolumes': self.BAD_STATUS_RESPONSE, 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: self.BAD_STATUS_RESPONSE, - self.snapshot_name_2x_enc: self.BAD_STATUS_RESPONSE, }, self.RESPONSE_MODE.Invalid: { + 'instances/System/action/snapshotVolumes': + mocks.MockHTTPSResponse( + { + 'errorCode': self.VOLUME_NOT_FOUND_ERROR, + 'message': 'BadStatus Volume Test', + }, 400 + ), 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: None, }, diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_snapshot.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_snapshot.py index 8e46c7ad4..0ae28d077 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_snapshot.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_snapshot.py @@ -14,8 +14,6 @@ # under the License. import urllib -import six - from cinder import context from cinder import exception from cinder.tests.unit.fake_snapshot import fake_snapshot_obj @@ -25,13 +23,6 @@ from cinder.tests.unit.volume.drivers.emc.scaleio import mocks class TestDeleteSnapShot(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.delete_snapshot()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' - VOLUME_NOT_FOUND_ERROR = 3 - def setUp(self): """Setup a test case environment. @@ -41,10 +32,11 @@ class TestDeleteSnapShot(scaleio.TestScaleIODriver): super(TestDeleteSnapShot, self).setUp() ctx = context.RequestContext('fake', 'fake', auth_token=True) - self.snapshot = fake_snapshot_obj(ctx) + self.snapshot = fake_snapshot_obj( + ctx, **{'provider_id': 'snap_1'}) self.snapshot_name_2x_enc = urllib.quote( urllib.quote( - self.driver.id_to_base64(self.snapshot.id) + self.driver._id_to_base64(self.snapshot.id) ) ) @@ -53,15 +45,18 @@ class TestDeleteSnapShot(scaleio.TestScaleIODriver): 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: self.snapshot.id, 'instances/Volume::{}/action/removeMappedSdc'.format( - self.snapshot.id + self.snapshot.provider_id ): self.snapshot.id, 'instances/Volume::{}/action/removeVolume'.format( - self.snapshot.id + self.snapshot.provider_id ): self.snapshot.id, }, self.RESPONSE_MODE.BadStatus: { 'types/Volume/instances/getByName::' + self.snapshot_name_2x_enc: self.BAD_STATUS_RESPONSE, + 'instances/Volume::{}/action/removeVolume'.format( + self.snapshot.provider_id + ): self.BAD_STATUS_RESPONSE, }, self.RESPONSE_MODE.Invalid: { 'types/Volume/instances/getByName::' + @@ -71,6 +66,13 @@ class TestDeleteSnapShot(scaleio.TestScaleIODriver): 'message': 'Test Delete Invalid Snapshot', }, 400 ), + 'instances/Volume::{}/action/removeVolume'.format( + self.snapshot.provider_id): mocks.MockHTTPSResponse( + { + 'errorCode': self.VOLUME_NOT_FOUND_ERROR, + 'message': 'Test Delete Invalid Snapshot', + }, 400, + ) }, } diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_volume.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_volume.py index f082b7b7d..475982e98 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_volume.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_delete_volume.py @@ -14,8 +14,6 @@ # under the License. import urllib -import six - from cinder import context from cinder import exception from cinder.tests.unit import fake_volume @@ -25,12 +23,6 @@ from cinder.tests.unit.volume.drivers.emc.scaleio import mocks class TestDeleteVolume(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.delete_volume()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' - def setUp(self): """Setup a test case environment. @@ -39,9 +31,11 @@ class TestDeleteVolume(scaleio.TestScaleIODriver): super(TestDeleteVolume, self).setUp() ctx = context.RequestContext('fake', 'fake', auth_token=True) - self.volume = fake_volume.fake_volume_obj(ctx) + self.volume = fake_volume.fake_volume_obj( + ctx, **{'provider_id': 'pid_1'}) + self.volume_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.volume.id)) + urllib.quote(self.driver._id_to_base64(self.volume.id)) ) self.HTTPS_MOCK_RESPONSES = { @@ -49,10 +43,10 @@ class TestDeleteVolume(scaleio.TestScaleIODriver): 'types/Volume/instances/getByName::' + self.volume_name_2x_enc: self.volume.id, 'instances/Volume::{}/action/removeMappedSdc'.format( - self.volume.id): self.volume.id, + self.volume.provider_id): self.volume.provider_id, 'instances/Volume::{}/action/removeVolume'.format( - self.volume.id - ): self.volume.id, + self.volume.provider_id + ): self.volume.provider_id, }, self.RESPONSE_MODE.BadStatus: { 'types/Volume/instances/getByName::' + @@ -62,6 +56,14 @@ class TestDeleteVolume(scaleio.TestScaleIODriver): 'message': 'BadStatus Volume Test', }, 401 ), + 'instances/Volume::{}/action/removeVolume'.format( + self.volume.provider_id + ): mocks.MockHTTPSResponse( + { + 'errorCode': 401, + 'message': 'BadStatus Volume Test', + }, 401 + ), }, } diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_extend_volume.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_extend_volume.py index c1dc87482..fca55f40c 100644 --- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_extend_volume.py +++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_extend_volume.py @@ -14,8 +14,6 @@ # under the License. import urllib -import six - from cinder import context from cinder import exception from cinder.tests.unit.fake_volume import fake_volume_obj @@ -25,11 +23,6 @@ from cinder.tests.unit.volume.drivers.emc.scaleio import mocks class TestExtendVolume(scaleio.TestScaleIODriver): """Test cases for ``ScaleIODriver.extend_volume()``""" - STORAGE_POOL_ID = six.text_type('1') - STORAGE_POOL_NAME = 'SP1' - - PROT_DOMAIN_ID = six.text_type('1') - PROT_DOMAIN_NAME = 'PD1' """ New sizes for the volume. Since ScaleIO has a granularity of 8 GB, multiples of 8 always work. @@ -47,9 +40,10 @@ class TestExtendVolume(scaleio.TestScaleIODriver): super(TestExtendVolume, self).setUp() ctx = context.RequestContext('fake', 'fake', auth_token=True) - self.volume = fake_volume_obj(ctx, **{'id': 'fake_volume'}) + self.volume = fake_volume_obj(ctx, **{'id': 'fake_volume', + 'provider_id': 'pid_1'}) self.volume_name_2x_enc = urllib.quote( - urllib.quote(self.driver.id_to_base64(self.volume.id)) + urllib.quote(self.driver._id_to_base64(self.volume.id)) ) self.HTTPS_MOCK_RESPONSES = { @@ -57,23 +51,27 @@ class TestExtendVolume(scaleio.TestScaleIODriver): 'types/Volume/instances/getByName::' + self.volume_name_2x_enc: '"{}"'.format(self.volume.id), 'instances/Volume::{}/action/setVolumeSize'.format( - self.volume.id + self.volume.provider_id ): 'OK', }, self.RESPONSE_MODE.BadStatus: { 'types/Volume/instances/getByName::' + self.volume_name_2x_enc: self.BAD_STATUS_RESPONSE, 'types/Volume/instances/getByName::' + - self.volume_name_2x_enc: mocks.MockHTTPSResponse( - { - 'errorCode': 401, - 'message': 'BadStatus Extend Volume Test', - }, 401 - ), + self.volume_name_2x_enc: self.BAD_STATUS_RESPONSE, + 'instances/Volume::{}/action/setVolumeSize'.format( + self.volume.provider_id): self.BAD_STATUS_RESPONSE, }, self.RESPONSE_MODE.Invalid: { 'types/Volume/instances/getByName::' + self.volume_name_2x_enc: None, + 'instances/Volume::{}/action/setVolumeSize'.format( + self.volume.provider_id): mocks.MockHTTPSResponse( + { + 'errorCode': self.VOLUME_NOT_FOUND_ERROR, + 'message': 'BadStatus Volume Test', + }, 400 + ), }, } diff --git a/cinder/volume/drivers/emc/scaleio.py b/cinder/volume/drivers/emc/scaleio.py index d96ba836a..27a572e89 100644 --- a/cinder/volume/drivers/emc/scaleio.py +++ b/cinder/volume/drivers/emc/scaleio.py @@ -18,10 +18,8 @@ Driver for EMC ScaleIO based on ScaleIO remote CLI. import base64 import json -import os -import eventlet -from oslo_concurrency import processutils +from os_brick.initiator import connector from oslo_config import cfg from oslo_log import log as logging from oslo_utils import units @@ -90,7 +88,7 @@ BANDWIDTH_LIMIT = 'sio:bandwidth_limit' BLOCK_SIZE = 8 OK_STATUS_CODE = 200 -VOLUME_NOT_FOUND_ERROR = 3 +VOLUME_NOT_FOUND_ERROR = 78 VOLUME_NOT_MAPPED_ERROR = 84 VOLUME_ALREADY_MAPPED_ERROR = 81 @@ -128,7 +126,7 @@ class ScaleIODriver(driver.VolumeDriver): self.configuration.sio_storage_pools.split(',')] self.storage_pool_name = self.configuration.sio_storage_pool_name self.storage_pool_id = self.configuration.sio_storage_pool_id - if (self.storage_pool_name is None and self.storage_pool_id is None): + if self.storage_pool_name is None and self.storage_pool_id is None: LOG.warning(_LW("No storage pool name or id was found.")) else: LOG.info(_LI( @@ -148,6 +146,25 @@ class ScaleIODriver(driver.VolumeDriver): "Protection domain name: %(domain_id)s."), {'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(), + device_scan_attempts= + self.configuration.num_volume_device_scan_tries + ) + + self.connection_properties = {} + self.connection_properties['scaleIO_volname'] = None + self.connection_properties['hostIP'] = None + self.connection_properties['serverIP'] = self.server_ip + self.connection_properties['serverPort'] = self.server_port + self.connection_properties['serverUsername'] = self.server_username + self.connection_properties['serverPassword'] = self.server_password + self.connection_properties['serverToken'] = self.server_token + self.connection_properties['iopsLimit'] = None + self.connection_properties['bandwidthLimit'] = None + def check_for_setup_error(self): if (not self.protection_domain_name and not self.protection_domain_id): @@ -216,7 +233,7 @@ class ScaleIODriver(driver.VolumeDriver): def _find_bandwidth_limit(self, storage_type): return storage_type.get(BANDWIDTH_LIMIT) - def id_to_base64(self, id): + def _id_to_base64(self, id): # Base64 encode the id to get a volume name less than 32 characters due # to ScaleIO limitation. name = six.text_type(id).replace("-", "") @@ -234,7 +251,7 @@ class ScaleIODriver(driver.VolumeDriver): """Creates a scaleIO volume.""" self._check_volume_size(volume.size) - volname = self.id_to_base64(volume.id) + volname = self._id_to_base64(volume.id) storage_type = self._get_volumetype_extraspecs(volume) storage_pool_name = self._find_storage_pool_name_from_storage_type( @@ -383,6 +400,8 @@ class ScaleIODriver(driver.VolumeDriver): LOG.info(_LI("Created volume %(volname)s, volume id %(volid)s."), {'volname': volname, 'volid': volume.id}) + return {'provider_id': response['id']} + def _check_volume_size(self, size): if size % 8 != 0: round_volume_capacity = ( @@ -396,12 +415,13 @@ class ScaleIODriver(driver.VolumeDriver): def create_snapshot(self, snapshot): """Creates a scaleio snapshot.""" - volname = self.id_to_base64(snapshot.volume_id) - snapname = self.id_to_base64(snapshot.id) - self._snapshot_volume(volname, snapname) + volume_id = snapshot.volume.provider_id + snapname = self._id_to_base64(snapshot.id) + return self._snapshot_volume(volume_id, snapname) - def _snapshot_volume(self, volname, snapname): - vol_id = self._get_volume_id(volname) + def _snapshot_volume(self, vol_id, snapname): + LOG.info(_LI("Snapshot volume %(vol)s into snapshot %(id)s.") % + {'vol': vol_id, 'id': snapname}) params = { 'snapshotDefs': [{"volumeId": vol_id, "snapshotName": snapname}]} req_vars = {'server_ip': self.server_ip, @@ -418,15 +438,17 @@ class ScaleIODriver(driver.VolumeDriver): verify=self._get_verify_cert()) r = self._check_response(r, request, False, params) response = r.json() - LOG.info(_LI("snapshot volume response: %s."), response) + LOG.info(_LI("Snapshot volume response: %s."), response) if r.status_code != OK_STATUS_CODE and "errorCode" in response: msg = (_("Failed creating snapshot for volume %(volname)s: " "%(response)s.") % - {'volname': volname, + {'volname': vol_id, 'response': response['message']}) LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) + return {'provider_id': response['volumeIdList'][0]} + def _check_response(self, response, request, is_get_request=True, params=None): if response.status_code == 401 or response.status_code == 403: @@ -470,49 +492,15 @@ class ScaleIODriver(driver.VolumeDriver): # becomes a new unmapped volume in the system and the user # may manipulate it in the same manner as any other volume # exposed by the system - volname = self.id_to_base64(snapshot.id) - snapname = self.id_to_base64(volume.id) + volume_id = snapshot.provider_id + snapname = self._id_to_base64(volume.id) LOG.info(_LI( "ScaleIO create volume from snapshot: snapshot %(snapname)s " "to volume %(volname)s."), - {'volname': volname, + {'volname': volume_id, 'snapname': snapname}) - self._snapshot_volume(volname, snapname) - - def _get_volume_id(self, volname): - volname_encoded = urllib.quote(volname, '') - volname_double_encoded = urllib.quote(volname_encoded, '') - LOG.info(_LI("Volume name after double encoding is %s."), - volname_double_encoded) - req_vars = {'server_ip': self.server_ip, - 'server_port': self.server_port, - 'encoded': volname_double_encoded} - request = ("https://%(server_ip)s:%(server_port)s" - "/api/types/Volume/instances/getByName::" - "%(encoded)s") % req_vars - LOG.info(_LI("ScaleIO get volume id by name request: %s"), request) - r = requests.get( - request, - auth=(self.server_username, - self.server_token), - verify=self._get_verify_cert()) - r = self._check_response(r, request) - - vol_id = r.json() - - if not vol_id: - msg = _("Volume with name %s wasn't found.") % volname - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - if r.status_code != OK_STATUS_CODE and "errorCode" in vol_id: - msg = (_("Error getting volume id from name %(volname)s: %(err)s.") - % {'volname': volname, - 'err': vol_id['message']}) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - LOG.info(_LI("volume id is %s."), vol_id) - return vol_id + return self._snapshot_volume(volume_id, snapname) def _get_headers(self): return {'content-type': 'application/json'} @@ -528,14 +516,12 @@ class ScaleIODriver(driver.VolumeDriver): self._check_volume_size(new_size) - volname = self.id_to_base64(volume.id) - + vol_id = volume['provider_id'] LOG.info(_LI( "ScaleIO extend volume: volume %(volname)s to size %(new_size)s."), - {'volname': volname, + {'volname': vol_id, 'new_size': new_size}) - vol_id = self._get_volume_id(volname) req_vars = {'server_ip': self.server_ip, 'server_port': self.server_port, 'vol_id': vol_id} @@ -557,69 +543,34 @@ class ScaleIODriver(driver.VolumeDriver): if r.status_code != OK_STATUS_CODE: response = r.json() msg = (_("Error extending volume %(vol)s: %(err)s.") - % {'vol': volname, + % {'vol': vol_id, 'err': response['message']}) LOG.error(msg) raise exception.VolumeBackendAPIException(data=msg) def create_cloned_volume(self, volume, src_vref): """Creates a cloned volume.""" - volname = self.id_to_base64(src_vref.id) - snapname = self.id_to_base64(volume.id) + volume_id = src_vref['provider_id'] + snapname = self._id_to_base64(volume.id) LOG.info(_LI( "ScaleIO create cloned volume: source volume %(src)s to target " "volume %(tgt)s."), - {'src': volname, + {'src': volume_id, 'tgt': snapname}) - self._snapshot_volume(volname, snapname) + + return self._snapshot_volume(volume_id, snapname) def delete_volume(self, volume): """Deletes a self.logical volume""" - volname = self.id_to_base64(volume.id) - self._delete_volume(volname) - - def _delete_volume(self, volname): - volname_encoded = urllib.quote(volname, '') - volname_double_encoded = urllib.quote(volname_encoded, '') - LOG.info(_LI("Volume name after double encoding is %s."), - volname_double_encoded) + volume_id = volume['provider_id'] + self._delete_volume(volume_id) + def _delete_volume(self, vol_id): verify_cert = self._get_verify_cert() - # convert volume name to id req_vars = {'server_ip': self.server_ip, 'server_port': self.server_port, - 'encoded': volname_double_encoded} - request = ("https://%(server_ip)s:%(server_port)s" - "/api/types/Volume/instances/getByName::" - "%(encoded)s") % req_vars - LOG.info(_LI("ScaleIO get volume id by name request: %s."), request) - r = requests.get( - request, - auth=( - self.server_username, - self.server_token), - verify=verify_cert) - r = self._check_response(r, request) - LOG.info(_LI("Get by name response: %s."), r.text) - vol_id = r.json() - LOG.info(_LI("ScaleIO volume id to delete is %s."), vol_id) - - if r.status_code != OK_STATUS_CODE and "errorCode" in vol_id: - msg = (_("Error getting volume id from name %(vol)s: %(err)s.") - % {'vol': volname, 'err': vol_id['message']}) - LOG.error(msg) - - error_code = vol_id['errorCode'] - if (error_code == VOLUME_NOT_FOUND_ERROR): - force_delete = self.configuration.sio_force_delete - if force_delete: - LOG.warning(_LW( - "Ignoring error in delete volume %s: volume not found " - "due to force delete settings."), volname) - return - - raise exception.VolumeBackendAPIException(data=msg) + 'vol_id': six.text_type(vol_id)} unmap_before_delete = ( self.configuration.sio_unmap_volume_before_deletion) @@ -627,9 +578,6 @@ class ScaleIODriver(driver.VolumeDriver): # case unmap_before_deletion is enabled. if unmap_before_delete: params = {'allSdcs': ''} - req_vars = {'server_ip': self.server_ip, - 'server_port': self.server_port, - 'vol_id': vol_id} request = ("https://%(server_ip)s:%(server_port)s" "/api/instances/Volume::%(vol_id)s" "/action/removeMappedSdc") % req_vars @@ -643,31 +591,30 @@ class ScaleIODriver(driver.VolumeDriver): auth=( self.server_username, self.server_token), - verify=verify_cert) + verify=verify_cert + ) r = self._check_response(r, request, False, params) LOG.debug("Unmap volume response: %s.", r.text) params = {'removeMode': 'ONLY_ME'} + request = ("https://%(server_ip)s:%(server_port)s" + "/api/instances/Volume::%(vol_id)s" + "/action/removeVolume") % req_vars r = requests.post( - "https://" + - self.server_ip + - ":" + - self.server_port + - "/api/instances/Volume::" + - six.text_type(vol_id) + - "/action/removeVolume", + request, data=json.dumps(params), headers=self._get_headers(), auth=(self.server_username, self.server_token), - verify=verify_cert) + verify=verify_cert + ) r = self._check_response(r, request, False, params) if r.status_code != OK_STATUS_CODE: response = r.json() error_code = response['errorCode'] if error_code == 78: - force_delete = self.configuration.sio_orce_delete + force_delete = self.configuration.sio_force_delete if force_delete: LOG.warning(_LW( "Ignoring error in delete volume %s: volume not found " @@ -686,9 +633,9 @@ class ScaleIODriver(driver.VolumeDriver): def delete_snapshot(self, snapshot): """Deletes a ScaleIO snapshot.""" - snapname = self.id_to_base64(snapshot.id) + snap_id = snapshot.provider_id LOG.info(_LI("ScaleIO delete snapshot.")) - self._delete_volume(snapname) + return self._delete_volume(snap_id) def initialize_connection(self, volume, connector): """Initializes the connection and returns connection info. @@ -697,28 +644,21 @@ class ScaleIODriver(driver.VolumeDriver): """ LOG.debug("Connector is %s.", connector) - volname = self.id_to_base64(volume.id) - properties = {} - - properties['scaleIO_volname'] = volname - properties['hostIP'] = connector['ip'] - properties['serverIP'] = self.server_ip - properties['serverPort'] = self.server_port - properties['serverUsername'] = self.server_username - properties['serverPassword'] = self.server_password - properties['serverToken'] = self.server_token + connection_properties = dict(self.connection_properties) + volname = self._id_to_base64(volume.id) + connection_properties['scaleIO_volname'] = volname storage_type = self._get_volumetype_extraspecs(volume) LOG.info(_LI("Volume type is %s."), storage_type) iops_limit = self._find_iops_limit(storage_type) LOG.info(_LI("iops limit is: %s."), iops_limit) bandwidth_limit = self._find_bandwidth_limit(storage_type) LOG.info(_LI("Bandwidth limit is: %s."), bandwidth_limit) - properties['iopsLimit'] = iops_limit - properties['bandwidthLimit'] = bandwidth_limit + connection_properties['iopsLimit'] = iops_limit + connection_properties['bandwidthLimit'] = bandwidth_limit return {'driver_volume_type': 'scaleio', - 'data': properties} + 'data': connection_properties} def terminate_connection(self, volume, connector, **kwargs): LOG.debug("scaleio driver terminate connection.") @@ -902,198 +842,23 @@ class ScaleIODriver(driver.VolumeDriver): return specs - def find_volume_path(self, volume_id): - - LOG.info(_LI("looking for volume %s."), volume_id) - # Look for the volume in /dev/disk/by-id directory. - disk_filename = "" - tries = 0 - while not disk_filename: - if tries > self.configuration.num_volume_device_scan_tries: - msg = (_( - "scaleIO volume %s not found at expected path.") - % volume_id) - raise exception.VolumeBackendAPIException(msg) - by_id_path = "/dev/disk/by-id" - if not os.path.isdir(by_id_path): - LOG.warning(_LW( - "scaleIO volume %(vol)s not yet found (no directory " - "/dev/disk/by-id yet). Try number: %(tries)d."), - {'vol': volume_id, - 'tries': tries}) - tries = tries + 1 - eventlet.sleep(1) - continue - filenames = os.listdir(by_id_path) - LOG.info(_LI( - "Files found in path %(path)s: %(file)s."), - {'path': by_id_path, - 'file': filenames}) - for filename in filenames: - if (filename.startswith("emc-vol") and - filename.endswith(volume_id)): - disk_filename = filename - if not disk_filename: - LOG.warning(_LW( - "scaleIO volume %(vol)s not yet found. " - "Try number: %(tries)d."), - {'vol': volume_id, - 'tries': tries}) - tries = tries + 1 - eventlet.sleep(1) - - if (tries != 0): - LOG.info(_LI( - "Found scaleIO device %(file)s after %(tries)d retries "), - {'file': disk_filename, - 'tries': tries}) - full_disk_name = by_id_path + "/" + disk_filename - LOG.info(_LI("Full disk name is %s."), full_disk_name) - return full_disk_name - - def _get_client_id( - self, server_ip, server_username, server_password, sdc_ip): - req_vars = {'server_ip': server_ip, - 'server_port': self.server_port, - 'sdc_ip': sdc_ip} - request = ("https://%(server_ip)s:%(server_port)s" - "/api/types/Client/instances/getByIp::" - "%(sdc_ip)s/") % req_vars - LOG.info(_LI("ScaleIO get client id by ip request: %s."), request) - r = requests.get( - request, - auth=( - server_username, - self.server_token), - verify=self._get_verify_cert()) - r = self._check_response(r, request) - - sdc_id = r.json() - if not sdc_id: - msg = _("Client with ip %s wasn't found.") % sdc_ip - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - if r.status_code != 200 and "errorCode" in sdc_id: - msg = (_("Error getting sdc id from ip %(ip)s: %(id)s.") - % {'ip': sdc_ip, 'id': sdc_id['message']}) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - LOG.info(_LI("ScaleIO sdc id is %s."), sdc_id) - return sdc_id + def _sio_attach_volume(self, volume): + """Call connector.connect_volume() and return the path. """ + LOG.debug("Calling os-brick to attach ScaleIO volume.") + connection_properties = dict(self.connection_properties) + connection_properties['scaleIO_volname'] = self._id_to_base64( + volume.id) + device_info = self.connector.connect_volume(connection_properties) - def _sio_attach_volume(self, volume, sdc_ip): - # We need to make sure we even *have* a local path - LOG.info(_LI("ScaleIO attach volume in scaleio cinder driver.")) - volname = self.id_to_base64(volume.id) + return device_info['path'] - cmd = ['drv_cfg'] - cmd += ["--query_guid"] - - LOG.info(_LI("ScaleIO sdc query guid command: %s"), six.text_type(cmd)) - - try: - (out, err) = utils.execute(*cmd, run_as_root=True) - LOG.debug("Map volume %(cmd)s: stdout=%(out)s stderr=%(err)s", - {'cmd': cmd, 'out': out, 'err': err}) - except processutils.ProcessExecutionError as e: - msg = _("Error querying sdc guid: %s.") % six.text_type(e.stderr) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - - guid = out - LOG.info(_LI("Current sdc guid: %s."), guid) - - params = {'guid': guid} - - volume_id = self._get_volume_id(volname) - req_vars = {'server_ip': self.server_ip, - 'server_port': self.server_port, - 'volume_id': volume_id} - request = ("https://%(server_ip)s:%(server_port)s" - "/api/instances/Volume::%(volume_id)s" - "/action/addMappedSdc") % req_vars - LOG.info(_LI("Map volume request: %s."), request) - 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) - - if r.status_code != OK_STATUS_CODE: - response = r.json() - error_code = response['errorCode'] - if (error_code == VOLUME_ALREADY_MAPPED_ERROR): - LOG.warning(_LW("Ignoring error mapping volume %s: " - "volume already mapped."), volname) - else: - msg = (_("Error mapping volume %(vol)s: %(err)s.") - % {'vol': volname, - 'err': response['message']}) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - - formated_id = volume_id - - return self.find_volume_path(formated_id) - - def _sio_detach_volume(self, volume, sdc_ip): - LOG.info(_LI("ScaleIO detach volume in scaleio cinder driver.")) - volname = self.id_to_base64(volume.id) - - cmd = ['drv_cfg'] - cmd += ["--query_guid"] - - LOG.info(_LI("ScaleIO sdc query guid command: %s."), cmd) - - try: - (out, err) = utils.execute(*cmd, run_as_root=True) - LOG.debug("Unmap volume %(cmd)s: stdout=%(out)s stderr=%(err)s", - {'cmd': cmd, 'out': out, 'err': err}) - - except processutils.ProcessExecutionError as e: - msg = _("Error querying sdc guid: %s.") % six.text_type(e.stderr) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) - - guid = out - LOG.info(_LI("Current sdc guid: %s."), guid) - - params = {'guid': guid} - - volume_id = self._get_volume_id(volname) - req_vars = {'server_ip': self.server_ip, - 'server_port': self.server_port, - 'vol_id': volume_id} - request = ("https://%(server_ip)s:%(server_port)s" - "/api/instances/Volume::%(vol_id)s" - "/action/removeMappedSdc") % req_vars - LOG.info(_LI("Unmap volume request: %s."), request) - 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() - error_code = response['errorCode'] - if error_code == VOLUME_NOT_MAPPED_ERROR: - LOG.warning(_LW("Ignoring error unmapping volume %s: " - "volume not mapped."), volname) - else: - msg = (_("Error unmapping volume %(vol)s: %(err)s.") - % {'vol': volname, - 'err': response['message']}) - LOG.error(msg) - raise exception.VolumeBackendAPIException(data=msg) + def _sio_detach_volume(self, volume): + """Call the connector.disconnect() """ + LOG.info(_LI("Calling os-brick to detach ScaleIO volume.")) + connection_properties = dict(self.connection_properties) + connection_properties['scaleIO_volname'] = self._id_to_base64( + volume.id) + self.connector.disconnect_volume(connection_properties, volume) def copy_image_to_volume(self, context, volume, image_service, image_id): """Fetch the image from image_service and write it to the volume.""" @@ -1103,20 +868,17 @@ class ScaleIODriver(driver.VolumeDriver): {'vol': volume, 'service': six.text_type(image_service), 'id': six.text_type(image_id)}) - properties = utils.brick_get_connector_properties() - sdc_ip = properties['ip'] - LOG.debug("SDC ip is: %s", sdc_ip) try: image_utils.fetch_to_raw(context, image_service, image_id, - self._sio_attach_volume(volume, sdc_ip), + self._sio_attach_volume(volume), BLOCK_SIZE, size=volume['size']) finally: - self._sio_detach_volume(volume, sdc_ip) + self._sio_detach_volume(volume) def copy_volume_to_image(self, context, volume, image_service, image_meta): """Copy the volume to the specified image.""" @@ -1126,16 +888,13 @@ class ScaleIODriver(driver.VolumeDriver): {'vol': volume, 'service': six.text_type(image_service), 'meta': six.text_type(image_meta)}) - properties = utils.brick_get_connector_properties() - sdc_ip = properties['ip'] - LOG.debug("SDC ip is: {0}".format(sdc_ip)) try: image_utils.upload_volume(context, image_service, image_meta, - self._sio_attach_volume(volume, sdc_ip)) + self._sio_attach_volume(volume)) finally: - self._sio_detach_volume(volume, sdc_ip) + self._sio_detach_volume(volume) def ensure_export(self, context, volume): """Driver entry point to get the export info for an existing volume."""