From 984be92ae962f5c25ba5823ed1143df7acef9df4 Mon Sep 17 00:00:00 2001 From: odonos12 Date: Wed, 3 Jun 2020 15:25:07 +0100 Subject: [PATCH] PowerMax Driver - RDF State Validation Enhancements Update exception message for volumes being missing from their management storage groups for additional clarity. Add RDF state validation call during extend volume operations if the volume is replication enabled. Add additional validation check for comparing device count in local and remote RDF groups to ensure they match. Change-Id: Ib4e7fb0a1a42fd2da8ee6244d78a16c308bfd1c1 --- .../dell_emc/powermax/test_powermax_common.py | 16 ++++-- .../powermax/test_powermax_replication.py | 55 ++++++++++++++++++- .../drivers/dell_emc/powermax/common.py | 24 +++++++- 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py index 61b50ceea87..db324686000 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py @@ -708,6 +708,7 @@ class PowerMaxCommonTest(test.TestCase): mck_extend.assert_called_once_with( array, device_id, new_size, ref_extra_specs, None) + @mock.patch.object(common.PowerMaxCommon, '_validate_rdfg_status') @mock.patch.object(provision.PowerMaxProvision, 'extend_volume') @mock.patch.object(common.PowerMaxCommon, '_array_ode_capabilities_check', return_value=[True] * 4) @@ -717,7 +718,8 @@ class PowerMaxCommonTest(test.TestCase): @mock.patch.object(common.PowerMaxCommon, '_initial_setup', return_value=tpd.PowerMaxData.ex_specs_rep_config) def test_extend_vol_rep_success_next_gen( - self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode, mck_extend): + self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode, mck_extend, + mck_validate): self.common.next_gen = True volume = self.data.test_volume array = self.data.array @@ -731,7 +733,9 @@ class PowerMaxCommonTest(test.TestCase): array, device_id, new_size, ref_extra_specs, '1') mck_ode.assert_called_once_with( array, ref_extra_specs[utils.REP_CONFIG], True) + mck_validate.assert_called_once_with(array, ref_extra_specs) + @mock.patch.object(common.PowerMaxCommon, '_validate_rdfg_status') @mock.patch.object(provision.PowerMaxProvision, 'extend_volume') @mock.patch.object(common.PowerMaxCommon, '_extend_legacy_replicated_vol') @mock.patch.object(common.PowerMaxCommon, '_array_ode_capabilities_check', @@ -743,7 +747,7 @@ class PowerMaxCommonTest(test.TestCase): return_value=tpd.PowerMaxData.ex_specs_rep_config) def test_extend_vol_rep_success_next_gen_legacy_r2( self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode, mck_leg_extend, - mck_extend): + mck_extend, mck_validate): self.common.next_gen = True self.common.rep_config = self.data.rep_config volume = self.data.test_volume @@ -760,7 +764,9 @@ class PowerMaxCommonTest(test.TestCase): mck_ode.assert_called_once_with( array, ref_extra_specs[utils.REP_CONFIG], True) mck_extend.assert_not_called() + mck_validate.assert_called_once_with(array, ref_extra_specs) + @mock.patch.object(common.PowerMaxCommon, '_validate_rdfg_status') @mock.patch.object(provision.PowerMaxProvision, 'extend_volume') @mock.patch.object(common.PowerMaxCommon, '_extend_legacy_replicated_vol') @mock.patch.object(common.PowerMaxCommon, '_array_ode_capabilities_check', @@ -772,7 +778,7 @@ class PowerMaxCommonTest(test.TestCase): return_value=tpd.PowerMaxData.ex_specs_rep_config) def test_extend_vol_rep_success_legacy( self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode, mck_leg_extend, - mck_extend): + mck_extend, mck_validate): self.common.rep_config = self.data.rep_config self.common.next_gen = False volume = self.data.test_volume @@ -789,7 +795,9 @@ class PowerMaxCommonTest(test.TestCase): mck_ode.assert_called_once_with( array, ref_extra_specs[utils.REP_CONFIG], True) mck_extend.assert_not_called() + mck_validate.assert_called_once_with(array, ref_extra_specs) + @mock.patch.object(common.PowerMaxCommon, '_validate_rdfg_status') @mock.patch.object(common.PowerMaxCommon, '_array_ode_capabilities_check', return_value=[False, False, False, False]) @mock.patch.object(common.PowerMaxCommon, 'get_rdf_details', @@ -799,7 +807,7 @@ class PowerMaxCommonTest(test.TestCase): common.PowerMaxCommon, '_initial_setup', return_value=tpd.PowerMaxData.ex_specs_rep_config_no_extend) def test_extend_vol_rep_success_legacy_allow_extend_false( - self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode): + self, mck_setup, mck_val_chk, mck_get_rdf, mck_ode, mck_validate): self.common.rep_config = self.data.rep_config self.common.next_gen = False volume = self.data.test_volume diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py index 239b94904e1..901c98e64e2 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py @@ -1312,6 +1312,8 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual(extra_specs[utils.REP_CONFIG], rep_extra_specs) self.assertTrue(resume_rdf) + @mock.patch.object(rest.PowerMaxRest, 'get_rdf_group', + return_value=tpd.PowerMaxData.rdf_group_details) @mock.patch.object( provision.PowerMaxProvision, 'verify_slo_workload', return_value=(True, True)) @@ -1333,7 +1335,7 @@ class PowerMaxReplicationTest(test.TestCase): return_value=tpd.PowerMaxData.sg_details[0]) def test_validate_rdfg_status_success( self, mck_get, mck_is_rep, mck_is_excl, mck_states, mck_cons, - mck_mgrp_name, mck_slo): + mck_mgrp_name, mck_slo, mck_rdf): array = self.data.array extra_specs = deepcopy(self.data.rep_extra_specs6) extra_specs[utils.REP_MODE] = utils.REP_ASYNC @@ -1350,6 +1352,7 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual(2, mck_states.call_count) self.assertEqual(1, mck_cons.call_count) self.assertEqual(1, mck_mgrp_name.call_count) + self.assertEqual(3, mck_rdf.call_count) mck_is_rep.assert_called_with(array, management_sg_name) mck_is_excl.assert_called_with(array, management_sg_name) mck_states.assert_called_with(array, management_sg_name, rdfg, mode) @@ -1439,6 +1442,56 @@ class PowerMaxReplicationTest(test.TestCase): mck_states.assert_called_with(array, management_sg_name, rdfg, mode) mck_cons.assert_called_with(array, management_sg_name, rdfg) + @mock.patch.object(rest.PowerMaxRest, 'get_rdf_group', + side_effect=(tpd.PowerMaxData.rdf_group_details, + tpd.PowerMaxData.rdf_group_details, + {'numDevices': '1000'})) + @mock.patch.object( + provision.PowerMaxProvision, 'verify_slo_workload', + return_value=(True, True)) + @mock.patch.object(utils.PowerMaxUtils, 'get_rdf_management_group_name', + return_value=tpd.PowerMaxData.rdf_managed_async_grp) + @mock.patch.object(common.PowerMaxCommon, + '_validate_management_group_volume_consistency', + return_value=True) + @mock.patch.object(common.PowerMaxCommon, + '_validate_storage_group_rdf_states', + side_effect=[True, True]) + @mock.patch.object(common.PowerMaxCommon, + '_validate_rdf_group_storage_group_exclusivity', + side_effect=[True, True]) + @mock.patch.object(common.PowerMaxCommon, + '_validate_storage_group_is_replication_enabled', + side_effect=[True, True]) + @mock.patch.object(rest.PowerMaxRest, 'get_storage_group', + return_value=tpd.PowerMaxData.sg_details[0]) + def test_validate_rdfg_status_failure_device_counts( + self, mck_get, mck_is_rep, mck_is_excl, mck_states, mck_cons, + mck_mgrp_name, mck_slo, mck_rdf): + array = self.data.array + extra_specs = deepcopy(self.data.rep_extra_specs6) + extra_specs[utils.REP_MODE] = utils.REP_ASYNC + extra_specs[utils.REP_CONFIG] = self.data.rep_config_async + management_sg_name = self.data.rdf_managed_async_grp + rdfg = self.data.rdf_group_no_2 + mode = utils.REP_ASYNC + + self.assertRaises(exception.VolumeDriverException, + self.common._validate_rdfg_status, + array, extra_specs) + + self.assertEqual(2, mck_get.call_count) + self.assertEqual(2, mck_is_rep.call_count) + self.assertEqual(2, mck_is_excl.call_count) + self.assertEqual(2, mck_states.call_count) + self.assertEqual(1, mck_cons.call_count) + self.assertEqual(1, mck_mgrp_name.call_count) + self.assertEqual(3, mck_rdf.call_count) + mck_is_rep.assert_called_with(array, management_sg_name) + mck_is_excl.assert_called_with(array, management_sg_name) + mck_states.assert_called_with(array, management_sg_name, rdfg, mode) + mck_cons.assert_called_with(array, management_sg_name, rdfg) + @mock.patch.object(rest.PowerMaxRest, 'get_storage_group_rep', return_value={'rdf': True}) def test_validate_storage_group_is_replication_enabled_success( diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 4ab758fda44..fe80bffcaf8 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -1091,6 +1091,7 @@ class PowerMaxCommon(object): if rep_enabled: rep_config = ex_specs[utils.REP_CONFIG] rdf_grp_no, __ = self.get_rdf_details(array, rep_config) + self._validate_rdfg_status(array, ex_specs) r1_ode, r1_ode_metro, r2_ode, r2_ode_metro = ( self._array_ode_capabilities_check(array, rep_config, True)) @@ -6161,6 +6162,26 @@ class PowerMaxCommon(object): % management_sg_name) raise exception.VolumeBackendAPIException(msg) + # Perform check to make sure we have the same number of devices + remote_array = rep_extra_specs[utils.ARRAY] + rdf_group = self.rest.get_rdf_group( + array, rdf_group_no) + remote_rdf_group_no = rdf_group.get('remoteRdfgNumber') + remote_rdf_group = self.rest.get_rdf_group( + remote_array, remote_rdf_group_no) + local_rdfg_device_count = rdf_group.get('numDevices') + remote_rdfg_device_count = remote_rdf_group.get('numDevices') + if local_rdfg_device_count != remote_rdfg_device_count: + msg = (_( + 'RDF validation failed. Different device counts found for ' + 'local and remote RDFGs. Local RDFG %s has %s devices. Remote ' + 'RDFG %s has %s devices. The same number of devices is ' + 'expected. Check RDFGs for broken RDF pairs and cleanup or ' + 'recreate the pairs as needed.') % ( + rdf_group_no, local_rdfg_device_count, remote_rdf_group_no, + remote_rdfg_device_count)) + raise exception.VolumeDriverException(msg) + def _validate_storage_group_is_replication_enabled( self, array, storage_group_name): """Validate that a storage groups is marked as RDF enabled @@ -6264,7 +6285,8 @@ class PowerMaxCommon(object): 'Inconsistency found between management group %s and RDF ' 'group %s. The following volumes are not in the management ' 'storage group %s. All Asynchronous and Metro volumes must ' - 'be managed together.', + 'be managed together in their respective management storage ' + 'groups.', management_sg_name, rdf_group_number, missing_volumes_str) is_valid = False return is_valid