Skip to content

Commit

Permalink
Remove unnecessary metadata from the 3PAR drivers
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@kumartin
kumartin committed Aug 5, 2013
1 parent 2380fef commit ba87053
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 82 deletions.
102 changes: 47 additions & 55 deletions cinder/tests/test_hp3par.py
View file
Expand Up @@ -18,6 +18,7 @@
"""
Unit tests for OpenStack Cinder volume drivers
"""
import ast
import mox
import shutil
import tempfile
Expand Down Expand Up @@ -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'),
Expand Down Expand Up @@ -383,6 +384,8 @@ def setup_fakes(self):
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)

Expand All @@ -393,6 +396,9 @@ def clear_mox(self):
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

Expand Down Expand Up @@ -440,16 +446,14 @@ def fake_add_volume_to_volume_set(self, volume, volume_name,
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)
Expand All @@ -461,12 +465,11 @@ def test_create_volume_qos(self):
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)
Expand All @@ -475,6 +478,23 @@ def test_delete_volume(self):
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)
Expand Down Expand Up @@ -520,12 +540,10 @@ def test_create_volume_from_snapshot_qos(self):
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
Expand Down Expand Up @@ -635,25 +653,6 @@ def test_initialize_connection(self):
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)

Expand Down Expand Up @@ -687,6 +686,8 @@ def test_create_host(self):

#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)
Expand All @@ -710,6 +711,8 @@ def test_create_invalid_host(self):

#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)
Expand Down Expand Up @@ -737,6 +740,8 @@ def test_create_modify_host(self):

#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)
Expand Down Expand Up @@ -839,25 +844,6 @@ def test_initialize_connection(self):
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)

Expand Down Expand Up @@ -891,6 +877,8 @@ def test_create_host(self):

#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)
Expand All @@ -915,6 +903,8 @@ def test_create_invalid_host(self):

#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)
Expand Down Expand Up @@ -942,6 +932,8 @@ def test_create_modify_host(self):

#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)
Expand Down
43 changes: 18 additions & 25 deletions cinder/volume/drivers/san/hp/hp_3par_common.py
View file
Expand Up @@ -38,7 +38,7 @@
array.
"""


import ast
import base64
import json
import paramiko
Expand Down Expand Up @@ -653,6 +653,15 @@ def _remove_volume_set(self, vvs_name):
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)
Expand Down Expand Up @@ -692,16 +701,13 @@ def create_volume(self, volume):
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)
hp3par_keys = self._get_keys_by_volume_type(volume_type)
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)
Expand Down Expand Up @@ -780,11 +786,6 @@ def create_volume(self, volume):
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)

Expand All @@ -798,12 +799,10 @@ def get_next_word(self, s, search_string):
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):
Expand Down Expand Up @@ -833,11 +832,12 @@ def create_cloned_volume(self, volume, src_vref):
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:
Expand Down Expand Up @@ -878,16 +878,13 @@ def create_volume_from_snapshot(self, volume, snapshot):
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)
hp3par_keys = self._get_keys_by_volume_type(volume_type)
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:
Expand Down Expand Up @@ -920,10 +917,6 @@ def create_volume_from_snapshot(self, volume, snapshot):
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))

Expand Down
2 changes: 1 addition & 1 deletion cinder/volume/drivers/san/hp/hp_3par_fc.py
View file
Expand Up @@ -220,7 +220,7 @@ def _create_host(self, volume, connector):
"""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)
Expand Down
2 changes: 1 addition & 1 deletion cinder/volume/drivers/san/hp/hp_3par_iscsi.py
View file
Expand Up @@ -278,7 +278,7 @@ def _create_host(self, volume, connector):
# 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)
Expand Down

0 comments on commit ba87053

Please sign in to comment.