Fix creation of iscsi targets

Previously when creating iscsi volumes, we were using

tgt-admin -e -c <config file> --update volume-id

Unfortunately the side affect of this is that tgt-admin
removed other volumes that wasnt connected to an iscsi
connector. Which is obvlously not what we want.

In order to fix this we create the targets.conf for the
volume but we call tgt-admin --update icssi qualified name.
We also set the tid in the configuration file now as well.

Fixes LP: #1038062

Change-Id: I23719390fbfaea5b55389a5c8ebaa8966cc283a8
Signed-off-by: Chuck Short <chuck.short@canonical.com>

nova/exception.py

@@ -425,6 +425,10 @@ class VirtDriverNotFound(NotFound):
     message = _("Could not find driver for connection_type %(name)s")
 
 
+class PersistentVolumeFileNotFound(NotFound):
+    message = _("Volume %(volume_id)s persistence file could not be found.")
+
+
 class VolumeNotFound(NotFound):
     message = _("Volume %(volume_id)s could not be found.")
 
@@ -470,6 +474,14 @@ class ISCSITargetNotFoundForVolume(NotFound):
     message = _("No target id found for volume %(volume_id)s.")
 
 
+class ISCSITargetCreateFailed(NovaException):
+    message = _("Failed to create iscsi target for volume %(volume_id)s.")
+
+
+class ISCSITargetRemoveFailed(NovaException):
+    message = _("Failed to remove iscsi target for volume %(volume_id)s.")
+
+
 class DiskNotFound(NotFound):
     message = _("No disk at %(location)s")
 

nova/tests/test_iscsi.py

@@ -35,6 +35,10 @@ class TargetAdminTestCase(object):
         self.script_template = None
         self.stubs.Set(os.path, 'isfile', lambda _: True)
         self.stubs.Set(os, 'unlink', lambda _: '')
+        self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
+
+    def fake_get_target(obj, iqn):
+        return 1
 
     def get_script_params(self):
         return {'tid': self.tid,
@@ -71,7 +75,7 @@ class TargetAdminTestCase(object):
         tgtadm.set_execute(self.fake_execute)
         tgtadm.create_iscsi_target(self.target_name, self.tid,
                 self.lun, self.path)
-        tgtadm.show_target(self.tid)
+        tgtadm.show_target(self.tid, iqn=self.target_name)
         tgtadm.remove_iscsi_target(self.tid, self.lun, self.vol_id)
 
     def test_target_admin(self):
@@ -88,9 +92,8 @@ class TgtAdmTestCase(test.TestCase, TargetAdminTestCase):
         self.flags(iscsi_helper='tgtadm')
         self.flags(volumes_dir="./")
         self.script_template = "\n".join([
-        "tgt-admin --execute --conf ./blaa --update blaa",
+        'tgt-admin --update iqn.2011-09.org.foo.bar:blaa',
-        "tgtadm --op show --lld=iscsi --mode=target --tid=1",
+        'tgt-admin --delete iqn.2010-10.org.openstack:volume-blaa'])
-        "tgt-admin --delete iqn.2010-10.org.openstack:volume-blaa"])
 
 
 class IetAdmTestCase(test.TestCase, TargetAdminTestCase):
@@ -100,9 +103,9 @@ class IetAdmTestCase(test.TestCase, TargetAdminTestCase):
         TargetAdminTestCase.setUp(self)
         self.flags(iscsi_helper='ietadm')
         self.script_template = "\n".join([
-        "ietadm --op new --tid=%(tid)s --params Name=%(target_name)s",
+        'ietadm --op new --tid=%(tid)s --params Name=%(target_name)s',
-        "ietadm --op new --tid=%(tid)s --lun=%(lun)s "
+        'ietadm --op new --tid=%(tid)s --lun=%(lun)s '
-                "--params Path=%(path)s,Type=fileio",
+                '--params Path=%(path)s,Type=fileio',
-        "ietadm --op show --tid=%(tid)s",
+        'ietadm --op show --tid=%(tid)s',
-        "ietadm --op delete --tid=%(tid)s",
+        'ietadm --op delete --tid=%(tid)s',
-        "ietadm --op delete --tid=%(tid)s --lun=%(lun)s"])
+        'ietadm --op delete --tid=%(tid)s --lun=%(lun)s'])

nova/tests/test_volume.py

@@ -37,7 +37,7 @@ from nova.openstack.common import rpc
 import nova.policy
 from nova import quota
 from nova import test
-import nova.volume.api
+from nova.volume import iscsi
 
 QUOTAS = quota.QUOTAS
 FLAGS = flags.FLAGS
@@ -54,6 +54,7 @@ class VolumeTestCase(test.TestCase):
                    volumes_dir=vol_tmpdir)
         self.stubs.Set(nova.flags.FLAGS, 'notification_driver',
                 ['nova.openstack.common.notifier.test_notifier'])
+        self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
         self.volume = importutils.import_object(FLAGS.volume_manager)
         self.context = context.get_admin_context()
         instance = db.instance_create(self.context, {})
@@ -70,6 +71,9 @@ class VolumeTestCase(test.TestCase):
         notifier_api._reset_drivers()
         super(VolumeTestCase, self).tearDown()
 
+    def fake_get_target(obj, iqn):
+        return 1
+
     @staticmethod
     def _create_volume(size=0, snapshot_id=None):
         """Create a volume object."""
@@ -214,23 +218,6 @@ class VolumeTestCase(test.TestCase):
         except TypeError:
             pass
 
-    def test_too_many_volumes(self):
-        """Ensure that NoMoreTargets is raised when we run out of volumes."""
-        vols = []
-        total_slots = FLAGS.iscsi_num_targets
-        for _index in xrange(total_slots):
-            volume = self._create_volume()
-            self.volume.create_volume(self.context, volume['id'])
-            vols.append(volume['id'])
-        volume = self._create_volume()
-        self.assertRaises(db.NoMoreTargets,
-                          self.volume.create_volume,
-                          self.context,
-                          volume['id'])
-        db.volume_destroy(context.get_admin_context(), volume['id'])
-        for volume_id in vols:
-            self.volume.delete_volume(self.context, volume_id)
-
     def test_run_attach_detach_volume(self):
         """Make sure volume can be attached and detached from instance."""
         inst = {}
@@ -295,11 +282,10 @@ class VolumeTestCase(test.TestCase):
                                                           volume_id)
             self.assert_(iscsi_target not in targets)
             targets.append(iscsi_target)
+
         total_slots = FLAGS.iscsi_num_targets
         for _index in xrange(total_slots):
-            volume = self._create_volume()
+            self._create_volume()
-            d = self.volume.create_volume(self.context, volume['id'])
-            _check(d)
         for volume_id in volume_ids:
             self.volume.delete_volume(self.context, volume_id)
 
@@ -518,6 +504,7 @@ class DriverTestCase(test.TestCase):
         self.volume = importutils.import_object(FLAGS.volume_manager)
         self.context = context.get_admin_context()
         self.output = ""
+        self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
 
         def _fake_execute(_command, *_args, **_kwargs):
             """Fake _execute."""
@@ -535,6 +522,9 @@ class DriverTestCase(test.TestCase):
             pass
         super(DriverTestCase, self).tearDown()
 
+    def fake_get_target(obj, iqn):
+        return 1
+
     def _attach_volume(self):
         """Attach volumes to an instance."""
         return []
@@ -593,39 +583,6 @@ class ISCSITestCase(DriverTestCase):
     def test_check_for_export_with_no_volume(self):
         self.volume.check_for_export(self.context, self.instance_id)
 
-    def test_check_for_export_with_all_volume_exported(self):
-        volume_id_list = self._attach_volume()
-
-        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.tgtadm.show_target(tid)
-
-        self.mox.ReplayAll()
-        self.volume.check_for_export(self.context, self.instance_id)
-        self.mox.UnsetStubs()
-
-        self._detach_volume(volume_id_list)
-
-    def test_check_for_export_with_some_volume_missing(self):
-        """Output a warning message when some volumes are not recognied
-           by ietd."""
-        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.tgtadm, 'show_target')
-        self.volume.driver.tgtadm.show_target(tid).AndRaise(
-            exception.ProcessExecutionError())
-
-        self.mox.ReplayAll()
-        self.assertRaises(exception.ProcessExecutionError,
-                          self.volume.check_for_export,
-                          self.context,
-                          self.instance_id)
-        self.mox.UnsetStubs()
-
-        self._detach_volume(volume_id_list)
-
 
 class VolumePolicyTestCase(test.TestCase):