openstack/ironic-python-agent
Code Issues Proposed changes

determine tgtd ready status through tgtadm

Browse Source 
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
Change-Id: If3284e0b441fe6c2da507640c08a4bbefd9dad2b
This commit is contained in:
dparalen 2015-10-19 17:21:12 +02:00
parent 1aeef4dc0d
commit fb920f41ed
2 changed files with 26 additions and 34 deletions
ironic_python_agent
extensions
iscsi.py
tests/unit/extensions
test_iscsi.py

23
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):

37
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')