diff options
author | Dmitry Tantsur <dtantsur@protonmail.com> | 2021-05-10 17:01:45 +0200 |
---|---|---|
committer | Dmitry Tantsur <dtantsur@protonmail.com> | 2021-06-02 11:19:39 +0200 |
commit | f65752680700fc8df8877620b2354cb06ea54fd3 (patch) | |
tree | ba5d755dc0a5b3836d8e2efceee4d431b6c0c016 | |
parent | 6fb4cec7aa0a3edc76b0e07cc9e7cc0b1d4a4e99 (diff) | |
download | ironic-python-agent-f65752680700fc8df8877620b2354cb06ea54fd3.tar.gz |
Stop accepting duplicated configdrive
We're currently requiring it twice: in image_info and in a separate
configdrive argument. I think we should eventually settle on separate
arguments for separate entities, so this change makes the value in
image_info optional with a goal to stop accepting it.
We could probably just remove the handling in image_info, but a
deprecation is safer.
The (unused in ironic) cache_image call is updated with an optional
configdrive arguments.
Story: #2008904
Task: #42480
Change-Id: I1e2efa28efa3ea7e389774cb7633d916757bc6ed
-rw-r--r-- | ironic_python_agent/extensions/standby.py | 48 | ||||
-rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_standby.py | 62 | ||||
-rw-r--r-- | releasenotes/notes/configdrive-dup-3fc46a878fe82485.yaml | 6 |
3 files changed, 77 insertions, 39 deletions
diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index ab1414df..a04a229a 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -139,7 +139,7 @@ def _fetch_checksum(checksum, image_info): checksum, "Checksum file does not contain name %s" % expected_fname) -def _write_partition_image(image, image_info, device): +def _write_partition_image(image, image_info, device, configdrive=None): """Call disk_util to create partition and write the partition image. :param image: Local path to image file to be written to the partition. @@ -147,6 +147,9 @@ def _write_partition_image(image, image_info, device): :param image_info: Image information dictionary. :param device: The device name, as a string, on which to store the image. Example: '/dev/sda' + :param configdrive: A string containing the location of the config + drive as a URL OR the contents (as gzip/base64) + of the configdrive. Optional, defaults to None. :raises: InvalidCommandParamsError if the partition is too small for the provided image. @@ -159,7 +162,6 @@ def _write_partition_image(image, image_info, device): node_uuid = image_info.get('node_uuid') preserve_ep = image_info['preserve_ephemeral'] - configdrive = image_info['configdrive'] boot_option = image_info.get('boot_option', 'local') boot_mode = utils.get_node_boot_mode(cached_node) disk_label = utils.get_partition_table_type_from_specs(cached_node) @@ -212,12 +214,15 @@ def _write_whole_disk_image(image, image_info, device): raise errors.ImageWriteError(device, e.exit_code, e.stdout, e.stderr) -def _write_image(image_info, device): +def _write_image(image_info, device, configdrive=None): """Writes an image to the specified device. :param image_info: Image information dictionary. :param device: The disk name, as a string, on which to store the image. Example: '/dev/sda' + :param configdrive: A string containing the location of the config + drive as a URL OR the contents (as gzip/base64) + of the configdrive. Optional, defaults to None. :raises: ImageWriteError if the command to write the image encounters an error. """ @@ -225,7 +230,7 @@ def _write_image(image_info, device): image = _image_location(image_info) uuids = {} if image_info.get('image_type') == 'partition': - uuids = _write_partition_image(image, image_info, device) + uuids = _write_partition_image(image, image_info, device, configdrive) else: _write_whole_disk_image(image, image_info, device) totaltime = time.time() - starttime @@ -536,12 +541,15 @@ class StandbyExtension(base.BaseAgentExtension): self.cached_image_id = None self.partition_uuids = None - def _cache_and_write_image(self, image_info, device): + def _cache_and_write_image(self, image_info, device, configdrive=None): """Cache an image and write it to a local device. :param image_info: Image information dictionary. :param device: The disk name, as a string, on which to store the image. Example: '/dev/sda' + :param configdrive: A string containing the location of the config + drive as a URL OR the contents (as gzip/base64) + of the configdrive. Optional, defaults to None. :raises: ImageDownloadError if the image download fails for any reason. :raises: ImageChecksumError if the downloaded image's checksum does not @@ -549,7 +557,7 @@ class StandbyExtension(base.BaseAgentExtension): :raises: ImageWriteError if writing the image fails. """ _download_image(image_info) - self.partition_uuids = _write_image(image_info, device) + self.partition_uuids = _write_image(image_info, device, configdrive) self.cached_image_id = image_info['id'] def _stream_raw_image_onto_device(self, image_info, device): @@ -623,13 +631,16 @@ class StandbyExtension(base.BaseAgentExtension): self.partition_uuids['root uuid'] = root_uuid @base.async_command('cache_image', _validate_image_info) - def cache_image(self, image_info=None, force=False): + def cache_image(self, image_info, force=False, configdrive=None): """Asynchronously caches specified image to the local OS device. :param image_info: Image information dictionary. :param force: Optional. If True forces cache_image to download and cache image, even if the same image already exists on the local OS install device. Defaults to False. + :param configdrive: A string containing the location of the config + drive as a URL OR the contents (as gzip/base64) + of the configdrive. Optional, defaults to None. :raises: ImageDownloadError if the image download fails for any reason. :raises: ImageChecksumError if the downloaded image's checksum does not @@ -645,7 +656,10 @@ class StandbyExtension(base.BaseAgentExtension): if self.cached_image_id != image_info['id'] or force: LOG.debug('Already had %s cached, overwriting', self.cached_image_id) - self._cache_and_write_image(image_info, device) + # NOTE(dtantsur): backward compatibility + if configdrive is None: + configdrive = image_info.pop('configdrive', None) + self._cache_and_write_image(image_info, device, configdrive) msg = 'image ({}) cached to device {} ' self._fix_up_partition_uuids(image_info, device) @@ -656,9 +670,7 @@ class StandbyExtension(base.BaseAgentExtension): return result_msg @base.async_command('prepare_image', _validate_image_info) - def prepare_image(self, - image_info=None, - configdrive=None): + def prepare_image(self, image_info, configdrive=None): """Asynchronously prepares specified image on local OS install device. In this case, 'prepare' means make local machine completely ready to @@ -680,6 +692,9 @@ class StandbyExtension(base.BaseAgentExtension): large to store on the given device. """ LOG.debug('Preparing image %s', image_info['id']) + # NOTE(dtantsur): backward compatibility + if configdrive is None: + configdrive = image_info.pop('configdrive', None) device = hardware.dispatch_to_managers('get_os_install_device', permit_refresh=True) @@ -695,7 +710,8 @@ class StandbyExtension(base.BaseAgentExtension): if image_info.get('image_type') == 'partition': self.partition_uuids = _write_partition_image(None, image_info, - device) + device, + configdrive) stream_to = self.partition_uuids['partitions']['root'] else: self.partition_uuids = {} @@ -703,12 +719,14 @@ class StandbyExtension(base.BaseAgentExtension): self._stream_raw_image_onto_device(image_info, stream_to) else: - self._cache_and_write_image(image_info, device) + self._cache_and_write_image(image_info, device, configdrive) _validate_partitioning(device) - # the configdrive creation is taken care by ironic-lib's - # work_on_disk(). + # For partition images the configdrive creation is taken care by + # partition_utils.work_on_disk(), invoked from either + # _write_partition_image or _cache_and_write_image above. + # Handle whole disk images explicitly now. if image_info.get('image_type') != 'partition': if configdrive is not None: # Will use dummy value of 'local' for 'node_uuid', diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index 1f985652..ba58b29f 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -52,7 +52,6 @@ def _build_fake_partition_image_info(): 'ephemeral_mb': '10', 'ephemeral_format': 'abc', 'preserve_ephemeral': 'False', - 'configdrive': 'configdrive', 'image_type': 'partition', 'boot_option': 'netboot', 'disk_label': 'msdos', @@ -214,7 +213,6 @@ class TestStandbyExtension(base.IronicAgentTest): ephemeral_format = image_info['ephemeral_format'] node_uuid = image_info['node_uuid'] pr_ep = image_info['preserve_ephemeral'] - configdrive = image_info['configdrive'] boot_mode = image_info['deploy_boot_mode'] boot_option = image_info['boot_option'] disk_label = image_info['disk_label'] @@ -229,14 +227,14 @@ class TestStandbyExtension(base.IronicAgentTest): work_on_disk_mock.side_effect = Exception_returned self.assertRaises(exc, standby._write_image, image_info, - device) + device, 'configdrive') image_mb_mock.assert_called_once_with(image_path) work_on_disk_mock.assert_called_once_with(device, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, node_uuid, - configdrive=configdrive, + configdrive='configdrive', preserve_ephemeral=pr_ep, boot_mode=boot_mode, boot_option=boot_option, @@ -262,7 +260,6 @@ class TestStandbyExtension(base.IronicAgentTest): ephemeral_format = image_info['ephemeral_format'] node_uuid = image_info['node_uuid'] pr_ep = image_info['preserve_ephemeral'] - configdrive = image_info['configdrive'] boot_mode = image_info['deploy_boot_mode'] boot_option = image_info['boot_option'] disk_label = image_info['disk_label'] @@ -277,14 +274,14 @@ class TestStandbyExtension(base.IronicAgentTest): image_mb_mock.return_value = 1 work_on_disk_mock.return_value = uuids - standby._write_image(image_info, device) + standby._write_image(image_info, device, 'configdrive') image_mb_mock.assert_called_once_with(image_path) work_on_disk_mock.assert_called_once_with(device, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, node_uuid, - configdrive=configdrive, + configdrive='configdrive', preserve_ephemeral=pr_ep, boot_mode=boot_mode, boot_option=boot_option, @@ -335,7 +332,6 @@ class TestStandbyExtension(base.IronicAgentTest): ephemeral_format = image_info['ephemeral_format'] node_uuid = image_info['node_uuid'] pr_ep = image_info['preserve_ephemeral'] - configdrive = image_info['configdrive'] boot_mode = image_info['deploy_boot_mode'] boot_option = image_info['boot_option'] disk_label = image_info['disk_label'] @@ -348,14 +344,14 @@ class TestStandbyExtension(base.IronicAgentTest): dispatch_mock.return_value = self.fake_cpu work_on_disk_mock.return_value = uuids - standby._write_image(image_info, device) + standby._write_image(image_info, device, 'configdrive') image_mb_mock.assert_called_once_with(image_path) work_on_disk_mock.assert_called_once_with(device, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, node_uuid, - configdrive=configdrive, + configdrive='configdrive', preserve_ephemeral=pr_ep, boot_mode=boot_mode, boot_option=boot_option, @@ -578,7 +574,7 @@ class TestStandbyExtension(base.IronicAgentTest): async_result = self.agent_extension.cache_image(image_info=image_info) async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', None) dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) self.assertEqual(image_info['id'], @@ -602,10 +598,12 @@ class TestStandbyExtension(base.IronicAgentTest): download_mock.return_value = None write_mock.return_value = {'root uuid': 'root_uuid'} dispatch_mock.return_value = 'manager' - async_result = self.agent_extension.cache_image(image_info=image_info) + async_result = self.agent_extension.cache_image( + image_info=image_info, configdrive='configdrive_data') async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', + 'configdrive_data') dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) self.assertEqual(image_info['id'], @@ -637,7 +635,7 @@ class TestStandbyExtension(base.IronicAgentTest): ) async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', None) dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) self.assertEqual(image_info['id'], @@ -712,7 +710,8 @@ class TestStandbyExtension(base.IronicAgentTest): async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', + 'configdrive_data') dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) configdrive_copy_mock.assert_called_once_with(image_info['node_uuid'], @@ -763,7 +762,8 @@ class TestStandbyExtension(base.IronicAgentTest): async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', + 'configdrive_data') dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) self.assertFalse(configdrive_copy_mock.called) @@ -836,7 +836,7 @@ class TestStandbyExtension(base.IronicAgentTest): async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', None) dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) @@ -887,7 +887,7 @@ class TestStandbyExtension(base.IronicAgentTest): async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', None) dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) @@ -930,7 +930,8 @@ class TestStandbyExtension(base.IronicAgentTest): async_result.join() download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, 'manager') + write_mock.assert_called_once_with(image_info, 'manager', + 'configdrive_data') dispatch_mock.assert_called_once_with('get_os_install_device', permit_refresh=True) configdrive_copy_mock.assert_called_once_with(image_info['node_uuid'], @@ -998,7 +999,7 @@ class TestStandbyExtension(base.IronicAgentTest): self.assertIs(partition, work_on_disk_mock.called) else: cache_write_mock.assert_called_once_with(mock.ANY, image_info, - '/dev/foo') + '/dev/foo', None) self.assertFalse(stream_mock.called) def test_prepare_image_raw_stream_true(self): @@ -1169,7 +1170,21 @@ class TestStandbyExtension(base.IronicAgentTest): device = '/dev/foo' self.agent_extension._cache_and_write_image(image_info, device) download_mock.assert_called_once_with(image_info) - write_mock.assert_called_once_with(image_info, device) + write_mock.assert_called_once_with(image_info, device, None) + + @mock.patch('ironic_python_agent.extensions.standby._write_image', + autospec=True) + @mock.patch('ironic_python_agent.extensions.standby._download_image', + autospec=True) + def test_cache_and_write_image_configdirve(self, download_mock, + write_mock): + image_info = _build_fake_image_info() + device = '/dev/foo' + self.agent_extension._cache_and_write_image(image_info, device, + 'configdrive_data') + download_mock.assert_called_once_with(image_info) + write_mock.assert_called_once_with(image_info, device, + 'configdrive_data') @mock.patch('ironic_lib.disk_utils.block_uuid', autospec=True) @mock.patch('ironic_lib.disk_utils.fix_gpt_partition', autospec=True) @@ -1395,7 +1410,6 @@ class TestStandbyExtension(base.IronicAgentTest): ephemeral_format = image_info['ephemeral_format'] node_uuid = image_info['node_uuid'] pr_ep = image_info['preserve_ephemeral'] - configdrive = image_info['configdrive'] boot_option = image_info['boot_option'] cpu_arch = self.fake_cpu.architecture @@ -1408,14 +1422,14 @@ class TestStandbyExtension(base.IronicAgentTest): image_mb_mock.return_value = 1 work_on_disk_mock.return_value = uuids - standby._write_image(image_info, device) + standby._write_image(image_info, device, 'configdrive') image_mb_mock.assert_called_once_with(image_path) work_on_disk_mock.assert_called_once_with(device, root_mb, swap_mb, ephemeral_mb, ephemeral_format, image_path, node_uuid, - configdrive=configdrive, + configdrive='configdrive', preserve_ephemeral=pr_ep, boot_mode='uefi', boot_option=boot_option, diff --git a/releasenotes/notes/configdrive-dup-3fc46a878fe82485.yaml b/releasenotes/notes/configdrive-dup-3fc46a878fe82485.yaml new file mode 100644 index 00000000..74a509d7 --- /dev/null +++ b/releasenotes/notes/configdrive-dup-3fc46a878fe82485.yaml @@ -0,0 +1,6 @@ +--- +other: + - | + The API call ``prepare_image`` and the deploy step ``write_image`` + will now expect configdrive to be passed as an explicit argument + rather than through ``image_info``. |