determine tgtd ready status through tgtadm

The intention of the iscsi extension _wait_for_iscsi_daemon function
is to wait till the tgtd process boots completely before running any
iscsi commands through the tgtadm command line utility.  The tgtd
process utilises a unix socket for communication with the user and the
_wait_for_iscsi_daemon function asserts the tgtd process status based
on the socket presence.  The socket name/path to use is not specified
by iscsi extension configuration but rather falls-back to a
(system-wide) default value[1].  Since tgtd version 1.0.60, this path
changed from /var/run/tgtd.ipc_abstract_namespace to
/var/run/tgtd/socket[2] and now causes a false-negative while waiting
for the tgtd process to boot[3].

This patch follows implementation suggestion from the bug report[3] to
avoid relying on the socket presence but rather to check the tgtd
status by trying to communicate through the tgtadm tool.

[1] https://github.com/openstack/ironic-python-agent/blob/
5bbb9ded082f2cfde5e8877e9f33294eb93a5fb7/
ironic_python_agent/extensions/iscsi.py#L47 [2]
https://github.com/fujita/tgt/commit/
d1aa4dcfd691c9409dbad5db49ce6754ce9c1b5a
[3] https://bugs.launchpad.net/ironic-python-agent/+bug/1505923

Closes-Bug: #1505923
Partial-Bug: #1551250
Depends-On: I4c19c96d2d5cd8accb9ed8df6241fc9f0c229e0e
Change-Id: If3284e0b441fe6c2da507640c08a4bbefd9dad2b
(cherry-picked from commit fb920f41ed181c7ec74e3367f2735bccce07e574)

2 files changed, 26 insertions, 34 deletions

diff --git a/ironic_python_agent/extensions/iscsi.py b/ironic_python_agent/extensions/iscsi.py
index 4cd7de0b..9993da8c 100644
--- a/ironic_python_agent/extensions/iscsi.py
+++ b/ironic_python_agent/extensions/iscsi.py
@@ -15,8 +15,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import os
-import time
 
 from oslo_concurrency import processutils
 from oslo_log import log
@@ -30,27 +28,20 @@ from ironic_python_agent import utils
 LOG = log.getLogger(__name__)
 
 
-def _execute(cmd, error_msg, check_exit_code=None):
-    if check_exit_code is None:
-        check_exit_code = [0]
-
+def _execute(cmd, error_msg, **kwargs):
     try:
-        stdout, stderr = utils.execute(*cmd, check_exit_code=check_exit_code)
+        stdout, stderr = utils.execute(*cmd, **kwargs)
     except processutils.ProcessExecutionError as e:
         LOG.error(error_msg)
         raise errors.ISCSIError(error_msg, e.exit_code, e.stdout, e.stderr)
 
 
-def _wait_for_iscsi_daemon(interval=1, attempts=10):
+def _wait_for_iscsi_daemon(attempts=10):
     """Wait for the ISCSI daemon to start."""
-    for attempt in range(attempts):
-        if os.path.exists("/var/run/tgtd.ipc_abstract_namespace.0"):
-            break
-        time.sleep(interval)
-    else:
-        error_msg = "ISCSI daemon didn't initialize"
-        LOG.error(error_msg)
-        raise errors.ISCSIError(error_msg, 1, '', error_msg)
+    # here, iscsi daemon is considered not running in case
+    # tgtadm is not able to talk to tgtd to show iscsi targets
+    cmd = ['tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', 'show']
+    _execute(cmd, "ISCSI daemon didn't initialize", attempts=attempts)
 
 
 def _start_iscsi_daemon(iqn, device):
diff --git a/ironic_python_agent/tests/unit/extensions/test_iscsi.py b/ironic_python_agent/tests/unit/extensions/test_iscsi.py
index 646a4e26..ca96b549 100644
--- a/ironic_python_agent/tests/unit/extensions/test_iscsi.py
+++ b/ironic_python_agent/tests/unit/extensions/test_iscsi.py
@@ -16,7 +16,6 @@
 #    under the License.
 
 import mock
-import os
 import time
 
 from oslo_concurrency import processutils
@@ -39,41 +38,44 @@ class TestISCSIExtension(test_base.BaseTestCase):
         self.fake_dev = '/dev/fake'
         self.fake_iqn = 'iqn-fake'
 
-    @mock.patch.object(iscsi, '_wait_for_iscsi_daemon')
-    def test_start_iscsi_target(self, mock_wait_iscsi, mock_execute,
-                                mock_dispatch):
+    def test_start_iscsi_target(self, mock_execute, mock_dispatch):
         mock_dispatch.return_value = self.fake_dev
         mock_execute.return_value = ('', '')
         result = self.agent_extension.start_iscsi_target(iqn=self.fake_iqn)
 
-        expected = [mock.call('tgtd', check_exit_code=[0]),
+        expected = [mock.call('tgtd'),
+                    mock.call('tgtadm', '--lld', 'iscsi', '--mode',
+                              'target', '--op', 'show', attempts=10),
                     mock.call('tgtadm', '--lld', 'iscsi', '--mode',
                               'target', '--op', 'new', '--tid', '1',
-                              '--targetname', self.fake_iqn,
-                              check_exit_code=[0]),
+                              '--targetname', self.fake_iqn),
                     mock.call('tgtadm', '--lld', 'iscsi', '--mode',
                               'logicalunit', '--op', 'new', '--tid', '1',
-                              '--lun', '1', '--backing-store',
-                              self.fake_dev, check_exit_code=[0]),
+                              '--lun', '1', '--backing-store', self.fake_dev),
                     mock.call('tgtadm', '--lld', 'iscsi', '--mode', 'target',
                               '--op', 'bind', '--tid', '1',
-                              '--initiator-address', 'ALL',
-                              check_exit_code=[0])]
+                              '--initiator-address', 'ALL')]
         mock_execute.assert_has_calls(expected)
         mock_dispatch.assert_called_once_with('get_os_install_device')
-        mock_wait_iscsi.assert_called_once_with()
         self.assertEqual({'iscsi_target_iqn': self.fake_iqn},
                           result.command_result)
 
-    @mock.patch.object(os.path, 'exists', lambda x: False)
     def test_start_iscsi_target_fail_wait_daemon(self, mock_execute,
                                                  mock_dispatch):
         mock_dispatch.return_value = self.fake_dev
-        mock_execute.return_value = ('', '')
+        # side effects here:
+        # - execute tgtd: stdout=='', stderr==''
+        # - induce tgtadm failure while in _wait_for_scsi_daemon
+        mock_execute.side_effect = [('', ''),
+                                    processutils.ProcessExecutionError('blah')]
         self.assertRaises(errors.ISCSIError,
                           self.agent_extension.start_iscsi_target,
                           iqn=self.fake_iqn)
-        mock_execute.assert_called_once_with('tgtd', check_exit_code=[0])
+        expected = [mock.call('tgtd'),
+                    mock.call('tgtadm', '--lld', 'iscsi', '--mode', 'target',
+                              '--op', 'show', attempts=10)]
+
+        mock_execute.assert_has_calls(expected)
         mock_dispatch.assert_called_once_with('get_os_install_device')
 
     @mock.patch.object(iscsi, '_wait_for_iscsi_daemon')
@@ -86,10 +88,9 @@ class TestISCSIExtension(test_base.BaseTestCase):
                           self.agent_extension.start_iscsi_target,
                           iqn=self.fake_iqn)
 
-        expected = [mock.call('tgtd', check_exit_code=[0]),
+        expected = [mock.call('tgtd'),
                     mock.call('tgtadm', '--lld', 'iscsi', '--mode',
                               'target', '--op', 'new', '--tid', '1',
-                              '--targetname', self.fake_iqn,
-                              check_exit_code=[0])]
+                              '--targetname', self.fake_iqn)]
         mock_execute.assert_has_calls(expected)
         mock_dispatch.assert_called_once_with('get_os_install_device')