openstack/ironic
Code Issues Proposed changes

Make image_checksum optional if other checksum is present

Browse Source 
In a previous change, we discovered that our validation
logic still required an image_checksum value to be present.
Since support was added some time ago for improved image
checksum validation, we no longer need to explicitly require
image_checksum be present.

Change-Id: Iec2fb7139f098f2782c440808d71b00a71785c5c
This commit is contained in:
Julia Kreger 2020-03-07 07:16:17 -08:00
parent 06c5997267
commit fbf92e6126
5 changed files with 104 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
ironic/drivers/modules/agent.py
View File

@ -208,7 +208,6 @@ class AgentDeployMixin(agent_base_vendor.AgentDeployMixin):
image_info = {
'id': image_source.split('/')[-1],
'urls': [node.instance_info['image_url']],
'checksum': node.instance_info['image_checksum'],
# NOTE(comstud): Older versions of ironic do not set
# 'disk_format' nor 'container_format', so we use .get()
# to maintain backwards compatibility in case code was
@ -219,6 +218,9 @@ class AgentDeployMixin(agent_base_vendor.AgentDeployMixin):
'stream_raw_images': CONF.agent.stream_raw_images,
}
if node.instance_info.get('image_checksum'):
image_info['checksum'] = node.instance_info['image_checksum']
if (node.instance_info.get('image_os_hash_algo') and
node.instance_info.get('image_os_hash_value')):
image_info['os_hash_algo'] = node.instance_info[
@ -414,6 +416,10 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
params = {}
image_source = node.instance_info.get('image_source')
image_checksum = node.instance_info.get('image_checksum')
os_hash_algo = node.instance_info.get('image_os_hash_algo')
os_hash_value = node.instance_info.get('image_os_hash_value')
params['instance_info.image_source'] = image_source
error_msg = _('Node %s failed to validate deploy image info. Some '
'parameters were missing') % node.uuid
@ -421,10 +427,25 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
deploy_utils.check_for_missing_params(params, error_msg)
if not service_utils.is_glance_image(image_source):
if not node.instance_info.get('image_checksum'):
def _raise_missing_checksum_exception(node):
raise exception.MissingParameterValue(_(
"image_source's image_checksum must be provided in "
"instance_info for node %s") % node.uuid)
'image_source\'s "image_checksum", or '
'"image_os_hash_algo" and "image_os_hash_value" '
'must be provided in instance_info for '
'node %s') % node.uuid)
if os_hash_value and not os_hash_algo:
# We are missing a piece of information,
# so we still need to raise an error.
_raise_missing_checksum_exception(node)
elif not os_hash_value and os_hash_algo:
# We have the hash setting, but not the hash.
_raise_missing_checksum_exception(node)
elif not os_hash_value and not image_checksum:
# We are lacking the original image_checksum,
# so we raise the error.
_raise_missing_checksum_exception(node)
validate_http_provisioning_configuration(node)

2
ironic/drivers/modules/deploy_utils.py
View File

@ -1312,7 +1312,7 @@ def build_instance_info_for_deploy(task):
os_hash_algo = 'sha256'
LOG.debug('Recalculating checksum for image %(image)s due to '
'image conversion.', {'image': image_path})
instance_info['image_checksum'] = 'md5-not-supported'
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

69
ironic/tests/unit/drivers/modules/test_agent.py
View File

@ -257,6 +257,75 @@ class TestAgentDeploy(db_base.DbTestCase):
pxe_boot_validate_mock.assert_called_once_with(
task.driver.boot, task)
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
def test_validate_nonglance_image_no_checksum_os_algo(
self, pxe_boot_validate_mock):
i_info = self.node.instance_info
i_info['image_source'] = 'http://image-ref'
i_info['image_os_hash_value'] = 'az'
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.assertRaises(exception.MissingParameterValue,
self.driver.validate, task)
pxe_boot_validate_mock.assert_called_once_with(
task.driver.boot, task)
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
def test_validate_nonglance_image_no_os_image_hash(
self, pxe_boot_validate_mock, autospec=True):
i_info = self.node.instance_info
i_info['image_source'] = 'http://image-ref'
i_info['image_os_hash_algo'] = 'magicalgo'
self.node.instance_info = i_info
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.assertRaises(exception.MissingParameterValue,
self.driver.validate, task)
pxe_boot_validate_mock.assert_called_once_with(
task.driver.boot, task)
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
def test_validate_nonglance_image_no_os_algo(
self, pxe_boot_validate_mock):
i_info = self.node.instance_info
i_info['image_source'] = 'http://image-ref'
i_info['image_os_hash_value'] = 'az'
self.node.instance_info = i_info
self.node.save()
with task_manager.acquire(
self.context, self.node.uuid, shared=False) as task:
self.assertRaises(exception.MissingParameterValue,
self.driver.validate, task)
pxe_boot_validate_mock.assert_called_once_with(
task.driver.boot, task)
@mock.patch.object(images, 'image_show', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
def test_validate_nonglance_image_no_os_checksum(
self, pxe_boot_validate_mock, show_mock):
i_info = self.node.instance_info
i_info['image_source'] = 'http://image-ref'
del i_info['image_checksum']
i_info['image_os_hash_algo'] = 'whacky-algo-1'
i_info['image_os_hash_value'] = '1234567890'
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)
show_mock.assert_called_once_with(self.context,
'http://image-ref')
@mock.patch.object(agent, 'validate_http_provisioning_configuration',
autospec=True)
@mock.patch.object(images, 'image_show', autospec=True)

4
ironic/tests/unit/drivers/modules/test_deploy_utils.py
View File

@ -2641,7 +2641,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
image_path, instance_info = self._test_build_instance_info(
image_info=self.image_info, expect_raw=True)
self.assertEqual('md5-not-supported', instance_info['image_checksum'])
self.assertIsNone(instance_info['image_checksum'])
self.assertEqual(instance_info['image_disk_format'], 'raw')
calls = [mock.call(image_path, algorithm='sha512')]
self.checksum_mock.assert_has_calls(calls)
@ -2652,7 +2652,7 @@ class TestBuildInstanceInfoForHttpProvisioning(db_base.DbTestCase):
image_path, instance_info = self._test_build_instance_info(
image_info=self.image_info, expect_raw=True)
self.assertEqual('md5-not-supported', instance_info['image_checksum'])
self.assertIsNone(instance_info['image_checksum'])
self.assertEqual(instance_info['image_disk_format'], 'raw')
calls = [mock.call(image_path, algorithm='sha256')]
self.checksum_mock.assert_has_calls(calls)

7
releasenotes/notes/image_checksum_optional-381acf9e441d2a58.yaml Normal file
View File

@ -0,0 +1,7 @@
---
features:
- |
Adds the cability for the ``instance_info\image_checksum`` value
to be optional in stand-alone deployments if the
``instance_info\image_os_hash_algo`` and
``instance_info\image_os_hash_value`` fields are populated.