From a1fa72b9e02973c064136c1f723d2af2558e578b Mon Sep 17 00:00:00 2001 From: Mark Sturdevant Date: Fri, 12 Sep 2014 23:48:16 -0700 Subject: [PATCH] HP 3PAR: Don't ignore extra-specs snap_cpg when missing cpg When snap_cpg is specified in extra-specs, it should be used. For some reason, it was being ignored when extra-specs did not also specify a user cpg. When using a volume-type, the snapCPG should come from (in this order of preference): 1. extra specs snap_cpg, 2. extra specs cpg, 3. config hp3par_cpg_snap, 4. config hp3par_cpg. Change-Id: I649e3dc49f7775a8071229fc315f0ee5f23aa833 Closes-Bug: 1368972 --- cinder/tests/test_hp3par.py | 75 ++++++++++++++++++- .../volume/drivers/san/hp/hp_3par_common.py | 16 ++-- 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index 78f2d9470..f11ea6116 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -559,6 +559,72 @@ class HP3PARBaseDriver(object): mock_client.assert_has_calls(expected) self.assertEqual(return_model, None) + @mock.patch.object(volume_types, 'get_volume_type') + def test_get_snap_cpg_from_volume_type(self, _mock_volume_types): + + self.setup_driver() + expected_type_snap_cpg = "type_snap_cpg" + _mock_volume_types.return_value = { + 'name': 'gold', + 'extra_specs': { + 'cpg': HP3PAR_CPG, + 'snap_cpg': expected_type_snap_cpg, + 'volume_type': self.volume_type}} + + result = self.driver.common.get_volume_settings_from_type_id( + "mock", self.driver.configuration.hp3par_cpg) + + self.assertEqual(expected_type_snap_cpg, result['snap_cpg']) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_get_snap_cpg_from_volume_type_cpg(self, _mock_volume_types): + + self.setup_driver() + expected_cpg = 'use_extra_specs_cpg' + _mock_volume_types.return_value = { + 'name': 'gold', + 'extra_specs': { + 'cpg': expected_cpg, + 'volume_type': self.volume_type}} + + result = self.driver.common.get_volume_settings_from_type_id( + "mock", self.driver.configuration.hp3par_cpg) + + self.assertEqual(expected_cpg, result['snap_cpg']) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_get_snap_cpg_from_volume_type_conf_snap_cpg( + self, _mock_volume_types): + _mock_volume_types.return_value = { + 'name': 'gold', + 'extra_specs': { + 'volume_type': self.volume_type}} + + conf = self.setup_configuration() + expected_snap_cpg = conf.hp3par_cpg_snap + self.setup_driver(config=conf) + result = self.driver.common.get_volume_settings_from_type_id( + "mock", self.driver.configuration.hp3par_cpg) + + self.assertEqual(expected_snap_cpg, result['snap_cpg']) + + @mock.patch.object(volume_types, 'get_volume_type') + def test_get_snap_cpg_from_volume_type_conf_cpg( + self, _mock_volume_types): + _mock_volume_types.return_value = { + 'name': 'gold', + 'extra_specs': { + 'volume_type': self.volume_type}} + + conf = self.setup_configuration() + conf.hp3par_cpg_snap = None + expected_cpg = conf.hp3par_cpg + self.setup_driver(config=conf) + result = self.driver.common.get_volume_settings_from_type_id( + "mock", self.driver.configuration.hp3par_cpg) + + self.assertEqual(expected_cpg, result['snap_cpg']) + @mock.patch.object(volume_types, 'get_volume_type') def test_create_volume_qos(self, _mock_volume_types): # setup_mock_client drive with default configuration @@ -1016,7 +1082,7 @@ class HP3PARBaseDriver(object): mock.call.modifyVolume( osv_matcher, {'comment': '{"qos": {}, "display_name": "Foo Volume"}', - 'snapCPG': 'CPG-FC1'}), + 'snapCPG': HP3PAR_CPG_SNAP}), mock.call.modifyVolume(osv_matcher, {'action': 6, 'userCPG': 'CPG-FC1', @@ -1171,7 +1237,7 @@ class HP3PARBaseDriver(object): mock.call.modifyVolume( osv_matcher, {'comment': '{"qos": {}, "display_name": "Foo Volume"}', - 'snapCPG': 'CPG-FC1'}), + 'snapCPG': HP3PAR_CPG_SNAP}), mock.call.modifyVolume(osv_matcher, {'action': 6, 'userCPG': 'CPG-FC1', @@ -1884,11 +1950,12 @@ class HP3PARBaseDriver(object): } } + expected_snap_cpg = self.volume_type['extra_specs']['cpg'] expected_retype_modify = [ mock.call.modifyVolume(osv_matcher, {'comment': self.CommentMatcher( self.assertEqual, retype_comment_qos), - 'snapCPG': 'OpenStackCPGSnap'}), + 'snapCPG': expected_snap_cpg}), mock.call.deleteVolumeSet(vvs_matcher), ] @@ -2151,7 +2218,7 @@ class HP3PARBaseDriver(object): mock.call.getVolume(unm_matcher), mock.call.modifyVolume( unm_matcher, {'newName': osv_matcher, 'comment': mock.ANY}), - mock.call.getCPG('POOL1'), + mock.call.getCPG('OpenStackCPG'), mock.call.getVolume(osv_matcher), mock.call.getCPG('testUserCpg0'), mock.call.getCPG('OpenStackCPG'), diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index f4c75f651..b93e204fd 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -156,10 +156,11 @@ class HP3PARCommon(object): 2.0.23 - Increase the hostname size from 23 to 31 Bug #1371242 2.0.24 - Add pools (hp3par_cpg now accepts a list of CPGs) 2.0.25 - Migrate without losing type settings bug #1356608 + 2.0.26 - Don't ignore extra-specs snap_cpg when missing cpg #1368972 """ - VERSION = "2.0.25" + VERSION = "2.0.26" stats = {} @@ -965,7 +966,7 @@ class HP3PARCommon(object): default_cpg = pool or self.config.hp3par_cpg[0] cpg = self._get_key_value(hp3par_keys, 'cpg', default_cpg) - if cpg not in self.config.hp3par_cpg: + if cpg is not default_cpg: # The cpg was specified in a volume type extra spec so it # needs to be validated that it's in the correct domain. self.validate_cpg(cpg) @@ -974,12 +975,13 @@ class HP3PARCommon(object): # default. snap_cpg = self._get_key_value(hp3par_keys, 'snap_cpg', cpg) else: - # default snap_cpg to hp3par_cpg_snap if it's not specified - # in the volume type extra specs. + # Look to see if the snap_cpg was specified in volume type + # extra spec, if not use hp3par_cpg_snap from config as the + # default. snap_cpg = self.config.hp3par_cpg_snap - # if it's still not set or empty then set it to the cpg - # specified in the cinder.conf file. - if not self.config.hp3par_cpg_snap: + snap_cpg = self._get_key_value(hp3par_keys, 'snap_cpg', snap_cpg) + # If it's still not set or empty then set it to the cpg. + if not snap_cpg: snap_cpg = cpg # if provisioning is not set use thin -- 2.45.2