openstack
/
cinder
Code Issues Proposed changes

Merge "Pure Storage: check volumename length does not exceed maximum"

This commit is contained in:
Zuul 2021-03-09 19:34:13 +00:00 committed by Gerrit Code Review
parent a4e64d839f bb219d6b9f
commit 884c72dff9
3 changed files with 49 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
cinder/tests/unit/volume/drivers/test_pure.py
View File

@ -52,6 +52,7 @@ BASE_DRIVER_OBJ = DRIVER_PATH + ".PureBaseVolumeDriver"
ISCSI_DRIVER_OBJ = DRIVER_PATH + ".PureISCSIDriver"
FC_DRIVER_OBJ = DRIVER_PATH + ".PureFCDriver"
ARRAY_OBJ = DRIVER_PATH + ".FlashArray"
UNMANAGED_SUFFIX = "-unmanaged"
GET_ARRAY_PRIMARY = {"version": "99.9.9",
"revision": "201411230504+8a400f7",
@ -495,6 +496,7 @@ MANAGEABLE_PURE_SNAP_REFS = [
'source_reference': {'name': MANAGEABLE_PURE_SNAPS[2]['source']},
}
]
MAX_SNAP_LENGTH = 96
class FakePureStorageHTTPError(Exception):
@ -643,7 +645,7 @@ class PureBaseSharedDriverTestCase(PureDriverTestCase):
return group_snap, group_snap_name
@ddt.ddt
@ddt.ddt(testNameFormat=ddt.TestNameFormat.INDEX_ONLY)
class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
def _setup_mocks_for_replication(self):
# Mock config values
@ -2042,7 +2044,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
def test_unmanage(self):
vol, vol_name = self.new_fake_vol()
unmanaged_vol_name = vol_name + "-unmanaged"
unmanaged_vol_name = vol_name + UNMANAGED_SUFFIX
self.driver.unmanage(vol)
@ -2057,7 +2059,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
def test_unmanage_with_deleted_volume(self):
vol, vol_name = self.new_fake_vol()
unmanaged_vol_name = vol_name + "-unmanaged"
unmanaged_vol_name = vol_name + UNMANAGED_SUFFIX
self.array.rename_volume.side_effect = \
self.purestorage_module.PureHTTPError(
text="Volume does not exist.",
@ -2206,12 +2208,23 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
self.driver.manage_existing_snapshot_get_size,
snap, {'name': PURE_SNAPSHOT['name']})
def test_unmanage_snapshot(self):
snap, snap_name = self.new_fake_snap()
unmanaged_snap_name = snap_name + "-unmanaged"
@ddt.data(
# 96 chars, will exceed allowable length
'volume-1e5177e7-95e5-4a0f-b170-e45f4b469f6a-cinder.'
'snapshot-253b2878-ec60-4793-ad19-e65496ec7aab',
# short_name that will require no adjustment
'volume-1e5177e7-cinder.snapshot-e65496ec7aab')
@mock.patch(BASE_DRIVER_OBJ + "._get_snap_name")
def test_unmanage_snapshot(self, fake_name, mock_get_snap_name):
snap, _ = self.new_fake_snap()
mock_get_snap_name.return_value = fake_name
self.driver.unmanage_snapshot(snap)
self.array.rename_volume.assert_called_with(snap_name,
unmanaged_snap_name)
self.array.rename_volume.assert_called_once()
old_name = self.array.rename_volume.call_args[0][0]
new_name = self.array.rename_volume.call_args[0][1]
self.assertEqual(fake_name, old_name)
self.assertLessEqual(len(new_name), MAX_SNAP_LENGTH)
self.assertTrue(new_name.endswith(UNMANAGED_SUFFIX))
def test_unmanage_snapshot_error_propagates(self):
snap, _ = self.new_fake_snap()
@ -2221,7 +2234,11 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase):
def test_unmanage_snapshot_with_deleted_snapshot(self):
snap, snap_name = self.new_fake_snap()
unmanaged_snap_name = snap_name + "-unmanaged"
if len(snap_name + UNMANAGED_SUFFIX) > MAX_SNAP_LENGTH:
unmanaged_snap_name = snap_name[:-len(UNMANAGED_SUFFIX)] + \
UNMANAGED_SUFFIX
else:
unmanaged_snap_name = snap_name
self.array.rename_volume.side_effect = \
self.purestorage_module.PureHTTPError(
text="Snapshot does not exist.",

21
cinder/volume/drivers/pure.py
View File

@ -125,6 +125,8 @@ ERR_MSG_ALREADY_IN_USE = "already in use"
EXTRA_SPECS_REPL_ENABLED = "replication_enabled"
EXTRA_SPECS_REPL_TYPE = "replication_type"
MAX_VOL_LENGTH = 63
MAX_SNAP_LENGTH = 96
UNMANAGED_SUFFIX = '-unmanaged'
SYNC_REPLICATION_REQUIRED_API_VERSIONS = ['1.13', '1.14']
ASYNC_REPLICATION_REQUIRED_API_VERSIONS = [
@ -1271,7 +1273,11 @@ class PureBaseVolumeDriver(san.SanDriver):
"""
vol_name = self._get_vol_name(volume)
unmanaged_vol_name = vol_name + UNMANAGED_SUFFIX
if len(vol_name + UNMANAGED_SUFFIX) > MAX_VOL_LENGTH:
unmanaged_vol_name = vol_name[:-len(UNMANAGED_SUFFIX)] + \
UNMANAGED_SUFFIX
else:
unmanaged_vol_name = vol_name + UNMANAGED_SUFFIX
LOG.info("Renaming existing volume %(ref_name)s to %(new_name)s",
{"ref_name": vol_name, "new_name": unmanaged_vol_name})
self._rename_volume_object(vol_name, unmanaged_vol_name)
@ -1328,7 +1334,11 @@ class PureBaseVolumeDriver(san.SanDriver):
"""
self._verify_manage_snap_api_requirements()
snap_name = self._get_snap_name(snapshot)
unmanaged_snap_name = snap_name + UNMANAGED_SUFFIX
if len(snap_name + UNMANAGED_SUFFIX) > MAX_SNAP_LENGTH:
unmanaged_snap_name = snap_name[:-len(UNMANAGED_SUFFIX)] + \
UNMANAGED_SUFFIX
else:
unmanaged_snap_name = snap_name + UNMANAGED_SUFFIX
LOG.info("Renaming existing snapshot %(ref_name)s to "
"%(new_name)s", {"ref_name": snap_name,
"new_name": unmanaged_snap_name})
@ -1557,13 +1567,14 @@ class PureBaseVolumeDriver(san.SanDriver):
base_name = volume.name
# Some OpenStack deployments, eg PowerVC, create a volume.name that
# when appended with out '-cinder' string will exceed the maximum
# when appended with our '-cinder' string will exceed the maximum
# volume name length for Pure, so here we left truncate the true volume
# name before the opennstack volume_name_template affected it and
# then put back the template format
if len(base_name) > 56:
actual_name = base_name[7:]
base_name = "volume-" + actual_name[-52:]
actual_name = base_name[(len(CONF.volume_name_template) - 2):]
base_name = CONF.volume_name_template % \
actual_name[-(56 - len(CONF.volume_name_template)):]
repl_type = self._get_replication_type_from_vol_type(
volume.volume_type)

7
releasenotes/notes/bug-1870103-013e314e9a5b8e08.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
Pure Storage driver `bug 1870103
<https://bugs.launchpad.net/cinder/+bug/1870103>`_:
Ensure that unmanaged volumes do not exceed maximum
character length on FlashArray.