From ba87053896ef80586719565d07ec76ea846cc5a4 Mon Sep 17 00:00:00 2001 From: Kurt Martin Date: Wed, 31 Jul 2013 13:56:29 -0700 Subject: [PATCH] Remove unnecessary metadata from the 3PAR drivers Currently, both the HP 3PAR iSCSI and FC drivers are populating the volume and snapshot metadata field with unnecessary data that should actually be stored on the backend. This data should not be accessible by the user which it is today and could be updated as well. This patch will remove all custom 3PAR data from the metadata fields. Change-Id: I3086be3856192a27fa1db1ada89d95c290c68df7 Fixes: bug 1203144 --- cinder/tests/test_hp3par.py | 102 ++++++++---------- .../volume/drivers/san/hp/hp_3par_common.py | 43 ++++---- cinder/volume/drivers/san/hp/hp_3par_fc.py | 2 +- cinder/volume/drivers/san/hp/hp_3par_iscsi.py | 2 +- 4 files changed, 67 insertions(+), 82 deletions(-) diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index 9e6935382..7b562a7c6 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -18,6 +18,7 @@ """ Unit tests for OpenStack Cinder volume drivers """ +import ast import mox import shutil import tempfile @@ -305,7 +306,7 @@ class HP3PARBaseDriver(): VOLUME_ID_SNAP = '761fc5e5-5191-4ec7-aeba-33e36de44156' FAKE_DESC = 'test description name' FAKE_FC_PORTS = ['0987654321234', '123456789000987'] - QOS = "{'qos:maxIOPS': 1000, 'qos:maxBWS': 50}" + QOS = {'qos:maxIOPS': '1000', 'qos:maxBWS': '50'} FAKE_ISCSI_PORTS = {'1.1.1.2': {'nsp': '8:1:1', 'iqn': ('iqn.2000-05.com.3pardata:' '21810002ac00383d'), @@ -383,6 +384,8 @@ class HP3PARBaseDriver(): self.fake_create_3par_vlun) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_ports", self.fake_get_ports) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) @@ -393,6 +396,9 @@ class HP3PARBaseDriver(): def fake_create_client(self): return FakeHP3ParClient(self.driver.configuration.hp3par_api_url) + def fake_get_cpg(self, volume): + return HP3PAR_CPG + def fake_get_domain(self, cpg): return HP3PAR_DOMAIN @@ -440,16 +446,14 @@ class HP3PARBaseDriver(): def fake_copy_volume(self, src_name, dest_name): pass - def fake_get_volume_state(self, vol_name): + def fake_get_volume_stats(self, vol_name): return "normal" def test_create_volume(self): self.flags(lock_path=self.tempdir) - model_update = self.driver.create_volume(self.volume) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) + self.driver.create_volume(self.volume) + volume = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME) + self.assertEqual(volume['name'], self.VOLUME_3PAR_NAME) def test_create_volume_qos(self): self.flags(lock_path=self.tempdir) @@ -461,12 +465,11 @@ class HP3PARBaseDriver(): self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_add_volume_to_volume_set", self.fake_add_volume_to_volume_set) - model_update = self.driver.create_volume(self.volume_qos) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) - self.assertEqual(metadata['qos'], True) + self.driver.create_volume(self.volume_qos) + volume = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME) + + self.assertEqual(volume['name'], self.VOLUME_3PAR_NAME) + self.assertNotIn(self.QOS, dict(ast.literal_eval(volume['comment']))) def test_delete_volume(self): self.flags(lock_path=self.tempdir) @@ -475,6 +478,23 @@ class HP3PARBaseDriver(): self.driver.common.client.getVolume, self.VOLUME_ID) + def test_create_cloned_volume(self): + self.flags(lock_path=self.tempdir) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_volume_stats", + self.fake_get_volume_stats) + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume", + self.fake_copy_volume) + self.state_tries = 0 + volume = {'name': HP3PARBaseDriver.VOLUME_NAME, + 'id': HP3PARBaseDriver.CLONE_ID, + 'display_name': 'Foo Volume', + 'size': 2, + 'host': HP3PARBaseDriver.FAKE_HOST, + 'source_volid': HP3PARBaseDriver.VOLUME_ID} + src_vref = {} + model_update = self.driver.create_cloned_volume(volume, src_vref) + self.assertTrue(model_update is not None) + def test_create_snapshot(self): self.flags(lock_path=self.tempdir) self.driver.create_snapshot(self.snapshot) @@ -520,12 +540,10 @@ class HP3PARBaseDriver(): self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_add_volume_to_volume_set", self.fake_add_volume_to_volume_set) - model_update = self.driver.create_volume_from_snapshot(self.volume_qos, - self.snapshot) - metadata = model_update['metadata'] - self.assertEqual(metadata['qos'], True) + self.driver.create_volume_from_snapshot(self.volume_qos, self.snapshot) snap_vol = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME) self.assertEqual(snap_vol['name'], self.VOLUME_3PAR_NAME) + self.assertNotIn(self.QOS, dict(ast.literal_eval(snap_vol['comment']))) volume = self.volume.copy() volume['size'] = 1 @@ -635,25 +653,6 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): self.assertEquals(self.VOLUME_3PAR_NAME, vlun['volumeName']) self.assertEquals(self.FAKE_HOST, vlun['hostname']) - def test_create_cloned_volume(self): - self.flags(lock_path=self.tempdir) - self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume", - self.fake_copy_volume) - self.state_tries = 0 - volume = {'name': HP3PARBaseDriver.VOLUME_NAME, - 'id': HP3PARBaseDriver.CLONE_ID, - 'display_name': 'Foo Volume', - 'size': 2, - 'host': HP3PARBaseDriver.FAKE_HOST, - 'source_volid': HP3PARBaseDriver.VOLUME_ID} - src_vref = {} - model_update = self.driver.create_cloned_volume(volume, src_vref) - self.assertTrue(model_update is not None) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) - def test_get_volume_stats(self): self.flags(lock_path=self.tempdir) @@ -687,6 +686,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) @@ -710,6 +711,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) @@ -737,6 +740,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) @@ -839,25 +844,6 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): self.assertEquals(self.VOLUME_3PAR_NAME, vlun['volumeName']) self.assertEquals(self.FAKE_HOST, vlun['hostname']) - def test_create_cloned_volume(self): - self.flags(lock_path=self.tempdir) - self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume", - self.fake_copy_volume) - self.state_tries = 0 - volume = {'name': HP3PARBaseDriver.VOLUME_NAME, - 'id': HP3PARBaseDriver.CLONE_ID, - 'display_name': 'Foo Volume', - 'size': 2, - 'host': HP3PARBaseDriver.FAKE_HOST, - 'source_volid': HP3PARBaseDriver.VOLUME_ID} - src_vref = {} - model_update = self.driver.create_cloned_volume(volume, src_vref) - self.assertTrue(model_update is not None) - metadata = model_update['metadata'] - self.assertFalse(metadata['3ParName'] is None) - self.assertEqual(metadata['CPG'], HP3PAR_CPG) - self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP) - def test_get_volume_stats(self): self.flags(lock_path=self.tempdir) @@ -891,6 +877,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) @@ -915,6 +903,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) @@ -942,6 +932,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase): #record self.clear_mox() + self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg", + self.fake_get_cpg) self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain", self.fake_get_domain) _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh) diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index 60bc48d1a..6f20a6589 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -38,7 +38,7 @@ for credentials to talk to the REST service on the 3PAR array. """ - +import ast import base64 import json import paramiko @@ -653,6 +653,15 @@ exit def _remove_volume_from_volume_set(self, volume_name, vvs_name): self._cli_run('removevvset -f %s %s' % (vvs_name, volume_name), None) + def get_cpg(self, volume): + volume_name = self._get_3par_vol_name(volume['id']) + vol = self.client.getVolume(volume_name) + return vol['userCPG'] + + def _get_3par_vol_comment(self, volume_name): + vol = self.client.getVolume(volume_name) + return vol['comment'] + def get_persona_type(self, volume, hp3par_keys=None): default_persona = self.valid_persona_values[0] type_id = volume.get('volume_type_id', None) @@ -692,7 +701,6 @@ exit vvs_name = None hp3par_keys = {} qos = {} - qos_on_volume = False type_id = volume.get('volume_type_id', None) if type_id is not None: volume_type = self._get_volume_type(type_id) @@ -700,8 +708,6 @@ exit vvs_name = self._get_key_value(hp3par_keys, 'vvs') if vvs_name is None: qos = self._get_qos_by_volume_type(volume_type) - if qos: - qos_on_volume = True cpg = self._get_key_value(hp3par_keys, 'cpg', self.config.hp3par_cpg) @@ -780,11 +786,6 @@ exit LOG.error(str(ex)) raise exception.CinderException(ex.get_description()) - metadata = {'3ParName': volume_name, 'CPG': cpg, - 'snapCPG': extras['snapCPG'], 'qos': qos_on_volume, - 'vvs': vvs_name} - return metadata - def _copy_volume(self, src_name, dest_name): self._cli_run('createvvcopy -p %s %s' % (src_name, dest_name), None) @@ -798,12 +799,10 @@ exit word = re.search(search_string.strip(' ') + ' ([^ ]*)', s) return word.groups()[0].strip(' ') - def get_volume_metadata_value(self, volume, key): - metadata = volume.get('volume_metadata') - if metadata: - for i in volume['volume_metadata']: - if i['key'] == key: - return i['value'] + def _get_3par_vol_comment_value(self, vol_comment, key): + comment_dict = dict(ast.literal_eval(vol_comment)) + if key in comment_dict: + return comment_dict[key] return None def create_cloned_volume(self, volume, src_vref): @@ -833,11 +832,12 @@ exit def delete_volume(self, volume): try: volume_name = self._get_3par_vol_name(volume['id']) - qos = self.get_volume_metadata_value(volume, 'qos') - vvs_name = self.get_volume_metadata_value(volume, 'vvs') + vol_comment = self._get_3par_vol_comment(volume_name) + qos = self._get_3par_vol_comment_value(vol_comment, 'qos') + vvs_name = self._get_3par_vol_comment_value(vol_comment, 'vvs') if vvs_name is not None: self._remove_volume_from_volume_set(volume_name, vvs_name) - elif qos: + elif qos is not None: self._remove_volume_set(self._get_3par_vvs_name(volume['id'])) self.client.deleteVolume(volume_name) except hpexceptions.HTTPNotFound as ex: @@ -878,7 +878,6 @@ exit type_id = volume.get('volume_type_id', None) vvs_name = None qos = {} - qos_on_volume = False hp3par_keys = {} if type_id is not None: volume_type = self._get_volume_type(type_id) @@ -886,8 +885,6 @@ exit vvs_name = self._get_key_value(hp3par_keys, 'vvs') if vvs_name is None: qos = self._get_qos_by_volume_type(volume_type) - if qos: - qos_on_volume = True name = snapshot.get('display_name', None) if name: @@ -920,10 +917,6 @@ exit LOG.error(str(ex)) raise exception.CinderException(ex.get_description()) - metadata = {'3ParName': volume_name, 'qos': qos_on_volume, - 'vvs': vvs_name} - return metadata - def create_snapshot(self, snapshot): LOG.debug("Create Snapshot\n%s" % pprint.pformat(snapshot)) diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py index 0553ae6f0..e627886d2 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_fc.py +++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py @@ -220,7 +220,7 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver): """Creates or modifies existing 3PAR host.""" host = None hostname = self.common._safe_hostname(connector['host']) - cpg = self.common.get_volume_metadata_value(volume, 'CPG') + cpg = self.common.get_cpg(volume) domain = self.common.get_domain(cpg) try: host = self.common._get_3par_host(hostname) diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py index 48db1bee7..656ef423a 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py @@ -278,7 +278,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver): # make sure we don't have the host already host = None hostname = self.common._safe_hostname(connector['host']) - cpg = self.common.get_volume_metadata_value(volume, 'CPG') + cpg = self.common.get_cpg(volume) domain = self.common.get_domain(cpg) try: host = self.common._get_3par_host(hostname) -- 2.45.2