openstack/ironic
Code Issues Proposed changes

Merge "Stop requiring root_gb for whole-disk images"

Browse Source
This commit is contained in:
Zuul 2019-03-11 16:17:31 +00:00 committed by Gerrit Code Review
commit 68afa6d8e6
6 changed files with 98 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/install/standalone.rst
View File

@ -117,10 +117,12 @@ Steps to start a deployment are pretty similar to those when using Compute:
while :ref:`iscsi-deploy` also accepts links to local files (prefixed
with ``file://``).
* ``root_gb`` - size of the root partition, mandatory.
* ``root_gb`` - size of the root partition, required for partition images.
.. TODO(dtantsur): root_gb should not be mandatory for whole disk images,
but it seems to be.
.. note::
Older versions of the Bare Metal service used to require a positive
integer for ``root_gb`` even for whole-disk images. You may want to set
it for compatibility.
* ``image_checksum`` - MD5 checksum of the image specified by
``image_source``, only required for :ref:`direct-deploy`.

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

@ -978,6 +978,10 @@ def get_image_instance_info(node):
return info
_ERR_MSG_INVALID_DEPLOY = _("Cannot validate parameter for driver deploy. "
"Invalid parameter %(param)s. Reason: %(reason)s")
def parse_instance_info(node):
"""Gets the instance specific Node deployment info.
@ -1003,57 +1007,64 @@ def parse_instance_info(node):
i_info['image_source'])):
i_info['kernel'] = info.get('kernel')
i_info['ramdisk'] = info.get('ramdisk')
i_info['root_gb'] = info.get('root_gb')
i_info['root_gb'] = info.get('root_gb')
error_msg = _("Cannot validate driver deploy. Some parameters were missing"
" in node's instance_info")
check_for_missing_params(i_info, error_msg)
# NOTE(vdrok): We're casting disk layout parameters to int only after
# ensuring that it is possible
i_info['swap_mb'] = info.get('swap_mb', 0)
i_info['ephemeral_gb'] = info.get('ephemeral_gb', 0)
err_msg_invalid = _("Cannot validate parameter for driver deploy. "
"Invalid parameter %(param)s. Reason: %(reason)s")
for param in DISK_LAYOUT_PARAMS:
try:
int(i_info[param])
except ValueError:
reason = _("%s is not an integer value.") % i_info[param]
raise exception.InvalidParameterValue(err_msg_invalid %
{'param': param,
'reason': reason})
i_info['root_mb'] = 1024 * int(i_info['root_gb'])
i_info['swap_mb'] = int(i_info['swap_mb'])
i_info['ephemeral_mb'] = 1024 * int(i_info['ephemeral_gb'])
if iwdi:
if i_info['swap_mb'] > 0 or i_info['ephemeral_mb'] > 0:
err_msg_invalid = _("Cannot deploy whole disk image with "
"swap or ephemeral size set")
raise exception.InvalidParameterValue(err_msg_invalid)
i_info['ephemeral_format'] = info.get('ephemeral_format')
i_info['configdrive'] = info.get('configdrive')
if i_info['ephemeral_gb'] and not i_info['ephemeral_format']:
i_info['ephemeral_format'] = CONF.pxe.default_ephemeral_format
# This is used in many places, so keep it even for whole-disk images.
# There is also a potential use case of creating an ephemeral partition via
# cloud-init and telling ironic to avoid metadata wipe via setting
# preserve_ephemeral (not saying it will work, but it seems possible).
preserve_ephemeral = info.get('preserve_ephemeral', False)
try:
i_info['preserve_ephemeral'] = (
strutils.bool_from_string(preserve_ephemeral, strict=True))
except ValueError as e:
raise exception.InvalidParameterValue(
err_msg_invalid % {'param': 'preserve_ephemeral', 'reason': e})
_ERR_MSG_INVALID_DEPLOY % {'param': 'preserve_ephemeral',
'reason': e})
if iwdi:
if i_info.get('swap_mb') or i_info.get('ephemeral_mb'):
err_msg_invalid = _("Cannot deploy whole disk image with "
"swap or ephemeral size set")
raise exception.InvalidParameterValue(err_msg_invalid)
else:
_validate_layout_properties(node, info, i_info)
i_info['configdrive'] = info.get('configdrive')
return i_info
def _validate_layout_properties(node, info, i_info):
i_info['swap_mb'] = info.get('swap_mb', 0)
i_info['ephemeral_gb'] = info.get('ephemeral_gb', 0)
# NOTE(vdrok): We're casting disk layout parameters to int only after
# ensuring that it is possible
for param in DISK_LAYOUT_PARAMS:
try:
int(i_info[param])
except ValueError:
reason = _("%s is not an integer value.") % i_info[param]
raise exception.InvalidParameterValue(_ERR_MSG_INVALID_DEPLOY %
{'param': param,
'reason': reason})
i_info['root_mb'] = 1024 * int(i_info['root_gb'])
i_info['swap_mb'] = int(i_info['swap_mb'])
i_info['ephemeral_mb'] = 1024 * int(i_info['ephemeral_gb'])
i_info['ephemeral_format'] = info.get('ephemeral_format')
if i_info['ephemeral_gb'] and not i_info['ephemeral_format']:
i_info['ephemeral_format'] = CONF.pxe.default_ephemeral_format
# NOTE(Zhenguo): If rebuilding with preserve_ephemeral option, check
# that the disk layout is unchanged.
if i_info['preserve_ephemeral']:
_check_disk_layout_unchanged(node, i_info)
return i_info
def _check_disk_layout_unchanged(node, i_info):
"""Check whether disk layout is unchanged.

3
ironic/tests/unit/drivers/modules/ansible/test_deploy.py
View File

@ -806,7 +806,10 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase):
prepare_vars_mock.return_value = _vars
driver_internal_info = self.node.driver_internal_info
driver_internal_info['is_whole_disk_image'] = True
instance_info = self.node.instance_info
del instance_info['root_mb']
self.node.driver_internal_info = driver_internal_info
self.node.instance_info = instance_info
self.node.extra = {'ham': 'spam'}
self.node.save()

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

@ -2268,17 +2268,26 @@ class InstanceInfoTestCase(db_base.DbTestCase):
)
instance_info = utils.parse_instance_info(node)
self.assertIsNotNone(instance_info['image_source'])
self.assertIsNotNone(instance_info['root_mb'])
self.assertEqual(0, instance_info['swap_mb'])
self.assertEqual(0, instance_info['ephemeral_mb'])
self.assertNotIn('root_mb', instance_info)
self.assertNotIn('ephemeral_mb', instance_info)
self.assertNotIn('swap_mb', instance_info)
self.assertIsNone(instance_info['configdrive'])
def test_parse_instance_info_whole_disk_image_missing_root(self):
driver_internal_info = dict(DRV_INTERNAL_INFO_DICT)
driver_internal_info['is_whole_disk_image'] = True
info = dict(INST_INFO_DICT)
del info['root_gb']
node = obj_utils.create_test_node(self.context, instance_info=info)
self.assertRaises(exception.InvalidParameterValue,
utils.parse_instance_info, node)
node = obj_utils.create_test_node(
self.context, instance_info=info,
driver_internal_info=driver_internal_info
)
instance_info = utils.parse_instance_info(node)
self.assertIsNotNone(instance_info['image_source'])
self.assertNotIn('root_mb', instance_info)
self.assertNotIn('ephemeral_mb', instance_info)
self.assertNotIn('swap_mb', instance_info)
class TestBuildInstanceInfoForDeploy(db_base.DbTestCase):

24
ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
View File

@ -128,6 +128,18 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
iscsi_deploy.check_image_size(task)
self.assertFalse(get_image_mb_mock.called)
@mock.patch.object(disk_utils, 'get_image_mb', autospec=True)
def test_check_image_size_whole_disk_image_no_root(self,
get_image_mb_mock):
get_image_mb_mock.return_value = 1025
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
del task.node.instance_info['root_gb']
task.node.driver_internal_info['is_whole_disk_image'] = True
# No error for whole disk images
iscsi_deploy.check_image_size(task)
self.assertFalse(get_image_mb_mock.called)
@mock.patch.object(disk_utils, 'get_image_mb', autospec=True)
def test_check_image_size_fails(self, get_image_mb_mock):
get_image_mb_mock.return_value = 1025
@ -452,6 +464,18 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase):
self.assertEqual('target-iqn', ret_val['iqn'])
self.assertEqual('My configdrive', ret_val['configdrive'])
def test_get_deploy_info_whole_disk_image_no_root(self):
instance_info = self.node.instance_info
instance_info['configdrive'] = 'My configdrive'
del instance_info['root_gb']
self.node.instance_info = instance_info
self.node.driver_internal_info['is_whole_disk_image'] = True
kwargs = {'address': '1.1.1.1', 'iqn': 'target-iqn'}
ret_val = iscsi_deploy.get_deploy_info(self.node, **kwargs)
self.assertEqual('1.1.1.1', ret_val['address'])
self.assertEqual('target-iqn', ret_val['iqn'])
self.assertEqual('My configdrive', ret_val['configdrive'])
@mock.patch.object(iscsi_deploy, 'continue_deploy', autospec=True)
def test_do_agent_iscsi_deploy_okay(self, continue_deploy_mock):
agent_client_mock = mock.MagicMock(spec_set=agent_client.AgentClient)

6
releasenotes/notes/whole-disk-root-gb-9132e5a354e6cb9d.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
The ``instance_info[root_gb]`` property is no longer required for
whole-disk images. It has always been ignored for them, but the validation
code still expected it to be present.