openstack
/
os-brick
Code Issues Proposed changes

NVMe-oF: Create /etc/nvme/hostid 

The NVMe-oF connector currently create the `/etc/nvme/hostnqn` file if
it doesn't exist, but it may still be missing the `/etc/nvme/hostid`
value.

Some distribution packages create the file on installation but others
may not.

It is recommended for the file to be present so that nvme doesn't
randomly generate it.

Randomly generating it means that the value will be different for the
same storage array and the same volume if we connect, disconnect, and
connect the volume again.

This patch ensures that the file will exist and will try to use the
system's UUID as reported by DMI or a randomly generated one.

Closes-Bug: #2016029
Change-Id: I0b60f9078f23f8464d8234841645ed520e8ba655
This commit is contained in:
Gorka Eguileor 2023-04-12 20:23:47 +02:00
parent 28ffcdbfa1
commit 16c90d5fe9
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.