From: Mark Sturdevant Date: Fri, 4 Jul 2014 00:34:46 +0000 (-0700) Subject: HP 3PAR retype implementation X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=e99d1a007d5757959a91c735e97aa47597f9a5b5;p=openstack-build%2Fcinder-build.git HP 3PAR retype implementation Add retype support to the HP 3PAR driver. The driver will modify the volume's current userCPG, snapCPG provisioningType, and QOS settings, as needed, to make the volume behave appropriately for the new type. The existing retype framework handles what to do if the driver cannot retype (when it returns False or raises an exception). The combination of optional type-specified VV set, old-style scoped extra-specs and QOS-specs will be resolved using existing code. So no new interpretation of a mixed-up type specifications will be introduced (no need to re-document these driver caveats). Other than the usual validity checking, the 3PAR retype will have one limitation. Retype will not be allowed if the volume has snapshots and the retype would require modifying/tuning the snapCPG or userCPG. Change-Id: I6c7816233635e369dd7ff349c2a437b22a849a84 Implements: blueprint retype-hp-3par --- diff --git a/cinder/tests/fake_hp_client_exceptions.py b/cinder/tests/fake_hp_client_exceptions.py index 07b5baf8c..5f61e8b5c 100644 --- a/cinder/tests/fake_hp_client_exceptions.py +++ b/cinder/tests/fake_hp_client_exceptions.py @@ -16,6 +16,58 @@ """Fake HP client exceptions to use when mocking HP clients.""" +class ClientException(Exception): + """The base exception class for these fake exceptions.""" + _error_code = None + _error_desc = None + _error_ref = None + + _debug1 = None + _debug2 = None + + def __init__(self, error=None): + if error: + if 'code' in error: + self._error_code = error['code'] + if 'desc' in error: + self._error_desc = error['desc'] + if 'ref' in error: + self._error_ref = error['ref'] + + if 'debug1' in error: + self._debug1 = error['debug1'] + if 'debug2' in error: + self._debug2 = error['debug2'] + + def get_code(self): + return self._error_code + + def get_description(self): + return self._error_desc + + def get_ref(self): + return self._error_ref + + def __str__(self): + formatted_string = self.message + if self.http_status: + formatted_string += " (HTTP %s)" % self.http_status + if self._error_code: + formatted_string += " %s" % self._error_code + if self._error_desc: + formatted_string += " - %s" % self._error_desc + if self._error_ref: + formatted_string += " - %s" % self._error_ref + + if self._debug1: + formatted_string += " (1: '%s')" % self._debug1 + + if self._debug2: + formatted_string += " (2: '%s')" % self._debug2 + + return formatted_string + + class HTTPConflict(Exception): http_status = 409 message = "Conflict" @@ -33,17 +85,10 @@ class HTTPNotFound(Exception): message = "Not found" -class HTTPForbidden(Exception): +class HTTPForbidden(ClientException): http_status = 403 message = "Forbidden" - def __init__(self, error=None): - if error and 'code' in error: - self._error_code = error['code'] - - def get_code(self): - return self._error_code - class HTTPBadRequest(Exception): http_status = 400 diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index e417587db..b423522b3 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -158,18 +158,167 @@ class HP3PARBaseDriver(object): 'state': 1, 'uuid': '29c214aa-62b9-41c8-b198-543f6cf24edf'}] + TASK_DONE = 1 + TASK_ACTIVE = 2 + STATUS_DONE = {'status': 1} + STATUS_ACTIVE = {'status': 2} + mock_client_conf = { 'PORT_MODE_TARGET': 2, 'PORT_STATE_READY': 4, 'PORT_PROTO_ISCSI': 2, 'PORT_PROTO_FC': 1, - 'TASK_DONE': 1, + 'TASK_DONE': TASK_DONE, + 'TASK_ACTIVE': TASK_ACTIVE, 'HOST_EDIT_ADD': 1, 'getPorts.return_value': { 'members': FAKE_FC_PORTS + [FAKE_ISCSI_PORT] } } + RETYPE_VVS_NAME = "yourvvs" + + RETYPE_HOST = { + u'host': u'mark-stack1@3parfc', + u'capabilities': { + 'QoS_support': True, + u'location_info': u'HP3PARDriver:1234567:MARK_TEST_CPG', + u'timestamp': u'2014-06-04T19:03:32.485540', + u'allocated_capacity_gb': 0, + u'volume_backend_name': u'3parfc', + u'free_capacity_gb': u'infinite', + u'driver_version': u'2.0.3', + u'total_capacity_gb': u'infinite', + u'reserved_percentage': 0, + u'vendor_name': u'Hewlett-Packard', + u'storage_protocol': u'FC' + } + } + + RETYPE_HOST_NOT3PAR = { + u'host': u'mark-stack1@3parfc', + u'capabilities': { + u'location_info': u'XXXDriverXXX:1610771:MARK_TEST_CPG', + } + } + + RETYPE_QOS_SPECS = {'maxIOPS': '1000', 'maxBWS': '50', + 'minIOPS': '100', 'minBWS': '25', + 'latency': '25', 'priority': 'high'} + + RETYPE_VOLUME_TYPE_ID = "FakeVolId" + + RETYPE_VOLUME_TYPE_0 = { + 'name': 'red', + 'id': RETYPE_VOLUME_TYPE_ID, + 'extra_specs': { + 'cpg': HP3PAR_CPG, + 'snap_cpg': HP3PAR_CPG_SNAP, + 'vvs': RETYPE_VVS_NAME, + 'qos': RETYPE_QOS_SPECS, + 'tpvv': True, + 'volume_type': volume_type + } + } + + RETYPE_VOLUME_TYPE_1 = { + 'name': 'white', + 'id': RETYPE_VOLUME_TYPE_ID, + 'extra_specs': { + 'cpg': HP3PAR_CPG, + 'snap_cpg': HP3PAR_CPG_SNAP, + 'vvs': VVS_NAME, + 'qos': QOS, + 'tpvv': True, + 'volume_type': volume_type + } + } + + RETYPE_VOLUME_TYPE_2 = { + 'name': 'blue', + 'id': RETYPE_VOLUME_TYPE_ID, + 'extra_specs': { + 'cpg': HP3PAR_CPG, + 'snap_cpg': HP3PAR_CPG_SNAP, + 'vvs': RETYPE_VVS_NAME, + 'qos': RETYPE_QOS_SPECS, + 'tpvv': True, + 'volume_type': volume_type + } + } + + RETYPE_VOLUME_TYPE_BAD_PERSONA = { + 'name': 'bad_persona', + 'id': 'any_id', + 'extra_specs': { + 'hp3par:persona': '99 - invalid' + } + } + + RETYPE_VOLUME_TYPE_BAD_CPG = { + 'name': 'bad_cpg', + 'id': 'any_id', + 'extra_specs': { + 'cpg': 'bogus', + 'snap_cpg': 'bogus', + 'hp3par:persona': '1 - Generic' + } + } + + RETYPE_TEST_COMMENT = "{'retype_test': 'test comment'}" + + RETYPE_VOLUME_INFO_0 = { + 'name': VOLUME_NAME, + 'id': VOLUME_ID, + 'display_name': 'Retype Vol0', + 'size': 1, + 'host': RETYPE_HOST, + 'userCPG': 'testUserCpg0', + 'snapCPG': 'testSnapCpg0', + 'provisioningType': 1, + 'comment': RETYPE_TEST_COMMENT + } + + RETYPE_TEST_COMMENT_1 = "{'retype_test': 'test comment 1'}" + + RETYPE_VOLUME_INFO_1 = { + 'name': VOLUME_NAME, + 'id': VOLUME_ID, + 'display_name': 'Retype Vol1', + 'size': 1, + 'host': RETYPE_HOST, + 'userCPG': HP3PAR_CPG, + 'snapCPG': HP3PAR_CPG_SNAP, + 'provisioningType': 1, + 'comment': RETYPE_TEST_COMMENT + } + + # Test for when we don't get a snapCPG. + RETYPE_VOLUME_INFO_NO_SNAP = { + 'name': VOLUME_NAME, + 'id': VOLUME_ID, + 'display_name': 'Retype Vol2', + 'size': 1, + 'host': RETYPE_HOST, + 'userCPG': 'testUserCpg2', + 'provisioningType': 1, + 'comment': '{}' + } + + RETYPE_CONF = { + 'TASK_ACTIVE': TASK_ACTIVE, + 'TASK_DONE': TASK_DONE, + 'getTask.return_value': STATUS_DONE, + 'getStorageSystemInfo.return_value': {'serialNumber': '1234567'}, + 'getVolume.return_value': RETYPE_VOLUME_INFO_0, + 'modifyVolume.return_value': ("anyResponse", {'taskid': 1}) + } + + # 3PAR retype currently doesn't use the diff. Existing code and fresh info + # from the array work better for the most part. Some use of the diff was + # intentionally removed to make _retype more usable for other use cases. + RETYPE_DIFF = None + def setup_configuration(self): configuration = mock.Mock() configuration.hp3par_debug = False @@ -212,6 +361,29 @@ class HP3PARBaseDriver(object): self.driver.do_setup(None) return _m_client + def test_task_waiter(self): + + task_statuses = [self.STATUS_ACTIVE, self.STATUS_ACTIVE] + + def side_effect(*args): + return task_statuses and task_statuses.pop(0) or self.STATUS_DONE + + conf = {'getTask.side_effect': side_effect} + mock_client = self.setup_driver(mock_conf=conf) + + task_id = 1234 + interval = .001 + waiter = self.driver.common.TaskWaiter(mock_client, task_id, interval) + status = waiter.wait_for_task() + + expected = [ + mock.call.getTask(task_id), + mock.call.getTask(task_id), + mock.call.getTask(task_id) + ] + mock_client.assert_has_calls(expected) + self.assertEqual(status, self.STATUS_DONE) + def test_create_volume(self): # setup_mock_client drive with default configuration @@ -271,6 +443,324 @@ class HP3PARBaseDriver(object): mock_client.assert_has_calls(expected) + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_not_3par(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + self.assertRaises(exception.InvalidHost, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST_NOT3PAR) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.logout()] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_volume_not_found(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + mock_client.getVolume.side_effect = hpexceptions.HTTPNotFound + + self.assertRaises(hpexceptions.HTTPNotFound, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.logout()] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_snap_cpg_check(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + mock_client.getVolume.return_value = self.RETYPE_VOLUME_INFO_NO_SNAP + + self.assertRaises(exception.InvalidVolume, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_NO_SNAP, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.getStorageSystemInfo(), + mock.call.logout()] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_specs_error_reverts_snap_cpg(self, _mock_volume_types): + _mock_volume_types.side_effect = [ + self.RETYPE_VOLUME_TYPE_1, self.RETYPE_VOLUME_TYPE_0] + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + mock_client.getVolume.return_value = self.RETYPE_VOLUME_INFO_0 + + # Fail the QOS setting to test the revert of the snap CPG rename. + mock_client.addVolumeToVolumeSet.side_effect = \ + hpexceptions.HTTPForbidden + + self.assertRaises(hpexceptions.HTTPForbidden, + self.driver.retype, + self.ctxt, + {'id': self.VOLUME_ID}, + self.RETYPE_VOLUME_TYPE_0, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + old_settings = { + 'snapCPG': self.RETYPE_VOLUME_INFO_0['snapCPG'], + 'comment': self.RETYPE_VOLUME_INFO_0['comment']} + new_settings = { + 'snapCPG': self.RETYPE_VOLUME_TYPE_1['extra_specs']['snap_cpg'], + 'comment': mock.ANY} + + expected = [ + mock.call.modifyVolume(self.VOLUME_3PAR_NAME, new_settings) + ] + mock_client.assert_has_calls(expected) + expected = [ + mock.call.modifyVolume(self.VOLUME_3PAR_NAME, old_settings), + mock.call.logout()] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_revert_comment(self, _mock_volume_types): + _mock_volume_types.side_effect = [ + self.RETYPE_VOLUME_TYPE_2, self.RETYPE_VOLUME_TYPE_1] + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + mock_client.getVolume.return_value = self.RETYPE_VOLUME_INFO_1 + + # Fail the QOS setting to test the revert of the snap CPG rename. + mock_client.deleteVolumeSet.side_effect = hpexceptions.HTTPForbidden + + self.assertRaises(hpexceptions.HTTPForbidden, + self.driver.retype, + self.ctxt, + {'id': self.VOLUME_ID}, + self.RETYPE_VOLUME_TYPE_2, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + original = { + 'snapCPG': self.RETYPE_VOLUME_INFO_1['snapCPG'], + 'comment': self.RETYPE_VOLUME_INFO_1['comment']} + + expected = [ + mock.call.modifyVolume('osv-0DM4qZEVSKON-DXN-NwVpw', original), + mock.call.logout()] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_different_array(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + mock_client.getStorageSystemInfo.return_value = { + 'serialNumber': 'XXXXXXX'} + + self.assertRaises(exception.InvalidHost, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.getStorageSystemInfo(), + mock.call.logout()] + + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_across_cpg_domains(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + mock_client.getCPG.side_effect = [ + {'domain': 'domain1'}, + {'domain': 'domain2'}, + ] + + self.assertRaises(exception.Invalid3PARDomain, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.getStorageSystemInfo(), + mock.call.getCPG(self.RETYPE_VOLUME_INFO_0['userCPG']), + mock.call.getCPG(self.RETYPE_VOLUME_TYPE_1['extra_specs']['cpg']), + mock.call.logout() + ] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_across_snap_cpg_domains(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + mock_client.getCPG.side_effect = [ + {'domain': 'cpg_domain'}, + {'domain': 'cpg_domain'}, + {'domain': 'snap_cpg_domain_1'}, + {'domain': 'snap_cpg_domain_2'}, + ] + + self.assertRaises(exception.Invalid3PARDomain, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_1, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.getStorageSystemInfo(), + mock.call.getCPG(self.RETYPE_VOLUME_INFO_0['userCPG']), + mock.call.getCPG(self.RETYPE_VOLUME_TYPE_1['extra_specs']['cpg']), + mock.call.getCPG(self.RETYPE_VOLUME_INFO_0['snapCPG']), + mock.call.getCPG( + self.RETYPE_VOLUME_TYPE_1['extra_specs']['snap_cpg']), + mock.call.logout() + ] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_to_bad_persona(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_BAD_PERSONA + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + self.assertRaises(exception.InvalidInput, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_BAD_PERSONA, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getVolume(self.VOLUME_3PAR_NAME), + mock.call.logout() + ] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_to_bad_cpg(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_BAD_CPG + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + mock_client.getCPG.side_effect = hpexceptions.HTTPNotFound + + self.assertRaises(exception.InvalidInput, + self.driver.retype, + self.ctxt, + self.RETYPE_VOLUME_INFO_0, + self.RETYPE_VOLUME_TYPE_BAD_CPG, + self.RETYPE_DIFF, + self.RETYPE_HOST) + + expected = [ + mock.call.login(HP3PAR_USER_NAME, HP3PAR_USER_PASS), + mock.call.getCPG( + self.RETYPE_VOLUME_TYPE_BAD_CPG['extra_specs']['cpg']), + mock.call.logout() + ] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_tune(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + qos_ref = qos_specs.create(self.ctxt, 'qos-specs-1', self.QOS) + type_ref = volume_types.create(self.ctxt, + "type1", {"qos:maxIOPS": "100", + "qos:maxBWS": "50", + "qos:minIOPS": "10", + "qos:minBWS": "20", + "qos:latency": "5", + "qos:priority": "high"}) + qos_specs.associate_qos_with_type(self.ctxt, + qos_ref['id'], + type_ref['id']) + + type_ref = volume_types.get_volume_type(self.ctxt, type_ref['id']) + + volume = {'id': HP3PARBaseDriver.CLONE_ID} + + self.driver.retype(self.ctxt, volume, type_ref, None, self.RETYPE_HOST) + + expected = [ + mock.call.modifyVolume('osv-0DM4qZEVSKON-AAAAAAAAA', + {'comment': mock.ANY, + 'snapCPG': 'OpenStackCPGSnap'}), + mock.call.deleteVolumeSet('vvs-0DM4qZEVSKON-AAAAAAAAA'), + mock.call.addVolumeToVolumeSet('myvvs', + 'osv-0DM4qZEVSKON-AAAAAAAAA'), + mock.call.modifyVolume('osv-0DM4qZEVSKON-AAAAAAAAA', + {'action': 6, + 'userCPG': 'OpenStackCPG', + 'conversionOperation': 1, + 'tuneOperation': 1}), + mock.call.getTask(1), + mock.call.logout() + ] + mock_client.assert_has_calls(expected) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_retype_qos_spec(self, _mock_volume_types): + _mock_volume_types.return_value = self.RETYPE_VOLUME_TYPE_1 + mock_client = self.setup_driver(mock_conf=self.RETYPE_CONF) + + cpg = "any_cpg" + snap_cpg = "any_cpg" + self.driver.common._retype(self.volume, + HP3PARBaseDriver.VOLUME_3PAR_NAME, + "old_type", "old_type_id", + HP3PARBaseDriver.RETYPE_HOST, + None, cpg, cpg, snap_cpg, snap_cpg, + True, True, None, None, + self.QOS_SPECS, self.RETYPE_QOS_SPECS, + "{}") + + expected = [ + mock.call.createVolumeSet('vvs-0DM4qZEVSKON-DXN-NwVpw', None), + mock.call.createQoSRules( + 'vvs-0DM4qZEVSKON-DXN-NwVpw', + {'ioMinGoal': 100, 'ioMaxLimit': 1000, + 'bwMinGoalKB': 25600, 'bwMaxLimitKB': 51200, + 'priority': 3, + 'latencyGoal': 25} + ), + mock.call.addVolumeToVolumeSet( + 'vvs-0DM4qZEVSKON-DXN-NwVpw', 'osv-0DM4qZEVSKON-DXN-NwVpw')] + mock_client.assert_has_calls(expected) + def test_delete_volume(self): # setup_mock_client drive with default configuration diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index 23d23f7b6..615d5c1c7 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -52,6 +52,7 @@ from oslo.config import cfg from cinder import context from cinder import exception +from cinder import flow_utils from cinder.openstack.common import excutils from cinder.openstack.common.gettextutils import _ from cinder.openstack.common import log as logging @@ -60,6 +61,8 @@ from cinder.openstack.common import units from cinder.volume import qos_specs from cinder.volume import volume_types +import taskflow.engines +from taskflow.patterns import linear_flow LOG = logging.getLogger(__name__) @@ -133,10 +136,11 @@ class HP3PARCommon(object): 2.0.12 - Volume detach hangs when host is in a host set bug #1317134 2.0.13 - Added support for managing/unmanaging of volumes 2.0.14 - Modified manage volume to use standard 'source-name' element. + 2.0.15 - Added support for volume retype """ - VERSION = "2.0.14" + VERSION = "2.0.15" stats = {} @@ -147,6 +151,10 @@ class HP3PARCommon(object): VLUN_TYPE_MATCHED_SET = 4 VLUN_TYPE_HOST_SET = 5 + THIN = 2 + CONVERT_TO_THIN = 1 + CONVERT_TO_FULL = 2 + # Valid values for volume type extra specs # The first value in the list is the default value valid_prov_values = ['thin', 'full'] @@ -815,42 +823,70 @@ class HP3PARCommon(object): return vol['comment'] return None + def validate_persona(self, persona_value): + """Validate persona value. + + If the passed in persona_value is not valid, raise InvalidInput, + otherwise return the persona ID. + + :param persona_value: + :raises: exception.InvalidInput + :return: persona ID + """ + if persona_value not in self.valid_persona_values: + err = (_("Must specify a valid persona %(valid)s," + "value '%(persona)s' is invalid.") % + ({'valid': self.valid_persona_values, + 'persona': persona_value})) + LOG.error(err) + raise exception.InvalidInput(reason=err) + # persona is set by the id so remove the text and return the id + # i.e for persona '1 - Generic' returns 1 + persona_id = persona_value.split(' ') + return persona_id[0] + def get_persona_type(self, volume, hp3par_keys=None): default_persona = self.valid_persona_values[0] type_id = volume.get('volume_type_id', None) - volume_type = None if type_id is not None: volume_type = self._get_volume_type(type_id) if hp3par_keys is None: hp3par_keys = self._get_keys_by_volume_type(volume_type) persona_value = self._get_key_value(hp3par_keys, 'persona', default_persona) - if persona_value not in self.valid_persona_values: - err = _("Must specify a valid persona %(valid)s, " - "value '%(persona)s' is invalid.") % \ - ({'valid': self.valid_persona_values, - 'persona': persona_value}) - LOG.error(err) - raise exception.InvalidInput(reason=err) - # persona is set by the id so remove the text and return the id - # i.e for persona '1 - Generic' returns 1 - persona_id = persona_value.split(' ') - return persona_id[0] + return self.validate_persona(persona_value) - def get_volume_settings_from_type(self, volume): - cpg = None - snap_cpg = None + def get_type_info(self, type_id): + """Get 3PAR type info for the given type_id. + + Reconciles VV Set, old-style extra-specs, and QOS specs + and returns commonly used info about the type. + + :returns: hp3par_keys, qos, volume_type, vvs_name + """ volume_type = None vvs_name = None hp3par_keys = {} qos = {} - type_id = volume.get('volume_type_id', None) if type_id is not None: volume_type = self._get_volume_type(type_id) hp3par_keys = self._get_keys_by_volume_type(volume_type) vvs_name = self._get_key_value(hp3par_keys, 'vvs') if vvs_name is None: qos = self._get_qos_by_volume_type(volume_type) + return hp3par_keys, qos, volume_type, vvs_name + + def get_volume_settings_from_type_id(self, type_id): + """Get 3PAR volume settings given a type_id. + + Combines type info and config settings to return a dictionary + describing the 3PAR volume settings. Does some validation (CPG). + + :param type_id: + :return: dict + """ + + hp3par_keys, qos, volume_type, vvs_name = self.get_type_info(type_id) cpg = self._get_key_value(hp3par_keys, 'cpg', self.config.hp3par_cpg) @@ -888,14 +924,32 @@ class HP3PARCommon(object): if prov_value == "full": tpvv = False + return {'hp3par_keys': hp3par_keys, + 'cpg': cpg, 'snap_cpg': snap_cpg, + 'vvs_name': vvs_name, 'qos': qos, + 'tpvv': tpvv, 'volume_type': volume_type} + + def get_volume_settings_from_type(self, volume): + """Get 3PAR volume settings given a volume. + + Combines type info and config settings to return a dictionary + describing the 3PAR volume settings. Does some validation (CPG and + persona). + + :param volume: + :return: dict + """ + + type_id = volume.get('volume_type_id', None) + + volume_settings = self.get_volume_settings_from_type_id(type_id) + # check for valid persona even if we don't use it until # attach time, this will give the end user notice that the # persona type is invalid at volume creation time - self.get_persona_type(volume, hp3par_keys) + self.get_persona_type(volume, volume_settings['hp3par_keys']) - return {'cpg': cpg, 'snap_cpg': snap_cpg, - 'vvs_name': vvs_name, 'qos': qos, - 'tpvv': tpvv, 'volume_type': volume_type} + return volume_settings def create_volume(self, volume): LOG.debug("CREATE VOLUME (%s : %s %s)" % @@ -1092,17 +1146,10 @@ class HP3PARCommon(object): extra = {'volume_id': volume['id'], 'snapshot_id': snapshot['id']} - volume_type = None type_id = volume.get('volume_type_id', None) - vvs_name = None - qos = {} - hp3par_keys = {} - if type_id is not None: - volume_type = self._get_volume_type(type_id) - hp3par_keys = self._get_keys_by_volume_type(volume_type) - vvs_name = self._get_key_value(hp3par_keys, 'vvs') - if vvs_name is None: - qos = self._get_qos_by_volume_type(volume_type) + + hp3par_keys, qos, volume_type, vvs_name = self.get_type_info( + type_id) name = volume.get('display_name', None) if name: @@ -1466,3 +1513,438 @@ class HP3PARCommon(object): portPos['slot'] = int(split[1]) portPos['cardPort'] = int(split[2]) return portPos + + def tune_vv(self, old_tpvv, new_tpvv, old_cpg, new_cpg, volume_name): + """Tune the volume to change the userCPG and/or provisioningType. + + The volume will be modified/tuned/converted to the new userCPG and + provisioningType, as needed. + + TaskWaiter is used to make this function wait until the 3PAR task + is no longer active. When the task is no longer active, then it must + either be done or it is in a state that we need to treat as an error. + """ + + if old_tpvv == new_tpvv: + if new_cpg != old_cpg: + LOG.info(_("Modifying %(volume_name)s userCPG from %(old_cpg)s" + " to %(new_cpg)s") % + {'volume_name': volume_name, + 'old_cpg': old_cpg, 'new_cpg': new_cpg}) + response, body = self.client.modifyVolume( + volume_name, + {'action': 6, + 'tuneOperation': 1, + 'userCPG': new_cpg}) + task_id = body['taskid'] + status = self.TaskWaiter(self.client, task_id).wait_for_task() + if status['status'] is not self.client.TASK_DONE: + msg = (_('Tune volume task stopped before it was done: ' + 'volume_name=%(volume_name)s, ' + 'task-status=%(status)s.') % + {'status': status, 'volume_name': volume_name}) + raise exception.VolumeBackendAPIException(msg) + else: + if old_tpvv: + cop = self.CONVERT_TO_FULL + LOG.info(_("Converting %(volume_name)s to full provisioning " + "with userCPG=%(new_cpg)s") % + {'volume_name': volume_name, 'new_cpg': new_cpg}) + else: + cop = self.CONVERT_TO_THIN + LOG.info(_("Converting %(volume_name)s to thin provisioning " + "with userCPG=%(new_cpg)s") % + {'volume_name': volume_name, 'new_cpg': new_cpg}) + + try: + response, body = self.client.modifyVolume( + volume_name, + {'action': 6, + 'tuneOperation': 1, + 'userCPG': new_cpg, + 'conversionOperation': cop}) + except hpexceptions.HTTPBadRequest as ex: + if ex.get_code() == 40 and "keepVV" in str(ex): + # Cannot retype with snapshots because we don't want to + # use keepVV and have straggling volumes. Log additional + # info and then raise. + LOG.info(_("tunevv failed because the volume '%s' " + "has snapshots.") % volume_name) + raise ex + + task_id = body['taskid'] + status = self.TaskWaiter(self.client, task_id).wait_for_task() + if status['status'] is not self.client.TASK_DONE: + msg = (_('Tune volume task stopped before it was done: ' + 'volume_name=%(volume_name)s, ' + 'task-status=%(status)s.') % + {'status': status, 'volume_name': volume_name}) + raise exception.VolumeBackendAPIException(msg) + + def _retype_pre_checks(self, host, new_persona, + old_cpg, new_cpg, + old_snap_cpg, new_snap_cpg): + """Test retype parameters before making retype changes. + + Do pre-retype parameter validation. These checks will + raise an exception if we should not attempt this retype. + """ + + if new_persona: + self.validate_persona(new_persona) + + (host_type, host_id, host_cpg) = ( + host['capabilities']['location_info']).split(':') + + if not (host_type == 'HP3PARDriver'): + reason = (_("Cannot retype from HP3PARDriver to %s.") % host_type) + raise exception.InvalidHost(reason) + + sys_info = self.client.getStorageSystemInfo() + if not (host_id == sys_info['serialNumber']): + reason = (_("Cannot retype from one 3PAR array to another.")) + raise exception.InvalidHost(reason) + + if not old_snap_cpg: + reason = (_("Invalid current snapCPG name for retype. The volume " + "may be in a transitioning state. snapCpg='%s'.") % + old_snap_cpg) + raise exception.InvalidVolume(reason) + + # Validate new_snap_cpg. A white-space snapCPG will fail eventually, + # but we'd prefer to fail fast -- if this ever happens. + if not new_snap_cpg or new_snap_cpg.isspace(): + reason = (_("Invalid new snapCPG name for retype. " + "new_snap_cpg='%s'.") % new_snap_cpg) + raise exception.InvalidInput(reason) + + # Check to make sure CPGs are in the same domain + if self.get_domain(old_cpg) != self.get_domain(new_cpg): + reason = (_('Cannot retype to a CPG in a different domain.')) + raise exception.Invalid3PARDomain(reason) + + if self.get_domain(old_snap_cpg) != self.get_domain(new_snap_cpg): + reason = (_('Cannot retype to a snap CPG in a different domain.')) + raise exception.Invalid3PARDomain(reason) + + def _retype(self, volume, volume_name, new_type_name, new_type_id, host, + new_persona, old_cpg, new_cpg, old_snap_cpg, new_snap_cpg, + old_tpvv, new_tpvv, old_vvs, new_vvs, old_qos, new_qos, + old_comment): + + action = "volume:retype" + + self._retype_pre_checks(host, new_persona, + old_cpg, new_cpg, + old_snap_cpg, new_snap_cpg) + + flow_name = action.replace(":", "_") + "_api" + retype_flow = linear_flow.Flow(flow_name) + # Keep this linear and do the big tunevv last. Everything leading + # up to that is reversible, but we'd let the 3PAR deal with tunevv + # errors on its own. + retype_flow.add( + ModifyVolumeTask(action), + ModifySpecsTask(action), + TuneVolumeTask(action)) + + taskflow.engines.run( + retype_flow, + store={'common': self, + 'volume_name': volume_name, 'volume': volume, + 'old_tpvv': old_tpvv, 'new_tpvv': new_tpvv, + 'old_cpg': old_cpg, 'new_cpg': new_cpg, + 'old_snap_cpg': old_snap_cpg, 'new_snap_cpg': new_snap_cpg, + 'old_vvs': old_vvs, 'new_vvs': new_vvs, + 'old_qos': old_qos, 'new_qos': new_qos, + 'new_type_name': new_type_name, 'new_type_id': new_type_id, + 'old_comment': old_comment + }) + + def retype(self, volume, new_type, diff, host): + """Convert the volume to be of the new type. + + Returns True if the retype was successful. + Uses taskflow to revert changes if errors occur. + + :param volume: A dictionary describing the volume to retype + :param new_type: A dictionary describing the volume type to convert to + :param diff: A dictionary with the difference between the two types + :param host: A dictionary describing the host, where + host['host'] is its name, and host['capabilities'] is a + dictionary of its reported capabilities. + """ + LOG.debug(("enter: retype: id=%(id)s, new_type=%(new_type)s," + "diff=%(diff)s, host=%(host)s") % {'id': volume['id'], + 'new_type': new_type, + 'diff': diff, + 'host': host}) + volume_id = volume['id'] + volume_name = self._get_3par_vol_name(volume_id) + + new_type_name = new_type['name'] + new_type_id = new_type['id'] + new_volume_settings = self.get_volume_settings_from_type_id( + new_type_id) + new_cpg = new_volume_settings['cpg'] + new_snap_cpg = new_volume_settings['snap_cpg'] + new_tpvv = new_volume_settings['tpvv'] + new_qos = new_volume_settings['qos'] + new_vvs = new_volume_settings['vvs_name'] + new_persona = None + new_hp3par_keys = new_volume_settings['hp3par_keys'] + if 'persona' in new_hp3par_keys: + new_persona = new_hp3par_keys['persona'] + + old_volume_settings = self.get_volume_settings_from_type(volume) + old_qos = old_volume_settings['qos'] + old_vvs = old_volume_settings['vvs_name'] + + # Get the current volume info because we can get in a bad state + # if we trust that all the volume type settings are still the + # same settings that were used with this volume. + old_volume_info = self.client.getVolume(volume_name) + old_tpvv = old_volume_info['provisioningType'] == self.THIN + old_cpg = old_volume_info['userCPG'] + old_comment = old_volume_info['comment'] + old_snap_cpg = None + if 'snapCPG' in old_volume_info: + old_snap_cpg = old_volume_info['snapCPG'] + + LOG.debug("retype old_volume_info=%s" % old_volume_info) + LOG.debug("retype old_volume_settings=%s" % old_volume_settings) + LOG.debug("retype new_volume_settings=%s" % new_volume_settings) + + self._retype(volume, volume_name, new_type_name, new_type_id, + host, new_persona, old_cpg, new_cpg, + old_snap_cpg, new_snap_cpg, old_tpvv, new_tpvv, + old_vvs, new_vvs, old_qos, new_qos, old_comment) + return True + + class TaskWaiter(object): + """TaskWaiter waits for task to be not active and returns status.""" + + def __init__(self, client, task_id, interval=1, initial_delay=0): + self.client = client + self.task_id = task_id + self.interval = interval + self.initial_delay = initial_delay + + def _wait_for_task(self): + status = self.client.getTask(self.task_id) + LOG.debug("3PAR Task id %(id)s status = %(status)s" % + {'id': self.task_id, + 'status': status['status']}) + if status['status'] is not self.client.TASK_ACTIVE: + raise loopingcall.LoopingCallDone(status) + + def wait_for_task(self): + timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_task) + return timer.start(interval=self.interval, + initial_delay=self.initial_delay).wait() + + +class ModifyVolumeTask(flow_utils.CinderTask): + + """Task to change a volume's snapCPG and comment. + + This is a task for changing the snapCPG and comment. It is intended for + use during retype(). These changes are done together with a single + modify request which should be fast and easy to revert. + + Because we do not support retype with existing snapshots, we can change + the snapCPG without using a keepVV. If snapshots exist, then this will + fail, as desired. + + This task does not change the userCPG or provisioningType. Those changes + may require tunevv, so they are done by the TuneVolumeTask. + + The new comment will contain the new type, VVS and QOS information along + with whatever else was in the old comment dict. + + The old comment and snapCPG are restored if revert is called. + """ + + def __init__(self, action): + self.needs_revert = False + super(ModifyVolumeTask, self).__init__(addons=[action]) + + def _get_new_comment(self, old_comment, new_vvs, new_qos, + new_type_name, new_type_id): + # Modify the comment during ModifyVolume + comment_dict = dict(ast.literal_eval(old_comment)) + if 'vvs' in comment_dict: + del comment_dict['vvs'] + if 'qos' in comment_dict: + del comment_dict['qos'] + if new_vvs: + comment_dict['vvs'] = new_vvs + elif new_qos: + comment_dict['qos'] = new_qos + else: + comment_dict['qos'] = {} + comment_dict['volume_type_name'] = new_type_name + comment_dict['volume_type_id'] = new_type_id + return comment_dict + + def execute(self, common, volume_name, old_snap_cpg, new_snap_cpg, + old_comment, new_vvs, new_qos, new_type_name, new_type_id): + + comment_dict = self._get_new_comment( + old_comment, new_vvs, new_qos, new_type_name, new_type_id) + + if new_snap_cpg != old_snap_cpg: + # Modify the snap_cpg. This will fail with snapshots. + LOG.info(_("Modifying %(volume_name)s snap_cpg from " + "%(old_snap_cpg)s to %(new_snap_cpg)s.") % + {'volume_name': volume_name, + 'old_snap_cpg': old_snap_cpg, + 'new_snap_cpg': new_snap_cpg}) + common.client.modifyVolume( + volume_name, + {'snapCPG': new_snap_cpg, + 'comment': json.dumps(comment_dict)}) + self.needs_revert = True + else: + LOG.info(_("Modifying %s comments.") % volume_name) + common.client.modifyVolume( + volume_name, + {'comment': json.dumps(comment_dict)}) + self.needs_revert = True + + def revert(self, common, volume_name, old_snap_cpg, new_snap_cpg, + old_comment, **kwargs): + if self.needs_revert: + LOG.info(_("Retype revert %(volume_name)s snap_cpg from " + "%(new_snap_cpg)s back to %(old_snap_cpg)s.") % + {'volume_name': volume_name, + 'new_snap_cpg': new_snap_cpg, + 'old_snap_cpg': old_snap_cpg}) + try: + common.client.modifyVolume( + volume_name, + {'snapCPG': old_snap_cpg, 'comment': old_comment}) + except Exception as ex: + LOG.error(_("Exception during snapCPG revert: %s") % ex) + + +class TuneVolumeTask(flow_utils.CinderTask): + + """Task to change a volume's CPG and/or provisioning type. + + This is a task for changing the CPG and/or provisioning type. It is + intended for use during retype(). This task has no revert. The current + design is to do this task last and do revert-able tasks first. Un-doing a + tunevv can be expensive and should be avoided. + """ + + def __init__(self, action, **kwargs): + super(TuneVolumeTask, self).__init__(addons=[action]) + + def execute(self, common, old_tpvv, new_tpvv, old_cpg, new_cpg, + volume_name): + common.tune_vv(old_tpvv, new_tpvv, old_cpg, new_cpg, volume_name) + + +class ModifySpecsTask(flow_utils.CinderTask): + + """Set/unset the QOS settings and/or VV set for the volume's new type. + + This is a task for changing the QOS settings and/or VV set. It is intended + for use during retype(). If changes are made during execute(), then they + need to be undone if revert() is called (i.e., if a later task fails). + + For 3PAR, we ignore QOS settings if a VVS is explicitly set, otherwise we + create a VV set and use that for QOS settings. That is why they are lumped + together here. Most of the decision-making about VVS vs. QOS settings vs. + old-style scoped extra-specs is handled in existing reusable code. Here + we mainly need to know what old stuff to remove before calling the function + that knows how to set the new stuff. + + Basic task flow is as follows: Remove the volume from the old externally + created VVS (when appropriate), delete the old cinder-created VVS, call + the function that knows how to set a new VVS or QOS settings. + + If any changes are made during execute, then revert needs to reverse them. + """ + + def __init__(self, action): + self.needs_revert = False + super(ModifySpecsTask, self).__init__(addons=[action]) + + def execute(self, common, volume_name, volume, old_cpg, new_cpg, + old_vvs, new_vvs, old_qos, new_qos): + + if old_vvs != new_vvs or old_qos != new_qos: + + # Remove VV from old VV Set. + if old_vvs is not None and old_vvs != new_vvs: + common.client.removeVolumeFromVolumeSet(old_vvs, + volume_name) + self.needs_revert = True + + # If any extra or qos specs changed then remove the old + # special VV set that we create. We'll recreate it + # as needed. + vvs_name = common._get_3par_vvs_name(volume['id']) + try: + common.client.deleteVolumeSet(vvs_name) + self.needs_revert = True + except hpexceptions.HTTPNotFound as ex: + # HTTPNotFound(code=102) is OK. Set does not exist. + if ex.get_code() != 102: + LOG.error( + _("Unexpected error when retype() tried to " + "deleteVolumeSet(%s)") % vvs_name) + raise ex + + if new_vvs or new_qos: + common._add_volume_to_volume_set( + volume, volume_name, new_cpg, new_vvs, new_qos) + self.needs_revert = True + + def revert(self, common, volume_name, volume, old_vvs, new_vvs, old_qos, + old_cpg, **kwargs): + if self.needs_revert: + # If any extra or qos specs changed then remove the old + # special VV set that we create and recreate it per + # the old type specs. + vvs_name = common._get_3par_vvs_name(volume['id']) + try: + common.client.deleteVolumeSet(vvs_name) + except hpexceptions.HTTPNotFound as ex: + # HTTPNotFound(code=102) is OK. Set does not exist. + if ex.get_code() != 102: + LOG.error( + _("Unexpected error when retype() revert " + "tried to deleteVolumeSet(%s)") % vvs_name) + except Exception: + LOG.error( + _("Unexpected error when retype() revert " + "tried to deleteVolumeSet(%s)") % vvs_name) + + if old_vvs is not None or old_qos is not None: + try: + common._add_volume_to_volume_set( + volume, volume_name, old_cpg, old_vvs, old_qos) + except Exception as ex: + LOG.error( + _("%(exception)s: Exception during revert of " + "retype for volume %(volume_name)s. " + "Original volume set/QOS settings may not " + "have been fully restored.") % + {'exception': ex, 'volume_name': volume_name}) + + if new_vvs is not None and old_vvs != new_vvs: + try: + common.client.removeVolumeFromVolumeSet( + new_vvs, volume_name) + except Exception as ex: + LOG.error( + _("%(exception)s: Exception during revert of " + "retype for volume %(volume_name)s. " + "Failed to remove from new volume set " + "%(new_vvs)s.") % + {'exception': ex, + 'volume_name': volume_name, + 'new_vvs': new_vvs}) diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py index 13b200b0b..ffa9ca80f 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_fc.py +++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py @@ -65,10 +65,11 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): 2.0.3 - Added initiator-target map for FC Zone Manager 2.0.4 - Added support for managing/unmanaging of volumes 2.0.5 - Only remove FC Zone on last volume detach + 2.0.6 - Added support for volume retype """ - VERSION = "2.0.5" + VERSION = "2.0.6" def __init__(self, *args, **kwargs): super(HP3PARFCDriver, self).__init__(*args, **kwargs) @@ -404,6 +405,15 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): def detach_volume(self, context, volume): self.common.detach_volume(volume) + @utils.synchronized('3par', external=True) + def retype(self, context, volume, new_type, diff, host): + """Convert the volume to be of the new type.""" + self.common.client_login() + try: + return self.common.retype(volume, new_type, diff, host) + finally: + self.common.client_logout() + @utils.synchronized('3par', external=True) def migrate_volume(self, context, volume, host): self.common.client_login() diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py index a631a427d..e8374814d 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py @@ -66,10 +66,11 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): 2.0.0 - Update hp3parclient API uses 3.0.x 2.0.2 - Add back-end assisted volume migrate 2.0.3 - Added support for managing/unmanaging of volumes + 2.0.4 - Added support for volume retype """ - VERSION = "2.0.3" + VERSION = "2.0.4" def __init__(self, *args, **kwargs): super(HP3PARISCSIDriver, self).__init__(*args, **kwargs) @@ -479,6 +480,15 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): def detach_volume(self, context, volume): self.common.detach_volume(volume) + @utils.synchronized('3par', external=True) + def retype(self, context, volume, new_type, diff, host): + """Convert the volume to be of the new type.""" + self.common.client_login() + try: + return self.common.retype(volume, new_type, diff, host) + finally: + self.common.client_logout() + @utils.synchronized('3par', external=True) def migrate_volume(self, context, volume, host): self.common.client_login()