From ac15e1df04bef7aed8c11f89ceb0d051582f7938 Mon Sep 17 00:00:00 2001 From: Ann Kamyshnikova Date: Tue, 3 Sep 2013 17:44:50 +0400 Subject: [PATCH] Remove duplication of brick.iscsi in brick.iser TargetAdmin and TgtAdm in iser is mostly a copy of TargetAdmin and TgtAdm in iscsi. Change-Id: I8d89a35c478485370b41f4dfd376de07b9fc9314 --- cinder/brick/exception.py | 8 -- cinder/brick/iscsi/iscsi.py | 47 ++++++-- cinder/brick/iser/__init__.py | 16 --- cinder/brick/iser/iser.py | 206 ---------------------------------- cinder/tests/test_iscsi.py | 7 ++ cinder/tests/test_iser.py | 117 ------------------- cinder/volume/driver.py | 13 ++- cinder/volume/drivers/lvm.py | 39 ++++--- 8 files changed, 69 insertions(+), 384 deletions(-) delete mode 100644 cinder/brick/iser/__init__.py delete mode 100644 cinder/brick/iser/iser.py delete mode 100644 cinder/tests/test_iser.py diff --git a/cinder/brick/exception.py b/cinder/brick/exception.py index 33ccfb391..0e9ed4b43 100644 --- a/cinder/brick/exception.py +++ b/cinder/brick/exception.py @@ -95,14 +95,6 @@ class VolumeDeviceNotFound(BrickException): message = _("Volume device not found at %(device)s.") -class ISERTargetCreateFailed(BrickException): - message = _("Failed to create iser target for volume %(volume_id)s.") - - -class ISERTargetRemoveFailed(BrickException): - message = _("Failed to remove iser target for volume %(volume_id)s.") - - class VolumeGroupNotFound(BrickException): message = _('Unable to find Volume Group: %(vg_name)s') diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py index 103014e98..098ce5624 100644 --- a/cinder/brick/iscsi/iscsi.py +++ b/cinder/brick/iscsi/iscsi.py @@ -82,6 +82,17 @@ class TargetAdmin(executor.Executor): class TgtAdm(TargetAdmin): """iSCSI target administration using tgtadm.""" + VOLUME_CONF = """ + + backing-store %s + + """ + VOLUME_CONF_WITH_CHAP_AUTH = """ + + backing-store %s + %s + + """ def __init__(self, root_helper, volumes_dir, target_prefix='iqn.2010-10.org.openstack:', @@ -156,18 +167,10 @@ class TgtAdm(TargetAdmin): vol_id = name.split(':')[1] if chap_auth is None: - volume_conf = """ - - backing-store %s - - """ % (name, path) + volume_conf = self.VOLUME_CONF % (name, path) else: - volume_conf = """ - - backing-store %s - %s - - """ % (name, path, chap_auth) + volume_conf = self.VOLUME_CONF_WITH_CHAP_AUTH % (name, + path, chap_auth) LOG.info(_('Creating iscsi_target for: %s') % vol_id) volumes_dir = self.volumes_dir @@ -550,3 +553,25 @@ class LioAdm(TargetAdmin): LOG.error(_("Failed to add initiator iqn %s to target") % connector['initiator']) raise exception.ISCSITargetAttachFailed(volume_id=volume['id']) + + +class ISERTgtAdm(TgtAdm): + VOLUME_CONF = """ + + driver iser + backing-store %s + + """ + VOLUME_CONF_WITH_CHAP_AUTH = """ + + driver iser + backing-store %s + %s + + """ + + def __init__(self, root_helper, volumes_dir, + target_prefix='iqn.2010-10.org.iser.openstack:', + execute=putils.execute): + super(ISERTgtAdm, self).__init__(root_helper, volumes_dir, + target_prefix, execute) diff --git a/cinder/brick/iser/__init__.py b/cinder/brick/iser/__init__.py deleted file mode 100644 index 5e8da711f..000000000 --- a/cinder/brick/iser/__init__.py +++ /dev/null @@ -1,16 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright 2013 OpenStack Foundation. -# All Rights Reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. diff --git a/cinder/brick/iser/iser.py b/cinder/brick/iser/iser.py deleted file mode 100644 index 2522b8f66..000000000 --- a/cinder/brick/iser/iser.py +++ /dev/null @@ -1,206 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright (c) 2013 Mellanox Technologies. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. -""" -Helper code for the iSER volume driver. - -""" - - -import os - -from cinder.brick import exception -from cinder.brick import executor -from cinder.openstack.common import fileutils -from cinder.openstack.common.gettextutils import _ -from cinder.openstack.common import log as logging -from cinder.openstack.common import processutils as putils - -LOG = logging.getLogger(__name__) - - -class TargetAdmin(executor.Executor): - """iSER target administration. - - Base class for iSER target admin helpers. - """ - - def __init__(self, cmd, root_helper, execute): - super(TargetAdmin, self).__init__(root_helper, execute=execute) - self._cmd = cmd - - def _run(self, *args, **kwargs): - self._execute(self._cmd, *args, run_as_root=True, **kwargs) - - def create_iser_target(self, name, tid, lun, path, - chap_auth=None, **kwargs): - """Create a iSER target and logical unit.""" - raise NotImplementedError() - - def remove_iser_target(self, tid, lun, vol_id, vol_name, **kwargs): - """Remove a iSER target and logical unit.""" - raise NotImplementedError() - - def _new_target(self, name, tid, **kwargs): - """Create a new iSER target.""" - raise NotImplementedError() - - def _delete_target(self, tid, **kwargs): - """Delete a target.""" - raise NotImplementedError() - - def show_target(self, tid, iqn=None, **kwargs): - """Query the given target ID.""" - raise NotImplementedError() - - def _new_logicalunit(self, tid, lun, path, **kwargs): - """Create a new LUN on a target using the supplied path.""" - raise NotImplementedError() - - def _delete_logicalunit(self, tid, lun, **kwargs): - """Delete a logical unit from a target.""" - raise NotImplementedError() - - -class TgtAdm(TargetAdmin): - """iSER target administration using tgtadm.""" - - def __init__(self, root_helper, volumes_dir, execute=putils.execute, - target_prefix='iqn.2010-10.org.iser.openstack:'): - super(TgtAdm, self).__init__('tgtadm', root_helper, execute) - self.volumes_dir = volumes_dir - self.iser_target_prefix = target_prefix - - def _get_target(self, iqn): - (out, err) = self._execute('tgt-admin', '--show', run_as_root=True) - lines = out.split('\n') - for line in lines: - if iqn in line: - parsed = line.split() - tid = parsed[1] - return tid[:-1] - - return None - - def create_iser_target(self, name, tid, lun, path, - chap_auth=None, **kwargs): - # Note(jdg) tid and lun aren't used by TgtAdm but remain for - # compatibility - - fileutils.ensure_tree(self.volumes_dir) - - vol_id = name.split(':')[1] - if chap_auth is None: - volume_conf = """ - - driver iser - backing-store %s - - """ % (name, path) - else: - volume_conf = """ - - driver iser - backing-store %s - %s - - """ % (name, path, chap_auth) - - LOG.info(_('Creating iser_target for: %s') % vol_id) - volume_path = os.path.join(self.volumes_dir, vol_id) - - f = open(volume_path, 'w+') - f.write(volume_conf) - f.close() - - old_persist_file = None - old_name = kwargs.get('old_name', None) - if old_name is not None: - old_persist_file = os.path.join(self.volumes_dir, old_name) - - try: - (out, err) = self._execute('tgt-admin', - '--update', - name, - run_as_root=True) - except putils.ProcessExecutionError as e: - LOG.error(_("Failed to create iser target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': str(e)}) - - #Don't forget to remove the persistent file we created - os.unlink(volume_path) - raise exception.ISERTargetCreateFailed(volume_id=vol_id) - - iqn = '%s%s' % (self.iser_target_prefix, vol_id) - tid = self._get_target(iqn) - if tid is None: - LOG.error(_("Failed to create iser target for volume " - "id:%(vol_id)s. Please ensure your tgtd config file " - "contains 'include %(volumes_dir)s/*'") % - {'vol_id': vol_id, 'volumes_dir': self.volumes_dir}) - raise exception.NotFound() - - if old_persist_file is not None and os.path.exists(old_persist_file): - os.unlink(old_persist_file) - - return tid - - def remove_iser_target(self, tid, lun, vol_id, vol_name, **kwargs): - LOG.info(_('Removing iser_target for: %s') % vol_id) - vol_uuid_file = vol_name - volume_path = os.path.join(self.volumes_dir, vol_uuid_file) - if os.path.isfile(volume_path): - iqn = '%s%s' % (self.iser_target_prefix, - vol_uuid_file) - else: - raise exception.ISERTargetRemoveFailed(volume_id=vol_id) - try: - # NOTE(vish): --force is a workaround for bug: - # https://bugs.launchpad.net/cinder/+bug/1159948 - self._execute('tgt-admin', - '--force', - '--delete', - iqn, - run_as_root=True) - except putils.ProcessExecutionError as e: - LOG.error(_("Failed to remove iser target for volume " - "id:%(vol_id)s: %(e)s") - % {'vol_id': vol_id, 'e': str(e)}) - raise exception.ISERTargetRemoveFailed(volume_id=vol_id) - - os.unlink(volume_path) - - def show_target(self, tid, iqn=None, **kwargs): - if iqn is None: - raise exception.InvalidParameterValue( - err=_('valid iqn needed for show_target')) - - tid = self._get_target(iqn) - if tid is None: - raise exception.NotFound() - - -class FakeIserHelper(object): - - def __init__(self): - self.tid = 1 - - def set_execute(self, execute): - self._execute = execute - - def create_iser_target(self, *args, **kwargs): - self.tid += 1 - return self.tid diff --git a/cinder/tests/test_iscsi.py b/cinder/tests/test_iscsi.py index f7b62bce8..bf2b6056e 100644 --- a/cinder/tests/test_iscsi.py +++ b/cinder/tests/test_iscsi.py @@ -201,3 +201,10 @@ class LioAdmTestCase(test.TestCase, TargetAdminTestCase): 'rtstool create ' '/foo iqn.2011-09.org.foo.bar:blaa test_id test_pass', 'rtstool delete iqn.2010-10.org.openstack:volume-blaa']) + + +class ISERTgtAdmTestCase(TgtAdmTestCase): + + def setUp(self): + super(ISERTgtAdmTestCase, self).setUp() + self.flags(iscsi_helper='iseradm') diff --git a/cinder/tests/test_iser.py b/cinder/tests/test_iser.py deleted file mode 100644 index 79863712b..000000000 --- a/cinder/tests/test_iser.py +++ /dev/null @@ -1,117 +0,0 @@ -# vim: tabstop=4 shiftwidth=4 softtabstop=4 - -# Copyright (c) 2013 Mellanox Technologies. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import os.path -import shutil -import string -import tempfile - -from cinder.brick.iser import iser -from cinder.openstack.common import fileutils -from cinder import test -from cinder.volume import driver -from cinder.volume import utils as volume_utils - - -class TargetAdminTestCase(object): - - def setUp(self): - self.cmds = [] - - self.tid = 1 - self.target_name = 'iqn.2011-09.org.foo.bar:blaa' - self.lun = 10 - self.path = '/foo' - self.vol_id = 'blaa' - self.vol_name = 'volume-blaa' - - self.script_template = None - self.stubs.Set(os.path, 'isfile', lambda _: True) - self.stubs.Set(os, 'unlink', lambda _: '') - self.stubs.Set(iser.TgtAdm, '_get_target', self.fake_get_target) - self.persist_tempdir = tempfile.mkdtemp() - self.driver = driver.ISERDriver() - - def fake_init(obj): - return - - def fake_get_target(obj, iqn): - return 1 - - def get_script_params(self): - return {'tid': self.tid, - 'target_name': self.target_name, - 'lun': self.lun, - 'path': self.path} - - def get_script(self): - return self.script_template % self.get_script_params() - - def fake_execute(self, *cmd, **kwargs): - self.cmds.append(string.join(cmd)) - return "", None - - def clear_cmds(self): - self.cmds = [] - - def verify_cmds(self, cmds): - self.assertEqual(len(cmds), len(self.cmds)) - for a, b in zip(cmds, self.cmds): - self.assertEqual(a, b) - - def verify(self): - script = self.get_script() - cmds = [] - for line in script.split('\n'): - if not line.strip(): - continue - cmds.append(line) - self.verify_cmds(cmds) - - def run_commands(self): - tgtadm = self.driver.get_target_admin() - tgtadm.set_execute(self.fake_execute) - tgtadm.create_iser_target(self.target_name, self.tid, - self.lun, self.path) - tgtadm.show_target(self.tid, iqn=self.target_name) - tgtadm.remove_iser_target(self.tid, self.lun, self.vol_id, - self.vol_name) - - def test_target_admin(self): - self.clear_cmds() - self.run_commands() - self.verify() - - -class TgtAdmTestCase(test.TestCase, TargetAdminTestCase): - - def setUp(self): - super(TgtAdmTestCase, self).setUp() - TargetAdminTestCase.setUp(self) - self.persist_tempdir = tempfile.mkdtemp() - self.flags(iser_helper='tgtadm') - self.flags(volumes_dir=self.persist_tempdir) - self.script_template = "\n".join([ - 'tgt-admin --update iqn.2011-09.org.foo.bar:blaa', - 'tgt-admin --force ' - '--delete iqn.2010-10.org.iser.openstack:volume-blaa']) - - def tearDown(self): - try: - shutil.rmtree(self.persist_tempdir) - except OSError: - pass - super(TgtAdmTestCase, self).tearDown() diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 9fc9f8534..64b958e1f 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -27,7 +27,6 @@ from oslo.config import cfg from cinder.brick.initiator import connector as initiator from cinder.brick.iscsi import iscsi -from cinder.brick.iser import iser from cinder import exception from cinder.image import image_utils from cinder.openstack.common import excutils @@ -691,8 +690,10 @@ class ISCSIDriver(VolumeDriver): def get_target_admin(self): root_helper = utils.get_root_helper() - - if CONF.iscsi_helper == 'tgtadm': + if CONF.iscsi_helper == 'iseradm': + return iscsi.ISERTgtAdm(root_helper, CONF.volumes_dir, + CONF.iscsi_target_prefix) + elif CONF.iscsi_helper == 'tgtadm': return iscsi.TgtAdm(root_helper, CONF.volumes_dir, CONF.iscsi_target_prefix) @@ -994,10 +995,10 @@ class ISERDriver(ISCSIDriver): root_helper = utils.get_root_helper() if CONF.iser_helper == 'fake': - return iser.FakeIserHelper() + return iscsi.FakeIscsiHelper() else: - return iser.TgtAdm(root_helper, - CONF.volumes_dir) + return iscsi.ISERTgtAdm(root_helper, + CONF.volumes_dir) class FakeISERDriver(FakeISCSIDriver): diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 862fe238f..6983a8c82 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -28,7 +28,6 @@ from oslo.config import cfg from cinder.brick import exception as brick_exception from cinder.brick.iscsi import iscsi -from cinder.brick.iser import iser from cinder.brick.local_dev import lvm as lvm from cinder import exception from cinder.image import image_utils @@ -761,7 +760,7 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): def ensure_export(self, context, volume): """Synchronously recreates an export for a logical volume.""" - if not isinstance(self.tgtadm, iser.TgtAdm): + if not isinstance(self.tgtadm, iscsi.TgtAdm): try: iser_target = self.db.volume_get_iscsi_target_num( context, @@ -788,19 +787,19 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): volume_name = old_name old_name = None - iser_name = "%s%s" % (self.configuration.iser_target_prefix, + iser_name = "%s%s" % (self.configuration.iscsi_target_prefix, volume_name) volume_path = "/dev/%s/%s" % (self.configuration.volume_group, volume_name) - self.tgtadm.create_iser_target(iser_name, iser_target, - 0, volume_path, chap_auth, - check_exit_code=False, - old_name=old_name) + self.tgtadm.create_iscsi_target(iser_name, iser_target, + 0, volume_path, chap_auth, + check_exit_code=False, + old_name=old_name) def _ensure_iser_targets(self, context, host): """Ensure that target ids have been created in datastore.""" - if not isinstance(self.tgtadm, iser.TgtAdm): + if not isinstance(self.tgtadm, iscsi.TgtAdm): host_iser_targets = self.db.iscsi_target_count_by_host(context, host) if host_iser_targets >= self.configuration.iser_num_targets: @@ -815,7 +814,7 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): def create_export(self, context, volume): """Creates an export for a logical volume.""" - iser_name = "%s%s" % (self.configuration.iser_target_prefix, + iser_name = "%s%s" % (self.configuration.iscsi_target_prefix, volume['name']) volume_path = "/dev/%s/%s" % (self.configuration.volume_group, volume['name']) @@ -823,7 +822,7 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): # TODO(jdg): In the future move all of the dependent stuff into the # cooresponding target admin class - if not isinstance(self.tgtadm, iser.TgtAdm): + if not isinstance(self.tgtadm, iscsi.TgtAdm): lun = 0 self._ensure_iser_targets(context, volume['host']) iser_target = self.db.volume_allocate_iscsi_target(context, @@ -838,13 +837,13 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): chap_password = utils.generate_password() chap_auth = self._iser_authentication('IncomingUser', chap_username, chap_password) - tid = self.tgtadm.create_iser_target(iser_name, - iser_target, - 0, - volume_path, - chap_auth) + tid = self.tgtadm.create_iscsi_target(iser_name, + iser_target, + 0, + volume_path, + chap_auth) model_update['provider_location'] = self._iser_location( - self.configuration.iser_ip_address, tid, iser_name, lun) + self.configuration.iscsi_ip_address, tid, iser_name, lun) model_update['provider_auth'] = self._iser_authentication( 'CHAP', chap_username, chap_password) return model_update @@ -852,7 +851,7 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): def remove_export(self, context, volume): """Removes an export for a logical volume.""" - if not isinstance(self.tgtadm, iser.TgtAdm): + if not isinstance(self.tgtadm, iscsi.TgtAdm): try: iser_target = self.db.volume_get_iscsi_target_num( context, @@ -878,11 +877,11 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): "is presently exported for volume: %s"), volume['id']) return - self.tgtadm.remove_iser_target(iser_target, 0, volume['id'], - volume['name']) + self.tgtadm.remove_iscsi_target(iser_target, 0, volume['id'], + volume['name']) def _iser_location(self, ip, target, iqn, lun=None): - return "%s:%s,%s %s %s" % (ip, self.configuration.iser_port, + return "%s:%s,%s %s %s" % (ip, self.configuration.iscsi_port, target, iqn, lun) def _iser_authentication(self, chap, name, password): -- 2.45.2