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>

3 files changed, 32 insertions, 64 deletions

diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index b085b9ab1..18071b193 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -46,6 +46,7 @@ from nova.tests import fake_network
 from nova.tests.image import fake
 from nova import utils
 from nova.virt import fake as fake_virt
+from nova.volume import iscsi
 
 
 LOG = logging.getLogger(__name__)
@@ -97,6 +98,7 @@ class CloudTestCase(test.TestCase):
         vol_tmpdir = tempfile.mkdtemp()
         self.flags(compute_driver='nova.virt.fake.FakeDriver',
                    volumes_dir=vol_tmpdir)
+        self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target)
 
         def fake_show(meh, context, id):
             return {'id': id,
@@ -158,6 +160,9 @@ class CloudTestCase(test.TestCase):
         super(CloudTestCase, self).tearDown()
         fake.FakeImageService_reset()
 
+    def fake_get_target(obj, iqn):
+        return 1
+
     def _stub_instance_get_with_fixed_ips(self, func_name):
         orig_func = getattr(self.cloud.compute_api, func_name)
 
@@ -1993,9 +1998,12 @@ class CloudTestCase(test.TestCase):
         self.assertTrue(result)
 
     def _volume_create(self, volume_id=None):
+        location = '10.0.2.15:3260'
+        iqn = 'iqn.2010-10.org.openstack:%s' % volume_id
         kwargs = {'status': 'available',
                   'host': self.volume.host,
                   'size': 1,
+                  'provider_location': '1 %s,fake %s' % (location, iqn),
                   'attach_status': 'detached', }
         if volume_id:
             kwargs['id'] = volume_id
diff --git a/nova/tests/test_iscsi.py b/nova/tests/test_iscsi.py
index b88bd3fce..d375313e2 100644
--- a/nova/tests/test_iscsi.py
+++ b/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",
-        "tgtadm --op show --lld=iscsi --mode=target --tid=1",
-        "tgt-admin --delete iqn.2010-10.org.openstack:volume-blaa"])
+        'tgt-admin --update iqn.2011-09.org.foo.bar: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 --lun=%(lun)s "
-                "--params Path=%(path)s,Type=fileio",
-        "ietadm --op show --tid=%(tid)s",
-        "ietadm --op delete --tid=%(tid)s",
-        "ietadm --op delete --tid=%(tid)s --lun=%(lun)s"])
+        'ietadm --op new --tid=%(tid)s --params Name=%(target_name)s',
+        'ietadm --op new --tid=%(tid)s --lun=%(lun)s '
+                '--params Path=%(path)s,Type=fileio',
+        'ietadm --op show --tid=%(tid)s',
+        'ietadm --op delete --tid=%(tid)s',
+        'ietadm --op delete --tid=%(tid)s --lun=%(lun)s'])
diff --git a/nova/tests/test_volume.py b/nova/tests/test_volume.py
index 56ee53ee6..44735bf7c 100644
--- a/nova/tests/test_volume.py
+++ b/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()
-            d = self.volume.create_volume(self.context, volume['id'])
-            _check(d)
+            self._create_volume()
         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):