From 4708aa16a7fd43a8fba95b51bf26021498776868 Mon Sep 17 00:00:00 2001 From: Tom Swanson Date: Wed, 14 Sep 2016 11:13:41 -0500 Subject: [PATCH] Dell SC: ISCSI initialize_connection fixes The driver could return duplicate iSCSI portal/IQN/Lun combinations. This patch returns the condensed list. Also sometimes when Cinder attempts to attach to a volume it will use the first item in the multipath list rather than the specified single path entries. So this patch simply moves a known up and active entry to the front of the list. Change-Id: Idb371825b6520d4bdb09a1dbb37429cb10643e93 Closes-Bug: #1622703 --- .../volume/drivers/dell/test_dellscapi.py | 80 +++++++++++++++---- .../drivers/dell/dell_storagecenter_api.py | 22 +++-- 2 files changed, 82 insertions(+), 20 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell/test_dellscapi.py b/cinder/tests/unit/volume/drivers/dell/test_dellscapi.py index f52e91e5164..a884191f00a 100644 --- a/cinder/tests/unit/volume/drivers/dell/test_dellscapi.py +++ b/cinder/tests/unit/volume/drivers/dell/test_dellscapi.py @@ -3543,8 +3543,7 @@ class DellSCSanAPITestCase(test.TestCase): '_find_active_controller', return_value='64702.64702') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, - '_find_controller_port', - return_value=ISCSI_CTRLR_PORT) + '_find_controller_port') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, '_find_domains', return_value=ISCSI_FLT_DOMAINS_MULTI_PORTALS) @@ -3564,6 +3563,59 @@ class DellSCSanAPITestCase(test.TestCase): mock_open_connection, mock_init): # Test case where there are multiple portals + mock_find_ctrl_port.side_effect = [ + {'iscsiName': 'iqn.2002-03.com.compellent:5000d31000fcbe43'}, + {'iscsiName': 'iqn.2002-03.com.compellent:5000d31000fcbe44'}] + res = self.scapi.find_iscsi_properties(self.VOLUME) + self.assertTrue(mock_find_mappings.called) + self.assertTrue(mock_find_domains.called) + self.assertTrue(mock_find_ctrl_port.called) + self.assertTrue(mock_find_active_controller.called) + self.assertTrue(mock_is_virtualport_mode.called) + expected = {'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe44', + 'target_iqns': + [u'iqn.2002-03.com.compellent:5000d31000fcbe44', + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + u'iqn.2002-03.com.compellent:5000d31000fcbe44'], + 'target_lun': 1, + 'target_luns': [1, 1, 1, 1], + 'target_portal': u'192.168.0.25:3260', + 'target_portals': [u'192.168.0.25:3260', + u'192.168.0.21:3260', + u'192.168.0.25:3260', + u'192.168.0.21:3260']} + self.assertEqual(expected, res, 'Wrong Target Info') + + @mock.patch.object(dell_storagecenter_api.StorageCenterApi, + '_find_active_controller', + return_value='64702.64702') + @mock.patch.object(dell_storagecenter_api.StorageCenterApi, + '_find_controller_port') + @mock.patch.object(dell_storagecenter_api.StorageCenterApi, + '_find_domains', + return_value=ISCSI_FLT_DOMAINS_MULTI_PORTALS) + @mock.patch.object(dell_storagecenter_api.StorageCenterApi, + '_find_mappings', + return_value=MAPPINGS_MULTI_PORTAL) + @mock.patch.object(dell_storagecenter_api.StorageCenterApi, + '_is_virtualport_mode', + return_value=True) + def test_find_iscsi_properties_multi_portals_duplicates( + self, + mock_is_virtualport_mode, + mock_find_mappings, + mock_find_domains, + mock_find_ctrl_port, + mock_find_active_controller, + mock_close_connection, + mock_open_connection, + mock_init): + # Test case where there are multiple portals and + mock_find_ctrl_port.return_value = { + 'iscsiName': 'iqn.2002-03.com.compellent:5000d31000fcbe43'} res = self.scapi.find_iscsi_properties(self.VOLUME) self.assertTrue(mock_find_mappings.called) self.assertTrue(mock_find_domains.called) @@ -3575,16 +3627,12 @@ class DellSCSanAPITestCase(test.TestCase): u'iqn.2002-03.com.compellent:5000d31000fcbe43', 'target_iqns': [u'iqn.2002-03.com.compellent:5000d31000fcbe43', - u'iqn.2002-03.com.compellent:5000d31000fcbe43', - u'iqn.2002-03.com.compellent:5000d31000fcbe43', u'iqn.2002-03.com.compellent:5000d31000fcbe43'], 'target_lun': 1, - 'target_luns': [1, 1, 1, 1], + 'target_luns': [1, 1], 'target_portal': u'192.168.0.25:3260', - 'target_portals': [u'192.168.0.21:3260', - u'192.168.0.25:3260', - u'192.168.0.21:3260', - u'192.168.0.25:3260']} + 'target_portals': [u'192.168.0.25:3260', + u'192.168.0.21:3260']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, @@ -3709,8 +3757,7 @@ class DellSCSanAPITestCase(test.TestCase): '_find_active_controller', return_value='64702.64702') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, - '_find_controller_port', - return_value=ISCSI_CTRLR_PORT) + '_find_controller_port') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, '_find_mappings', return_value=MAPPINGS_MULTI_PORTAL) @@ -3730,6 +3777,9 @@ class DellSCSanAPITestCase(test.TestCase): mock_close_connection, mock_open_connection, mock_init): + mock_find_ctrl_port.side_effect = [ + {'iscsiName': 'iqn.2002-03.com.compellent:5000d31000fcbe43'}, + {'iscsiName': 'iqn.2002-03.com.compellent:5000d31000fcbe44'}] # Test case where there are multiple portals res = self.scapi.find_iscsi_properties(self.VOLUME) self.assertTrue(mock_find_mappings.called) @@ -3737,13 +3787,13 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_active_controller.called) self.assertTrue(mock_is_virtualport_mode.called) self.assertTrue(mock_find_controller_port_iscsi_config.called) - # Since we're feeding the same info back multiple times the information - # will be duped. + # We're feeding the same info back multiple times the information + # will be scrubbed to a single item. expected = {'target_discovered': False, 'target_iqn': - u'iqn.2002-03.com.compellent:5000d31000fcbe43', + u'iqn.2002-03.com.compellent:5000d31000fcbe44', 'target_iqns': - [u'iqn.2002-03.com.compellent:5000d31000fcbe43', + [u'iqn.2002-03.com.compellent:5000d31000fcbe44', u'iqn.2002-03.com.compellent:5000d31000fcbe43'], 'target_lun': 1, 'target_luns': [1, 1], diff --git a/cinder/volume/drivers/dell/dell_storagecenter_api.py b/cinder/volume/drivers/dell/dell_storagecenter_api.py index d2903d094da..308be9e58c6 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_api.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_api.py @@ -1684,7 +1684,15 @@ class StorageCenterApi(object): :return: Nothing """ if self.excluded_domain_ips.count(address) == 0: - portals.append(address + ':' + six.text_type(port)) + # Make sure this isn't a duplicate. + newportal = address + ':' + six.text_type(port) + for idx, portal in enumerate(portals): + if portal == newportal and iqns[idx] == iqn: + LOG.debug('Skipping duplicate portal %(ptrl)s and' + 'iqn %(iqn)s.', {'ptrl': portal, 'iqn': iqn}) + return + # It isn't in the list so process it. + portals.append(newportal) iqns.append(iqn) luns.append(lun) @@ -1776,13 +1784,17 @@ class StorageCenterApi(object): 'Volume is not yet active on any controller.') pdata['active'] = 0 + # Make sure we have a good item at the top of the list. + iqns.insert(0, iqns.pop(pdata['active'])) + portals.insert(0, portals.pop(pdata['active'])) + luns.insert(0, luns.pop(pdata['active'])) data = {'target_discovered': False, - 'target_iqn': iqns[pdata['active']], + 'target_iqn': iqns[0], 'target_iqns': iqns, - 'target_portal': portals[pdata['active']], + 'target_portal': portals[0], 'target_portals': portals, - 'target_lun': luns[pdata['active']], - 'target_luns': luns, + 'target_lun': luns[0], + 'target_luns': luns } LOG.debug('find_iscsi_properties: %s', data) return data