From c0a5be259e608808d1866dd8f54bcacf8ab6365b Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Tue, 1 May 2018 16:25:08 +0100 Subject: [PATCH] VMAX driver - remove deprecated XML option 'cinder_dell_emc_config_file' was deprecated in Queens. We are now removing all reference to it and all XML parsing. Now we rely solely on the config tags in cinder.conf. Closes-Bug: #1768273 Change-Id: If6f7c58f445dd813df7161cc6cd6dd84484e8a7e --- .../volume/drivers/dell_emc/vmax/test_vmax.py | 352 ++++-------------- cinder/volume/drivers/dell_emc/vmax/common.py | 123 ++---- cinder/volume/drivers/dell_emc/vmax/utils.py | 132 ------- .../drivers/dell-emc-vmax-driver.rst | 8 +- ...emove_deprecated_xml-4065b893d781f65c.yaml | 4 + 5 files changed, 104 insertions(+), 515 deletions(-) create mode 100644 releasenotes/notes/remove_deprecated_xml-4065b893d781f65c.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py index 97160420c6d..56530c5682c 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/vmax/test_vmax.py @@ -16,9 +16,7 @@ import ast from copy import deepcopy import datetime -import tempfile import time -from xml.dom import minidom import mock import requests @@ -154,7 +152,7 @@ class VMAXCommonData(object): # cinder volume info ctx = context.RequestContext('admin', 'fake', True) - provider_location = {'array': six.text_type(array), + provider_location = {'array': array, 'device_id': device_id} provider_location2 = {'array': six.text_type(array), @@ -283,8 +281,8 @@ class VMAXCommonData(object): extra_specs_rep_enabled['replication_enabled'] = True rep_extra_specs = deepcopy(extra_specs_rep_enabled) rep_extra_specs['array'] = remote_array - rep_extra_specs['interval'] = 0 - rep_extra_specs['retries'] = 0 + rep_extra_specs['interval'] = 1 + rep_extra_specs['retries'] = 1 rep_extra_specs['srp'] = srp2 rep_extra_specs['rep_mode'] = 'Synchronous' rep_extra_specs2 = deepcopy(rep_extra_specs) @@ -1183,89 +1181,15 @@ class FakeConfiguration(object): pass -class FakeXML(object): - - def __init__(self): - """""" - self.tempdir = tempfile.mkdtemp() - self.data = VMAXCommonData() - - def create_fake_config_file(self, config_group, portgroup, - ssl_verify=False): - - doc = minidom.Document() - emc = doc.createElement("EMC") - doc.appendChild(emc) - doc = self.add_array_info(doc, emc, portgroup, ssl_verify) - filename = 'cinder_dell_emc_config_%s.xml' % config_group - config_file_path = self.tempdir + '/' + filename - - f = open(config_file_path, 'w') - doc.writexml(f) - f.close() - return config_file_path - - def add_array_info(self, doc, emc, portgroup_name, ssl_verify): - array = doc.createElement("Array") - arraytext = doc.createTextNode(self.data.array) - emc.appendChild(array) - array.appendChild(arraytext) - - ecomserverip = doc.createElement("RestServerIp") - ecomserveriptext = doc.createTextNode("1.1.1.1") - emc.appendChild(ecomserverip) - ecomserverip.appendChild(ecomserveriptext) - - ecomserverport = doc.createElement("RestServerPort") - ecomserverporttext = doc.createTextNode("8443") - emc.appendChild(ecomserverport) - ecomserverport.appendChild(ecomserverporttext) - - ecomusername = doc.createElement("RestUserName") - ecomusernametext = doc.createTextNode("smc") - emc.appendChild(ecomusername) - ecomusername.appendChild(ecomusernametext) - - ecompassword = doc.createElement("RestPassword") - ecompasswordtext = doc.createTextNode("smc") - emc.appendChild(ecompassword) - ecompassword.appendChild(ecompasswordtext) - - portgroup = doc.createElement("PortGroup") - portgrouptext = doc.createTextNode(portgroup_name) - portgroup.appendChild(portgrouptext) - - portgroups = doc.createElement("PortGroups") - portgroups.appendChild(portgroup) - emc.appendChild(portgroups) - - srp = doc.createElement("SRP") - srptext = doc.createTextNode("SRP_1") - emc.appendChild(srp) - srp.appendChild(srptext) - - if ssl_verify: - restcert = doc.createElement("SSLCert") - restcerttext = doc.createTextNode("/path/cert.crt") - emc.appendChild(restcert) - restcert.appendChild(restcerttext) - - restverify = doc.createElement("SSLVerify") - restverifytext = doc.createTextNode("/path/cert.pem") - emc.appendChild(restverify) - restverify.appendChild(restverifytext) - return doc - - class VMAXUtilsTest(test.TestCase): def setUp(self): self.data = VMAXCommonData() volume_utils.get_max_over_subscription_ratio = mock.Mock() super(VMAXUtilsTest, self).setUp() - config_group = 'UtilsTests' - fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_i, True) - configuration = FakeConfiguration(fake_xml, config_group) + configuration = FakeConfiguration( + None, 'UtilsTests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = iscsi.VMAXISCSIDriver(configuration=configuration) @@ -1298,48 +1222,6 @@ class VMAXUtilsTest(test.TestCase): {'name': 'no_type_id'}) self.assertEqual({}, extra_specs) - def test_get_random_portgroup(self): - # 4 portgroups - data = ("\n\n" - "" - "OS-PG1\n" - "OS-PG2\n" - "OS-PG3\n" - "OS-PG4\n" - "" - "") - dom = minidom.parseString(data) - portgroup = self.utils._get_random_portgroup(dom) - self.assertIn('OS-PG', portgroup) - - # Duplicate portgroups - data = ("\n\n" - "" - "OS-PG1\n" - "OS-PG1\n" - "OS-PG1\n" - "OS-PG2\n" - "" - "") - dom = minidom.parseString(data) - portgroup = self.utils._get_random_portgroup(dom) - self.assertIn('OS-PG', portgroup) - - def test_get_random_portgroup_none(self): - # Missing PortGroup tag - data = ("\n\n" - "") - dom = minidom.parseString(data) - self.assertIsNone(self.utils._get_random_portgroup(dom)) - - # Missing portgroups - data = ("\n\n" - "" - "" - "") - dom = minidom.parseString(data) - self.assertIsNone(self.utils._get_random_portgroup(dom)) - def test_get_host_short_name(self): host_under_16_chars = 'host_13_chars' host1 = self.utils.get_host_short_name( @@ -1365,53 +1247,6 @@ class VMAXUtilsTest(test.TestCase): expect_vol_element_name = ('OS-' + volume_id) self.assertEqual(expect_vol_element_name, volume_element_name) - def test_parse_file_to_get_array_map(self): - kwargs = ( - {'RestServerIp': '1.1.1.1', - 'RestServerPort': '8443', - 'RestUserName': 'smc', - 'RestPassword': 'smc', - 'SSLCert': '/path/cert.crt', - 'SSLVerify': '/path/cert.pem', - 'SerialNumber': self.data.array, - 'srpName': 'SRP_1', - 'PortGroup': self.data.port_group_name_i}) - array_info = self.utils.parse_file_to_get_array_map( - self.common.configuration.cinder_dell_emc_config_file) - self.assertEqual(kwargs, array_info) - - @mock.patch.object(utils.VMAXUtils, - '_get_connection_info') - @mock.patch.object(utils.VMAXUtils, - '_get_random_portgroup') - def test_parse_file_to_get_array_map_errors(self, mock_port, mock_conn): - tempdir = tempfile.mkdtemp() - doc = minidom.Document() - emc = doc.createElement("EMC") - doc.appendChild(emc) - filename = 'cinder_dell_emc_config_%s.xml' % 'fake_xml' - config_file_path = tempdir + '/' + filename - f = open(config_file_path, 'w') - doc.writexml(f) - f.close() - array_info = self.utils.parse_file_to_get_array_map( - config_file_path) - self.assertIsNone(array_info['SerialNumber']) - - def test_parse_file_to_get_array_map_conn_errors(self): - tempdir = tempfile.mkdtemp() - doc = minidom.Document() - emc = doc.createElement("EMC") - doc.appendChild(emc) - filename = 'cinder_dell_emc_config_%s.xml' % 'fake_xml' - config_file_path = tempdir + '/' + filename - f = open(config_file_path, 'w') - doc.writexml(f) - f.close() - self.assertRaises(exception.VolumeBackendAPIException, - self.utils.parse_file_to_get_array_map, - config_file_path) - def test_truncate_string(self): # string is less than max number str_to_truncate = 'string' @@ -1796,10 +1631,10 @@ class VMAXRestTest(test.TestCase): super(VMAXRestTest, self).setUp() volume_utils.get_max_over_subscription_ratio = mock.Mock() - config_group = 'RestTests' - fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_f) - configuration = FakeConfiguration(fake_xml, config_group) + configuration = FakeConfiguration( + None, 'RestTests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = fc.VMAXFCDriver(configuration=configuration) @@ -3255,10 +3090,10 @@ class VMAXProvisionTest(test.TestCase): super(VMAXProvisionTest, self).setUp() volume_utils.get_max_over_subscription_ratio = mock.Mock() - config_group = 'ProvisionTests' - self.fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_i) - configuration = FakeConfiguration(self.fake_xml, config_group) + configuration = FakeConfiguration( + None, 'ProvisionTests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = iscsi.VMAXISCSIDriver(configuration=configuration) @@ -3760,11 +3595,10 @@ class VMAXCommonTest(test.TestCase): super(VMAXCommonTest, self).setUp() self.mock_object(volume_utils, 'get_max_over_subscription_ratio', return_value=1.0) - config_group = 'CommonTests' - self.fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_f) - configuration = FakeConfiguration(self.fake_xml, config_group, - 1, 1) + configuration = FakeConfiguration( + None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_f]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = fc.VMAXFCDriver(configuration=configuration) @@ -3782,16 +3616,15 @@ class VMAXCommonTest(test.TestCase): @mock.patch.object(common.VMAXCommon, '_get_slo_workload_combinations', return_value=[]) - @mock.patch.object(utils.VMAXUtils, - 'parse_file_to_get_array_map', + @mock.patch.object(common.VMAXCommon, + 'get_attributes_from_cinder_config', return_value=[]) def test_gather_info_no_opts(self, mock_parse, mock_combo, mock_rest): configuration = FakeConfiguration(None, 'config_group', None, None) fc.VMAXFCDriver(configuration=configuration) def test_get_slo_workload_combinations_success(self): - array_info = self.utils.parse_file_to_get_array_map( - self.common.pool_info['config_file']) + array_info = self.common.get_attributes_from_cinder_config() finalarrayinfolist = self.common._get_slo_workload_combinations( array_info) self.assertTrue(len(finalarrayinfolist) > 1) @@ -4123,23 +3956,6 @@ class VMAXCommonTest(test.TestCase): data = self.common.update_volume_stats() self.assertEqual('CommonTests', data['volume_backend_name']) - def test_set_config_file_and_get_extra_specs(self): - volume = self.data.test_volume - extra_specs, config_file, qos_specs = ( - self.common._set_config_file_and_get_extra_specs(volume)) - self.assertEqual(self.data.vol_type_extra_specs, extra_specs) - self.assertEqual(self.fake_xml, config_file) - - def test_set_config_file_and_get_extra_specs_no_specs(self): - volume = self.data.test_volume - ref_config = '/etc/cinder/cinder_dell_emc_config.xml' - with mock.patch.object(self.utils, 'get_volumetype_extra_specs', - return_value=None): - extra_specs, config_file, qos_specs = ( - self.common._set_config_file_and_get_extra_specs(volume)) - self.assertIsNone(extra_specs) - self.assertEqual(ref_config, config_file) - def test_find_device_on_array_success(self): volume = self.data.test_volume extra_specs = self.data.extra_specs @@ -4240,18 +4056,6 @@ class VMAXCommonTest(test.TestCase): volume, None, extra_specs) self.assertEqual(ref_masked, maskedvols) - def test_register_config_file_from_config_group_exists(self): - config_group_name = 'CommonTests' - config_file = self.common._register_config_file_from_config_group( - config_group_name) - self.assertEqual(self.fake_xml, config_file) - - def test_register_config_file_from_config_group_does_not_exist(self): - config_group_name = 'IncorrectName' - self.assertRaises(exception.VolumeBackendAPIException, - self.common._register_config_file_from_config_group, - config_group_name) - def test_initial_setup_success(self): volume = self.data.test_volume ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set) @@ -4261,8 +4065,9 @@ class VMAXCommonTest(test.TestCase): def test_initial_setup_failed(self): volume = self.data.test_volume - with mock.patch.object(self.utils, 'parse_file_to_get_array_map', - return_value=None): + with mock.patch.object( + self.common, 'get_attributes_from_cinder_config', + return_value=None): self.assertRaises(exception.VolumeBackendAPIException, self.common._initial_setup, volume) @@ -4475,8 +4280,7 @@ class VMAXCommonTest(test.TestCase): volume_name, volume_size, extra_specs) def test_set_vmax_extra_specs(self): - srp_record = self.utils.parse_file_to_get_array_map( - self.fake_xml) + srp_record = self.common.get_attributes_from_cinder_config() extra_specs = self.common._set_vmax_extra_specs( self.data.vol_type_extra_specs, srp_record) ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set) @@ -4484,16 +4288,14 @@ class VMAXCommonTest(test.TestCase): self.assertEqual(ref_extra_specs, extra_specs) def test_set_vmax_extra_specs_no_srp_name(self): - srp_record = self.utils.parse_file_to_get_array_map( - self.fake_xml) + srp_record = self.common.get_attributes_from_cinder_config() extra_specs = self.common._set_vmax_extra_specs({}, srp_record) self.assertEqual('Optimized', extra_specs['slo']) def test_set_vmax_extra_specs_compr_disabled(self): with mock.patch.object(self.rest, 'is_compression_capable', return_value=True): - srp_record = self.utils.parse_file_to_get_array_map( - self.fake_xml) + srp_record = self.common.get_attributes_from_cinder_config() extra_specs = self.common._set_vmax_extra_specs( self.data.vol_type_extra_specs_compr_disabled, srp_record) ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set) @@ -4502,8 +4304,7 @@ class VMAXCommonTest(test.TestCase): self.assertEqual(ref_extra_specs, extra_specs) def test_set_vmax_extra_specs_compr_disabled_not_compr_capable(self): - srp_record = self.utils.parse_file_to_get_array_map( - self.fake_xml) + srp_record = self.common.get_attributes_from_cinder_config() extra_specs = self.common._set_vmax_extra_specs( self.data.vol_type_extra_specs_compr_disabled, srp_record) ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set) @@ -4511,16 +4312,20 @@ class VMAXCommonTest(test.TestCase): self.assertEqual(ref_extra_specs, extra_specs) def test_set_vmax_extra_specs_portgroup_as_spec(self): - srp_record = self.utils.parse_file_to_get_array_map( - self.fake_xml) + srp_record = self.common.get_attributes_from_cinder_config() extra_specs = self.common._set_vmax_extra_specs( {utils.PORTGROUPNAME: 'extra_spec_pg'}, srp_record) self.assertEqual('extra_spec_pg', extra_specs[utils.PORTGROUPNAME]) def test_set_vmax_extra_specs_no_portgroup_set(self): - fake_xml = FakeXML().create_fake_config_file( - 'test_no_pg_set', '') - srp_record = self.utils.parse_file_to_get_array_map(fake_xml) + srp_record = {'srpName': 'SRP_1', + 'RestServerIp': '1.1.1.1', + 'RestPassword': 'smc', + 'SSLCert': None, + 'RestServerPort': 8443, + 'SSLVerify': False, + 'RestUserName': 'smc', + 'SerialNumber': '000197800123'} self.assertRaises(exception.VolumeBackendAPIException, self.common._set_vmax_extra_specs, {}, srp_record) @@ -5375,7 +5180,7 @@ class VMAXCommonTest(test.TestCase): self.assertEqual(ref_dev_id, src_dev_id1) self.assertEqual(ref_dev_id, src_dev_id2) - def test_get_attributes_from_cinder_config_new(self): + def test_get_attributes_from_cinder_config_new_and_old(self): kwargs_expected = ( {'RestServerIp': '1.1.1.1', 'RestServerPort': 8443, @@ -5386,7 +5191,7 @@ class VMAXCommonTest(test.TestCase): 'SerialNumber': self.data.array, 'srpName': 'SRP_1', 'PortGroup': self.data.port_group_name_i}) - backup_conf = self.common.configuration + old_conf = FakeConfiguration(None, 'CommonTests', 1, 1) configuration = FakeConfiguration( None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc', vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', @@ -5394,30 +5199,7 @@ class VMAXCommonTest(test.TestCase): self.common.configuration = configuration kwargs_returned = self.common.get_attributes_from_cinder_config() self.assertEqual(kwargs_expected, kwargs_returned) - self.common.configuration = backup_conf - kwargs = self.common.get_attributes_from_cinder_config() - self.assertIsNone(kwargs) - - def test_get_attributes_from_cinder_config_old(self): - kwargs_expected = ( - {'RestServerIp': '1.1.1.1', - 'RestServerPort': 8443, - 'RestUserName': 'smc', - 'RestPassword': 'smc', - 'SSLCert': None, - 'SSLVerify': False, - 'SerialNumber': self.data.array, - 'srpName': 'SRP_1', - 'PortGroup': self.data.port_group_name_i}) - backup_conf = self.common.configuration - configuration = FakeConfiguration( - None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc', - vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', - san_rest_port=8443, vmax_port_groups=[self.data.port_group_name_i]) - self.common.configuration = configuration - kwargs_returned = self.common.get_attributes_from_cinder_config() - self.assertEqual(kwargs_expected, kwargs_returned) - self.common.configuration = backup_conf + self.common.configuration = old_conf kwargs = self.common.get_attributes_from_cinder_config() self.assertIsNone(kwargs) @@ -5703,11 +5485,11 @@ class VMAXFCTest(test.TestCase): self.data = VMAXCommonData() super(VMAXFCTest, self).setUp() - config_group = 'FCTests' volume_utils.get_max_over_subscription_ratio = mock.Mock() - self.fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_f) - self.configuration = FakeConfiguration(self.fake_xml, config_group) + self.configuration = FakeConfiguration( + None, 'FCTests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = fc.VMAXFCDriver(configuration=self.configuration) @@ -5961,11 +5743,10 @@ class VMAXISCSITest(test.TestCase): self.data = VMAXCommonData() super(VMAXISCSITest, self).setUp() - config_group = 'ISCSITests' - self.fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_i) - volume_utils.get_max_over_subscription_ratio = mock.Mock() - configuration = FakeConfiguration(self.fake_xml, config_group) + configuration = FakeConfiguration( + None, 'ISCSITests', 1, 1, san_ip='1.1.1.1', san_login='smc', + vmax_array=self.data.array, vmax_srp='SRP_1', san_password='smc', + san_api_port=8443, vmax_port_groups=[self.data.port_group_name_i]) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = iscsi.VMAXISCSIDriver(configuration=configuration) @@ -7266,9 +7047,6 @@ class VMAXCommonReplicationTest(test.TestCase): self.data = VMAXCommonData() super(VMAXCommonReplicationTest, self).setUp() - config_group = 'CommonReplicationTests' - self.fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_f) self.replication_device = { 'target_device_id': self.data.remote_array, 'remote_port_group': self.data.port_group_name_f, @@ -7276,16 +7054,21 @@ class VMAXCommonReplicationTest(test.TestCase): 'rdf_group_label': self.data.rdf_group_name, 'allow_extend': 'True'} volume_utils.get_max_over_subscription_ratio = mock.Mock() + configuration = FakeConfiguration( - self.fake_xml, config_group, + None, 'CommonReplicationTests', 1, 1, san_ip='1.1.1.1', + san_login='smc', vmax_array=self.data.array, vmax_srp='SRP_1', + san_password='smc', san_api_port=8443, + vmax_port_groups=[self.data.port_group_name_f], replication_device=self.replication_device) rest.VMAXRest._establish_rest_session = mock.Mock( return_value=FakeRequestsSession()) driver = fc.VMAXFCDriver(configuration=configuration) - iscsi_fake_xml = FakeXML().create_fake_config_file( - config_group, self.data.port_group_name_i) iscsi_config = FakeConfiguration( - iscsi_fake_xml, config_group, + None, 'CommonReplicationTests', 1, 1, san_ip='1.1.1.1', + san_login='smc', vmax_array=self.data.array, vmax_srp='SRP_1', + san_password='smc', san_api_port=8443, + vmax_port_groups=[self.data.port_group_name_i], replication_device=self.replication_device) iscsi_driver = iscsi.VMAXISCSIDriver(configuration=iscsi_config) self.iscsi_common = iscsi_driver.common @@ -7299,8 +7082,8 @@ class VMAXCommonReplicationTest(test.TestCase): mock.Mock( return_value=self.data.vol_type_extra_specs_rep_enabled)) self.extra_specs = deepcopy(self.data.extra_specs_rep_enabled) - self.extra_specs['retries'] = 0 - self.extra_specs['interval'] = 0 + self.extra_specs['retries'] = 1 + self.extra_specs['interval'] = 1 self.extra_specs['rep_mode'] = 'Synchronous' self.async_rep_device = { 'target_device_id': self.data.remote_array, @@ -7309,7 +7092,10 @@ class VMAXCommonReplicationTest(test.TestCase): 'rdf_group_label': self.data.rdf_group_name, 'allow_extend': 'True', 'mode': 'async'} async_configuration = FakeConfiguration( - self.fake_xml, config_group, + None, 'CommonReplicationTests', 1, 1, san_ip='1.1.1.1', + san_login='smc', vmax_array=self.data.array, vmax_srp='SRP_1', + san_password='smc', san_api_port=8443, + vmax_port_groups=[self.data.port_group_name_f], replication_device=self.async_rep_device) self.async_driver = fc.VMAXFCDriver(configuration=async_configuration) self.metro_rep_device = { @@ -7319,7 +7105,10 @@ class VMAXCommonReplicationTest(test.TestCase): 'rdf_group_label': self.data.rdf_group_name, 'allow_extend': 'True', 'mode': 'metro'} metro_configuration = FakeConfiguration( - self.fake_xml, config_group, + None, 'CommonReplicationTests', 1, 1, san_ip='1.1.1.1', + san_login='smc', vmax_array=self.data.array, vmax_srp='SRP_1', + san_password='smc', san_api_port=8443, + vmax_port_groups=[self.data.port_group_name_f], replication_device=self.metro_rep_device) self.metro_driver = fc.VMAXFCDriver(configuration=metro_configuration) @@ -7521,7 +7310,7 @@ class VMAXCommonReplicationTest(test.TestCase): self.data.device_id, volume_name, "5", extra_specs) def test_set_config_file_get_extra_specs_rep_enabled(self): - extra_specs, _, _ = self.common._set_config_file_and_get_extra_specs( + extra_specs, _ = self.common._set_config_file_and_get_extra_specs( self.data.test_volume) self.assertTrue(extra_specs['replication_enabled']) @@ -7853,8 +7642,7 @@ class VMAXCommonReplicationTest(test.TestCase): def test_get_secondary_stats(self): rep_config = self.utils.get_replication_config( [self.replication_device]) - array_map = self.utils.parse_file_to_get_array_map( - self.common.pool_info['config_file']) + array_map = self.common.get_attributes_from_cinder_config() finalarrayinfolist = self.common._get_slo_workload_combinations( array_map) array_info = finalarrayinfolist[0] diff --git a/cinder/volume/drivers/dell_emc/vmax/common.py b/cinder/volume/drivers/dell_emc/vmax/common.py index 02b8081d911..45ea63d78d5 100644 --- a/cinder/volume/drivers/dell_emc/vmax/common.py +++ b/cinder/volume/drivers/dell_emc/vmax/common.py @@ -16,7 +16,6 @@ import ast from copy import deepcopy import math -import os.path import random import sys import time @@ -41,9 +40,6 @@ LOG = logging.getLogger(__name__) CONF = cfg.CONF -CINDER_EMC_CONFIG_FILE = '/etc/cinder/cinder_dell_emc_config.xml' -CINDER_EMC_CONFIG_FILE_PREFIX = '/etc/cinder/cinder_dell_emc_config_' -CINDER_EMC_CONFIG_FILE_POSTFIX = '.xml' BACKENDNAME = 'volume_backend_name' PREFIXBACKENDNAME = 'capabilities:volume_backend_name' @@ -56,11 +52,6 @@ REPLICATION_ERROR = fields.ReplicationStatus.ERROR vmax_opts = [ - cfg.StrOpt('cinder_dell_emc_config_file', - default=CINDER_EMC_CONFIG_FILE, - deprecated_for_removal=True, - help='Use this file for cinder emc plugin ' - 'config data.'), cfg.IntOpt('interval', default=3, help='Use this value to specify ' @@ -137,8 +128,10 @@ class VMAXCommon(object): self._get_attributes_from_config() array_info = self.get_attributes_from_cinder_config() if array_info is None: - array_info = self.utils.parse_file_to_get_array_map( - self.pool_info['config_file']) + LOG.error("Unable to get attributes from cinder.conf. Please " + "refer to the current online documentation for correct " + "configuration and note that the xml file is no " + "longer supported.") self.rest.set_rest_credentials(array_info) finalarrayinfolist = self._get_slo_workload_combinations( array_info) @@ -146,12 +139,6 @@ class VMAXCommon(object): def _get_attributes_from_config(self): """Get relevent details from configuration file.""" - if hasattr(self.configuration, 'cinder_dell_emc_config_file'): - self.pool_info['config_file'] = ( - self.configuration.cinder_dell_emc_config_file) - else: - self.pool_info['config_file'] = ( - self.configuration.safe_get('cinder_dell_emc_config_file')) self.interval = self.configuration.safe_get('interval') self.retries = self.configuration.safe_get('retries') self.pool_info['backend_name'] = ( @@ -563,8 +550,8 @@ class VMAXCommon(object): storage_group_name = OS----SG e.g OS-myShortHost-SRP_1-I-SG port_group_name = OS--PG The port_group_name will come from - the EMC configuration xml file. - These are precreated. If the portGroup does not + the cinder.conf or as an extra spec on the volume + type. These are precreated. If the portGroup does not exist then an error will be returned to the user maskingview_name = OS----MV e.g OS-myShortHost-SRP_1-I-MV @@ -939,8 +926,7 @@ class VMAXCommon(object): array_reserve_percent) def _set_config_file_and_get_extra_specs(self, volume, - volume_type_id=None, - register_config_file=True): + volume_type_id=None): """Given the volume object get the associated volumetype. Given the volume object get the associated volumetype and the @@ -950,7 +936,7 @@ class VMAXCommon(object): :param volume: the volume object including the volume_type_id :param volume_type_id: Optional override of volume.volume_type_id :returns: dict -- the extra specs dict - :returns: string -- configuration file + :returns: dict -- QoS specs """ qos_specs = {} extra_specs = self.utils.get_volumetype_extra_specs( @@ -960,11 +946,8 @@ class VMAXCommon(object): res = volume_types.get_volume_type_qos_specs(type_id) qos_specs = res['qos_specs'] - config_group = None - config_file = None # If there are no extra specs then the default case is assumed. if extra_specs: - config_group = self.configuration.config_group if extra_specs.get('replication_enabled') == ' True': extra_specs[utils.IS_RE] = True if self.rep_config and self.rep_config.get('mode'): @@ -972,10 +955,7 @@ class VMAXCommon(object): if self.rep_config and self.rep_config.get(utils.METROBIAS): extra_specs[utils.METROBIAS] = self.rep_config[ utils.METROBIAS] - if register_config_file: - config_file = self._register_config_file_from_config_group( - config_group) - return extra_specs, config_file, qos_specs + return extra_specs, qos_specs def _find_device_on_array(self, volume, extra_specs): """Given the volume get the VMAX device Id. @@ -1126,45 +1106,6 @@ class VMAXCommon(object): all_masking_view_list) return maskingview_list, all_masking_view_list - def _register_config_file_from_config_group(self, config_group_name): - """Given the config group name register the file. - - :param config_group_name: the config group name - :returns: string -- configurationFile - name of the configuration file - :raises: VolumeBackendAPIException: - """ - if config_group_name is None: - return CINDER_EMC_CONFIG_FILE - if hasattr(self.configuration, 'cinder_dell_emc_config_file'): - config_file = self.configuration.cinder_dell_emc_config_file - else: - config_file = ( - ("%(prefix)s%(configGroupName)s%(postfix)s" - % {'prefix': CINDER_EMC_CONFIG_FILE_PREFIX, - 'configGroupName': config_group_name, - 'postfix': CINDER_EMC_CONFIG_FILE_POSTFIX})) - - # The file saved in self.configuration may not be the correct one, - # double check. - if config_group_name not in config_file: - config_file = ( - ("%(prefix)s%(configGroupName)s%(postfix)s" - % {'prefix': CINDER_EMC_CONFIG_FILE_PREFIX, - 'configGroupName': config_group_name, - 'postfix': CINDER_EMC_CONFIG_FILE_POSTFIX})) - - if os.path.isfile(config_file): - LOG.debug("Configuration file : %(configurationFile)s exists.", - {'configurationFile': config_file}) - else: - exception_message = (_( - "Configuration file %(configurationFile)s does not exist.") - % {'configurationFile': config_file}) - LOG.error(exception_message) - raise exception.VolumeBackendAPIException(data=exception_message) - - return config_file - def _initial_setup(self, volume, volume_type_id=None): """Necessary setup to accumulate the relevant information. @@ -1180,18 +1121,15 @@ class VMAXCommon(object): try: array_info = self.get_attributes_from_cinder_config() if array_info: - extra_specs, config_file, qos_specs = ( - self._set_config_file_and_get_extra_specs( - volume, volume_type_id, register_config_file=False)) - else: - extra_specs, config_file, qos_specs = ( + extra_specs, qos_specs = ( self._set_config_file_and_get_extra_specs( volume, volume_type_id)) - array_info = self.utils.parse_file_to_get_array_map( - self.pool_info['config_file']) - if not array_info: + else: exception_message = (_( - "Unable to get corresponding record for srp.")) + "Unable to get corresponding record for srp. Please " + "refer to the current online documentation for correct " + "configuration and note that the xml file is no longer " + "supported.")) raise exception.VolumeBackendAPIException( data=exception_message) @@ -1491,11 +1429,7 @@ class VMAXCommon(object): The pool_name extra spec must be set, otherwise a default slo/workload will be chosen. The portgroup can either be passed as an extra spec on the volume type (e.g. 'storagetype:portgroupname = os-pg1-pg'), or - can be chosen from a list provided in the xml file, e.g.: - - OS-PORTGROUP1-PG - OS-PORTGROUP2-PG - . + can be chosen from a list provided in the cinder.conf :param extra_specs: extra specifications :param pool_record: pool record @@ -1504,19 +1438,16 @@ class VMAXCommon(object): # set extra_specs from pool_record extra_specs[utils.SRP] = pool_record['srpName'] extra_specs[utils.ARRAY] = pool_record['SerialNumber'] - if not extra_specs.get(utils.PORTGROUPNAME): - extra_specs[utils.PORTGROUPNAME] = pool_record['PortGroup'] - if not extra_specs[utils.PORTGROUPNAME]: + try: + if not extra_specs.get(utils.PORTGROUPNAME): + extra_specs[utils.PORTGROUPNAME] = pool_record['PortGroup'] + except Exception: error_message = (_("Port group name has not been provided - " "please configure the " "'storagetype:portgroupname' extra spec on " "the volume type, or enter a list of " - "portgroups to the xml file associated with " - "this backend e.g." - "" - " OS-PORTGROUP1-PG" - " OS-PORTGROUP2-PG" - ".")) + "portgroups in the cinder.conf associated with " + "this backend.")) LOG.exception(error_message) raise exception.VolumeBackendAPIException(data=error_message) @@ -1544,7 +1475,7 @@ class VMAXCommon(object): if not workload_from_extra_spec: workload_from_extra_spec = 'NONE' LOG.info("Pool_name is not present in the extra_specs " - "- using slo/ workload from xml file: %(slo)s/%(wl)s.", + "- using slo/ workload from cinder.conf: %(slo)s/%(wl)s.", {'slo': slo_from_extra_spec, 'wl': workload_from_extra_spec}) @@ -1558,10 +1489,10 @@ class VMAXCommon(object): else: slo_from_extra_spec = 'None' workload_from_extra_spec = 'NONE' - LOG.warning("Pool_name is not present in the extra_specs" - "and no slo/ workload information is present " - "in the xml file - using default slo/ workload " - "combination: %(slo)s/%(wl)s.", + LOG.warning("Pool_name is not present in the extra_specs " + "so no slo/ workload information is present " + "using default slo/ workload combination: " + "%(slo)s/%(wl)s.", {'slo': slo_from_extra_spec, 'wl': workload_from_extra_spec}) # Standardize slo and workload 'NONE' naming conventions diff --git a/cinder/volume/drivers/dell_emc/vmax/utils.py b/cinder/volume/drivers/dell_emc/vmax/utils.py index 40edee5ff38..48f74f48e3e 100644 --- a/cinder/volume/drivers/dell_emc/vmax/utils.py +++ b/cinder/volume/drivers/dell_emc/vmax/utils.py @@ -15,9 +15,7 @@ from copy import deepcopy import datetime -from defusedxml import minidom import hashlib -import random import re from cinder.objects.group import Group @@ -309,136 +307,6 @@ class VMAXUtils(object): max_over_sub_ratio = 20.0 return max_over_sub_ratio - @staticmethod - def _process_tag(element, tag_name): - """Process the tag to get the value. - - :param element: the parent element - :param tag_name: the tag name - :returns: nodeValue(can be None) - """ - node_value = None - try: - processed_element = element.getElementsByTagName(tag_name)[0] - node_value = processed_element.childNodes[0].nodeValue - if node_value: - node_value = node_value.strip() - except IndexError: - pass - return node_value - - def _get_connection_info(self, rest_element): - """Given the filename get the rest server connection details. - - :param rest_element: the rest element - :returns: dict -- connargs - the connection info dictionary - :raises: VolumeBackendAPIException - """ - connargs = { - 'RestServerIp': ( - self._process_tag(rest_element, 'RestServerIp')), - 'RestServerPort': ( - self._process_tag(rest_element, 'RestServerPort')), - 'RestUserName': ( - self._process_tag(rest_element, 'RestUserName')), - 'RestPassword': ( - self._process_tag(rest_element, 'RestPassword'))} - - for k, __ in connargs.items(): - if connargs[k] is None: - exception_message = (_( - "RestServerIp, RestServerPort, RestUserName, " - "RestPassword must have valid values.")) - LOG.error(exception_message) - raise exception.VolumeBackendAPIException( - data=exception_message) - - # These can be None - connargs['SSLCert'] = self._process_tag(rest_element, 'SSLCert') - connargs['SSLVerify'] = ( - self._process_tag(rest_element, 'SSLVerify')) - - return connargs - - def parse_file_to_get_array_map(self, file_name): - """Parses a file and gets array map. - - Given a file, parse it to get array and pool(srp). - - .. code:: ini - - - 10.108.246.202 - 8443 - smc - smc - /path/client.cert - /path/to/certfile.pem - - OS-PORTGROUP1-PG - - 000198700439 - SRP_1 - - - :param file_name: the configuration file - :returns: list - """ - LOG.warning("Use of xml file in backend configuration is deprecated " - "in Queens and will not be supported in future releases.") - kwargs = {} - my_file = open(file_name, 'r') - data = my_file.read() - my_file.close() - dom = minidom.parseString(data) - try: - connargs = self._get_connection_info(dom) - portgroup = self._get_random_portgroup(dom) - serialnumber = self._process_tag(dom, 'Array') - if serialnumber is None: - LOG.error("Array Serial Number must be in the file %(file)s.", - {'file': file_name}) - srp_name = self._process_tag(dom, 'SRP') - if srp_name is None: - LOG.error("SRP Name must be in the file %(file)s.", - {'file': file_name}) - slo = self._process_tag(dom, 'ServiceLevel') - workload = self._process_tag(dom, 'Workload') - kwargs = ( - {'RestServerIp': connargs['RestServerIp'], - 'RestServerPort': connargs['RestServerPort'], - 'RestUserName': connargs['RestUserName'], - 'RestPassword': connargs['RestPassword'], - 'SSLCert': connargs['SSLCert'], - 'SSLVerify': connargs['SSLVerify'], - 'SerialNumber': serialnumber, - 'srpName': srp_name, - 'PortGroup': portgroup}) - if slo is not None: - kwargs.update({'ServiceLevel': slo, 'Workload': workload}) - - except IndexError: - pass - return kwargs - - @staticmethod - def _get_random_portgroup(element): - """Randomly choose a portgroup from list of portgroups. - - :param element: the parent element - :returns: the randomly chosen port group - """ - portgroupelements = element.getElementsByTagName('PortGroup') - if portgroupelements and len(portgroupelements) > 0: - portgroupnames = [portgroupelement.childNodes[0].nodeValue.strip() - for portgroupelement in portgroupelements - if portgroupelement.childNodes] - portgroupnames = list(set(filter(None, portgroupnames))) - pg_len = len(portgroupnames) - if pg_len > 0: - return portgroupnames[random.randint(0, pg_len - 1)] - return None - def get_temp_snap_name(self, clone_name, source_device_id): """Construct a temporary snapshot name for clone operation. diff --git a/doc/source/configuration/block-storage/drivers/dell-emc-vmax-driver.rst b/doc/source/configuration/block-storage/drivers/dell-emc-vmax-driver.rst index 2dcf0a6e82d..38168d0fe2e 100644 --- a/doc/source/configuration/block-storage/drivers/dell-emc-vmax-driver.rst +++ b/doc/source/configuration/block-storage/drivers/dell-emc-vmax-driver.rst @@ -154,9 +154,8 @@ VMAX Driver Integration .. note:: For security and backend uniformity, the use of the XML file for VMAX - backend configuration has been deprecated in Queens. While the xml file - usage will still be supported, a warning will be issued on its impending - deprecation. + backend configuration was deprecated in Queens and removed entirely + in Rocky. +-----------------+------------------------+---------+----------+---------------------------+ | VMAX parameter | cinder.conf parameter | Default | Required | Description | @@ -1182,8 +1181,7 @@ Configure the source and target arrays #. Configure an SRDF group between the chosen source and target arrays for the VMAX cinder driver to use. The source array must correspond - with the 'vmax_array' entry in the cinder.conf (or the ```` entry - in the VMAX XML file for legacy setups). + with the 'vmax_array' entry in the cinder.conf. #. Select both the director and the ports for the SRDF emulation to use on both sides. Bear in mind that network topology is important when choosing director endpoints. Supported modes are `Synchronous`, `Asynchronous`, diff --git a/releasenotes/notes/remove_deprecated_xml-4065b893d781f65c.yaml b/releasenotes/notes/remove_deprecated_xml-4065b893d781f65c.yaml new file mode 100644 index 00000000000..eb946357982 --- /dev/null +++ b/releasenotes/notes/remove_deprecated_xml-4065b893d781f65c.yaml @@ -0,0 +1,4 @@ +--- +upgrade: + - | + VMAX driver - Removed deprecated option ``cinder_dell_emc_config_file``