openstack
/
os-brick
Code Issues Proposed changes

Don't log error trace when nvme is not used 

When nvme is not configured, the connector still logs error
traceback and creates /etc/nvme directory.
This patch checks if nvme is configured and avoids the case
if nvme is not used.

Closes-Bug: #1929074
Closes-Bug: #1929075
Change-Id: I2473f50671b9c3f1840efd0d12d2a2572a21d850
This commit is contained in:
Rajat Dhasmana 2021-05-03 10:07:54 -04:00 committed by whoami-rajat
parent 1b2e229542
commit f2bc082b07
4 changed files with 67 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -10,6 +10,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import errno
import glob
import json
import os.path
@ -27,6 +28,7 @@ try:
from os_brick.initiator.connectors import nvmeof_agent
except ImportError:
nvmeof_agent = None
from os_brick.privileged import rootwrap as priv_rootwrap
from os_brick import utils
DEV_SEARCH_PATH = '/dev/'
@ -77,15 +79,31 @@ class NVMeOFConnector(base.BaseLinuxConnector):
except exception.VolumeDeviceNotFound:
return []
@staticmethod
def get_connector_properties(root_helper, *args, **kwargs):
@classmethod
def nvme_present(cls):
try:
priv_rootwrap.custom_execute('nvme', 'version')
return True
except Exception as exc:
if isinstance(exc, OSError) and exc.errno == errno.ENOENT:
LOG.debug('nvme not present on system')
else:
LOG.warning('Unknown error when checking presence of nvme: %s',
exc)
return False
@classmethod
def get_connector_properties(cls, root_helper, *args, **kwargs):
"""The NVMe-oF connector properties (initiator uuid and nqn.)"""
nvmf = NVMeOFConnector(root_helper=root_helper,
execute=kwargs.get('execute'))
execute = kwargs.get('execute') or priv_rootwrap.execute
nvmf = NVMeOFConnector(root_helper=root_helper, execute=execute)
ret = {}
nqn = None
uuid = nvmf._get_host_uuid()
suuid = nvmf._get_system_uuid()
nqn = nvmf._get_host_nqn()
if cls.nvme_present():
nqn = nvmf._get_host_nqn()
if uuid:
ret['uuid'] = uuid
if suuid:

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

@ -22,6 +22,7 @@ from os_brick import exception
from os_brick import executor
from os_brick.initiator.connectors import nvmeof
from os_brick.initiator import linuxscsi
from os_brick.privileged import rootwrap as priv_rootwrap
from os_brick.tests.initiator import test_connector
@ -59,6 +60,21 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
execute=self.fake_execute,
use_multipath=False)
@mock.patch.object(priv_rootwrap, 'custom_execute', autospec=True)
def test_nvme_present(self, mock_execute):
nvme_present = self.connector.nvme_present()
self.assertTrue(nvme_present)
@ddt.data(OSError(2, 'FileNotFoundError'), Exception())
@mock.patch('os_brick.initiator.connectors.nvmeof.LOG')
@mock.patch.object(priv_rootwrap, 'custom_execute', autospec=True)
def test_nvme_present_exception(self, exc, mock_execute, mock_log):
mock_execute.side_effect = exc
nvme_present = self.connector.nvme_present()
log = mock_log.debug if isinstance(exc, OSError) else mock_log.warning
log.assert_called_once()
self.assertFalse(nvme_present)
@mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True)
def test_get_sysuuid_without_newline(self, mock_execute):
mock_execute.return_value = (
@ -73,6 +89,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
uuid = self.connector._get_host_uuid()
self.assertIsNone(uuid)
@mock.patch.object(nvmeof.NVMeOFConnector, 'nvme_present',
return_value=True)
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_host_nqn',
return_value='fakenqn')
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_system_uuid',
@ -80,20 +98,24 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_host_uuid',
return_value=None)
def test_get_connector_properties_without_sysuuid(self, mock_host_uuid,
mock_sysuuid, mock_nqn):
mock_sysuuid, mock_nqn,
mock_nvme_present):
props = self.connector.get_connector_properties('sudo')
expected_props = {'nqn': 'fakenqn'}
self.assertEqual(expected_props, props)
@mock.patch.object(nvmeof.NVMeOFConnector, 'nvme_present')
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_host_nqn', autospec=True)
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_system_uuid',
autospec=True)
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_host_uuid', autospec=True)
def test_get_connector_properties_with_sysuuid(self, mock_host_uuid,
mock_sysuuid, mock_nqn):
mock_sysuuid, mock_nqn,
mock_nvme_present):
mock_host_uuid.return_value = HOST_UUID
mock_sysuuid.return_value = SYS_UUID
mock_nqn.return_value = HOST_NQN
mock_nvme_present.return_value = True
props = self.connector.get_connector_properties('sudo')
expected_props = {"system uuid": SYS_UUID, "nqn": HOST_NQN,
"uuid": HOST_UUID}

12
os_brick/tests/initiator/test_connector.py
View File

@ -106,20 +106,28 @@ class ConnectorUtilsTestCase(test_base.TestCase):
def test_brick_get_connector_properties(self):
self._test_brick_get_connector_properties(False, False, False)
@mock.patch.object(priv_rootwrap, 'custom_execute',
side_effect=OSError(2))
@mock.patch.object(priv_rootwrap, 'execute', return_value=('', ''))
def test_brick_get_connector_properties_multipath(self, mock_execute):
def test_brick_get_connector_properties_multipath(self, mock_execute,
mock_custom_execute):
self._test_brick_get_connector_properties(True, True, True)
mock_execute.assert_called_once_with('multipathd', 'show', 'status',
run_as_root=True,
root_helper='sudo')
mock_custom_execute.assert_called_once_with('nvme', 'version')
@mock.patch.object(priv_rootwrap, 'custom_execute',
side_effect=OSError(2))
@mock.patch.object(priv_rootwrap, 'execute',
side_effect=putils.ProcessExecutionError)
def test_brick_get_connector_properties_fallback(self, mock_execute):
def test_brick_get_connector_properties_fallback(self, mock_execute,
mock_custom_execute):
self._test_brick_get_connector_properties(True, False, False)
mock_execute.assert_called_once_with('multipathd', 'show', 'status',
run_as_root=True,
root_helper='sudo')
mock_custom_execute.assert_called_once_with('nvme', 'version')
@mock.patch.object(priv_rootwrap, 'execute',
side_effect=putils.ProcessExecutionError)

10
releasenotes/notes/fix-nvme-issues-8dfc15cb691389fe.yaml Normal file
View File

@ -0,0 +1,10 @@
---
fixes:
- |
NVMe-oF connector `bug #1929074
<https://bugs.launchpad.net/os-brick/+bug/1929074>`_: Fixed issue with
nvme logging error trace when not present.
- |
NVMe-oF connector `bug #1929075
<https://bugs.launchpad.net/os-brick/+bug/1929075>`_: Fixed issue with
nvme connector creating /etc/nvme directory when not present.