Merge changes I1c0064e5,I86d491cf

* changes:
  Allow the user to choose either ietadm or tgtadm (lp:819997)
  Remove VolumeDriver.sync_exec method (lp:819997)

4 files changed, 331 insertions, 75 deletions

diff --git a/nova/tests/test_iscsi.py b/nova/tests/test_iscsi.py
new file mode 100644
index 000000000..d7aed0fbd
--- /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 88a73f550..588b7a328 100644
--- a/nova/tests/test_volume.py
+++ b/nova/tests/test_volume.py
@@ -270,8 +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._sync_execute = _fake_execute
+        self.volume.driver.set_execute(_fake_execute)
 
         log = logging.getLogger()
         self.stream = cStringIO.StringIO()
@@ -334,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()
@@ -355,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 2692f5e9c..ea7386d86 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
 
 
@@ -52,12 +53,13 @@ flags.DEFINE_string('rbd_pool', 'rbd',
 
 class VolumeDriver(object):
     """Executes commands relating to Volumes."""
-    def __init__(self, execute=utils.execute,
-                 sync_exec=utils.execute, *args, **kwargs):
+    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
-        self._sync_exec = sync_exec
 
     def _try_execute(self, *command, **kwargs):
         # NOTE(vish): Volume commands can partially fail due to timing, but
@@ -226,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:
@@ -238,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._sync_exec('ietadm', '--op', 'new',
-                        "--tid=%s" % iscsi_target,
-                        '--params',
-                        "Name=%s" % iscsi_name,
-                        run_as_root=True,
-                        check_exit_code=False)
-        self._sync_exec('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."""
@@ -270,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."""
@@ -291,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
@@ -424,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
@@ -439,7 +431,6 @@ class FakeISCSIDriver(ISCSIDriver):
     """Logs calls instead of executing."""
     def __init__(self, *args, **kwargs):
         super(FakeISCSIDriver, self).__init__(execute=self.fake_execute,
-                                              sync_exec=self.fake_execute,
                                               *args, **kwargs)
 
     def check_for_setup_error(self):
@@ -718,14 +709,14 @@ class ZadaraBEDriver(ISCSIDriver):
                 break
 
         try:
-            self._sync_exec('/var/lib/zadara/bin/zadara_sncfg',
-                            'create_qospart',
-                            '--qos', qosstr,
-                            '--pname', volume['name'],
-                            '--psize', sizestr,
-                            '--vsaid', vsa_id,
-                            run_as_root=True,
-                            check_exit_code=0)
+            self._execute('/var/lib/zadara/bin/zadara_sncfg',
+                          'create_qospart',
+                          '--qos', qosstr,
+                          '--pname', volume['name'],
+                          '--psize', sizestr,
+                          '--vsaid', vsa_id,
+                          run_as_root=True,
+                          check_exit_code=0)
         except exception.ProcessExecutionError:
             LOG.debug(_("VSA BE create_volume for %s failed"), volume['name'])
             raise
@@ -743,11 +734,11 @@ class ZadaraBEDriver(ISCSIDriver):
             return
 
         try:
-            self._sync_exec('/var/lib/zadara/bin/zadara_sncfg',
-                            'delete_partition',
-                            '--pname', volume['name'],
-                            run_as_root=True,
-                            check_exit_code=0)
+            self._execute('/var/lib/zadara/bin/zadara_sncfg',
+                          'delete_partition',
+                          '--pname', volume['name'],
+                          run_as_root=True,
+                          check_exit_code=0)
         except exception.ProcessExecutionError:
             LOG.debug(_("VSA BE delete_volume for %s failed"), volume['name'])
             return
@@ -883,12 +874,12 @@ class ZadaraBEDriver(ISCSIDriver):
             return
 
         try:
-            self._sync_exec('/var/lib/zadara/bin/zadara_sncfg',
-                        'remove_export',
-                        '--pname', volume['name'],
-                        '--tid', iscsi_target,
-                        run_as_root=True,
-                        check_exit_code=0)
+            self._execute('/var/lib/zadara/bin/zadara_sncfg',
+                          'remove_export',
+                          '--pname', volume['name'],
+                          '--tid', iscsi_target,
+                          run_as_root=True,
+                          check_exit_code=0)
         except exception.ProcessExecutionError:
             LOG.debug(_("VSA BE remove_export for %s failed"), volume['name'])
             return
@@ -913,13 +904,12 @@ class ZadaraBEDriver(ISCSIDriver):
         Common logic that asks zadara_sncfg to setup iSCSI target/lun for
         this volume
         """
-        (out, err) = self._sync_exec(
-                                '/var/lib/zadara/bin/zadara_sncfg',
-                                'create_export',
-                                '--pname', volume['name'],
-                                '--tid', iscsi_target,
-                                run_as_root=True,
-                                check_exit_code=0)
+        (out, err) = self._execute('/var/lib/zadara/bin/zadara_sncfg',
+                                   'create_export',
+                                   '--pname', volume['name'],
+                                   '--tid', iscsi_target,
+                                   run_as_root=True,
+                                   check_exit_code=0)
 
         result_xml = ElementTree.fromstring(out)
         response_node = result_xml.find("Sn")
@@ -940,11 +930,10 @@ class ZadaraBEDriver(ISCSIDriver):
     def _get_qosgroup_summary(self):
         """gets the list of qosgroups from Zadara BE"""
         try:
-            (out, err) = self._sync_exec(
-                                        '/var/lib/zadara/bin/zadara_sncfg',
-                                        'get_qosgroups_xml',
-                                        run_as_root=True,
-                                        check_exit_code=0)
+            (out, err) = self._execute('/var/lib/zadara/bin/zadara_sncfg',
+                                       'get_qosgroups_xml',
+                                       run_as_root=True,
+                                       check_exit_code=0)
         except exception.ProcessExecutionError:
             LOG.debug(_("Failed to retrieve QoS info"))
             return {}
diff --git a/nova/volume/iscsi.py b/nova/volume/iscsi.py
new file mode 100644
index 000000000..d50dd5fd7
--- /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()