Properly passes arg to run_iscsiadm to fix logout

 * fixes 905471
 * includes test to verify expected calls
 * elevates context as expected for volume_api.detach

Change-Id: I0c2a5822d45df6f82f103f49ce8fcfe4a38a0049

3 files changed, 32 insertions, 13 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index d6912bb56..8b444b958 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1484,7 +1484,7 @@ class ComputeManager(manager.SchedulerDependentManager):
         bdm = self._get_instance_volume_bdm(context, instance_id, volume_id)
         self._detach_volume(context, instance_ref['name'], bdm)
         self.volume_api.terminate_connection(context, volume_id, FLAGS.my_ip)
-        self.volume_api.detach(context, volume_id)
+        self.volume_api.detach(context.elevated(), volume_id)
         self.db.block_device_mapping_destroy_by_instance_and_volume(
             context, instance_id, volume_id)
         return True
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 5c180a9fe..ebe487d04 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -104,14 +104,15 @@ class FakeVirtDomain(object):
 
 class LibvirtVolumeTestCase(test.TestCase):
 
-    @staticmethod
-    def fake_execute(*cmd, **kwargs):
-        LOG.debug("FAKE EXECUTE: %s" % ' '.join(cmd))
-        return None, None
-
     def setUp(self):
         super(LibvirtVolumeTestCase, self).setUp()
-        self.stubs.Set(utils, 'execute', self.fake_execute)
+        self.executes = []
+
+        def fake_execute(*cmd, **kwargs):
+            self.executes.append(cmd)
+            return None, None
+
+        self.stubs.Set(utils, 'execute', fake_execute)
 
         class FakeLibvirtConnection(object):
             def __init__(self, hyperv="QEMU"):
@@ -126,22 +127,38 @@ class LibvirtVolumeTestCase(test.TestCase):
         self.stubs.Set(os.path, 'exists', lambda x: True)
         vol_driver = volume_driver.ISCSIDriver()
         libvirt_driver = volume.LibvirtISCSIVolumeDriver(self.fake_conn)
+        location = '10.0.2.15:3260'
         name = 'volume-00000001'
+        iqn = 'iqn.2010-10.org.openstack:%s' % name
         vol = {'id': 1,
                'name': name,
                'provider_auth': None,
-               'provider_location': '10.0.2.15:3260,fake '
-                        'iqn.2010-10.org.openstack:volume-00000001'}
+               'provider_location': '%s,fake %s' % (location, iqn)}
         address = '127.0.0.1'
-        connection_info = vol_driver.initialize_connection(vol, address)
+        connection_info = vol_driver.initialize_connection(vol, '127.0.0.1')
         mount_device = "vde"
         xml = libvirt_driver.connect_volume(connection_info, mount_device)
         tree = xml_to_tree(xml)
-        dev_str = '/dev/disk/by-path/ip-10.0.2.15:3260-iscsi-iqn.' \
-                  '2010-10.org.openstack:%s-lun-0' % name
+        dev_str = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-0' % (location, iqn)
         self.assertEqual(tree.get('type'), 'block')
         self.assertEqual(tree.find('./source').get('dev'), dev_str)
         libvirt_driver.disconnect_volume(connection_info, mount_device)
+        connection_info = vol_driver.terminate_connection(vol, '127.0.0.1')
+        expected_commands = [('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location),
+                             ('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location, '--login'),
+                             ('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location, '--op', 'update',
+                              '-n', 'node.startup', '-v', 'automatic'),
+                             ('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location, '--op', 'update',
+                              '-n', 'node.startup', '-v', 'manual'),
+                             ('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location, '--logout'),
+                             ('iscsiadm', '-m', 'node', '-T', iqn,
+                              '-p', location, '--op', 'delete')]
+        self.assertEqual(self.executes, expected_commands)
 
     def test_libvirt_sheepdog_driver(self):
         vol_driver = volume_driver.SheepdogDriver()
@@ -157,6 +174,7 @@ class LibvirtVolumeTestCase(test.TestCase):
         self.assertEqual(tree.find('./source').get('protocol'), 'sheepdog')
         self.assertEqual(tree.find('./source').get('name'), name)
         libvirt_driver.disconnect_volume(connection_info, mount_device)
+        connection_info = vol_driver.terminate_connection(vol, '127.0.0.1')
 
     def test_libvirt_rbd_driver(self):
         vol_driver = volume_driver.RBDDriver()
@@ -173,6 +191,7 @@ class LibvirtVolumeTestCase(test.TestCase):
         rbd_name = '%s/%s' % (FLAGS.rbd_pool, name)
         self.assertEqual(tree.find('./source').get('name'), rbd_name)
         libvirt_driver.disconnect_volume(connection_info, mount_device)
+        connection_info = vol_driver.terminate_connection(vol, '127.0.0.1')
 
 
 class CacheConcurrencyTestCase(test.TestCase):
diff --git a/nova/virt/libvirt/volume.py b/nova/virt/libvirt/volume.py
index 352ba5115..d93680d59 100644
--- a/nova/virt/libvirt/volume.py
+++ b/nova/virt/libvirt/volume.py
@@ -172,7 +172,7 @@ class LibvirtISCSIVolumeDriver(LibvirtVolumeDriver):
         sup.disconnect_volume(connection_info, mount_device)
         iscsi_properties = connection_info['data']
         self._iscsiadm_update(iscsi_properties, "node.startup", "manual")
-        self._run_iscsiadm(iscsi_properties, ("--logout"),
+        self._run_iscsiadm(iscsi_properties, ("--logout",),
                            check_exit_code=[0, 255])
         self._run_iscsiadm(iscsi_properties, ('--op', 'delete'),
                            check_exit_code=[0, 255])