diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index 59d4fe731..ec2794d6c 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -28,6 +28,7 @@ try: from os_brick.initiator.connectors import nvmeof_agent except ImportError: nvmeof_agent = None +from os_brick.privileged import nvmeof as priv_nvme from os_brick.privileged import rootwrap as priv_rootwrap from os_brick import utils @@ -128,26 +129,13 @@ class NVMeOFConnector(base.BaseLinuxConnector): return None def _get_host_nqn(self): - host_nqn = None try: with open('/etc/nvme/hostnqn', 'r') as f: host_nqn = f.read().strip() - f.close() except IOError: - try: - self._execute( - 'mkdir', '-m', '755', '-p', '/etc/nvme', - root_helper=self._root_helper, run_as_root=True) - out, err = self._execute( - 'nvme', 'gen-hostnqn', '|', 'tee', '/etc/nvme/hostnqn', - root_helper=self._root_helper, run_as_root=True) - if out.strip(): - host_nqn = out.strip() - self._execute( - 'chmod', '644', '/etc/nvme/hostnqn', - root_helper=self._root_helper, run_as_root=True) - except Exception as e: - LOG.warning("Could not generate host nqn: %s" % str(e)) + host_nqn = priv_nvme.create_hostnqn() + except Exception: + host_nqn = None return host_nqn def _get_system_uuid(self): diff --git a/os_brick/privileged/nvmeof.py b/os_brick/privileged/nvmeof.py new file mode 100644 index 000000000..da5e510a5 --- /dev/null +++ b/os_brick/privileged/nvmeof.py @@ -0,0 +1,66 @@ +# Copyright (c) 2021, Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import errno +import os + +from oslo_concurrency import processutils as putils +from oslo_log import log as logging + +import os_brick.privileged +from os_brick.privileged import rootwrap + + +LOG = logging.getLogger(__name__) + + +@os_brick.privileged.default.entrypoint +def create_hostnqn(): + """Create the hostnqn file to speed up finding out the nqn. + + By having the /etc/nvme/hostnqn not only do we make sure that that value is + always used on this system, but we are also able to just open the file to + get the nqn on each get_connector_properties call instead of having to make + a call to nvme show-hostnqn command. + """ + host_nqn = '' + try: + os.makedirs('/etc/nvme', mode=0o755, exist_ok=True) + + # Try to get existing nqn generated from dmi or systemd + try: + host_nqn, err = rootwrap.custom_execute('nvme', 'show-hostnqn') + host_nqn = host_nqn.strip() + + # This is different from OSError's ENOENT, which is missing nvme + # command. This ENOENT is when nvme says there isn't an nqn. + except putils.ProcessExecutionError as e: + if e.exit_code != errno.ENOENT: + raise + LOG.debug('No nqn could be formed from dmi or systemd.') + + if not host_nqn: + LOG.debug('Generating nqn') + host_nqn, err = rootwrap.custom_execute('nvme', 'gen-hostnqn') + host_nqn = host_nqn.strip() + + with open('/etc/nvme/hostnqn', 'w') as f: + LOG.debug('Writing hostnqn file') + f.write(host_nqn) + os.chmod('/etc/nvme/hostnqn', 0o644) + except Exception as e: + LOG.warning("Could not generate host nqn: %s" % str(e)) + + return host_nqn diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index 19423bf41..b06c0f962 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -882,14 +882,26 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): mock_open.return_value.__enter__.return_value.read = ( lambda: HOST_NQN + "\n") host_nqn = self._get_host_nqn() - self.assertEqual(host_nqn, HOST_NQN) + mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r') + self.assertEqual(HOST_NQN, host_nqn) - @mock.patch.object(executor.Executor, '_execute') + @mock.patch.object(nvmeof.priv_nvme, 'create_hostnqn') @mock.patch.object(builtins, 'open') - def test_get_host_nqn_err(self, mock_open, mock_execute): - mock_execute.side_effect = Exception() + def test_get_host_nqn_io_err(self, mock_open, mock_create): + mock_create.return_value = mock.sentinel.nqn mock_open.side_effect = IOError() result = self.connector._get_host_nqn() + mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r') + mock_create.assert_called_once_with() + self.assertEqual(mock.sentinel.nqn, result) + + @mock.patch.object(nvmeof.priv_nvme, 'create_hostnqn') + @mock.patch.object(builtins, 'open') + def test_get_host_nqn_err(self, mock_open, mock_create): + mock_open.side_effect = Exception() + result = self.connector._get_host_nqn() + mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r') + mock_create.assert_not_called() self.assertIsNone(result) @mock.patch.object(executor.Executor, '_execute') diff --git a/os_brick/tests/privileged/test_nvmeof.py b/os_brick/tests/privileged/test_nvmeof.py new file mode 100644 index 000000000..2b5c8f5e3 --- /dev/null +++ b/os_brick/tests/privileged/test_nvmeof.py @@ -0,0 +1,97 @@ +# Copyright (c) 2021, Red Hat, Inc. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +import builtins +import errno +from unittest import mock + +import ddt +from oslo_concurrency import processutils as putils + +import os_brick.privileged as privsep_brick +import os_brick.privileged.nvmeof as privsep_nvme +from os_brick.privileged import rootwrap +from os_brick.tests import base + + +@ddt.ddt +class PrivNVMeTestCase(base.TestCase): + def setUp(self): + super(PrivNVMeTestCase, self).setUp() + + # Disable privsep server/client mode + privsep_brick.default.set_client_mode(False) + self.addCleanup(privsep_brick.default.set_client_mode, True) + + @mock.patch('os.chmod') + @mock.patch.object(builtins, 'open', new_callable=mock.mock_open) + @mock.patch('os.makedirs') + @mock.patch.object(rootwrap, 'custom_execute') + def test_create_hostnqn(self, mock_exec, mock_mkdirs, mock_open, + mock_chmod): + hostnqn = mock.Mock() + mock_exec.return_value = (hostnqn, mock.sentinel.err) + + res = privsep_nvme.create_hostnqn() + + mock_mkdirs.assert_called_once_with('/etc/nvme', + mode=0o755, + exist_ok=True) + mock_exec.assert_called_once_with('nvme', 'show-hostnqn') + mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'w') + stripped_hostnqn = hostnqn.strip.return_value + mock_open().write.assert_called_once_with(stripped_hostnqn) + mock_chmod.assert_called_once_with('/etc/nvme/hostnqn', 0o644) + self.assertEqual(stripped_hostnqn, res) + + @mock.patch('os.chmod') + @mock.patch.object(builtins, 'open', new_callable=mock.mock_open) + @mock.patch('os.makedirs') + @mock.patch.object(rootwrap, 'custom_execute') + def test_create_hostnqn_generate(self, mock_exec, mock_mkdirs, mock_open, + mock_chmod): + hostnqn = mock.Mock() + mock_exec.side_effect = [ + putils.ProcessExecutionError(exit_code=errno.ENOENT), + (hostnqn, mock.sentinel.err) + ] + + res = privsep_nvme.create_hostnqn() + + mock_mkdirs.assert_called_once_with('/etc/nvme', + mode=0o755, + exist_ok=True) + self.assertEqual(2, mock_exec.call_count) + mock_exec.assert_has_calls([mock.call('nvme', 'show-hostnqn'), + mock.call('nvme', 'gen-hostnqn')]) + + mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'w') + stripped_hostnqn = hostnqn.strip.return_value + mock_open().write.assert_called_once_with(stripped_hostnqn) + mock_chmod.assert_called_once_with('/etc/nvme/hostnqn', 0o644) + self.assertEqual(stripped_hostnqn, res) + + @ddt.data(OSError(errno.ENOENT), # nvme not present in system + putils.ProcessExecutionError(exit_code=123)) # nvme error + @mock.patch('os.makedirs') + @mock.patch.object(rootwrap, 'custom_execute') + def test_create_hostnqn_nvme_not_present(self, exception, + mock_exec, mock_mkdirs): + mock_exec.side_effect = exception + res = privsep_nvme.create_hostnqn() + mock_mkdirs.assert_called_once_with('/etc/nvme', + mode=0o755, + exist_ok=True) + mock_exec.assert_called_once_with('nvme', 'show-hostnqn') + self.assertEqual('', res) diff --git a/releasenotes/notes/nvme-hostnqn-c2611dc56729183b.yaml b/releasenotes/notes/nvme-hostnqn-c2611dc56729183b.yaml new file mode 100644 index 000000000..7cfea243d --- /dev/null +++ b/releasenotes/notes/nvme-hostnqn-c2611dc56729183b.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NVMe-oF connector `bug #1928944 + `_: Fixed not returning + the right nqn value on ``get_connector_properties`` when + ``/etc/nvme/hostnqn`` doesn't exist.