summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitry Tantsur <dtantsur@protonmail.com>2021-05-10 17:01:45 +0200
committerDmitry Tantsur <dtantsur@protonmail.com>2021-06-02 11:19:39 +0200
commitf65752680700fc8df8877620b2354cb06ea54fd3 (patch)
treeba5d755dc0a5b3836d8e2efceee4d431b6c0c016
parent6fb4cec7aa0a3edc76b0e07cc9e7cc0b1d4a4e99 (diff)
downloadironic-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.py48
-rw-r--r--ironic_python_agent/tests/unit/extensions/test_standby.py62
-rw-r--r--releasenotes/notes/configdrive-dup-3fc46a878fe82485.yaml6
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``.