From: John Griffith Date: Sun, 14 Dec 2014 19:02:30 +0000 (-0700) Subject: Remove iscsi_helper calls from base iscsi driver X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=7095e1c5a7477ca77c8698d96a969a7d78090a18;p=openstack-build%2Fcinder-build.git Remove iscsi_helper calls from base iscsi driver There are a number of drivers that inherit from the base iscsi driver, using the base implementation of initialize_connection and terminate_connection. These drivers also don't have iscsi_helper defined. We've kept all iscsi_helper specific code in the tgt helpers themselves, or in the various versions of the LVM driver which is where they belong. Over time however there have been some iscsi_helper specific calls that have crept in to the base volume.driver:ISCSIDriver class. These were LIO specific calls and as a result when a configuration is set with multi-backend to use lioadm as the target helper, or even if it's not used but the default iscsi_helper is set to lioadm the initialize_connection and terminate_connection calls will try and access the LIO specific calls and raise because the drivers inheriting from this class don't have target_helper members. This went unnoticed mostly because there's no significant LIO testing in place and there were no distros setting LIO as the default iscsi_helper, it appears that this has changed however. This patch moves the LIO specific calls back where they belong and keeps the base ISCSIDriver generic. I've also added a test case for this in the SolidFire driver because it's impacted by this. Change-Id: Ibe010b62bb2518685753dd515326e56ffcc99cea Closes-Bug: #1400804 --- diff --git a/cinder/tests/test_solidfire.py b/cinder/tests/test_solidfire.py index 1a37be387..a3da1435f 100644 --- a/cinder/tests/test_solidfire.py +++ b/cinder/tests/test_solidfire.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock import mox from oslo.utils import timeutils from oslo.utils import units @@ -280,6 +281,66 @@ class SolidFireVolumeTestCase(test.TestCase): sfv.create_cloned_volume(testvol_b, testvol) def test_initialize_connector_with_blocksizes(self): + expected_iqn = 'iqn.2010-01.com.solidfire:'\ + '87hg.uuid-2cc06226-cc74-4cb7-bd55-14aed659a0cc.4060' + expected_properties = \ + {'driver_volume_type': 'iscsi', + 'data': {'target_discovered': False, + 'encrypted': False, + 'logical_block_size': '4096', + 'physical_block_size': '4096', + 'target_iqn': expected_iqn, + 'target_portal': '10.10.7.1:3260', + 'volume_id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66', + 'target_lun': 0, + 'auth_password': '2FE0CQ8J196R', + 'auth_username': + 'stack-1-a60e2611875f40199931f2c76370d66b', + 'auth_method': 'CHAP'}} + + connector = {'initiator': 'iqn.2012-07.org.fake:01'} + testvol = {'project_id': 'testprjid', + 'name': 'testvol', + 'size': 1, + 'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66', + 'volume_type_id': None, + 'provider_location': '10.10.7.1:3260 iqn.2010-01.com.' + 'solidfire:87hg.uuid-2cc06226-cc' + '74-4cb7-bd55-14aed659a0cc.4060 0', + 'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2' + 'c76370d66b 2FE0CQ8J196R', + 'provider_geometry': '4096 4096', + 'created_at': timeutils.utcnow(), + } + + sfv = SolidFireDriver(configuration=self.configuration) + self.assertEqual(sfv.initialize_connection(testvol, connector), + expected_properties) + + @mock.patch('cinder.volume.driver.CONF') + def test_iscsi_helpers_not_in_base_iscsi_driver(self, mock_conf): + # This test is added to check for bug: 1400804 + # The base iscsi driver should be clean from specifics + # regarding tgtadm or LVM driver, this check is here + # to make sure nothing regarding specific iscsi_helpers + # sneak back in + expected_iqn = 'iqn.2010-01.com.solidfire:'\ + '87hg.uuid-2cc06226-cc74-4cb7-bd55-14aed659a0cc.4060' + expected_properties = \ + {'driver_volume_type': 'iscsi', + 'data': {'target_discovered': False, + 'encrypted': False, + 'logical_block_size': '4096', + 'physical_block_size': '4096', + 'target_iqn': expected_iqn, + 'target_portal': '10.10.7.1:3260', + 'volume_id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66', + 'target_lun': 0, + 'auth_password': '2FE0CQ8J196R', + 'auth_username': + 'stack-1-a60e2611875f40199931f2c76370d66b', + 'auth_method': 'CHAP'}} + connector = {'initiator': 'iqn.2012-07.org.fake:01'} testvol = {'project_id': 'testprjid', 'name': 'testvol', @@ -295,10 +356,20 @@ class SolidFireVolumeTestCase(test.TestCase): 'created_at': timeutils.utcnow(), } + mock_conf.iscsi_helper = 'lioadm' + sfv = SolidFireDriver(configuration=self.configuration) + self.assertEqual(sfv.initialize_connection(testvol, connector), + expected_properties) + + mock_conf.iscsi_helper = 'iseradm' + sfv = SolidFireDriver(configuration=self.configuration) + self.assertEqual(sfv.initialize_connection(testvol, connector), + expected_properties) + + mock_conf.iscsi_helper = 'tgtadm' sfv = SolidFireDriver(configuration=self.configuration) - properties = sfv.initialize_connection(testvol, connector) - self.assertEqual(properties['data']['physical_block_size'], '4096') - self.assertEqual(properties['data']['logical_block_size'], '4096') + self.assertEqual(sfv.initialize_connection(testvol, connector), + expected_properties) def test_create_volume_with_qos(self): preset_qos = {} diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 48611512f..4c07865ad 100755 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -1041,10 +1041,6 @@ class ISCSIDriver(VolumeDriver): } """ - - if CONF.iscsi_helper == 'lioadm': - self.target_helper.initialize_connection(volume, connector) - iscsi_properties = self._get_iscsi_properties(volume) return { 'driver_volume_type': 'iscsi', @@ -1060,8 +1056,7 @@ class ISCSIDriver(VolumeDriver): raise exception.VolumeBackendAPIException(data=err_msg) def terminate_connection(self, volume, connector, **kwargs): - if CONF.iscsi_helper == 'lioadm': - self.target_helper.terminate_connection(volume, connector) + pass def get_volume_stats(self, refresh=False): """Get volume stats. @@ -1111,6 +1106,8 @@ class ISCSIDriver(VolumeDriver): def get_target_helper(self, db): root_helper = utils.get_root_helper() + # FIXME(jdg): These work because the driver will overide + # but we need to move these to use self.configuraiton if CONF.iscsi_helper == 'iseradm': return iscsi.ISERTgtAdm(root_helper, CONF.volumes_dir, CONF.iscsi_target_prefix, db=db) @@ -1241,7 +1238,6 @@ class ISERDriver(ISCSIDriver): } """ - iser_properties = self._get_iscsi_properties(volume) return { 'driver_volume_type': 'iser', diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 95c885902..1ae4fc037 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -548,6 +548,23 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver): def create_export(self, context, volume): return self._create_export(context, volume) + def initialize_connection(self, volume, connector): + """Initializes the connection and returns connection info. """ + + # We have a special case for lioadm here, that's fine, we can + # keep the call in the parent class (driver:ISCSIDriver) generic + # and still use it throughout, just override and call super here + # no duplication, same effect but doesn't break things + # see bug: #1400804 + if self.configuration.iscsi_helper == 'lioadm': + self.target_helper.initialize_connection(volume, connector) + return super(LVMISCSIDriver, self).initialize_connection(volume, + connector) + + def terminate_connection(self, volume, connector, **kwargs): + if self.configuration.iscsi_helper == 'lioadm': + self.target_helper.terminate_connection(volume, connector) + def _create_export(self, context, volume, vg=None): """Creates an export for a logical volume.""" if vg is None: