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 274145a84c382ea3111d2abf6ddbc68e4ea76428..6756a8b9d4a3fbf516d65833f469192d83c0bc54 100644 (file)
--- 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 3e26e0a7fb06bac4cbceb3bd3304c513528f4c4a..1ff175081fd1c065a5cb78564e383f1ca66e65bb 100644 (file)
--- 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)