openstack
/
cinder
Code Issues Proposed changes

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
This commit is contained in:
Kurt Martin 2013-07-31 13:56:29 -07:00
parent 2380feff7c
commit ba87053896
4 changed files with 67 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
cinder/tests/test_hp3par.py
View File

@ -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)

43
cinder/volume/drivers/san/hp/hp_3par_common.py
View File

@ -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))

2
cinder/volume/drivers/san/hp/hp_3par_fc.py
View File

@ -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)

2
cinder/volume/drivers/san/hp/hp_3par_iscsi.py
View File

@ -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)