Pure Storage: check volumename length does not exceed maximum
Add in checks to ensure volume names in all cases, including snapshotting and unmanaging, do not exceed the name length limits on the FlashArray Change-Id: I6c153fd6c9c0f485092a82f0e6ae020ec5980fcc Closes-Bug: 1870103
This commit is contained in:
Simon Dodsley
parent
e39c0406de
commit
bb219d6b9f
|
|
@ -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.",
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue