diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index d9b2627cc..7cb46df63 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -24,7 +24,6 @@ import time from typing import (Callable, Optional, Sequence, Type, Union) # noqa: H301 import uuid as uuid_lib - from oslo_concurrency import processutils as putils from oslo_log import log as logging @@ -768,10 +767,16 @@ class NVMeOFConnector(base.BaseLinuxConnector): ret = {} nqn = None + hostid = None uuid = nvmf._get_host_uuid() suuid = priv_nvmeof.get_system_uuid() if cls.nvme_present(): nqn = utils.get_host_nqn() + # Ensure /etc/nvme/hostid exists and defaults to the system uuid, + # or a random value. + hostid = utils.get_nvme_host_id(suuid) + if hostid: + ret['nvme_hostid'] = hostid if uuid: ret['uuid'] = uuid if suuid: diff --git a/os_brick/privileged/nvmeof.py b/os_brick/privileged/nvmeof.py index b5a4622cb..0a45aca6c 100644 --- a/os_brick/privileged/nvmeof.py +++ b/os_brick/privileged/nvmeof.py @@ -13,8 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +from __future__ import annotations + import errno import os +from typing import Optional # noqa: H301 from oslo_concurrency import processutils as putils from oslo_log import log as logging @@ -93,3 +96,21 @@ def get_system_uuid() -> str: out = "" return out.strip() + + +@os_brick.privileged.default.entrypoint +def create_hostid(uuid: str) -> Optional[str]: + """Create the hostid to ensure it's always the same.""" + try: + os.makedirs('/etc/nvme', mode=0o755, exist_ok=True) + + with open('/etc/nvme/hostid', 'w') as f: + LOG.debug('Writing nvme hostid %s', uuid) + f.write(f'{uuid}\n') + os.chmod('/etc/nvme/hostid', 0o644) + + except Exception as e: + LOG.warning("Could not generate nvme host id: %s", e) + return None + + return uuid diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index f14e626c9..5d7e5fb9b 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -921,6 +921,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): uuid = self.connector._get_host_uuid() self.assertIsNone(uuid) + @mock.patch.object(utils, 'get_nvme_host_id', + return_value=SYS_UUID) @mock.patch.object(nvmeof.NVMeOFConnector, '_is_native_multipath_supported', return_value=True) @@ -935,11 +937,17 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_get_connector_properties_without_sysuuid(self, mock_host_uuid, mock_sysuuid, mock_nqn, mock_nvme_present, - mock_nat_mpath_support): + mock_nat_mpath_support, + mock_get_host_id): props = self.connector.get_connector_properties('sudo') - expected_props = {'nqn': 'fakenqn', 'nvme_native_multipath': False} + expected_props = {'nqn': 'fakenqn', + 'nvme_native_multipath': False, + 'nvme_hostid': SYS_UUID} self.assertEqual(expected_props, props) + mock_get_host_id.assert_called_once_with(None) + @mock.patch.object(utils, 'get_nvme_host_id', + return_value=SYS_UUID) @mock.patch.object(nvmeof.NVMeOFConnector, '_is_native_multipath_supported', return_value=True) @@ -951,15 +959,18 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): def test_get_connector_properties_with_sysuuid(self, mock_host_uuid, mock_sysuuid, mock_nqn, mock_nvme_present, - mock_native_mpath_support): + mock_native_mpath_support, + mock_get_host_id): 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, 'nvme_native_multipath': False} + "uuid": HOST_UUID, 'nvme_native_multipath': False, + 'nvme_hostid': SYS_UUID} self.assertEqual(expected_props, props) + mock_get_host_id.assert_called_once_with(SYS_UUID) def test_get_volume_paths_device_info(self): """Device info path has highest priority.""" diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index e5a2f4940..a353c3af5 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -60,6 +60,7 @@ class ConnectorUtilsTestCase(test_base.TestCase): return_value=None) @mock.patch.object(platform, 'machine', mock.Mock(return_value='s390x')) @mock.patch('sys.platform', 'linux2') + @mock.patch.object(utils, 'get_nvme_host_id', mock.Mock(return_value=None)) def _test_brick_get_connector_properties(self, multipath, enforce_multipath, multipath_result, diff --git a/os_brick/tests/privileged/test_nvmeof.py b/os_brick/tests/privileged/test_nvmeof.py index 6fa623d4a..35a964c30 100644 --- a/os_brick/tests/privileged/test_nvmeof.py +++ b/os_brick/tests/privileged/test_nvmeof.py @@ -126,6 +126,35 @@ class PrivNVMeTestCase(base.TestCase): mock_exec.assert_called_once_with('nvme', 'show-hostnqn') self.assertEqual('', res) + @mock.patch('os.chmod') + @mock.patch.object(builtins, 'open', new_callable=mock.mock_open) + @mock.patch('os.makedirs') + def test_create_hostid(self, mock_mkdirs, mock_open, mock_chmod): + res = privsep_nvme.create_hostid('uuid') + + mock_mkdirs.assert_called_once_with('/etc/nvme', + mode=0o755, + exist_ok=True) + mock_open.assert_called_once_with('/etc/nvme/hostid', 'w') + mock_open().write.assert_called_once_with('uuid\n') + mock_chmod.assert_called_once_with('/etc/nvme/hostid', 0o644) + self.assertEqual('uuid', res) + + @mock.patch('os.chmod') + @mock.patch.object(builtins, 'open', new_callable=mock.mock_open) + @mock.patch('os.makedirs') + def test_create_hostid_fails(self, mock_mkdirs, mock_open, mock_chmod): + mock_mkdirs.side_effect = OSError + + res = privsep_nvme.create_hostid(None) + + mock_mkdirs.assert_called_once_with('/etc/nvme', + mode=0o755, + exist_ok=True) + mock_open.assert_not_called() + mock_chmod.assert_not_called() + self.assertIsNone(res) + @mock.patch.object(builtins, 'open', new_callable=mock.mock_open) def test_get_system_uuid_product_uuid(self, mock_open): uuid = 'dbc6ba60-36ae-4b96-9310-628832bdfc3d' diff --git a/os_brick/tests/test_utils.py b/os_brick/tests/test_utils.py index e52a71fe5..8dab15f04 100644 --- a/os_brick/tests/test_utils.py +++ b/os_brick/tests/test_utils.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import builtins import functools import io import time @@ -43,6 +44,45 @@ class TestUtils(base.TestCase): 'blockdev', '--getsize64', device, run_as_root=True, root_helper=mock_execute._root_helper) + @mock.patch.object(builtins, 'open') + def test_get_nvme_host_id_file_available(self, mock_open): + mock_open.return_value.__enter__.return_value.read.return_value = ( + 'uuid\n') + result = utils.get_nvme_host_id(mock.sentinel.uuid) + mock_open.assert_called_once_with('/etc/nvme/hostid', 'r') + self.assertEqual('uuid', result) + + @mock.patch.object(utils.priv_nvme, 'create_hostid') + @mock.patch.object(builtins, 'open') + def test_get_nvme_host_id_io_err(self, mock_open, mock_create): + mock_create.return_value = mock.sentinel.uuid_return + mock_open.side_effect = IOError() + result = utils.get_nvme_host_id(mock.sentinel.uuid) + mock_open.assert_called_once_with('/etc/nvme/hostid', 'r') + mock_create.assert_called_once_with(mock.sentinel.uuid) + self.assertEqual(mock.sentinel.uuid_return, result) + + @mock.patch('uuid.uuid4') + @mock.patch.object(utils.priv_nvme, 'create_hostid') + @mock.patch.object(builtins, 'open') + def test_get_nvme_host_id_io_err_no_uuid(self, mock_open, mock_create, + mock_uuid): + mock_create.return_value = mock.sentinel.uuid_return + mock_open.side_effect = IOError() + result = utils.get_nvme_host_id(None) + mock_open.assert_called_once_with('/etc/nvme/hostid', 'r') + mock_create.assert_called_once_with(str(mock_uuid.return_value)) + self.assertEqual(mock.sentinel.uuid_return, result) + + @mock.patch.object(utils.priv_nvme, 'create_hostid') + @mock.patch.object(builtins, 'open') + def test_get_nvme_host_id_err(self, mock_open, mock_create): + mock_open.side_effect = Exception() + result = utils.get_nvme_host_id(None) + mock_open.assert_called_once_with('/etc/nvme/hostid', 'r') + mock_create.assert_not_called() + self.assertIsNone(result) + class TestRetryDecorator(base.TestCase): diff --git a/os_brick/utils.py b/os_brick/utils.py index facd1b4ab..b8af536de 100644 --- a/os_brick/utils.py +++ b/os_brick/utils.py @@ -20,6 +20,7 @@ import logging as py_logging import os import time from typing import Any, Callable, Optional, Type, Union # noqa: H301 +import uuid as uuid_lib from oslo_concurrency import processutils from oslo_log import log as logging @@ -231,6 +232,23 @@ def get_host_nqn() -> Optional[str]: return host_nqn +def get_nvme_host_id(uuid: Optional[str]) -> Optional[str]: + """Get the nvme host id + + If the hostid file doesn't exist create it either with the passed uuid or + a random one. + """ + try: + with open('/etc/nvme/hostid', 'r') as f: + host_id = f.read().strip() + except IOError: + uuid = uuid or str(uuid_lib.uuid4()) + host_id = priv_nvme.create_hostid(uuid) + except Exception: + host_id = None + return host_id + + def _symlink_name_from_device_path(device_path): """Generate symlink absolute path for encrypted devices. diff --git a/releasenotes/notes/nvmeof-create-hostid-15bf84ec00726fad.yaml b/releasenotes/notes/nvmeof-create-hostid-15bf84ec00726fad.yaml new file mode 100644 index 000000000..bb4b61422 --- /dev/null +++ b/releasenotes/notes/nvmeof-create-hostid-15bf84ec00726fad.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NVMe-oF `bug #2016029 `_: + The NVMe-oF connector now creates `/etc/nvme/hostid` when it's missing from + the system. That way the Host ID will persist and always be the same, + instead of being randomly generated.