From: Tom Swanson Date: Wed, 13 May 2015 17:00:54 +0000 (-0500) Subject: Dell SC: Added support for alternate iscsi portals X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3846830d237765cdb25678685538dc789c0f47d1;p=openstack-build%2Fcinder-build.git Dell SC: Added support for alternate iscsi portals Added support for returning alternate target portals, luns and iqns in a non multipath setup. This was done per https://review.openstack.org/#/c/140877/ Tests were modified to support this change. The iscsi initialize_connection and find_iscsi_properties functions took the brunt of this. The full list of portals, luns and iqns are returned along with the preferred portal, lun and iqn rather than a single iqn, lun and portal. Some minor log message changes for clarity. (Removed the one character vars.) Comment cleanup and clarification. Implements: blueprint dell-sc-add-alternate-iscsi-portals Change-Id: I40ad7c2e9b92fd6c6780ad181fcbccd25021d312 --- diff --git a/cinder/tests/unit/test_dellsc.py b/cinder/tests/unit/test_dellsc.py index f65fb0d80..efb4f0201 100644 --- a/cinder/tests/unit/test_dellsc.py +++ b/cinder/tests/unit/test_dellsc.py @@ -179,16 +179,16 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): ISCSI_PROPERTIES = {'access_mode': 'rw', 'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe43', 'target_iqns': - [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], - 'target_luns': [1], - 'target_portals': [u'192.168.0.21:3260']} - - ISCSI_PROPERTIES_EMPTY = {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': [], - 'target_luns': [], - 'target_portals': []} + [u'iqn.2002-03.com.compellent:5000d31000fcbe43', + u'iqn.2002-03.com.compellent:5000d31000fcbe44'], + 'target_lun': 1, + 'target_luns': [1, 1], + 'target_portal': u'192.168.0.21:3260', + 'target_portals': [u'192.168.0.21:3260', + u'192.168.0.22:3260']} def setUp(self): super(DellSCSanISCSIDriverTestCase, self).setUp() @@ -341,7 +341,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=MAPPINGS[0]) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES)) + return_value=ISCSI_PROPERTIES) def test_initialize_connection(self, mock_find_iscsi_props, mock_map_volume, @@ -359,13 +359,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): # verify find_volume has been called and that is has been called twice mock_find_volume.assert_any_call(self.volume_name) assert mock_find_volume.call_count == 2 - expected = {'data': - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqn': - u'iqn.2002-03.com.compellent:5000d31000fcbe43', - 'target_lun': 1, - 'target_portal': u'192.168.0.21:3260'}, + expected = {'data': self.ISCSI_PROPERTIES, 'driver_volume_type': 'iscsi'} self.assertEqual(expected, data, 'Unexpected return value') @@ -386,7 +380,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=MAPPINGS[0]) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES)) + return_value=ISCSI_PROPERTIES) def test_initialize_connection_multi_path(self, mock_find_iscsi_props, mock_map_volume, @@ -406,13 +400,8 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): # verify find_volume has been called and that is has been called twice mock_find_volume.assert_any_call(self.volume_name) assert mock_find_volume.call_count == 2 - expected = {'data': - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': - [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], - 'target_luns': [1], - 'target_portals': [u'192.168.0.21:3260']}, + props = self.ISCSI_PROPERTIES + expected = {'data': props, 'driver_volume_type': 'iscsi'} self.assertEqual(expected, data, 'Unexpected return value') @@ -430,7 +419,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=MAPPINGS) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES_EMPTY)) + return_value=None) def test_initialize_connection_no_iqn(self, mock_find_iscsi_properties, mock_map_volume, @@ -442,6 +431,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): mock_init): volume = {'id': self.volume_name} connector = {} + mock_find_iscsi_properties.side_effect = Exception('abc') self.assertRaises(exception.VolumeBackendAPIException, self.driver.initialize_connection, volume, @@ -464,7 +454,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=MAPPINGS) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES_EMPTY)) + return_value=None) def test_initialize_connection_no_server(self, mock_find_iscsi_properties, mock_map_volume, @@ -496,7 +486,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=MAPPINGS) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES_EMPTY)) + return_value=None) def test_initialize_connection_vol_not_found(self, mock_find_iscsi_properties, mock_map_volume, @@ -530,7 +520,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): return_value=None) @mock.patch.object(dell_storagecenter_api.StorageCenterApi, 'find_iscsi_properties', - return_value=(0, ISCSI_PROPERTIES)) + return_value=ISCSI_PROPERTIES) def test_initialize_connection_map_vol_fail(self, mock_find_iscsi_props, mock_map_volume, diff --git a/cinder/tests/unit/test_dellscapi.py b/cinder/tests/unit/test_dellscapi.py index 96bdf5458..79319d454 100644 --- a/cinder/tests/unit/test_dellscapi.py +++ b/cinder/tests/unit/test_dellscapi.py @@ -2949,13 +2949,16 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': + expected = {'access_mode': 'rw', + 'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + 'target_iqns': [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], - 'target_luns': [1], - 'target_portals': [u'192.168.0.21:3260']}) + 'target_lun': 1, + 'target_luns': [1], + 'target_portal': u'192.168.0.21:3260', + 'target_portals': [u'192.168.0.21:3260']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, @@ -2985,13 +2988,16 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': + expected = {'access_mode': 'rw', + 'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + 'target_iqns': [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], - 'target_luns': [1], - 'target_portals': [u'192.168.0.21:3260']}) + 'target_lun': 1, + 'target_luns': [1], + 'target_portal': u'192.168.0.21:3260', + 'target_portals': [u'192.168.0.21:3260']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, @@ -3021,12 +3027,16 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_ctrl.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': [], - 'target_luns': [], - 'target_portals': []}) + expected = {'access_mode': 'rw', + 'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + 'target_iqns': + [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], + 'target_lun': 1, + 'target_luns': [1], + 'target_portal': u'192.168.0.21:3260', + 'target_portals': [u'192.168.0.21:3260']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, @@ -3038,15 +3048,10 @@ class DellSCSanAPITestCase(test.TestCase): mock_open_connection, mock_init): # Test case where there are no ScMapping(s) - res = self.scapi.find_iscsi_properties(self.VOLUME) + self.assertRaises(exception.VolumeBackendAPIException, + self.scapi.find_iscsi_properties, + self.VOLUME) self.assertTrue(mock_find_mappings.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': [], - 'target_luns': [], - 'target_portals': []}) - self.assertEqual(expected, res, 'Expected empty Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, '_find_active_controller', @@ -3069,18 +3074,13 @@ class DellSCSanAPITestCase(test.TestCase): mock_open_connection, mock_init): # Test case where there are no ScFaultDomain(s) - res = self.scapi.find_iscsi_properties(self.VOLUME) + self.assertRaises(exception.VolumeBackendAPIException, + self.scapi.find_iscsi_properties, + self.VOLUME) self.assertTrue(mock_find_mappings.called) self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': [], - 'target_luns': [], - 'target_portals': []}) - self.assertEqual(expected, res, 'Expected empty Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, '_find_active_controller', @@ -3103,18 +3103,13 @@ class DellSCSanAPITestCase(test.TestCase): mock_open_connection, mock_init): # Test case where there are no ScFaultDomain(s) - res = self.scapi.find_iscsi_properties(self.VOLUME) + self.assertRaises(exception.VolumeBackendAPIException, + self.scapi.find_iscsi_properties, + self.VOLUME) self.assertTrue(mock_find_mappings.called) self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (0, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': [], - 'target_luns': [], - 'target_portals': []}) - self.assertEqual(expected, res, 'Expected empty Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, '_find_active_controller', @@ -3142,13 +3137,16 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (0, - {'access_mode': 'ro', - 'target_discovered': False, - 'target_iqns': + expected = {'access_mode': 'ro', + 'target_discovered': False, + 'target_iqn': + u'iqn.2002-03.com.compellent:5000d31000fcbe43', + 'target_iqns': [u'iqn.2002-03.com.compellent:5000d31000fcbe43'], - 'target_luns': [1], - 'target_portals': [u'192.168.0.21:3260']}) + 'target_lun': 1, + 'target_luns': [1], + 'target_portal': u'192.168.0.21:3260', + 'target_portals': [u'192.168.0.21:3260']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, @@ -3177,20 +3175,22 @@ class DellSCSanAPITestCase(test.TestCase): self.assertTrue(mock_find_domain.called) self.assertTrue(mock_find_ctrl_port.called) self.assertTrue(mock_find_active_controller.called) - expected = (3, - {'access_mode': 'rw', - 'target_discovered': False, - 'target_iqns': + expected = {'access_mode': 'rw', + 'target_discovered': False, + 'target_iqn': + 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_luns': [1, 1, 1, 1], - '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_lun': 1, + 'target_luns': [1, 1, 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']} self.assertEqual(expected, res, 'Wrong Target Info') @mock.patch.object(dell_storagecenter_api.StorageCenterApi, diff --git a/cinder/volume/drivers/dell/dell_storagecenter_api.py b/cinder/volume/drivers/dell/dell_storagecenter_api.py index 6053ecdb5..89fb072ed 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_api.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_api.py @@ -912,14 +912,18 @@ class StorageCenterApi(object): ipaddress = d.get('targetIpv4Address', d.get('wellKnownIpAddress')) portnumber = d.get('portNumber') - # We've all the information. Do we want to return - # it? + # We save our portal. + portals.append(ipaddress + ':' + + six.text_type(portnumber)) + iqns.append(iqn) + luns.append(lun) + + # We've all the information. We need to find + # the best single portal to return. So check + # this one if it is on the right IP, port and + # if the access and status are correct. if ((ip is None or ip == ipaddress) and (port is None or port == portnumber)): - portals.append(ipaddress + ':' + - six.text_type(portnumber)) - iqns.append(iqn) - luns.append(lun) # We need to point to the best link. # So state active and status up is preferred @@ -931,6 +935,12 @@ class StorageCenterApi(object): active = len(iqns) - 1 if status == 'Up': up = active + # Make sure we found something to return. + if len(luns) == 0: + # Since we just mapped this and can't find that mapping the world + # is wrong so we raise exception. + raise exception.VolumeBackendAPIException( + _('Unable to find iSCSI mappings.')) # Make sure we point to the best portal we can. This means it is # on the active controller and, preferably, up. If it isn't return @@ -946,17 +956,19 @@ class StorageCenterApi(object): active = 0 data = {'target_discovered': False, + 'target_iqn': iqns[active], 'target_iqns': iqns, + 'target_portal': portals[active], 'target_portals': portals, + 'target_lun': luns[active], 'target_luns': luns, 'access_mode': access_mode } - LOG.debug('find_iscsi_properties return: %(a)d %(d)s', - {'a': active, - 'd': data}) + LOG.debug('find_iscsi_properties return: %s', + data) - return active, data + return data def map_volume(self, scvolume, scserver): '''map_volume diff --git a/cinder/volume/drivers/dell/dell_storagecenter_common.py b/cinder/volume/drivers/dell/dell_storagecenter_common.py index 765651f94..596d88117 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_common.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_common.py @@ -111,7 +111,7 @@ class DellCommonDriver(san.SanDriver): def delete_volume(self, volume): deleted = False - # we use id as our name as it s unique + # We use id as our name as it is unique. volume_name = volume.get('id') LOG.debug('Deleting volume %s', volume_name) with self._client.open_connection() as api: diff --git a/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py b/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py index 86623aad0..141d22ec6 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_iscsi.py @@ -43,14 +43,28 @@ class DellStorageCenterISCSIDriver(san.SanISCSIDriver, or 'Dell-iSCSI') def initialize_connection(self, volume, connector): + # Initialize_connection will find or create a server identified by the + # connector on the Dell backend. It will then map the volume to it + # and return the properties as follows.. + # {'driver_volume_type': 'iscsi', + # data = {'target_discovered': False, + # 'target_iqn': preferred iqn, + # 'target_iqns': all iqns, + # 'target_portal': preferred portal, + # 'target_portals': all portals, + # 'target_lun': preferred lun, + # 'target_luns': all luns, + # 'access_mode': access_mode + # } + # We use id to name the volume name as it is a # known unique name. volume_name = volume.get('id') initiator_name = connector.get('initiator') multipath = connector.get('multipath', False) - LOG.info(_LI('initialize_ connection: %(n)s:%(i)s'), - {'n': volume_name, - 'i': initiator_name}) + LOG.info(_LI('initialize_ connection: %(vol)s:%(initiator)s'), + {'vol': volume_name, + 'initiator': initiator_name}) with self._client.open_connection() as api: try: @@ -70,50 +84,43 @@ class DellStorageCenterISCSIDriver(san.SanISCSIDriver, # Since we just mapped our volume we had best update # our sc volume object. scvolume = api.find_volume(volume_name) - - if multipath: - # Just return our properties with all the mappings - idx, iscsiproperties = ( - api.find_iscsi_properties(scvolume, - None, - None)) - return {'driver_volume_type': 'iscsi', - 'data': iscsiproperties} - else: - # Only return the iqn for the user specified port. + # Our return. + iscsiprops = {} + ip = None + port = None + if not multipath: + # We want to make sure we point to the specified + # ip address for our target_portal return. This + # isn't an issue with multipath since it should + # try all the alternate portal. ip = self.configuration.iscsi_ip_address port = self.configuration.iscsi_port - idx, iscsiproperties = ( - api.find_iscsi_properties(scvolume, - ip, - port)) - properties = {} - properties['target_discovered'] = False - portals = iscsiproperties['target_portals'] - # We'll key off of target_portals. If we have - # one listed we can assume that we found what - # we are looking for. Otherwise error. - if len(portals) > 0: - properties['target_portal'] = portals[idx] - properties['target_iqn'] = ( - iscsiproperties['target_iqns'][idx]) - properties['target_lun'] = ( - iscsiproperties['target_luns'][idx]) - properties['access_mode'] = ( - iscsiproperties['access_mode']) - LOG.debug(properties) - return {'driver_volume_type': 'iscsi', - 'data': properties - } - else: - LOG.error( - _LE('Volume mapped to invalid path.')) + + # Three cases that should all be satisfied with the + # same return of Target_Portal and Target_Portals. + # 1. Nova is calling us so we need to return the + # Target_Portal stuff. It should ignore the + # Target_Portals stuff. + # 2. OS brick is calling us in multipath mode so we + # want to return Target_Portals. It will ignore + # the Target_Portal stuff. + # 3. OS brick is calling us in single path mode so + # we want to return Target_Portal and + # Target_Portals as alternates. + iscsiprops = (api.find_iscsi_properties(scvolume, + ip, + port)) + + # Return our iscsi properties. + return {'driver_volume_type': 'iscsi', + 'data': iscsiprops} except Exception: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Failed to initialize connection ' - ' %(i)s %(n)s'), - {'i': initiator_name, - 'n': volume_name}) + error = (_('Failed to initialize connection ' + '%(initiator)s %(vol)s') % + {'initiator': initiator_name, + 'vol': volume_name}) + LOG.error(error) + raise exception.VolumeBackendAPIException(error) # We get here because our mapping is none or we have no valid iqn to # return so blow up. @@ -124,9 +131,9 @@ class DellStorageCenterISCSIDriver(san.SanISCSIDriver, # Grab some initial info. initiator_name = connector.get('initiator') volume_name = volume.get('id') - LOG.debug('Terminate connection: %(n)s:%(i)s', - {'n': volume_name, - 'i': initiator_name}) + LOG.debug('Terminate connection: %(vol)s:%(initiator)s', + {'vol': volume_name, + 'initiator': initiator_name}) with self._client.open_connection() as api: try: scserver = api.find_server(initiator_name) @@ -142,8 +149,8 @@ class DellStorageCenterISCSIDriver(san.SanISCSIDriver, except Exception: with excutils.save_and_reraise_exception(): LOG.error(_LE('Failed to terminate connection ' - '%(i)s %(n)s'), - {'i': initiator_name, - 'n': volume_name}) + '%(initiator)s %(vol)s'), + {'initiator': initiator_name, + 'vol': volume_name}) raise exception.VolumeBackendAPIException( _('Terminate connection failed'))