openstack/os-brick
Code Issues Proposed changes

NVMe-oF: Support nvme cli v2

Browse Source 
The nvme cli has changed its behavior, now they no longer differentiate
between errors returning a different exit code.

Exit code 1 is for errors and 0 for success.

This patch fixes the detection of race conditions to also look for the
message in case it's a newer CLI version.

Together with change I318f167baa0ba7789f4ca2c7c12a8de5568195e0 we are
ready for nvme CLI v2.

Closes-Bug: #1961222
Change-Id: Idf4d79527e1f03cec754ad708d069b2905b90d3f
(cherry picked from commit 16ba8ed396)
(cherry picked from commit 2bd9cee754)
This commit is contained in:
Gorka Eguileor 2023-09-14 14:34:07 +02:00
parent b1363ca36a
commit 776864b43d
3 changed files with 37 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
os_brick/initiator/connectors/nvmeof.py
View File

@ -926,8 +926,12 @@ class NVMeOFConnector(base.BaseLinuxConnector):
# c-vol running on same node with different lock paths) or
# an admin is touching things manually. Not passing these
# exit codes in check_exit_code parameter to _execute so we
# can log it.
if exc.exit_code not in (70, errno.EALREADY):
# can log it. nvme cli v2 returns 1, so we parse the
# message. Some nvme cli versions return errors in stdout,
# so we look in stderr and stdout.
if not (exc.exit_code in (70, errno.EALREADY) or
(exc.exit_code == 1 and
'already connected' in exc.stderr + exc.stdout)):
LOG.error('Could not connect to %s: exit_code: %s, '
'stdout: "%s", stderr: "%s",', portal,
exc.exit_code, exc.stdout, exc.stderr)

38
os_brick/tests/initiator/connectors/test_nvmeof.py
View File

@ -1505,7 +1505,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
'-n', target.nqn, '-Q', '128', '-l', '-1'])
for portal in target.portals])
@ddt.data(70, errno.EALREADY)
@ddt.data((70, '', ''),
(errno.EALREADY, '', ''),
(1, '', 'already connected'),
(1, 'already connected', ''))
@ddt.unpack
@mock.patch.object(nvmeof.LOG, 'warning')
@mock.patch.object(nvmeof.Target, 'find_device')
@mock.patch.object(nvmeof.Target, 'set_portals_controllers')
@ -1513,14 +1517,14 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(nvmeof.NVMeOFConnector, 'rescan')
@mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock)
def test__connect_target_race(
self, exit_code, mock_state, mock_rescan, mock_cli,
self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli,
mock_set_ctrls, mock_find_dev, mock_log):
"""Treat race condition with sysadmin as success."""
mock_state.side_effect = ['connecting', 'connecting', None, 'live']
dev_path = '/dev/nvme0n1'
mock_find_dev.return_value = dev_path
mock_cli.side_effect = putils.ProcessExecutionError(
exit_code=exit_code)
exit_code=exit_code, stdout=stdout, stderr=stderr)
target = self.conn_props.targets[0]
@ -1539,7 +1543,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
portal.transport, '-n', target.nqn, '-Q', '128', '-l', '-1'])
self.assertEqual(1, mock_log.call_count)
@ddt.data(70, errno.EALREADY)
@ddt.data((70, '', ''),
(errno.EALREADY, '', ''),
(1, '', 'already connected'),
(1, 'already connected', ''))
@ddt.unpack
@mock.patch.object(nvmeof.LOG, 'warning')
@mock.patch('time.sleep')
@mock.patch('time.time', side_effect=[0, 0.1, 0.6])
@ -1553,12 +1561,12 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(nvmeof.NVMeOFConnector, 'rescan')
@mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock)
def test__connect_target_race_connecting(
self, exit_code, mock_state, mock_rescan, mock_cli, mock_set_ctrls,
mock_find_dev, mock_is_live, mock_delay, mock_time, mock_sleep,
mock_log):
self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli,
mock_set_ctrls, mock_find_dev, mock_is_live, mock_delay, mock_time,
mock_sleep, mock_log):
"""Test connect target when portal is reconnecting after race."""
mock_cli.side_effect = putils.ProcessExecutionError(
exit_code=exit_code)
exit_code=exit_code, stdout=stdout, stderr=stderr)
mock_state.side_effect = ['connecting', 'connecting', None,
'connecting']
mock_is_live.side_effect = [False, False, False, False, True]
@ -1581,7 +1589,11 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
portal.transport, '-n', target.nqn, '-Q', '128', '-l', '-1'])
self.assertEqual(1, mock_log.call_count)
@ddt.data(70, errno.EALREADY)
@ddt.data((70, '', ''),
(errno.EALREADY, '', ''),
(1, '', 'already connected'),
(1, 'already connected', ''))
@ddt.unpack
@mock.patch.object(nvmeof.LOG, 'warning')
@mock.patch.object(nvmeof.LOG, 'error')
@mock.patch('time.sleep')
@ -1596,12 +1608,12 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(nvmeof.NVMeOFConnector, 'rescan')
@mock.patch.object(nvmeof.Portal, 'state', new_callable=mock.PropertyMock)
def test__connect_target_race_unknown(
self, exit_code, mock_state, mock_rescan, mock_cli, mock_set_ctrls,
mock_find_dev, mock_is_live, mock_delay, mock_time, mock_sleep,
mock_log_err, mock_log_warn):
self, exit_code, stdout, stderr, mock_state, mock_rescan, mock_cli,
mock_set_ctrls, mock_find_dev, mock_is_live, mock_delay, mock_time,
mock_sleep, mock_log_err, mock_log_warn):
"""Test connect target when portal is unknown after race."""
mock_cli.side_effect = putils.ProcessExecutionError(
exit_code=exit_code)
exit_code=exit_code, stdout=stdout, stderr=stderr)
mock_state.side_effect = ['connecting', 'connecting', None,
'unknown']
mock_is_live.side_effect = [False, False, False, True]

6
releasenotes/notes/nvmeof-support-v2-0d3a423c26eee003.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
NVMe-oF connector `bug #1961222
<https://bugs.launchpad.net/os-brick/+bug/1961222>`_: Fixed
support of newer NVMe CLI v2.