openstack/os-brick
Code Issues Proposed changes

Merge "NVMe-oF: Create /etc/nvme/hostid"

Browse Source
This commit is contained in:
Zuul 2023-08-24 08:35:34 +00:00 committed by Gerrit Code Review
commit eb76f8283e
7 changed files with 136 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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:

21
os_brick/privileged/nvmeof.py
View File

@ -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

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

@ -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."""

29
os_brick/tests/privileged/test_nvmeof.py
View File

@ -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'

40
os_brick/tests/test_utils.py
View File

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

18
os_brick/utils.py
View File

@ -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.

7
releasenotes/notes/nvmeof-create-hostid-15bf84ec00726fad.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
NVMe-oF `bug #2016029 <https://bugs.launchpad.net/os-brick/+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.