PowerMax Driver - Allowing for all types of boolean in extra specs
Allowing for <is> True, 'True', 'true', <is> False, 'False', 'false' in 'storagetype:disablecompression' to that retype won't default to host assisted migration. Also checking that the array is compression capable in a retype operation. Change-Id: I79724532202a19d02bf624dde6b3ba0c53080f98
This commit is contained in:
parent
d428e3f909
commit
b2f696361a
|
@ -2139,7 +2139,10 @@ class PowerMaxCommonTest(test.TestCase):
|
|||
device_id, volume, host, volume_name, new_type, extra_specs)
|
||||
self.assertFalse(migrate_status)
|
||||
|
||||
def test_slo_workload_migration_same_host_change_compression(self):
|
||||
@mock.patch.object(rest.PowerMaxRest, 'is_compression_capable',
|
||||
return_value=True)
|
||||
def test_slo_workload_migration_same_host_change_compression(
|
||||
self, mock_cap):
|
||||
device_id = self.data.device_id
|
||||
volume_name = self.data.test_volume.name
|
||||
extra_specs = self.data.extra_specs
|
||||
|
@ -2391,12 +2394,17 @@ class PowerMaxCommonTest(test.TestCase):
|
|||
self.data.workload, False)
|
||||
self.assertEqual(ref_return, return_val)
|
||||
# Already in correct sg
|
||||
host4 = {'host': self.data.fake_host}
|
||||
return_val = self.common._is_valid_for_storage_assisted_migration(
|
||||
device_id, host4, self.data.array,
|
||||
self.data.srp, volume_name, False, False, self.data.slo,
|
||||
self.data.workload, False)
|
||||
self.assertEqual(ref_return, return_val)
|
||||
with mock.patch.object(
|
||||
self.common.provision,
|
||||
'get_slo_workload_settings_from_storage_group',
|
||||
return_value='Diamond+DSS') as mock_settings:
|
||||
host4 = {'host': self.data.fake_host}
|
||||
return_val = self.common._is_valid_for_storage_assisted_migration(
|
||||
device_id, host4, self.data.array,
|
||||
self.data.srp, volume_name, False, False, self.data.slo,
|
||||
self.data.workload, False)
|
||||
self.assertEqual(ref_return, return_val)
|
||||
mock_settings.assert_called_once()
|
||||
|
||||
def test_is_valid_for_storage_assisted_migration_next_gen(self):
|
||||
device_id = self.data.device_id
|
||||
|
|
|
@ -219,42 +219,53 @@ class PowerMaxUtilsTest(test.TestCase):
|
|||
def test_is_compression_disabled_true(self):
|
||||
# Compression disabled in extra specs
|
||||
extra_specs = self.data.extra_specs_disable_compression
|
||||
do_disable_compression = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
self.assertTrue(do_disable_compression)
|
||||
self.assertTrue(self.utils.is_compression_disabled(extra_specs))
|
||||
# Compression disabled by no SL/WL combination
|
||||
extra_specs = deepcopy(self.data.vol_type_extra_specs_none_pool)
|
||||
self.assertTrue(self.utils.is_compression_disabled(
|
||||
extra_specs))
|
||||
self.assertTrue(self.utils.is_compression_disabled(extra_specs))
|
||||
extra_specs3 = deepcopy(extra_specs)
|
||||
extra_specs3.update({utils.DISABLECOMPRESSION: '<is> True'})
|
||||
self.assertTrue(self.utils.is_compression_disabled(extra_specs3))
|
||||
extra_specs4 = deepcopy(extra_specs)
|
||||
extra_specs4.update({utils.DISABLECOMPRESSION: 'True'})
|
||||
self.assertTrue(self.utils.is_compression_disabled(extra_specs4))
|
||||
|
||||
def test_is_compression_disabled_false(self):
|
||||
# Path 1: no compression extra spec set
|
||||
extra_specs = self.data.extra_specs
|
||||
do_disable_compression = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
self.assertFalse(do_disable_compression)
|
||||
self.assertFalse(self.utils.is_compression_disabled(extra_specs))
|
||||
# Path 2: compression extra spec set to false
|
||||
extra_specs2 = deepcopy(extra_specs)
|
||||
extra_specs2.update({utils.DISABLECOMPRESSION: 'false'})
|
||||
do_disable_compression2 = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
self.assertFalse(do_disable_compression2)
|
||||
self.assertFalse(self.utils.is_compression_disabled(extra_specs2))
|
||||
extra_specs3 = deepcopy(extra_specs)
|
||||
extra_specs3.update({utils.DISABLECOMPRESSION: '<is> False'})
|
||||
self.assertFalse(self.utils.is_compression_disabled(extra_specs3))
|
||||
extra_specs4 = deepcopy(extra_specs)
|
||||
extra_specs4.update({utils.DISABLECOMPRESSION: 'False'})
|
||||
self.assertFalse(self.utils.is_compression_disabled(extra_specs4))
|
||||
|
||||
def test_change_compression_type_true(self):
|
||||
source_compr_disabled = True
|
||||
new_type_compr_disabled = {
|
||||
new_type_compr_disabled_1 = {
|
||||
'extra_specs': {utils.DISABLECOMPRESSION: 'false'}}
|
||||
ans = self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled)
|
||||
self.assertTrue(ans)
|
||||
self.assertTrue(self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled_1))
|
||||
new_type_compr_disabled_2 = {
|
||||
'extra_specs': {utils.DISABLECOMPRESSION: '<is> False'}}
|
||||
self.assertTrue(self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled_2))
|
||||
|
||||
def test_change_compression_type_false(self):
|
||||
source_compr_disabled = True
|
||||
new_type_compr_disabled = {
|
||||
'extra_specs': {utils.DISABLECOMPRESSION: 'true'}}
|
||||
ans = self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled)
|
||||
self.assertFalse(ans)
|
||||
self.assertFalse(self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled))
|
||||
new_type_compr_disabled_2 = {
|
||||
'extra_specs': {utils.DISABLECOMPRESSION: '<is> True'}}
|
||||
self.assertFalse(self.utils.change_compression_type(
|
||||
source_compr_disabled, new_type_compr_disabled_2))
|
||||
|
||||
def test_is_replication_enabled(self):
|
||||
is_re = self.utils.is_replication_enabled(
|
||||
|
|
|
@ -23,7 +23,6 @@ import time
|
|||
from oslo_config import cfg
|
||||
from oslo_config import types
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import strutils
|
||||
import six
|
||||
|
||||
from cinder import coordination
|
||||
|
@ -2384,11 +2383,8 @@ class PowerMaxCommon(object):
|
|||
extra_specs[utils.SLO] = slo_from_extra_spec
|
||||
extra_specs[utils.WORKLOAD] = workload_from_extra_spec
|
||||
if self.rest.is_compression_capable(extra_specs[utils.ARRAY]):
|
||||
if extra_specs.get(utils.DISABLECOMPRESSION):
|
||||
# If not True remove it.
|
||||
if not strutils.bool_from_string(
|
||||
extra_specs[utils.DISABLECOMPRESSION]):
|
||||
extra_specs.pop(utils.DISABLECOMPRESSION, None)
|
||||
if not self.utils.is_compression_disabled(extra_specs):
|
||||
extra_specs.pop(utils.DISABLECOMPRESSION, None)
|
||||
else:
|
||||
extra_specs.pop(utils.DISABLECOMPRESSION, None)
|
||||
|
||||
|
@ -3871,14 +3867,16 @@ class PowerMaxCommon(object):
|
|||
:param extra_specs: extra specifications
|
||||
:returns: boolean -- True if migration succeeded, False if error.
|
||||
"""
|
||||
do_change_compression = False
|
||||
# Check if old type and new type have different replication types
|
||||
do_change_replication = self.utils.change_replication(
|
||||
extra_specs, new_type[utils.EXTRA_SPECS])
|
||||
is_compression_disabled = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
# Check if old type and new type have different compression types
|
||||
do_change_compression = (self.utils.change_compression_type(
|
||||
is_compression_disabled, new_type))
|
||||
if self.rest.is_compression_capable(extra_specs[utils.ARRAY]):
|
||||
is_compression_disabled = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
# Check if old type and new type have different compression types
|
||||
do_change_compression = (self.utils.change_compression_type(
|
||||
is_compression_disabled, new_type))
|
||||
is_tgt_rep = self.utils.is_replication_enabled(
|
||||
new_type[utils.EXTRA_SPECS])
|
||||
is_valid, target_slo, target_workload = (
|
||||
|
@ -4102,7 +4100,7 @@ class PowerMaxCommon(object):
|
|||
self.volume_metadata.capture_retype_info(
|
||||
volume, device_id, array, srp, target_slo,
|
||||
target_workload, target_sg_name, is_rep_enabled, rep_mode,
|
||||
target_extra_specs[utils.DISABLECOMPRESSION],
|
||||
self.utils.is_compression_disabled(target_extra_specs),
|
||||
target_backend_id)
|
||||
|
||||
return success, model_update
|
||||
|
@ -4229,7 +4227,8 @@ class PowerMaxCommon(object):
|
|||
|
||||
target_extra_specs[utils.PORTGROUPNAME] = (
|
||||
extra_specs.get(utils.PORTGROUPNAME, None))
|
||||
disable_compression = target_extra_specs[utils.DISABLECOMPRESSION]
|
||||
disable_compression = self.utils.is_compression_disabled(
|
||||
target_extra_specs)
|
||||
source_sg_list = device_info['storageGroupId']
|
||||
if mgmt_sg_name in source_sg_list:
|
||||
source_sg_list.remove(mgmt_sg_name)
|
||||
|
@ -4341,8 +4340,8 @@ class PowerMaxCommon(object):
|
|||
'storage groups.')
|
||||
storage_groups = self.rest.get_storage_group_list(array)
|
||||
if source_sg not in storage_groups:
|
||||
disable_compression = extra_specs.get(
|
||||
utils.DISABLECOMPRESSION, False)
|
||||
disable_compression = self.utils.is_compression_disabled(
|
||||
extra_specs)
|
||||
self.rest.create_storage_group(
|
||||
array, source_sg, extra_specs['srp'], extra_specs['slo'],
|
||||
extra_specs['workload'], extra_specs, disable_compression)
|
||||
|
|
|
@ -218,6 +218,10 @@ PORT_ID = 'portId'
|
|||
# Revert snapshot exception
|
||||
REVERT_SS_EXC = 'Link must be fully copied for this operation to proceed'
|
||||
|
||||
# extra specs
|
||||
IS_TRUE = ['<is> True', 'True', 'true', True]
|
||||
IS_FALSE = ['<is> False', 'False', 'false', False]
|
||||
|
||||
|
||||
class PowerMaxUtils(object):
|
||||
"""Utility class for Rest based PowerMax volume drivers.
|
||||
|
@ -510,7 +514,7 @@ class PowerMaxUtils(object):
|
|||
compression_disabled = False
|
||||
|
||||
if extra_specs.get(DISABLECOMPRESSION, False):
|
||||
if strutils.bool_from_string(extra_specs.get(DISABLECOMPRESSION)):
|
||||
if extra_specs.get(DISABLECOMPRESSION) in IS_TRUE:
|
||||
compression_disabled = True
|
||||
else:
|
||||
if extra_specs.get(SLO):
|
||||
|
@ -2078,6 +2082,7 @@ class PowerMaxUtils(object):
|
|||
:returns: array_id, srp, service_level, workload -- str, str, str, str
|
||||
"""
|
||||
array_id, srp, service_level, workload = str(), str(), str(), str()
|
||||
|
||||
pool_details = pool_name.split('+')
|
||||
if len(pool_details) == 4:
|
||||
array_id = pool_details[3]
|
||||
|
|
Loading…
Reference in New Issue