diff --git a/doc/source/install/standalone.rst b/doc/source/install/standalone.rst index 553c58306b..60c865355d 100644 --- a/doc/source/install/standalone.rst +++ b/doc/source/install/standalone.rst @@ -88,22 +88,21 @@ Preparing images If you don't use Image service, it's possible to provide images to Bare Metal service via a URL. -.. note:: - At the moment, only two types of URLs are acceptable instead of Image - service UUIDs: HTTP(S) URLs (for example, "http://my.server.net/images/img") - and file URLs (file:///images/img). +At the moment, only two types of URLs are acceptable instead of Image +service UUIDs: HTTP(S) URLs (for example, "http://my.server.net/images/img") +and file URLs (file:///images/img). There are however some limitations for different hardware interfaces: -* If you're using :ref:`direct-deploy`, you have to provide the Bare Metal - service with the MD5 checksum of your instance image. To compute it, you can - use the following command:: +* If you're using :ref:`direct-deploy` with HTTP(s) URLs, you have to provide + the Bare Metal service with the MD5 checksum of your instance image. + To compute it, you can use the following command:: md5sum image.qcow2 ed82def8730f394fb85aef8a208635f6 image.qcow2 -* :ref:`direct-deploy` requires the instance image be accessible through a - HTTP(s) URL. +* :ref:`direct-deploy` started supporting ``file://`` images in the Victoria + release cycle, before that only HTTP(s) had been supported. .. note:: The Bare Metal service tracks content changes for non-Glance images by diff --git a/ironic/common/images.py b/ironic/common/images.py index 31332df761..cdddddf8e3 100644 --- a/ironic/common/images.py +++ b/ironic/common/images.py @@ -414,7 +414,7 @@ def fetch(context, image_href, path, force_raw=False): image_to_raw(image_href, path, "%s.part" % path) -def force_raw_get_source_format(image_href, path): +def get_source_format(image_href, path): data = disk_utils.qemu_img_info(path) fmt = data.file_format @@ -435,7 +435,7 @@ def force_raw_get_source_format(image_href, path): def force_raw_will_convert(image_href, path_tmp): with fileutils.remove_path_on_error(path_tmp): - fmt = force_raw_get_source_format(image_href, path_tmp) + fmt = get_source_format(image_href, path_tmp) if fmt != "raw": return True return False @@ -443,7 +443,7 @@ def force_raw_will_convert(image_href, path_tmp): def image_to_raw(image_href, path, path_tmp): with fileutils.remove_path_on_error(path_tmp): - fmt = force_raw_get_source_format(image_href, path_tmp) + fmt = get_source_format(image_href, path_tmp) if fmt != "raw": staged = "%s.converted" % path diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 8418c533ce..73f0c72d5b 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -168,8 +168,12 @@ def validate_http_provisioning_configuration(node): :raises: MissingParameterValue if required option(s) is not set. """ image_source = node.instance_info.get('image_source') - if (not service_utils.is_glance_image(image_source) - or CONF.agent.image_download_source != 'http'): + # NOTE(dtantsur): local HTTP configuration is required in two cases: + # 1. Glance images with image_download_source == http + # 2. File images (since we need to serve them to IPA) + if (not image_source.startswith('file://') + and (not service_utils.is_glance_image(image_source) + or CONF.agent.image_download_source == 'swift')): return params = { @@ -379,8 +383,7 @@ class AgentDeployMixin(agent_base.AgentDeployMixin): manager_utils.node_set_boot_device(task, 'disk', persistent=True) # Remove symbolic link when deploy is done. - if CONF.agent.image_download_source == 'http': - deploy_utils.remove_http_instance_symlink(task.node.uuid) + deploy_utils.remove_http_instance_symlink(task.node.uuid) class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, @@ -442,7 +445,10 @@ class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, deploy_utils.check_for_missing_params(params, error_msg) - if not service_utils.is_glance_image(image_source): + # NOTE(dtantsur): glance images contain a checksum; for file images we + # will recalculate the checksum anyway. + if (not service_utils.is_glance_image(image_source) + and not image_source.startswith('file://')): def _raise_missing_checksum_exception(node): raise exception.MissingParameterValue(_( @@ -629,8 +635,7 @@ class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, :param task: a TaskManager instance. """ super(AgentDeploy, self).clean_up(task) - if CONF.agent.image_download_source == 'http': - deploy_utils.destroy_http_instance_images(task.node) + deploy_utils.destroy_http_instance_images(task.node) class AgentRAID(base.RAIDInterface): diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 648a38d0bd..3b68f85713 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -30,6 +30,7 @@ from ironic.common import faults from ironic.common.glance_service import service_utils from ironic.common.i18n import _ from ironic.common import image_service +from ironic.common import images from ironic.common import keystone from ironic.common import states from ironic.common import utils @@ -966,6 +967,85 @@ def destroy_http_instance_images(node): destroy_images(node.uuid) +def _validate_image_url(node, url, secret=False): + """Validates image URL through the HEAD request. + + :param url: URL to be validated + :param secret: if URL is secret (e.g. swift temp url), + it will not be shown in logs. + """ + try: + image_service.HttpImageService().validate_href(url, secret) + except exception.ImageRefValidationFailed as e: + with excutils.save_and_reraise_exception(): + LOG.error("The specified URL is not a valid HTTP(S) URL or is " + "not reachable for node %(node)s. Error: %(msg)s", + {'node': node.uuid, 'msg': e}) + + +def _cache_and_convert_image(task, instance_info, image_info=None): + """Cache an image locally and covert it to RAW if needed.""" + # Ironic cache and serve images from httpboot server + force_raw = direct_deploy_should_convert_raw_image(task.node) + _, image_path = cache_instance_image(task.context, task.node, + force_raw=force_raw) + if force_raw or image_info is None: + if force_raw: + instance_info['image_disk_format'] = 'raw' + else: + LOG.debug('Detecting image format for the locally cached image ' + '%(image)s for node %(node)s', + {'image': image_path, 'node': task.node.uuid}) + instance_info['image_disk_format'] = \ + images.get_source_format(instance_info['image_source'], + image_path) + + # Standard behavior is for image_checksum to be MD5, + # so if the hash algorithm is None, then we will use + # sha256. + if image_info is None: + os_hash_algo = instance_info.get('image_os_hash_algo') + else: + os_hash_algo = image_info.get('os_hash_algo') + + if not os_hash_algo or os_hash_algo == 'md5': + LOG.debug("Checksum algorithm for image %(image)s for node " + "%(node)s is set to '%(algo)s', changing to 'sha256'", + {'algo': os_hash_algo, 'node': task.node.uuid, + 'image': image_path}) + os_hash_algo = 'sha256' + + LOG.debug('Recalculating checksum for image %(image)s for node ' + '%(node)s due to image conversion', + {'image': image_path, 'node': task.node.uuid}) + instance_info['image_checksum'] = None + hash_value = compute_image_checksum(image_path, os_hash_algo) + instance_info['image_os_hash_algo'] = os_hash_algo + instance_info['image_os_hash_value'] = hash_value + else: + instance_info['image_checksum'] = image_info['checksum'] + instance_info['image_disk_format'] = image_info['disk_format'] + instance_info['image_os_hash_algo'] = image_info[ + 'os_hash_algo'] + instance_info['image_os_hash_value'] = image_info[ + 'os_hash_value'] + + # Create symlink and update image url + symlink_dir = _get_http_image_symlink_dir_path() + fileutils.ensure_tree(symlink_dir) + symlink_path = _get_http_image_symlink_file_path(task.node.uuid) + utils.create_link_without_raise(image_path, symlink_path) + + base_url = CONF.deploy.http_url + if base_url.endswith('/'): + base_url = base_url[:-1] + http_image_url = '/'.join( + [base_url, CONF.deploy.http_image_subdir, + task.node.uuid]) + _validate_image_url(task.node, http_image_url, secret=False) + instance_info['image_url'] = http_image_url + + @METRICS.timer('build_instance_info_for_deploy') def build_instance_info_for_deploy(task): """Build instance_info necessary for deploying to a node. @@ -976,23 +1056,6 @@ def build_instance_info_for_deploy(task): :raises: exception.ImageRefValidationFailed if image_source is not Glance href and is not HTTP(S) URL. """ - def validate_image_url(url, secret=False): - """Validates image URL through the HEAD request. - - :param url: URL to be validated - :param secret: if URL is secret (e.g. swift temp url), - it will not be shown in logs. - """ - try: - image_service.HttpImageService().validate_href(url, secret) - except exception.ImageRefValidationFailed as e: - with excutils.save_and_reraise_exception(): - LOG.error("Agent deploy supports only HTTP(S) URLs as " - "instance_info['image_source'] or swift " - "temporary URL. Either the specified URL is not " - "a valid HTTP(S) URL or is not reachable " - "for node %(node)s. Error: %(msg)s", - {'node': node.uuid, 'msg': e}) node = task.node instance_info = node.instance_info iwdi = node.driver_internal_info.get('is_whole_disk_image') @@ -1005,56 +1068,14 @@ def build_instance_info_for_deploy(task): {'info': image_info, 'node': node.uuid}) if CONF.agent.image_download_source == 'swift': swift_temp_url = glance.swift_temp_url(image_info) - validate_image_url(swift_temp_url, secret=True) + _validate_image_url(node, swift_temp_url, secret=True) instance_info['image_url'] = swift_temp_url instance_info['image_checksum'] = image_info['checksum'] instance_info['image_disk_format'] = image_info['disk_format'] instance_info['image_os_hash_algo'] = image_info['os_hash_algo'] instance_info['image_os_hash_value'] = image_info['os_hash_value'] else: - # Ironic cache and serve images from httpboot server - force_raw = direct_deploy_should_convert_raw_image(node) - _, image_path = cache_instance_image(task.context, node, - force_raw=force_raw) - if force_raw: - instance_info['image_disk_format'] = 'raw' - # Standard behavior is for image_checksum to be MD5, - # so if the hash algorithm is None, then we will use - # sha256. - os_hash_algo = image_info.get('os_hash_algo') - if os_hash_algo == 'md5': - LOG.debug('Checksum calculation for image %(image)s is ' - 'set to \'%(algo)s\', changing to \'sha256\'', - {'algo': os_hash_algo, - 'image': image_path}) - os_hash_algo = 'sha256' - LOG.debug('Recalculating checksum for image %(image)s due to ' - 'image conversion.', {'image': image_path}) - instance_info['image_checksum'] = None - hash_value = compute_image_checksum(image_path, os_hash_algo) - instance_info['image_os_hash_algo'] = os_hash_algo - instance_info['image_os_hash_value'] = hash_value - else: - instance_info['image_checksum'] = image_info['checksum'] - instance_info['image_disk_format'] = image_info['disk_format'] - instance_info['image_os_hash_algo'] = image_info[ - 'os_hash_algo'] - instance_info['image_os_hash_value'] = image_info[ - 'os_hash_value'] - - # Create symlink and update image url - symlink_dir = _get_http_image_symlink_dir_path() - fileutils.ensure_tree(symlink_dir) - symlink_path = _get_http_image_symlink_file_path(node.uuid) - utils.create_link_without_raise(image_path, symlink_path) - base_url = CONF.deploy.http_url - if base_url.endswith('/'): - base_url = base_url[:-1] - http_image_url = '/'.join( - [base_url, CONF.deploy.http_image_subdir, - node.uuid]) - validate_image_url(http_image_url, secret=True) - instance_info['image_url'] = http_image_url + _cache_and_convert_image(task, instance_info, image_info) instance_info['image_container_format'] = ( image_info['container_format']) @@ -1064,8 +1085,10 @@ def build_instance_info_for_deploy(task): if not iwdi: instance_info['kernel'] = image_info['properties']['kernel_id'] instance_info['ramdisk'] = image_info['properties']['ramdisk_id'] + elif image_source.startswith('file://'): + _cache_and_convert_image(task, instance_info) else: - validate_image_url(image_source) + _validate_image_url(node, image_source) instance_info['image_url'] = image_source if not iwdi: diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 926f665240..39baa2bdaf 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -166,7 +166,10 @@ class TestAgentMethods(db_base.DbTestCase): show_mock.assert_called_once_with(self.context, 'fake-image') @mock.patch.object(deploy_utils, 'check_for_missing_params', autospec=True) - def test_validate_http_provisioning_not_glance(self, utils_mock): + def test_validate_http_provisioning_http_image(self, utils_mock): + i_info = self.node.instance_info + i_info['image_source'] = 'http://image-ref' + self.node.instance_info = i_info agent.validate_http_provisioning_configuration(self.node) utils_mock.assert_not_called() @@ -189,6 +192,16 @@ class TestAgentMethods(db_base.DbTestCase): agent.validate_http_provisioning_configuration, self.node) + def test_validate_http_provisioning_missing_args_file(self): + CONF.set_override('http_url', None, group='deploy') + i_info = self.node.instance_info + i_info['image_source'] = 'file://image-ref' + self.node.instance_info = i_info + self.assertRaisesRegex(exception.MissingParameterValue, + 'failed to validate http provisoning', + agent.validate_http_provisioning_configuration, + self.node) + class TestAgentDeploy(db_base.DbTestCase): def setUp(self): @@ -211,6 +224,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.ports = [ object_utils.create_test_port(self.context, node_id=self.node.id)] dhcp_factory.DHCPFactory._dhcp_provider = None + CONF.set_override('http_url', 'http://example.com', group='deploy') def test_get_properties(self): expected = agent.COMMON_PROPERTIES @@ -353,6 +367,24 @@ class TestAgentDeploy(db_base.DbTestCase): show_mock.assert_called_once_with(self.context, 'http://image-ref') + @mock.patch.object(image_service.FileImageService, 'validate_href', + autospec=True) + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_file_image_no_checksum( + self, pxe_boot_validate_mock, validate_mock): + i_info = self.node.instance_info + i_info['image_source'] = 'file://image-ref' + del i_info['image_checksum'] + self.node.instance_info = i_info + self.node.save() + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.driver.validate(task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + validate_mock.assert_called_once_with(mock.ANY, 'file://image-ref') + @mock.patch.object(agent, 'validate_http_provisioning_configuration', autospec=True) @mock.patch.object(images, 'image_show', autospec=True) @@ -1045,11 +1077,14 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertFalse(build_options_mock.called) self.assertFalse(pxe_prepare_ramdisk_mock.called) + @mock.patch.object(deploy_utils, 'destroy_http_instance_images', + autospec=True) @mock.patch('ironic.common.dhcp_factory.DHCPFactory', autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) def test_clean_up(self, pxe_clean_up_ramdisk_mock, - pxe_clean_up_instance_mock, dhcp_factor_mock): + pxe_clean_up_instance_mock, dhcp_factor_mock, + destroy_images_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.driver.clean_up(task) @@ -1058,13 +1093,17 @@ class TestAgentDeploy(db_base.DbTestCase): pxe_clean_up_instance_mock.assert_called_once_with( task.driver.boot, task) dhcp_factor_mock.assert_called_once_with() + destroy_images_mock.assert_called_once_with(task.node) + @mock.patch.object(deploy_utils, 'destroy_http_instance_images', + autospec=True) @mock.patch('ironic.common.dhcp_factory.DHCPFactory', autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_instance', autospec=True) @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) def test_clean_up_manage_agent_boot_false(self, pxe_clean_up_ramdisk_mock, pxe_clean_up_instance_mock, - dhcp_factor_mock): + dhcp_factor_mock, + destroy_images_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.config(group='agent', manage_agent_boot=False) @@ -1073,6 +1112,7 @@ class TestAgentDeploy(db_base.DbTestCase): pxe_clean_up_instance_mock.assert_called_once_with( task.driver.boot, task) dhcp_factor_mock.assert_called_once_with() + destroy_images_mock.assert_called_once_with(task.node) @mock.patch.object(agent_base, 'get_steps', autospec=True) def test_get_clean_steps(self, mock_get_steps): diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 0307fd8d24..e8b3088e2b 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1921,12 +1921,12 @@ class TestBuildInstanceInfoForDeploy(db_base.DbTestCase): @mock.patch.object(image_service.HttpImageService, 'validate_href', autospec=True) - def test_build_instance_info_for_deploy_nonsupported_image( + def test_build_instance_info_for_deploy_image_not_found( self, validate_href_mock): validate_href_mock.side_effect = exception.ImageRefValidationFailed( - image_href='file://img.qcow2', reason='fail') + image_href='http://img.qcow2', reason='fail') i_info = self.node.instance_info - i_info['image_source'] = 'file://img.qcow2' + i_info['image_source'] = 'http://img.qcow2' i_info['image_checksum'] = 'aa' self.node.instance_info = i_info self.node.save() @@ -1958,9 +1958,11 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): self.checksum_mock.return_value = 'fake-checksum' self.cache_image_mock = self.useFixture(fixtures.MockPatchObject( utils, 'cache_instance_image', autospec=True)).mock + self.fake_path = '/var/lib/ironic/images/{}/disk'.format( + self.node.uuid) self.cache_image_mock.return_value = ( '733d1c44-a2ea-414b-aca7-69decf20d810', - '/var/lib/ironic/images/{}/disk'.format(self.node.uuid)) + self.fake_path) self.ensure_tree_mock = self.useFixture(fixtures.MockPatchObject( utils.fileutils, 'ensure_tree', autospec=True)).mock self.create_link_mock = self.useFixture(fixtures.MockPatchObject( @@ -2003,7 +2005,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): self.create_link_mock.assert_called_once_with(image_path, symlink_file) validate_mock.assert_called_once_with(mock.ANY, self.expected_url, - secret=True) + secret=False) return image_path, instance_info def test_build_instance_info_no_force_raw(self): @@ -2039,6 +2041,38 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase): calls = [mock.call(image_path, algorithm='sha256')] self.checksum_mock.assert_has_calls(calls) + @mock.patch.object(image_service.HttpImageService, 'validate_href', + autospec=True) + def test_build_instance_info_file_image(self, validate_href_mock): + i_info = self.node.instance_info + driver_internal_info = self.node.driver_internal_info + i_info['image_source'] = 'file://image-ref' + i_info['image_checksum'] = 'aa' + i_info['root_gb'] = 10 + i_info['image_checksum'] = 'aa' + driver_internal_info['is_whole_disk_image'] = True + self.node.instance_info = i_info + self.node.driver_internal_info = driver_internal_info + self.node.save() + + expected_url = ( + 'http://172.172.24.10:8080/agent_images/%s' % self.node.uuid) + + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + + info = utils.build_instance_info_for_deploy(task) + + self.assertEqual(expected_url, info['image_url']) + self.assertEqual('sha256', info['image_os_hash_algo']) + self.assertEqual('fake-checksum', info['image_os_hash_value']) + self.cache_image_mock.assert_called_once_with( + task.context, task.node, force_raw=True) + self.checksum_mock.assert_called_once_with( + self.fake_path, algorithm='sha256') + validate_href_mock.assert_called_once_with( + mock.ANY, expected_url, False) + class TestStorageInterfaceUtils(db_base.DbTestCase): def setUp(self): diff --git a/releasenotes/notes/direct-file-6f80728d76093530.yaml b/releasenotes/notes/direct-file-6f80728d76093530.yaml new file mode 100644 index 0000000000..0571560d2e --- /dev/null +++ b/releasenotes/notes/direct-file-6f80728d76093530.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + ``file://`` images are now supported in the ``direct`` deploy interface.