Do not use pregenerated tokens with a pre-built ISO

If an ISO is provided by a user and removable configuration is not used,
we cannot embed the token. Retrieve it normally instead.

Change-Id: Ie5f735e077f4cee89d7ae59c414909eb4e0c734b
(cherry picked from commit 8647aeca658a55a7fec67f6397d9eda8fcfe2341)

3 files changed, 87 insertions, 12 deletions

diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py
index 5b4971484..6297052f8 100644
--- a/ironic/drivers/modules/redfish/boot.py
+++ b/ironic/drivers/modules/redfish/boot.py
@@ -104,6 +104,7 @@ def _parse_driver_info(node):
     iso_ref = d_info.get(iso_param)
     if iso_ref is not None:
         deploy_info = {iso_param: iso_ref}
+        can_config = False
     else:
         params_to_check = KERNEL_RAMDISK_LABELS[mode]
 
@@ -121,6 +122,7 @@ def _parse_driver_info(node):
                       "parameters were missing in node's driver_info")
 
         deploy_utils.check_for_missing_params(deploy_info, error_msg)
+        can_config = True
 
     deploy_info.update(
         {option: d_info.get(option, getattr(CONF.conductor, option, None))
@@ -133,6 +135,11 @@ def _parse_driver_info(node):
         deploy_info['config_via_removable'] = d_info['config_via_floppy']
 
     deploy_info.update(redfish_utils.parse_driver_info(node))
+    # Configuration can be provided in one of two cases:
+    # 1) A removable disk is requested.
+    # 2) An ISO is built from a kernel/initramfs pair.
+    deploy_info['can_provide_config'] = \
+        deploy_info.get('config_via_removable') or can_config
 
     return deploy_info
 
@@ -466,19 +473,21 @@ class RedfishVirtualMediaBoot(base.BootInterface):
                                         states.INSPECTING):
             return
 
+        d_info = _parse_driver_info(node)
+
         # NOTE(TheJulia): Since we're deploying, cleaning, or rescuing,
         # with virtual media boot, we should generate a token!
-        manager_utils.add_secret_token(node, pregenerated=True)
-        node.save()
-        ramdisk_params['ipa-agent-token'] = \
-            node.driver_internal_info['agent_secret_token']
+        # However, we don't have a way to inject it with a pre-built ISO
+        # if a removable disk is not used.
+        can_config = d_info.pop('can_provide_config', True)
+        if can_config:
+            manager_utils.add_secret_token(node, pregenerated=True)
+            node.save()
+            ramdisk_params['ipa-agent-token'] = \
+                node.driver_internal_info['agent_secret_token']
 
         manager_utils.node_power_action(task, states.POWER_OFF)
 
-        d_info = _parse_driver_info(node)
-
-        config_via_removable = d_info.get('config_via_removable')
-
         deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task)
         if deploy_nic_mac is not None:
             ramdisk_params['BOOTIF'] = deploy_nic_mac
@@ -491,6 +500,7 @@ class RedfishVirtualMediaBoot(base.BootInterface):
         # based deployment operations.
         ramdisk_params['boot_method'] = 'vmedia'
 
+        config_via_removable = d_info.get('config_via_removable')
         if config_via_removable:
 
             removable = _has_vmedia_device(
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
index d75712b67..782ddfcf5 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py
@@ -71,6 +71,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
             self.assertIn('kernel', actual_driver_info['deploy_kernel'])
             self.assertIn('ramdisk', actual_driver_info['deploy_ramdisk'])
             self.assertIn('bootloader', actual_driver_info['bootloader'])
+            self.assertTrue(actual_driver_info['can_provide_config'])
 
     def test_parse_driver_info_iso(self):
         with task_manager.acquire(self.context, self.node.uuid,
@@ -82,19 +83,19 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             self.assertEqual('http://boot.iso',
                              actual_driver_info['redfish_deploy_iso'])
+            self.assertFalse(actual_driver_info['can_provide_config'])
 
     def test_parse_driver_info_removable(self):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
             task.node.driver_info.update(
-                {'deploy_kernel': 'kernel',
-                 'deploy_ramdisk': 'ramdisk',
-                 'bootloader': 'bootloader',
+                {'redfish_deploy_iso': 'http://boot.iso',
                  'config_via_removable': True}
             )
 
             actual_driver_info = redfish_boot._parse_driver_info(task.node)
             self.assertTrue(actual_driver_info['config_via_removable'])
+            self.assertTrue(actual_driver_info['can_provide_config'])
 
     @mock.patch.object(redfish_boot.LOG, 'warning', autospec=True)
     def test_parse_driver_info_removable_deprecated(self, mock_log):
@@ -445,8 +446,11 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
             mock__insert_vmedia.assert_called_once_with(
                 task, managers, 'image-url', sushy.VIRTUAL_MEDIA_CD)
 
+            token = task.node.driver_internal_info['agent_secret_token']
+            self.assertTrue(token)
+
             expected_params = {
-                'ipa-agent-token': mock.ANY,
+                'ipa-agent-token': token,
                 'ipa-debug': '1',
                 'boot_method': 'vmedia',
             }
@@ -459,6 +463,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task)
 
+            self.assertTrue(task.node.driver_internal_info[
+                'agent_secret_token_pregenerated'])
+
     @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device',
                        autospec=True)
     @mock.patch.object(image_utils, 'prepare_deploy_iso', autospec=True)
@@ -652,6 +659,59 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
 
             mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task)
 
+    @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device',
+                       autospec=True)
+    @mock.patch.object(image_utils, 'prepare_deploy_iso', autospec=True)
+    @mock.patch.object(redfish_boot, '_eject_vmedia', autospec=True)
+    @mock.patch.object(redfish_boot, '_insert_vmedia', autospec=True)
+    @mock.patch.object(redfish_boot, '_parse_driver_info', autospec=True)
+    @mock.patch.object(redfish_boot.manager_utils, 'node_power_action',
+                       autospec=True)
+    @mock.patch.object(redfish_boot, 'boot_mode_utils', autospec=True)
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_prepare_ramdisk_no_config(
+            self, mock_system, mock_boot_mode_utils, mock_node_power_action,
+            mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia,
+            mock_prepare_deploy_iso, mock_node_set_boot_device):
+
+        managers = mock_system.return_value.managers
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.provision_state = states.DEPLOYING
+
+            mock__parse_driver_info.return_value = {
+                'can_provide_config': False}
+            mock_prepare_deploy_iso.return_value = 'image-url'
+
+            task.driver.boot.prepare_ramdisk(task, {})
+
+            mock_node_power_action.assert_called_once_with(
+                task, states.POWER_OFF)
+
+            mock__eject_vmedia.assert_called_once_with(
+                task, managers, sushy.VIRTUAL_MEDIA_CD)
+
+            mock__insert_vmedia.assert_called_once_with(
+                task, managers, 'image-url', sushy.VIRTUAL_MEDIA_CD)
+
+            expected_params = {
+                'ipa-debug': '1',
+                'boot_method': 'vmedia',
+            }
+
+            mock_prepare_deploy_iso.assert_called_once_with(
+                task, expected_params, 'deploy', {})
+
+            mock_node_set_boot_device.assert_called_once_with(
+                task, boot_devices.CDROM, False)
+
+            mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task)
+
+            self.assertNotIn('agent_secret_token',
+                             task.node.driver_internal_info)
+            self.assertNotIn('agent_secret_token_pregenerated',
+                             task.node.driver_internal_info)
+
     @mock.patch.object(redfish_boot, '_eject_vmedia', autospec=True)
     @mock.patch.object(image_utils, 'cleanup_iso_image', autospec=True)
     @mock.patch.object(image_utils, 'cleanup_floppy_image', autospec=True)
diff --git a/releasenotes/notes/redfish-iso-pregenerated-97040711c4537726.yaml b/releasenotes/notes/redfish-iso-pregenerated-97040711c4537726.yaml
new file mode 100644
index 000000000..c53dfb076
--- /dev/null
+++ b/releasenotes/notes/redfish-iso-pregenerated-97040711c4537726.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes providing agent tokens with pre-built ISO images and the
+    ``redfish-virtual-media`` boot interface.