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

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``.