Merge "Stop accepting duplicated configdrive"

3 files changed, 77 insertions, 39 deletions

diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py
index bdf338a9..ce754f82 100644
--- a/ironic_python_agent/extensions/standby.py
+++ b/ironic_python_agent/extensions/standby.py
@@ -130,7 +130,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.
@@ -138,6 +138,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.
@@ -150,7 +153,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)
@@ -223,12 +225,15 @@ def _write_whole_disk_image(image, image_info, device):
     disk_utils.trigger_device_rescan(device)
 
 
-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.
     """
@@ -236,7 +241,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
@@ -547,12 +552,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
@@ -560,7 +568,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):
@@ -634,13 +642,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
@@ -656,7 +667,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)
@@ -667,9 +681,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
@@ -691,6 +703,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)
 
@@ -706,7 +721,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 = {}
@@ -714,12 +730,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 e7500027..f9f1d530 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',
@@ -234,7 +233,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']
@@ -249,14 +247,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,
@@ -282,7 +280,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']
@@ -297,14 +294,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,
@@ -355,7 +352,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']
@@ -368,14 +364,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,
@@ -598,7 +594,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'],
@@ -622,10 +618,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'],
@@ -657,7 +655,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'],
@@ -732,7 +730,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'],
@@ -783,7 +782,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)
@@ -856,7 +856,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)
 
@@ -907,7 +907,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)
 
@@ -950,7 +950,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'],
@@ -1018,7 +1019,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):
@@ -1189,7 +1190,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)
@@ -1415,7 +1430,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
 
@@ -1428,14 +1442,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``.