PowerMax Driver - Fix for create group from source

Fix for creating a group from the source when the source
storage group is replicated. The newly created target group
must not be replicated initially so the replicate group API can
succeed.  A few force flags are also required to add and
remove from a replicated storage group.  The modify snap API
also needs a force flag.

Change-Id: I46e676cfd885a38008772f8ae20742da4955ae81
This commit is contained in:
Helen Walsh 2021-03-16 10:19:43 +00:00
parent fe55090f9f
commit 9af47a2fc4
9 changed files with 254 additions and 53 deletions

View File

@ -546,6 +546,10 @@ class PowerMaxData(object):
id=test_vol_grp_name_id_only, host=fake_host,
replication_status=fields.ReplicationStatus.ENABLED)
test_rep_group2 = fake_group.fake_group_obj(
context=ctx,
replication_status=fields.ReplicationStatus.ENABLED)
test_group = fake_group.fake_group_obj(
context=ctx, name=storagegroup_name_source,
id=test_vol_grp_name_id_only, host=fake_host)

View File

@ -2955,6 +2955,28 @@ class PowerMaxCommonTest(test.TestCase):
model_update, __ = self.common._delete_group(group, volumes)
self.assertEqual(ref_model_update, model_update)
@mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members')
@mock.patch.object(common.PowerMaxCommon, '_cleanup_device_snapvx')
@mock.patch.object(common.PowerMaxCommon, '_get_members_of_volume_group',
return_value=[tpd.PowerMaxData.device_id])
@mock.patch.object(rest.PowerMaxRest, 'get_volume_snapshot_list',
return_value=[])
@mock.patch.object(volume_utils, 'is_group_a_type', return_value=False)
def test_delete_group_snapshot_and_volume_cleanup(
self, mock_check, mck_get_snaps, mock_members, mock_cleanup,
mock_remove):
group = self.data.test_group_1
volumes = [fake_volume.fake_volume_obj(
context='cxt', provider_location=None)]
with mock.patch.object(
volume_utils, 'is_group_a_cg_snapshot_type',
return_value=True), mock.patch.object(
self.rest, 'get_volumes_in_storage_group',
return_value=[]):
self.common._delete_group(group, volumes)
mock_cleanup.assert_called_once()
mock_remove.assert_called_once()
@mock.patch.object(rest.PowerMaxRest, 'get_volume_snapshot_list',
return_value=list())
def test_delete_group_already_deleted(self, mck_get_snaps):
@ -2994,7 +3016,7 @@ class PowerMaxCommonTest(test.TestCase):
@mock.patch.object(common.PowerMaxCommon, '_get_members_of_volume_group',
return_value=[tpd.PowerMaxData.device_id])
@mock.patch.object(common.PowerMaxCommon, '_find_device_on_array',
return_value= tpd.PowerMaxData.device_id)
return_value=tpd.PowerMaxData.device_id)
@mock.patch.object(masking.PowerMaxMasking,
'remove_volumes_from_storage_group')
def test_delete_group_cleanup_snapvx(

View File

@ -129,21 +129,39 @@ class PowerMaxReplicationTest(test.TestCase):
@mock.patch.object(
utils.PowerMaxUtils, 'is_volume_failed_over', return_value=True)
def test_initialize_connection_vol_failed_over(self, mock_fo):
extra_specs = deepcopy(self.extra_specs)
extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f
rep_extra_specs = deepcopy(tpd.PowerMaxData.rep_extra_specs)
rep_extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f
rep_config = deepcopy(self.data.rep_config_sync)
extra_specs[utils.REP_CONFIG] = rep_config
rep_config[utils.RDF_CONS_EXEMPT] = False
rep_extra_specs = {
'pool_name': 'Diamond+NONE+SRP_1+000197800123',
'slo': 'Diamond', 'workload': 'NONE', 'srp': 'SRP_1',
'array': '000197800124', 'interval': 1, 'retries': 1,
'replication_enabled': True, 'rep_mode': 'Synchronous',
'sync_interval': 3, 'sync_retries': 200,
'rdf_group_label': '23_24_007', 'rdf_group_no': '70',
'storagetype:portgroupname': 'OS-fibre-PG'}
rep_config = {
'backend_id': 'rep_backend_id_sync', 'array': '000197800124',
'portgroup': 'OS-fibre-PG', 'srp': 'SRP_1',
'rdf_group_label': '23_24_007', 'mode': 'Synchronous',
'allow_extend': True, 'sync_interval': 3, 'sync_retries': 200,
'exempt': False}
extra_specs = {
'pool_name': 'Diamond+NONE+SRP_1+000197800123', 'slo': 'Diamond',
'workload': 'NONE', 'srp': 'SRP_1', 'array': '000197800123',
'interval': 1, 'retries': 1, 'replication_enabled': True,
'rep_mode': 'Synchronous',
'storagetype:portgroupname': 'OS-fibre-PG', 'rep_config': {
'backend_id': 'rep_backend_id_sync', 'array': '000197800124',
'portgroup': 'OS-fibre-PG', 'srp': 'SRP_1',
'rdf_group_label': '23_24_007', 'mode': 'Synchronous',
'allow_extend': True, 'sync_interval': 3,
'sync_retries': 200, 'exempt': False}}
with mock.patch.object(self.common, '_get_replication_extra_specs',
return_value=rep_extra_specs) as mock_es:
self.common.initialize_connection(
self.data.test_volume, self.data.connector)
mock_es.assert_called_once_with(extra_specs, rep_config)
with mock.patch.object(self.common, '_initial_setup',
return_value=extra_specs):
self.common.initialize_connection(
self.data.test_volume, self.data.connector)
mock_es.assert_called_once_with(extra_specs, rep_config)
@mock.patch.object(utils.PowerMaxUtils, 'is_metro_device',
return_value=True)
@ -2156,3 +2174,53 @@ class PowerMaxReplicationTest(test.TestCase):
array, device_id, volume, volume_name, rep_extra_specs)
mck_resume.assert_called_once_with(
array, management_sg, rdf_group_no, rep_extra_specs)
@mock.patch.object(common.PowerMaxCommon,
'_add_replicated_volumes_to_default_storage_group')
@mock.patch.object(common.PowerMaxCommon,
'_replicate_group')
@mock.patch.object(provision.PowerMaxProvision, 'link_and_break_replica')
@mock.patch.object(rest.PowerMaxRest, 'get_storage_group_snap_id_list',
return_value=[tpd.PowerMaxData.snap_id])
@mock.patch.object(common.PowerMaxCommon, 'get_volume_metadata')
@mock.patch.object(common.PowerMaxCommon, '_create_non_replicated_volume')
@mock.patch.object(utils.PowerMaxUtils, 'get_volume_group_utils',
return_value=(None, {'interval': 1, 'retries': 1}))
def test_create_group_from_src_replication(
self, mock_grp_utils, mock_create, mock_metadata, mock_snap,
mock_link, mock_rep, mock_add):
context = None
group_snapshot = self.data.test_group_snapshot_1
snapshots = []
source_vols = [self.data.test_volume]
volumes = [self.data.test_volume]
source_group = self.data.test_group_1
with mock.patch.object(
volume_utils, 'is_group_a_cg_snapshot_type',
return_value=True), mock.patch.object(
volume_utils, 'is_group_a_type', return_value=True):
self.common.create_group_from_src(
context, self.data.test_rep_group2, volumes, group_snapshot,
snapshots, source_group, source_vols)
mock_create.assert_called_once()
mock_link.assert_called_once()
mock_metadata.assert_called_once()
mock_rep.assert_called_once()
mock_add.assert_called_once()
@mock.patch.object(masking.PowerMaxMasking,
'add_volumes_to_storage_group')
def test_add_replicated_volumes_to_default_storage_group(self, mock_add):
volumes_model_update = [{'provider_location': six.text_type(
self.data.provider_location),
'replication_driver_data': six.text_type(
{'array': self.data.remote_array,
'device_id': self.data.device_id2})}]
rep_extra_specs = deepcopy(self.data.rep_extra_specs)
self.common._add_replicated_volumes_to_default_storage_group(
self.data.array, volumes_model_update, rep_extra_specs)
mock_add.assert_has_calls(
[call(self.data.array, [self.data.device_id],
'OS-SRP_1-Diamond-DSS-RE-SG', rep_extra_specs),
call(self.data.remote_array, [self.data.device_id2],
'OS-SRP_1-Diamond-DSS-RE-SG', rep_extra_specs)])

View File

@ -1251,6 +1251,10 @@ class PowerMaxRestTest(test.TestCase):
'snapshotSrcs'][0]['linkedDevices'][0]['targetDevice']
snap_name = self.data.volume_snap_vx['snapshotSrcs'][0]['snapshotName']
extra_specs = self.data.extra_specs
try:
extra_specs.pop(utils.FORCE_VOL_EDIT)
except KeyError:
pass
payload = {'deviceNameListSource': [{'name': source_id}],
'deviceNameListTarget': [
{'name': target_id}],

View File

@ -535,6 +535,7 @@ class PowerMaxCommon(object):
if group_id is not None:
if group and (volume_utils.is_group_a_cg_snapshot_type(group)
or group.is_replicated):
extra_specs[utils.FORCE_VOL_EDIT] = True
group_name = self._add_new_volume_to_volume_group(
volume, device_id, volume_name,
extra_specs, rep_driver_data)
@ -5661,9 +5662,8 @@ class PowerMaxCommon(object):
# cleanup completes before doing any replication/storage group cleanup.
remaining_device_snapshots = list()
remaining_snapvx_targets = list()
for vol in volumes:
extra_specs = self._initial_setup(vol)
device_id = self._find_device_on_array(vol, extra_specs)
def _cleanup_snapshots(device_id):
self._cleanup_device_snapvx(array, device_id, extra_specs)
snapshots = self.rest.get_volume_snapshot_list(array, device_id)
__, snapvx_target_details = self.rest.find_snap_vx_sessions(
@ -5682,6 +5682,20 @@ class PowerMaxCommon(object):
'snapshot_name': snap_name}
remaining_snapvx_targets.append(target_details)
vol_not_deleted = list()
for vol in volumes:
extra_specs = self._initial_setup(vol)
device_id = self._find_device_on_array(vol, extra_specs)
if device_id:
_cleanup_snapshots(device_id)
else:
LOG.debug('Cannot find device id for volume. It is '
'possible this information was not persisted.')
vol_not_deleted.append(vol)
if len(vol_not_deleted) == len(volume_device_ids):
for volume_device_id in volume_device_ids:
_cleanup_snapshots(volume_device_id)
# Fail out if volumes to be deleted still have snapshots.
if remaining_device_snapshots:
for details in remaining_device_snapshots:
@ -5726,6 +5740,22 @@ class PowerMaxCommon(object):
interval_retries_dict, target_rep_config)
try:
if volume_device_ids:
def _delete_vol(dev_id):
if group.is_replicated:
# Set flag to True if replicated.
extra_specs[utils.FORCE_VOL_EDIT] = True
if dev_id in volume_device_ids:
self.masking.remove_and_reset_members(
array, vol, dev_id, vol.name,
extra_specs, False)
self._delete_from_srp(
array, dev_id, "group vol", extra_specs)
else:
LOG.debug("Volume not found on the array.")
# Add the device id to the deleted list
deleted_volume_device_ids.append(dev_id)
# First remove all the volumes from the SG
self.masking.remove_volumes_from_storage_group(
array, volume_device_ids, vol_grp_name,
@ -5733,16 +5763,14 @@ class PowerMaxCommon(object):
for vol in volumes:
extra_specs = self._initial_setup(vol)
device_id = self._find_device_on_array(vol, extra_specs)
if device_id in volume_device_ids:
self.masking.remove_and_reset_members(
array, vol, device_id, vol.name,
extra_specs, False)
self._delete_from_srp(
array, device_id, "group vol", extra_specs)
else:
LOG.debug("Volume not found on the array.")
# Add the device id to the deleted list
deleted_volume_device_ids.append(device_id)
if device_id:
_delete_vol(device_id)
if volume_device_ids != deleted_volume_device_ids:
new_list = list(set(volume_device_ids).difference(
deleted_volume_device_ids))
for device_id in new_list:
_delete_vol(device_id)
# Once all volumes are deleted then delete the SG
self.rest.delete_storage_group(array, vol_grp_name)
model_update = {'status': fields.GroupStatus.DELETED}
@ -5814,6 +5842,7 @@ class PowerMaxCommon(object):
:param extra_specs: the extra specifications
:param rep_config: the rep config to use for rdf operations
"""
extra_specs[utils.FORCE_VOL_EDIT] = True
rdf_group_no, remote_array = self.get_rdf_details(array, rep_config)
# Delete replication for group, if applicable
group_details = self.rest.get_storage_group_rep(
@ -6301,9 +6330,10 @@ class PowerMaxCommon(object):
# Create the target devices
list_volume_pairs = []
for volume in volumes:
volumes_model_update, rollback_dict, list_volume_pairs = (
self. _create_vol_and_add_to_group(
volume, group.id, tgt_name, rollback_dict,
(volumes_model_update, rollback_dict, list_volume_pairs,
extra_specs) = (
self._create_vol_and_add_to_group(
volume, group, tgt_name, rollback_dict,
source_vols, snapshots, list_volume_pairs,
volumes_model_update))
@ -6313,10 +6343,15 @@ class PowerMaxCommon(object):
rollback_dict, create_snapshot))
# Link and break the snapshot to the source group
self.provision.link_and_break_replica(
array, src_grp_name, tgt_name, snap_name,
interval_retries_dict, list_volume_pairs,
delete_snapshot=create_snapshot)
snap_id_list = self.rest.get_storage_group_snap_id_list(
array, src_grp_name, snap_name)
if snap_id_list:
if group.is_replicated:
interval_retries_dict[utils.FORCE_VOL_EDIT] = True
self.provision.link_and_break_replica(
array, src_grp_name, tgt_name, snap_name,
interval_retries_dict, list_volume_pairs,
delete_snapshot=create_snapshot, snap_id=snap_id_list[0])
# Update the replication status
if group.is_replicated:
@ -6327,6 +6362,10 @@ class PowerMaxCommon(object):
volumes_model_update = self._replicate_group(
array, volumes_model_update,
tgt_name, interval_retries_dict)
# Add the volumes to the default storage group
extra_specs[utils.FORCE_VOL_EDIT] = True
self._add_replicated_volumes_to_default_storage_group(
array, volumes_model_update, extra_specs)
model_update.update({
'replication_status': fields.ReplicationStatus.ENABLED})
except Exception:
@ -6343,6 +6382,48 @@ class PowerMaxCommon(object):
return model_update, volumes_model_update
def _add_replicated_volumes_to_default_storage_group(
self, array, volumes_model_update, extra_specs):
"""Add replicated volumes to the default storage group.
:param array: the serial number of the array
:param volumes_model_update: the list of volume updates
:param extra_specs: the extra specifications
"""
is_re = False
rep_mode = None
if self.utils.is_replication_enabled(extra_specs):
is_re, rep_mode = True, extra_specs['rep_mode']
do_disable_compression = self.utils.is_compression_disabled(
extra_specs)
storage_group_name = self.masking.get_or_create_default_storage_group(
array, extra_specs[utils.SRP], extra_specs[utils.SLO],
extra_specs[utils.WORKLOAD], extra_specs,
do_disable_compression, is_re, rep_mode)
local_device_list = list()
remote_device_list = list()
for volume_dict in volumes_model_update:
if volume_dict.get('provider_location'):
loc = ast.literal_eval(volume_dict.get('provider_location'))
device_id = loc.get('device_id')
local_array = loc.get('array')
local_device_list.append(device_id)
if volume_dict.get('replication_driver_data'):
loc = ast.literal_eval(volume_dict.get(
'replication_driver_data'))
remote_device_id = loc.get('device_id')
remote_array = loc.get('array')
remote_device_list.append(remote_device_id)
if local_device_list:
self.masking.add_volumes_to_storage_group(
local_array, local_device_list, storage_group_name,
extra_specs)
if remote_device_list:
self.masking.add_volumes_to_storage_group(
remote_array, remote_device_list, storage_group_name,
extra_specs)
def _create_group_replica_and_get_snap_name(
self, group_id, actual_source_grp, source_id, source_sg,
rollback_dict, create_snapshot):
@ -6376,12 +6457,12 @@ class PowerMaxCommon(object):
return snap_name, rollback_dict
def _create_vol_and_add_to_group(
self, volume, group_id, tgt_name, rollback_dict, source_vols,
self, volume, group, tgt_name, rollback_dict, source_vols,
snapshots, list_volume_pairs, volumes_model_update):
"""Creates the volume group from source.
:param volume: volume object
:param group_id: the group id
:param group: the group object
:param tgt_name: target name
:param rollback_dict: rollback dict
:param source_vols: source volumes
@ -6389,18 +6470,27 @@ class PowerMaxCommon(object):
:param list_volume_pairs: volume pairs list
:param volumes_model_update: volume model update
:returns: volumes_model_update, rollback_dict, list_volume_pairs
extra_specs
"""
src_dev_id, extra_specs, vol_size, tgt_vol_name = (
self._get_clone_vol_info(
volume, source_vols, snapshots))
volume_dict, __, __, = self._create_volume(
volume, tgt_vol_name, vol_size, extra_specs)
device_id = volume_dict['device_id']
# Add the volume to the volume group SG
self.masking.add_volume_to_storage_group(
extra_specs[utils.ARRAY], device_id, tgt_name,
tgt_vol_name, extra_specs)
array = extra_specs[utils.ARRAY]
volume_name = self.utils.get_volume_element_name(volume.id)
if group.is_replicated:
volume_dict = self._create_non_replicated_volume(
array, volume, volume_name, tgt_name,
vol_size, extra_specs)
device_id = volume_dict['device_id']
else:
volume_dict, __, __, = self._create_volume(
volume, tgt_vol_name, vol_size, extra_specs)
device_id = volume_dict['device_id']
# Add the volume to the volume group SG
self.masking.add_volume_to_storage_group(
extra_specs[utils.ARRAY], device_id, tgt_name,
tgt_vol_name, extra_specs)
# Record relevant information
list_volume_pairs.append((src_dev_id, device_id))
# Add details to rollback dict
@ -6411,11 +6501,12 @@ class PowerMaxCommon(object):
(device_id, extra_specs, volume))
volumes_model_update.append(
self.utils.get_grp_volume_model_update(
volume, volume_dict, group_id,
volume, volume_dict, group.id,
meta=self.get_volume_metadata(volume_dict['array'],
volume_dict['device_id'])))
return volumes_model_update, rollback_dict, list_volume_pairs
return (volumes_model_update, rollback_dict, list_volume_pairs,
extra_specs)
def _get_clone_vol_info(self, volume, source_vols, snapshots):
"""Get the clone volume info.

View File

@ -765,7 +765,7 @@ class PowerMaxMasking(object):
'sg_name': storagegroup_name})
else:
self.rest.add_vol_to_sg(serial_number, sg_name,
device_id, extra_specs, force)
device_id, extra_specs, force=force)
do_add_volume_to_sg(storagegroup_name, serial_number)
LOG.debug("Add volume to storagegroup took: %(delta)s H:MM:SS.",

View File

@ -689,15 +689,16 @@ class PowerMaxProvision(object):
'tgtGroup': target_group_name})
# Link the snapshot
self.rest.modify_volume_snap(
array, None, None, snap_name, extra_specs, link=True,
list_volume_pairs=list_volume_pairs)
array, None, None, snap_name, extra_specs, snap_id=snap_id,
link=True, list_volume_pairs=list_volume_pairs)
# Unlink the snapshot
LOG.debug("Unlinking Snap Vx snapshot: source group: %(srcGroup)s "
"targetGroup: %(tgtGroup)s.",
{'srcGroup': source_group_name,
'tgtGroup': target_group_name})
self._unlink_volume(array, None, None, snap_name, extra_specs,
list_volume_pairs=list_volume_pairs)
self._unlink_volume(
array, None, None, snap_name, extra_specs, snap_id=snap_id,
list_volume_pairs=list_volume_pairs)
# Delete the snapshot if necessary
if delete_snapshot:
LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s "

View File

@ -2114,6 +2114,8 @@ class PowerMaxRest(object):
"""
action, operation, payload = '', '', {}
copy = 'true' if copy else 'false'
force = (
"true" if utils.FORCE_VOL_EDIT in extra_specs else "false")
if link:
action = "Link"
@ -2143,7 +2145,7 @@ class PowerMaxRest(object):
payload = {"deviceNameListSource": src_list,
"deviceNameListTarget": tgt_list,
"copy": copy, "action": action,
"star": 'false', "force": 'false',
"star": 'false', "force": force,
"exact": 'false', "remote": 'false',
"symforce": 'false'}
elif action == "Rename":
@ -2277,7 +2279,7 @@ class PowerMaxRest(object):
snapvx_tgt = False
rdf_grp = None
volume_details = self.get_volume(array, device_id)
if volume_details:
if volume_details and isinstance(volume_details, dict):
if volume_details.get('snapvx_target'):
snapvx_tgt = volume_details['snapvx_target']
if volume_details.get('snapvx_source'):
@ -2461,9 +2463,11 @@ class PowerMaxRest(object):
snap_tgt_dict, snap_src_dict_list = dict(), list()
s_in = self.get_volume_snap_info(array, device_id)
snap_src = (
s_in['snapshotSrcs'] if s_in.get('snapshotSrcs') else list())
s_in['snapshotSrcs'] if s_in and s_in.get(
'snapshotSrcs') else list())
snap_tgt = (
s_in['snapshotLnks'][0] if s_in.get('snapshotLnks') else dict())
s_in['snapshotLnks'][0] if s_in and s_in.get(
'snapshotLnks') else dict())
if snap_src and not tgt_only:
for session in snap_src:
snap_src_dict = dict()

View File

@ -0,0 +1,7 @@
---
fixes:
- |
PowerMax driver: Previously, the target storage group created from a
replicated storage group was also replicated, which could cause failures.
This fix creates a non-replicated target initially, and lets the replicate
group API take care of replicating it.