diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index 08ff0b6e6fd..37a527d8c4c 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/cinder/tests/unit/volume/drivers/test_pure.py @@ -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): @@ -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.", diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 12706c6be4d..3ba40c6bb35 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -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 = [ @@ -1269,7 +1271,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) @@ -1326,7 +1332,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}) @@ -1555,13 +1565,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) diff --git a/releasenotes/notes/bug-1870103-013e314e9a5b8e08.yaml b/releasenotes/notes/bug-1870103-013e314e9a5b8e08.yaml new file mode 100644 index 00000000000..5d8751f5b05 --- /dev/null +++ b/releasenotes/notes/bug-1870103-013e314e9a5b8e08.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Pure Storage driver `bug 1870103 + `_: + Ensure that unmanaged volumes do not exceed maximum + character length on FlashArray.