Fix various issues in the anaconda deploy interface

The kickstart template expects a dictionary with 'ks_options'
as the key. Instead build_kickstart_config_options function
returns a dict with keys 'liveimg_url', 'agent_token' and
'heartbeat_url'.

This change fixes this problem by returning a dictionary of
dict with 'ks_options' as key and the dictionary with
keys 'liveimg_url', 'agent_token' and heartbeat_url' as
value.

Fix a bug where the deploy() method of anaconda deploy
interface where it did not return states.DEPLOYWAIT instead
it returned 'None' which caused the instance to go straight to
'active' instead of 'wait call-back'.

Fix issues in the default kickstart template where heartbeat was
missing 'callback_url' parameter and the HTTP method should be
'POST' not 'PUT'.

Fix issues with automated cleaning when anaconda deploy interface
is used.

Anaconda deploy interface could not deploy tarballs as
the disk image sent to the anaconda interface via liveimg --url
kickstart command does not include any file extension. When
no file extension is present the kickstart command liveimg --url
assumes the disk is a mountable partiton image. We fix this
problem by enabling the user to specify file extensions using
a glance image property named 'disk_file_extension' on the OS
image.

Conflicts:
    ironic/drivers/modules/deploy_utils.py
    ironic/tests/unit/drivers/modules/test_deploy_utils.py
    ironic/tests/unit/drivers/modules/test_pxe.py

NOTE(rloo): The conflicts are due to changes in master that
refactored the code (among other changes,
I4fc63be4e3cd4656d0ca7e893d4f3a98c07a8b4c) so that validation
of stage2_id is in deploy_utils.py. In wallaby, that validation
is done in pxe_base.py

Co-Authored-By: Ruby Loo <opensrloo@gmail.com>
Change-Id: I556f8c9efbc5ab0941513c3ecaa2aa3ca7f346ae
(cherry picked from commit df99dea001772c1c6691150060d3ebbf496e5d25)

9 files changed, 111 insertions, 32 deletions

diff --git a/doc/source/admin/anaconda-deploy-interface.rst b/doc/source/admin/anaconda-deploy-interface.rst
index c72c4378e..041bd9c31 100644
--- a/doc/source/admin/anaconda-deploy-interface.rst
+++ b/doc/source/admin/anaconda-deploy-interface.rst
@@ -21,6 +21,16 @@ option in ironic.conf. For example:
 This change takes effect after all the ironic conductors have been
 restarted.
 
+The default kickstart template is specified via the configuration option
+``[anaconda]default_ks_template``. It is set to this `ks.cfg.template`_
+but can be modified to be some other template.
+
+.. code-block::  ini
+
+   [anaconda]
+   default_ks_template = file:///etc/ironic/ks.cfg.template
+
+
 When creating an ironic node, specify ``anaconda`` as the deploy interface.
 For example:
 
@@ -92,12 +102,19 @@ The kernel and ramdisk can be found at ``/images/pxeboot/vmlinuz`` and
 image can be normally found at ``/LiveOS/squashfs.img`` or
 ``/images/install.img``.
 
-The OS tarball must be configured with the following properties in glance, in order
-to be used with the anaconda deploy driver:
+The OS tarball must be configured with the following properties in glance, in
+order to be used with the anaconda deploy driver:
 
 * ``kernel_id``
 * ``ramdisk_id``
 * ``stage2_id``
+* ``disk_file_extension`` (optional)
+
+Valid ``disk_file_extension`` values are ``.img``, ``.tar``, ``.tbz``,
+``.tgz``, ``.txz``, ``.tar.gz``, ``.tar.bz2``, and ``.tar.xz``. When
+``disk_file_extension`` property is not set to one of the above valid values
+the anaconda installer will assume that the image provided is a mountable
+OS disk.
 
 This is an example of adding the anaconda-related images and the OS tarball to
 glance:
@@ -114,7 +131,8 @@ glance:
             compressed --disk-format raw --shared \
             --property kernel_id=<glance_uuid_vmlinuz> \
             --property ramdisk_id=<glance_uuid_ramdisk> \
-            --property stage2_id=<glance_uuid_stage2> <disto-name-version>
+            --property stage2_id=<glance_uuid_stage2> disto-name-version \
+            --property disk_file_extension=.tgz
 
 Creating a bare metal server
 ----------------------------
@@ -127,10 +145,6 @@ specified via the OS image in glance. If no kickstart template is specified
 (via the node's ``instance_info``  or ``ks_template`` glance image property),
 the default kickstart template will be used to deploy the OS.
 
-The default kickstart template is specified via the configuration option
-``[anaconda]default_ks_template``. It is set to this `ks.cfg.template`_
-but can be modified to be some other template.
-
 This is an example of how to set the kickstart template for a specific
 ironic node:
 
diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index ddaea240d..54439ce69 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -959,14 +959,14 @@ def build_kickstart_config_options(task):
     :returns: A dictionary of kickstart options to be used in the kickstart
               template.
     """
-    ks_options = {}
+    params = {}
     node = task.node
     manager_utils.add_secret_token(node, pregenerated=True)
     node.save()
-    ks_options['liveimg_url'] = node.instance_info['image_url']
-    ks_options['agent_token'] = node.driver_internal_info['agent_secret_token']
-    ks_options['heartbeat_url'] = _build_heartbeat_url(node.uuid)
-    return ks_options
+    params['liveimg_url'] = node.instance_info['image_url']
+    params['agent_token'] = node.driver_internal_info['agent_secret_token']
+    params['heartbeat_url'] = _build_heartbeat_url(node.uuid)
+    return {'ks_options': params}
 
 
 def get_volume_pxe_options(task):
diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index dc57c85a4..b10038f8b 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -1083,6 +1083,24 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
     symlink_dir = _get_http_image_symlink_dir_path()
     fileutils.ensure_tree(symlink_dir)
     symlink_path = _get_http_image_symlink_file_path(task.node.uuid)
+    file_extension = None
+    if get_boot_option(task.node) == 'kickstart':
+        # 'liveimg --url' kickstart command uses the file extension to
+        # identify the OS image type. Without a valid file extension it will
+        # assume the disk image is a partition image and try to 'mount' it on
+        # the ramdisk. See 'liveimg' command for more details
+        # https://pykickstart.readthedocs.io/en/latest/kickstart-docs.html
+        valid_file_extensions = ['.img', '.tar', '.tbz', '.tgz', '.txz',
+                                 '.tar.gz', '.tar.bz2', '.tar.xz']
+        if image_info and 'disk_file_extension' in image_info['properties']:
+            ext = image_info['properties']['disk_file_extension']
+            file_extension = ext if ext in valid_file_extensions else None
+        if file_extension:
+            symlink_path = symlink_path + file_extension
+        else:
+            LOG.warning("The 'disk_file_extension' property was not set on "
+                        "the glance image or not a valid extension so the "
+                        "anaconda installer will try to 'mount' the OS image.")
     utils.create_link_without_raise(image_path, symlink_path)
 
     base_url = CONF.deploy.http_url
@@ -1091,6 +1109,8 @@ def _cache_and_convert_image(task, instance_info, image_info=None):
     http_image_url = '/'.join(
         [base_url, CONF.deploy.http_image_subdir,
          task.node.uuid])
+    if file_extension:
+        http_image_url = http_image_url + file_extension
     _validate_image_url(task.node, http_image_url, secret=False)
     instance_info['image_url'] = http_image_url
 
diff --git a/ironic/drivers/modules/ks.cfg.template b/ironic/drivers/modules/ks.cfg.template
index 1a2cecaf3..40552377b 100644
--- a/ironic/drivers/modules/ks.cfg.template
+++ b/ironic/drivers/modules/ks.cfg.template
@@ -6,8 +6,9 @@ text
 cmdline
 reboot
 selinux --enforcing
-firewall --enabled
+firewall --disabled
 firstboot --disabled
+rootpw --lock
 
 bootloader --location=mbr --append="rhgb quiet crashkernel=auto"
 zerombr
@@ -19,19 +20,18 @@ liveimg --url {{ ks_options.liveimg_url }}
 
 # Following %pre, %onerror and %trackback sections are mandatory
 %pre
-/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"agent_token": "{{ ks_options.agent_token }}", "agent_status": "start", "agent_status_message": "Deployment starting. Running pre-installation scripts."}' {{ ks_options.heartbeat_url }}
+/usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "start", "agent_status_message": "Deployment starting. Running pre-installation scripts."}' {{ ks_options.heartbeat_url }}
 %end
 
 %onerror
-/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Deploying using anaconda. Check console for more information."}' {{ ks_options.heartbeat_url }}
+/usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Deploying using anaconda. Check console for more information."}' {{ ks_options.heartbeat_url }}
 %end
 
 %traceback
-/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Installer crashed unexpectedly."}' {{ ks_options.heartbeat_url }}
+/usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "error", "agent_status_message": "Error: Installer crashed unexpectedly."}' {{ ks_options.heartbeat_url }}
 %end
 
 # Sending callback after the installation is mandatory
 %post
-/usr/bin/curl -X PUT -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"agent_token": "{{ ks_options.agent_token }}", "agent_status": "end", "agent_status_message": "Deployment completed successfully."}' {{ ks_options.heartbeat_url }}
+/usr/bin/curl -X POST -H 'Content-Type: application/json' -H 'Accept: application/json' -H 'X-OpenStack-Ironic-API-Version: 1.72' -d '{"callback_url": "", "agent_token": "{{ ks_options.agent_token }}", "agent_status": "end", "agent_status_message": "Deployment completed successfully."}' {{ ks_options.heartbeat_url }}
 %end
-
diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py
index a91a33e5b..23bcda924 100644
--- a/ironic/drivers/modules/pxe.py
+++ b/ironic/drivers/modules/pxe.py
@@ -130,7 +130,7 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin,
         # Power-on the instance, with PXE prepared, we're done.
         manager_utils.node_power_action(task, states.POWER_ON)
         LOG.info('Deployment setup for node %s done', task.node.uuid)
-        return None
+        return states.DEPLOYWAIT
 
     @METRICS.timer('AnacondaDeploy.prepare')
     @task_manager.require_exclusive_lock
@@ -164,7 +164,11 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin,
         return False
 
     def should_manage_boot(self, task):
-        return False
+        if task.node.provision_state in (
+                states.DEPLOYING, states.DEPLOYWAIT, states.DEPLOYFAIL):
+            return False
+        # For cleaning and rescue, we use IPA, not anaconda
+        return agent_base.AgentBaseMixin.should_manage_boot(self, task)
 
     def reboot_to_instance(self, task):
         node = task.node
diff --git a/ironic/drivers/modules/pxe_base.py b/ironic/drivers/modules/pxe_base.py
index 8b8cc4f13..34273278d 100644
--- a/ironic/drivers/modules/pxe_base.py
+++ b/ironic/drivers/modules/pxe_base.py
@@ -420,7 +420,7 @@ class PXEBaseMixin(object):
         elif service_utils.is_glance_image(d_info['image_source']):
             props = ['kernel_id', 'ramdisk_id']
             if boot_option == 'kickstart':
-                props.append('squashfs_id')
+                props.append('stage2_id')
         else:
             props = ['kernel', 'ramdisk']
         deploy_utils.validate_image_properties(task.context, d_info, props)
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index 9f6c36f41..f85034442 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -1376,9 +1376,9 @@ class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase):
             expected['heartbeat_url'] = (
                 'http://ironic-api/v1/heartbeat/%s' % task.node.uuid
             )
-            ks_options = pxe_utils.build_kickstart_config_options(task)
-            self.assertTrue(ks_options.pop('agent_token'))
-            self.assertEqual(expected, ks_options)
+            params = pxe_utils.build_kickstart_config_options(task)
+            self.assertTrue(params['ks_options'].pop('agent_token'))
+            self.assertEqual(expected, params['ks_options'])
 
     @mock.patch('ironic.common.utils.render_template', autospec=True)
     def test_prepare_instance_kickstart_config_not_anaconda_boot(self,
@@ -1400,15 +1400,16 @@ class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase):
             'ks_cfg': ['', '/http_root/node_uuid/ks.cfg'],
             'ks_template': ['tmpl_id', '/http_root/node_uuid/ks.cfg.template']
         }
-        ks_options = {'liveimg_url': 'http://fake', 'agent_token': 'faketoken',
-                      'heartbeat_url': 'http://fake_hb'}
+        params = {'liveimg_url': 'http://fake', 'agent_token': 'faketoken',
+                  'heartbeat_url': 'http://fake_hb'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=True) as task:
-            ks_options_mock.return_value = ks_options
+            ks_options_mock.return_value = {'ks_options': params}
             pxe_utils.prepare_instance_kickstart_config(task, image_info,
                                                         anaconda_boot=True)
-            render_mock.assert_called_with(image_info['ks_template'][1],
-                                           ks_options)
+            render_mock.assert_called_with(
+                image_info['ks_template'][1], {'ks_options': params}
+            )
             write_mock.assert_called_with(image_info['ks_cfg'][1],
                                           render_mock.return_value)
 
diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py
index 1d871982a..0ea6e9eb6 100644
--- a/ironic/tests/unit/drivers/modules/test_pxe.py
+++ b/ironic/tests/unit/drivers/modules/test_pxe.py
@@ -239,7 +239,7 @@ class PXEBootTestCase(db_base.DbTestCase):
 
     @mock.patch.object(deploy_utils, 'validate_image_properties',
                        autospec=True)
-    def test_validate_kickstart_has_squashfs_id(self, mock_validate_img):
+    def test_validate_kickstart_has_stage2_id(self, mock_validate_img):
         node = self.node
         node.deploy_interface = 'anaconda'
         node.save()
@@ -247,7 +247,7 @@ class PXEBootTestCase(db_base.DbTestCase):
         with task_manager.acquire(self.context, node.uuid) as task:
             task.driver.boot.validate(task)
             mock_validate_img.assert_called_once_with(
-                mock.ANY, mock.ANY, ['kernel_id', 'ramdisk_id', 'squashfs_id']
+                mock.ANY, mock.ANY, ['kernel_id', 'ramdisk_id', 'stage2_id']
             )
 
     def test_validate_kickstart_fail_http_url_not_set(self):
@@ -1088,7 +1088,9 @@ class PXEAnacondaDeployTestCase(db_base.DbTestCase):
                       'ks_cfg': ('', '/path/to/ks_cfg')}
         mock_image_info.return_value = image_info
         with task_manager.acquire(self.context, self.node.uuid) as task:
-            self.assertIsNone(task.driver.deploy.deploy(task))
+            self.assertEqual(
+                states.DEPLOYWAIT, task.driver.deploy.deploy(task)
+            )
             mock_image_info.assert_called_once_with(task, ipxe_enabled=False)
             mock_cache.assert_called_once_with(
                 task, image_info, ipxe_enabled=False)
@@ -1127,6 +1129,8 @@ class PXEAnacondaDeployTestCase(db_base.DbTestCase):
                       'ks_template': ('', '/path/to/ks_template'),
                       'ks_cfg': ('', '/path/to/ks_cfg')}
         mock_image_info.return_value = image_info
+        self.node.provision_state = states.DEPLOYWAIT
+        self.node.save()
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.deploy.reboot_to_instance(task)
             mock_set_boot_dev.assert_called_once_with(task, boot_devices.DISK)
@@ -1189,6 +1193,17 @@ class PXEAnacondaDeployTestCase(db_base.DbTestCase):
                 task.node.driver_internal_info['agent_status'])
             self.assertTrue(mock_reboot_to_instance.called)
 
+    @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', autospec=True)
+    def test_prepare_cleaning(self, prepare_inband_cleaning_mock):
+        prepare_inband_cleaning_mock.return_value = states.CLEANWAIT
+        self.node.provision_state = states.CLEANING
+        self.node.save()
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            self.assertEqual(
+                states.CLEANWAIT, self.deploy.prepare_cleaning(task))
+            prepare_inband_cleaning_mock.assert_called_once_with(
+                task, manage_boot=True)
+
 
 class PXEValidateRescueTestCase(db_base.DbTestCase):
 
diff --git a/releasenotes/notes/fix-anaconda-deploy-interface-bfa2cfca22b04680.yaml b/releasenotes/notes/fix-anaconda-deploy-interface-bfa2cfca22b04680.yaml
new file mode 100644
index 000000000..e791d7fdd
--- /dev/null
+++ b/releasenotes/notes/fix-anaconda-deploy-interface-bfa2cfca22b04680.yaml
@@ -0,0 +1,25 @@
+---
+fixes:
+  - |
+    Fixes a bug in the anaconda deploy interface where the 'ks_options'
+    key was not found when rendering the default kickstart template.
+  - |
+    Fixes issue where PXEAnacondaDeploy interface's deploy() method did not
+    return states.DEPLOYWAIT so the instance went straight to 'active' instead
+    of 'wait call-back'.
+  - |
+    Fixes an issue where the anaconda deploy interface mistakenly expected
+    'squashfs_id' instead of 'stage2_id'  property on the image.
+  - |
+    Fixes the heartbeat mechanism in the default kickstart template
+    ks.cfg.template as the heartbeat API only accepts 'POST' and expects a
+    mandatory 'callback_url' parameter.
+  - |
+    Fixes handling of tarball images in anaconda deploy interface. Allows user
+    specified file extensions to be appended to the disk image symlink. Users
+    can now set the file extensions by setting the 'disk_file_extension'
+    property on the OS image. This enables users to deploy tarballs with
+    anaconda deploy interface.
+  - |
+    Fixes issue where automated cleaning was not supported when anaconda deploy
+    interface is used.