diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index 5d3259b9e..ed4a99372 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -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: diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index 573137563..e1f1efd35 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -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} diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index f82ff6392..e18c8e798 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -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) diff --git a/releasenotes/notes/fix-nvme-issues-8dfc15cb691389fe.yaml b/releasenotes/notes/fix-nvme-issues-8dfc15cb691389fe.yaml new file mode 100644 index 000000000..e625e8b81 --- /dev/null +++ b/releasenotes/notes/fix-nvme-issues-8dfc15cb691389fe.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + NVMe-oF connector `bug #1929074 + `_: Fixed issue with + nvme logging error trace when not present. + - | + NVMe-oF connector `bug #1929075 + `_: Fixed issue with + nvme connector creating /etc/nvme directory when not present. \ No newline at end of file