From: Mark Sturdevant Date: Fri, 12 Sep 2014 21:03:28 +0000 (-0700) Subject: HP 3PAR: Allow retype when the old snapshot CPG (3PAR pool) is None X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=28539accb581e7632edbd88493fb72c165bdeaac;p=openstack-build%2Fcinder-build.git HP 3PAR: Allow retype when the old snapshot CPG (3PAR pool) is None A common provisioning group (CPG) is a virtual pool of logical disks. A volume is stored in a CPG. The volume's snapshots may be stored in the same CPG or in a separate CPG. In 3PAR this is the "snapCPG". Before the 3PAR driver supported "manage", the snapCPG was never None in OpenStack because when we create volumes, we explicitly default snapCPG to match the volume CPG unless otherwise specified. So, the original retype pre-checks raised an exception if this unexpected case occurred and a unit test was provided for that exception. Now that we support manage_existing(), it should be a valid use case to manage an existing volume created outside of OpenStack with a snapCPG set to None. Unfortunately, when we applied volume-type settings to this volume we would hit that old exception. That exception has been removed. Removing it required the following changes: 1. When retype pre-checks validate the domain of the new snapCPG, it will not use the optional old snapCPG. Instead it will compare the new snapCPG domain with the old volume CPG domain. This satisfies the requirement that domains cannot be mixed and avoids the need to have an old snapCPG setting. 2. Remove the no longer used old_snap_cpg parameter and remove the code that raised an exception "if not old_snap_cpg". 3. Remove the unit test that was just to test that obsolete exception. 4. Adjust the exiting test_retype_across_snap_cpg_domains to verify that a snapCPG domain that does not match the volume CPG domain will still raise Invalid3PARDomain Change-Id: Icd2c66898ef64f81116df102f5529a8da30c14d5 Closes-Bug: 1368927 --- diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index 274145a84..6756a8b9d 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -594,27 +594,6 @@ class HP3PARBaseDriver(object): 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 = [ @@ -738,7 +717,6 @@ class HP3PARBaseDriver(object): {'domain': 'cpg_domain'}, {'domain': 'cpg_domain'}, {'domain': 'snap_cpg_domain_1'}, - {'domain': 'snap_cpg_domain_2'}, ] self.assertRaises(exception.Invalid3PARDomain, @@ -755,7 +733,6 @@ class HP3PARBaseDriver(object): 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() diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index 3e26e0a7f..1ff175081 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -148,10 +148,11 @@ class HP3PARCommon(object): 2.0.18 - HP 3PAR manage_existing with volume-type support 2.0.19 - Update default persona from Generic to Generic-ALUA 2.0.20 - Configurable SSH missing key policy and known hosts file + 2.0.21 - Remove bogus invalid snapCPG=None exception """ - VERSION = "2.0.20" + VERSION = "2.0.21" stats = {} @@ -1634,7 +1635,7 @@ class HP3PARCommon(object): def _retype_pre_checks(self, host, new_persona, old_cpg, new_cpg, - old_snap_cpg, new_snap_cpg): + new_snap_cpg): """Test retype parameters before making retype changes. Do pre-retype parameter validation. These checks will @@ -1658,12 +1659,6 @@ class HP3PARCommon(object): 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(): @@ -1672,11 +1667,12 @@ class HP3PARCommon(object): 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): + domain = self.get_domain(old_cpg) + if domain != 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): + if domain != self.get_domain(new_snap_cpg): reason = (_('Cannot retype to a snap CPG in a different domain.')) raise exception.Invalid3PARDomain(reason) @@ -1689,7 +1685,7 @@ class HP3PARCommon(object): self._retype_pre_checks(host, new_persona, old_cpg, new_cpg, - old_snap_cpg, new_snap_cpg) + new_snap_cpg) flow_name = action.replace(":", "_") + "_api" retype_flow = linear_flow.Flow(flow_name)