From 871141d4d3cc0ac739de72ca010aef3e5c13fe1f Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Thu, 11 Aug 2011 07:44:38 -0400 Subject: [PATCH] Allow the user to choose either ietadm or tgtadm (lp:819997) Also, refactor ietadm/tgtadm calls out into helper classes. Add a new TargetAdmin abstract base class and implement it using ietadm and tgtadm. This cleans up the code greatly and gets us some code reuse. (Based on a patch by Chuck Short ) Change-Id: I1c0064e5d35483a6c4059cfc61a484f5f576b2da --- nova/tests/test_iscsi.py | 116 ++++++++++++++++++++++++++++ nova/tests/test_volume.py | 16 ++-- nova/volume/driver.py | 52 ++++++------- nova/volume/iscsi.py | 156 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 301 insertions(+), 39 deletions(-) create mode 100644 nova/tests/test_iscsi.py create mode 100644 nova/volume/iscsi.py diff --git a/nova/tests/test_iscsi.py b/nova/tests/test_iscsi.py new file mode 100644 index 000000000000..d7aed0fbdb31 --- /dev/null +++ b/nova/tests/test_iscsi.py @@ -0,0 +1,116 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2011 Red Hat, Inc. +# +# 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 string + +from nova import test +from nova.volume import iscsi + + +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/bar/blaa' + + self.script_template = None + + 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): + 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 = iscsi.get_target_admin() + tgtadm.set_execute(self.fake_execute) + tgtadm.new_target(self.target_name, self.tid) + tgtadm.show_target(self.tid) + tgtadm.new_logicalunit(self.tid, self.lun, self.path) + tgtadm.delete_logicalunit(self.tid, self.lun) + tgtadm.delete_target(self.tid) + + 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.flags(iscsi_helper='tgtadm') + self.script_template = """ +tgtadm --op new --lld=iscsi --mode=target --tid=%(tid)s \ +--targetname=%(target_name)s +tgtadm --op bind --lld=iscsi --mode=target --initiator-address=ALL \ +--tid=%(tid)s +tgtadm --op show --lld=iscsi --mode=target --tid=%(tid)s +tgtadm --op new --lld=iscsi --mode=logicalunit --tid=%(tid)s --lun=%(lun)d \ +--backing-store=%(path)s +tgtadm --op delete --lld=iscsi --mode=logicalunit --tid=%(tid)s --lun=%(lun)d +tgtadm --op delete --lld=iscsi --mode=target --tid=%(tid)s +""" + + def get_script_params(self): + params = super(TgtAdmTestCase, self).get_script_params() + params['lun'] += 1 + return params + + +class IetAdmTestCase(test.TestCase, TargetAdminTestCase): + + def setUp(self): + super(IetAdmTestCase, self).setUp() + TargetAdminTestCase.setUp(self) + self.flags(iscsi_helper='ietadm') + self.script_template = """ +ietadm --op new --tid=%(tid)s --params Name=%(target_name)s +ietadm --op show --tid=%(tid)s +ietadm --op new --tid=%(tid)s --lun=%(lun)d --params Path=%(path)s,Type=fileio +ietadm --op delete --tid=%(tid)s --lun=%(lun)d +ietadm --op delete --tid=%(tid)s +""" diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py index d1b11aaf849f..588b7a3288a6 100644 --- a/nova/tests/test_volume.py +++ b/nova/tests/test_volume.py @@ -270,7 +270,7 @@ class DriverTestCase(test.TestCase): def _fake_execute(_command, *_args, **_kwargs): """Fake _execute.""" return self.output, None - self.volume.driver._execute = _fake_execute + self.volume.driver.set_execute(_fake_execute) log = logging.getLogger() self.stream = cStringIO.StringIO() @@ -333,12 +333,10 @@ class ISCSITestCase(DriverTestCase): """No log message when all the processes are running.""" volume_id_list = self._attach_volume() - self.mox.StubOutWithMock(self.volume.driver, '_execute') + self.mox.StubOutWithMock(self.volume.driver.tgtadm, 'show_target') for i in volume_id_list: tid = db.volume_get_iscsi_target_num(self.context, i) - self.volume.driver._execute("ietadm", "--op", "show", - "--tid=%(tid)d" % locals(), - run_as_root=True) + self.volume.driver.tgtadm.show_target(tid) self.stream.truncate(0) self.mox.ReplayAll() @@ -354,11 +352,9 @@ class ISCSITestCase(DriverTestCase): volume_id_list = self._attach_volume() tid = db.volume_get_iscsi_target_num(self.context, volume_id_list[0]) - self.mox.StubOutWithMock(self.volume.driver, '_execute') - self.volume.driver._execute("ietadm", "--op", "show", - "--tid=%(tid)d" % locals(), - run_as_root=True).AndRaise( - exception.ProcessExecutionError()) + self.mox.StubOutWithMock(self.volume.driver.tgtadm, 'show_target') + self.volume.driver.tgtadm.show_target(tid).AndRaise( + exception.ProcessExecutionError()) self.mox.ReplayAll() self.assertRaises(exception.ProcessExecutionError, diff --git a/nova/volume/driver.py b/nova/volume/driver.py index f1e30dc94442..ea7386d86160 100644 --- a/nova/volume/driver.py +++ b/nova/volume/driver.py @@ -28,6 +28,7 @@ from nova import exception from nova import flags from nova import log as logging from nova import utils +from nova.volume import iscsi from nova.volume import volume_types @@ -55,6 +56,9 @@ class VolumeDriver(object): def __init__(self, execute=utils.execute, *args, **kwargs): # NOTE(vish): db is set by Manager self.db = None + self.set_execute(execute) + + def set_execute(self, execute): self._execute = execute def _try_execute(self, *command, **kwargs): @@ -224,6 +228,14 @@ class ISCSIDriver(VolumeDriver): `CHAP` is the only auth_method in use at the moment. """ + def __init__(self, *args, **kwargs): + self.tgtadm = iscsi.get_target_admin() + super(ISCSIDriver, self).__init__(*args, **kwargs) + + def set_execute(self, execute): + super(ISCSIDriver, self).set_execute(execute) + self.tgtadm.set_execute(execute) + def ensure_export(self, context, volume): """Synchronously recreates an export for a logical volume.""" try: @@ -236,19 +248,10 @@ class ISCSIDriver(VolumeDriver): iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume['name']) volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume['name']) - self._execute('ietadm', '--op', 'new', - "--tid=%s" % iscsi_target, - '--params', - "Name=%s" % iscsi_name, - run_as_root=True, - check_exit_code=False) - self._execute('ietadm', '--op', 'new', - "--tid=%s" % iscsi_target, - '--lun=0', - '--params', - "Path=%s,Type=fileio" % volume_path, - run_as_root=True, - check_exit_code=False) + + self.tgtadm.new_target(iscsi_name, iscsi_target, check_exit_code=False) + self.tgtadm.new_logicalunit(iscsi_target, 0, volume_path, + check_exit_code=False) def _ensure_iscsi_targets(self, context, host): """Ensure that target ids have been created in datastore.""" @@ -268,13 +271,9 @@ class ISCSIDriver(VolumeDriver): volume['host']) iscsi_name = "%s%s" % (FLAGS.iscsi_target_prefix, volume['name']) volume_path = "/dev/%s/%s" % (FLAGS.volume_group, volume['name']) - self._execute('ietadm', '--op', 'new', - '--tid=%s' % iscsi_target, - '--params', 'Name=%s' % iscsi_name, run_as_root=True) - self._execute('ietadm', '--op', 'new', - '--tid=%s' % iscsi_target, - '--lun=0', '--params', - 'Path=%s,Type=fileio' % volume_path, run_as_root=True) + + self.tgtadm.new_target(iscsi_name, iscsi_target) + self.tgtadm.new_logicalunit(iscsi_target, 0, volume_path) def remove_export(self, context, volume): """Removes an export for a logical volume.""" @@ -289,18 +288,14 @@ class ISCSIDriver(VolumeDriver): try: # ietadm show will exit with an error # this export has already been removed - self._execute('ietadm', '--op', 'show', - '--tid=%s' % iscsi_target, run_as_root=True) + self.tgtadm.show_target(iscsi_target) except Exception as e: LOG.info(_("Skipping remove_export. No iscsi_target " + "is presently exported for volume: %d"), volume['id']) return - self._execute('ietadm', '--op', 'delete', - '--tid=%s' % iscsi_target, - '--lun=0', run_as_root=True) - self._execute('ietadm', '--op', 'delete', - '--tid=%s' % iscsi_target, run_as_root=True) + self.tgtadm.delete_logicalunit(iscsi_target, 0) + self.tgtadm.delete_target(iscsi_target) def _do_iscsi_discovery(self, volume): #TODO(justinsb): Deprecate discovery and use stored info @@ -422,8 +417,7 @@ class ISCSIDriver(VolumeDriver): tid = self.db.volume_get_iscsi_target_num(context, volume_id) try: - self._execute('ietadm', '--op', 'show', - '--tid=%(tid)d' % locals(), run_as_root=True) + self.tgtadm.show_target(tid) except exception.ProcessExecutionError, e: # Instances remount read-only in this case. # /etc/init.d/iscsitarget restart and rebooting nova-volume diff --git a/nova/volume/iscsi.py b/nova/volume/iscsi.py new file mode 100644 index 000000000000..d50dd5fd7e1d --- /dev/null +++ b/nova/volume/iscsi.py @@ -0,0 +1,156 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# 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 iSCSI volume driver. + +""" + +from nova import flags +from nova import utils + + +FLAGS = flags.FLAGS +flags.DEFINE_string('iscsi_helper', 'ietadm', + 'iscsi target user-land tool to use') + + +class TargetAdmin(object): + """iSCSI target administration. + + Base class for iSCSI target admin helpers. + """ + + def __init__(self, cmd, execute): + self._cmd = cmd + self.set_execute(execute) + + def set_execute(self, execute): + """Set the function to be used to execute commands.""" + self._execute = execute + + def _run(self, *args, **kwargs): + self._execute(self._cmd, *args, run_as_root=True, **kwargs) + + def new_target(self, name, tid, **kwargs): + """Create a new iSCSI target.""" + raise NotImplementedError() + + def delete_target(self, tid, **kwargs): + """Delete a target.""" + raise NotImplementedError() + + def show_target(self, tid, **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): + """iSCSI target administration using tgtadm.""" + + def __init__(self, execute=utils.execute): + super(TgtAdm, self).__init__('tgtadm', execute) + + def new_target(self, name, tid, **kwargs): + self._run('--op', 'new', + '--lld=iscsi', '--mode=target', + '--tid=%s' % tid, + '--targetname=%s' % name, + **kwargs) + self._run('--op', 'bind', + '--lld=iscsi', '--mode=target', + '--initiator-address=ALL', + '--tid=%s' % tid, + **kwargs) + + def delete_target(self, tid, **kwargs): + self._run('--op', 'delete', + '--lld=iscsi', '--mode=target', + '--tid=%s' % tid, + **kwargs) + + def show_target(self, tid, **kwargs): + self._run('--op', 'show', + '--lld=iscsi', '--mode=target', + '--tid=%s' % tid, + **kwargs) + + def new_logicalunit(self, tid, lun, path, **kwargs): + self._run('--op', 'new', + '--lld=iscsi', '--mode=logicalunit', + '--tid=%s' % tid, + '--lun=%d' % (lun + 1), # lun0 is reserved + '--backing-store=%s' % path, + **kwargs) + + def delete_logicalunit(self, tid, lun, **kwargs): + self._run('--op', 'delete', + '--lld=iscsi', '--mode=logicalunit', + '--tid=%s' % tid, + '--lun=%d' % (lun + 1), + **kwargs) + + +class IetAdm(TargetAdmin): + """iSCSI target administration using ietadm.""" + + def __init__(self, execute=utils.execute): + super(IetAdm, self).__init__('ietadm', execute) + + def new_target(self, name, tid, **kwargs): + self._run('--op', 'new', + '--tid=%s' % tid, + '--params', 'Name=%s' % name, + **kwargs) + + def delete_target(self, tid, **kwargs): + self._run('--op', 'delete', + '--tid=%s' % tid, + **kwargs) + + def show_target(self, tid, **kwargs): + self._run('--op', 'show', + '--tid=%s' % tid, + **kwargs) + + def new_logicalunit(self, tid, lun, path, **kwargs): + self._run('--op', 'new', + '--tid=%s' % tid, + '--lun=%d' % lun, + '--params', 'Path=%s,Type=fileio' % path, + **kwargs) + + def delete_logicalunit(self, tid, lun, **kwargs): + self._run('--op', 'delete', + '--tid=%s' % tid, + '--lun=%d' % lun, + **kwargs) + + +def get_target_admin(): + if FLAGS.iscsi_helper == 'tgtadm': + return TgtAdm() + else: + return IetAdm()