diff options
| -rw-r--r-- | ironic/common/glance_service/image_service.py | 3 | ||||
| -rw-r--r-- | ironic/common/utils.py | 15 | ||||
| -rw-r--r-- | ironic/conf/glance.py | 1 | ||||
| -rw-r--r-- | ironic/drivers/modules/image_utils.py | 10 | ||||
| -rw-r--r-- | ironic/drivers/modules/redfish/raid.py | 8 | ||||
| -rw-r--r-- | ironic/tests/unit/common/test_glance_service.py | 27 | ||||
| -rw-r--r-- | ironic/tests/unit/drivers/modules/irmc/test_inspect.py | 25 | ||||
| -rw-r--r-- | ironic/tests/unit/drivers/modules/redfish/test_raid.py | 4 | ||||
| -rw-r--r-- | ironic/tests/unit/drivers/modules/test_image_utils.py | 47 | ||||
| -rw-r--r-- | releasenotes/config.yaml | 5 | ||||
| -rw-r--r-- | releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml | 7 | ||||
| -rw-r--r-- | releasenotes/notes/fix-nonetype-object-is-not-iterable-0592926d890d6c11.yaml | 7 | ||||
| -rw-r--r-- | reno.yaml | 4 | ||||
| -rw-r--r-- | tox.ini | 9 |
14 files changed, 124 insertions, 48 deletions
diff --git a/ironic/common/glance_service/image_service.py b/ironic/common/glance_service/image_service.py index 0a32eaf0a..1d9d6d4bc 100644 --- a/ironic/common/glance_service/image_service.py +++ b/ironic/common/glance_service/image_service.py @@ -33,6 +33,7 @@ from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common import keystone from ironic.common import swift +from ironic.common import utils from ironic.conf import CONF TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement', @@ -114,7 +115,7 @@ class GlanceImageService(object): @tenacity.retry( retry=tenacity.retry_if_exception_type( exception.GlanceConnectionFailed), - stop=tenacity.stop_after_attempt(CONF.glance.num_retries + 1), + stop=utils.stop_after_retries('num_retries', group='glance'), wait=tenacity.wait_fixed(1), reraise=True ) diff --git a/ironic/common/utils.py b/ironic/common/utils.py index e4d83c9b6..9ae88d4d6 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -681,3 +681,18 @@ def is_fips_enabled(): except Exception: pass return False + + +def stop_after_retries(option, group=None): + """A tenacity retry helper that stops after retries specified in conf.""" + # NOTE(dtantsur): fetch the option inside of the nested call, otherwise it + # cannot be changed in runtime. + def should_stop(retry_state): + if group: + conf = getattr(CONF, group) + else: + conf = CONF + num_retries = getattr(conf, option) + return retry_state.attempt_number >= num_retries + 1 + + return should_stop diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index a3286b1eb..317f213bc 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -114,6 +114,7 @@ opts = [ 'will determine how many containers are created.')), cfg.IntOpt('num_retries', default=0, + mutable=True, help=_('Number of retries when downloading an image from ' 'glance.')), ] diff --git a/ironic/drivers/modules/image_utils.py b/ironic/drivers/modules/image_utils.py index 304c199bf..86607ee25 100644 --- a/ironic/drivers/modules/image_utils.py +++ b/ironic/drivers/modules/image_utils.py @@ -211,6 +211,16 @@ class ImageHandler(object): try: os.link(image_file, published_file) os.chmod(image_file, self._file_permission) + try: + utils.execute( + '/usr/sbin/restorecon', '-i', '-R', 'v', public_dir) + except FileNotFoundError as exc: + LOG.debug( + "Could not restore SELinux context on " + "%(public_dir)s, restorecon command not found.\n" + "Error: %(error)s", + {'public_dir': public_dir, + 'error': exc}) except OSError as exc: LOG.debug( diff --git a/ironic/drivers/modules/redfish/raid.py b/ironic/drivers/modules/redfish/raid.py index 809ec59c6..154cd53d3 100644 --- a/ironic/drivers/modules/redfish/raid.py +++ b/ironic/drivers/modules/redfish/raid.py @@ -1120,7 +1120,9 @@ class RedfishRAID(base.RAIDInterface): raid_configs['pending'].setdefault(controller, []).append( logical_disk) - node.set_driver_internal_info('raid_configs', raid_configs) + # Store only when async operation + if reboot_required: + node.set_driver_internal_info('raid_configs', raid_configs) return raid_configs, reboot_required @@ -1182,7 +1184,9 @@ class RedfishRAID(base.RAIDInterface): response.task_monitor_uri) reboot_required = True - node.set_driver_internal_info('raid_configs', raid_configs) + # Store only when async operation + if reboot_required: + node.set_driver_internal_info('raid_configs', raid_configs) return raid_configs, reboot_required diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index 6be0fccd9..a6dd11d3e 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -24,7 +24,6 @@ from glanceclient import exc as glance_exc from keystoneauth1 import loading as ks_loading from oslo_config import cfg from oslo_utils import uuidutils -import tenacity import testtools from ironic.common import context @@ -204,20 +203,18 @@ class TestGlanceImageService(base.TestCase): image_id = uuidutils.generate_uuid() writer = NullWriter() - with mock.patch.object(tenacity, 'retry', autospec=True) as mock_retry: - # When retries are disabled, we should get an exception - self.config(num_retries=0, group='glance') - self.assertRaises(exception.GlanceConnectionFailed, - stub_service.download, image_id, writer) - - # Now lets enable retries. No exception should happen now. - self.config(num_retries=1, group='glance') - importlib.reload(image_service) - stub_service = image_service.GlanceImageService(stub_client, - stub_context) - tries = [0] - stub_service.download(image_id, writer) - mock_retry.assert_called_once() + # When retries are disabled, we should get an exception + self.config(num_retries=0, group='glance') + self.assertRaises(exception.GlanceConnectionFailed, + stub_service.download, image_id, writer) + + # Now lets enable retries. No exception should happen now. + self.config(num_retries=1, group='glance') + importlib.reload(image_service) + stub_service = image_service.GlanceImageService(stub_client, + stub_context) + tries = [0] + stub_service.download(image_id, writer) def test_download_no_data(self): self.client.fake_wrapped = None diff --git a/ironic/tests/unit/drivers/modules/irmc/test_inspect.py b/ironic/tests/unit/drivers/modules/irmc/test_inspect.py index 5c66cb96a..da91ec61d 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_inspect.py @@ -204,8 +204,8 @@ class IRMCInspectTestCase(test_common.BaseIRMCTest): _inspect_hardware_mock.return_value = (inspected_props, inspected_macs, new_traits) - new_port_mock1 = mock.MagicMock(spec=objects.Port) - new_port_mock2 = mock.MagicMock(spec=objects.Port) + new_port_mock1 = objects.Port + new_port_mock2 = objects.Port port_mock.side_effect = [new_port_mock1, new_port_mock2] @@ -220,11 +220,11 @@ class IRMCInspectTestCase(test_common.BaseIRMCTest): port_mock.assert_has_calls([ mock.call(task.context, address=inspected_macs[0], node_id=node_id), + mock.call.create(), mock.call(task.context, address=inspected_macs[1], - node_id=node_id) - ]) - new_port_mock1.create.assert_called_once_with() - new_port_mock2.create.assert_called_once_with() + node_id=node_id), + mock.call.create() + ], any_order=False) self.assertTrue(info_mock.called) task.node.refresh() @@ -259,8 +259,9 @@ class IRMCInspectTestCase(test_common.BaseIRMCTest): _inspect_hardware_mock.return_value = (inspected_props, inspected_macs, new_traits) - new_port_mock1 = mock.MagicMock(spec=objects.Port) - new_port_mock2 = mock.MagicMock(spec=objects.Port) + + new_port_mock1 = objects.Port + new_port_mock2 = objects.Port port_mock.side_effect = [new_port_mock1, new_port_mock2] @@ -276,11 +277,11 @@ class IRMCInspectTestCase(test_common.BaseIRMCTest): port_mock.assert_has_calls([ mock.call(task.context, address=inspected_macs[0], node_id=node_id), + mock.call.create(), mock.call(task.context, address=inspected_macs[1], - node_id=node_id) - ]) - new_port_mock1.create.assert_called_once_with() - new_port_mock2.create.assert_called_once_with() + node_id=node_id), + mock.call.create() + ], any_order=False) self.assertTrue(info_mock.called) task.node.refresh() diff --git a/ironic/tests/unit/drivers/modules/redfish/test_raid.py b/ironic/tests/unit/drivers/modules/redfish/test_raid.py index dfb3c1473..843be735c 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_raid.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_raid.py @@ -336,6 +336,8 @@ class RedfishRAIDTestCase(db_base.DbTestCase): self.assertEqual(mock_node_power_action.call_count, 0) self.assertEqual(mock_build_agent_options.call_count, 0) self.assertEqual(mock_prepare_ramdisk.call_count, 0) + self.assertIsNone( + task.node.driver_internal_info.get('raid_configs')) self.assertEqual( [{'controller': 'RAID controller 1', 'id': '1', @@ -1066,6 +1068,8 @@ class RedfishRAIDTestCase(db_base.DbTestCase): self.assertEqual(mock_node_power_action.call_count, 0) self.assertEqual(mock_build_agent_options.call_count, 0) self.assertEqual(mock_prepare_ramdisk.call_count, 0) + self.assertIsNone( + task.node.driver_internal_info.get('raid_configs')) self.assertEqual([], task.node.raid_config['logical_disks']) self.assertNotEqual( last_updated, task.node.raid_config['last_updated']) diff --git a/ironic/tests/unit/drivers/modules/test_image_utils.py b/ironic/tests/unit/drivers/modules/test_image_utils.py index 753452f5d..b6c572125 100644 --- a/ironic/tests/unit/drivers/modules/test_image_utils.py +++ b/ironic/tests/unit/drivers/modules/test_image_utils.py @@ -105,73 +105,96 @@ class RedfishImageHandlerTestCase(db_base.DbTestCase): mock_swift_api.delete_object.assert_called_once_with( 'ironic_redfish_container', object_name) + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_local_link( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) - url = img_handler_obj.publish_image('file.iso', 'boot.iso') - self.assertEqual( 'http://localhost/redfish/boot.iso', url) + mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) + mock_link.assert_called_once_with( + 'file.iso', '/httpboot/redfish/boot.iso') + mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(os, 'chmod', autospec=True) + @mock.patch.object(image_utils, 'shutil', autospec=True) + @mock.patch.object(os, 'link', autospec=True) + @mock.patch.object(os, 'mkdir', autospec=True) + def test_publish_image_local_link_no_restorecon( + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): + self.config(use_swift=False, group='redfish') + self.config(http_url='http://localhost', group='deploy') + img_handler_obj = image_utils.ImageHandler(self.node.driver) + url = img_handler_obj.publish_image('file.iso', 'boot.iso') + self.assertEqual( + 'http://localhost/redfish/boot.iso', url) mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.return_value = FileNotFoundError + mock_shutil.assert_not_called() + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_external_ip( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', external_http_url='http://non-local.host', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) - url = img_handler_obj.publish_image('file.iso', 'boot.iso') - self.assertEqual( 'http://non-local.host/redfish/boot.iso', url) - mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') + @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) @mock.patch.object(os, 'link', autospec=True) @mock.patch.object(os, 'mkdir', autospec=True) def test_publish_image_external_ip_node_override( - self, mock_mkdir, mock_link, mock_shutil, mock_chmod): + self, mock_mkdir, mock_link, mock_shutil, mock_chmod, + mock_execute): self.config(use_swift=False, group='redfish') self.config(http_url='http://localhost', external_http_url='http://non-local.host', group='deploy') img_handler_obj = image_utils.ImageHandler(self.node.driver) self.node.driver_info["external_http_url"] = "http://node.override.url" - override_url = self.node.driver_info.get("external_http_url") - url = img_handler_obj.publish_image('file.iso', 'boot.iso', override_url) - self.assertEqual( 'http://node.override.url/redfish/boot.iso', url) - mock_mkdir.assert_called_once_with('/httpboot/redfish', 0o755) mock_link.assert_called_once_with( 'file.iso', '/httpboot/redfish/boot.iso') mock_chmod.assert_called_once_with('file.iso', 0o644) + mock_execute.assert_called_once_with( + '/usr/sbin/restorecon', '-i', '-R', 'v', '/httpboot/redfish') @mock.patch.object(os, 'chmod', autospec=True) @mock.patch.object(image_utils, 'shutil', autospec=True) diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml new file mode 100644 index 000000000..26538010e --- /dev/null +++ b/releasenotes/config.yaml @@ -0,0 +1,5 @@ +--- +# Ignore the kilo-eol tag because that branch does not work with reno +# and contains no release notes. +# Ignore bugfix tags because their releasenotes are covered under stable +closed_branch_tag_re: 'r"(?!^(kilo-|bugfix-)).+-eol$"' diff --git a/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml b/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml new file mode 100644 index 000000000..90d38d5cc --- /dev/null +++ b/releasenotes/notes/fix-context-image-hardlink-16f452974abc7327.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue where if selinux is enabled and enforcing, and + the published image is a hardlink, the source selinux context + is preserved, causing access denied when retrieving the image + using hardlink URL. diff --git a/releasenotes/notes/fix-nonetype-object-is-not-iterable-0592926d890d6c11.yaml b/releasenotes/notes/fix-nonetype-object-is-not-iterable-0592926d890d6c11.yaml new file mode 100644 index 000000000..ec9043adb --- /dev/null +++ b/releasenotes/notes/fix-nonetype-object-is-not-iterable-0592926d890d6c11.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes ``'NoneType' object is not iterable`` in conductor logs for + ``redfish`` and ``idrac-redfish`` RAID clean and deploy steps. The message + should no longer appear. For affected nodes re-create the node or delete + ``raid_configs`` entry from ``driver_internal_info`` field. diff --git a/reno.yaml b/reno.yaml deleted file mode 100644 index dd0aac790..000000000 --- a/reno.yaml +++ /dev/null @@ -1,4 +0,0 @@ ---- -# Ignore the kilo-eol tag because that branch does not work with reno -# and contains no release notes. -closed_branch_tag_re: "(.+)(?<!kilo)-eol" @@ -8,7 +8,7 @@ ignore_basepython_conflict=true usedevelop = True basepython = python3 setenv = VIRTUAL_ENV={envdir} - PYTHONDONTWRITEBYTECODE = 1 + PYTHONDONTWRITEBYTECODE=1 LANGUAGE=en_US LC_ALL=en_US.UTF-8 deps = @@ -17,7 +17,12 @@ deps = -r{toxinidir}/test-requirements.txt commands = stestr run --slowest {posargs} -passenv = http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY +passenv = http_proxy + HTTP_PROXY + https_proxy + HTTPS_PROXY + no_proxy + NO_PROXY [testenv:unit-with-driver-libs] deps = {[testenv]deps} |