Merge "Fix various issues in the anaconda deploy interface" into bugfix/18.1

9 files changed, 112 insertions, 33 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 cd5a57084..136499db8 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -962,14 +962,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 012451651..673e4c664 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -573,7 +573,7 @@ def validate_image_properties(task, deploy_info):
         properties = ['kernel_id', 'ramdisk_id']
         boot_option = get_boot_option(task.node)
         if boot_option == 'kickstart':
-            properties.append('squashfs_id')
+            properties.append('stage2_id')
     else:
         properties = ['kernel', 'ramdisk']
 
@@ -1122,6 +1122,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
@@ -1130,6 +1148,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/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index c191686cb..86b6a5cba 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_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index 4021558bf..e9c4c2e58 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -1343,7 +1343,7 @@ class ValidateImagePropertiesTestCase(db_base.DbTestCase):
     @mock.patch.object(utils, 'get_boot_option', autospec=True,
                        return_value='kickstart')
     @mock.patch.object(image_service, 'get_image_service', autospec=True)
-    def test_validate_image_properties_glance_image_missing_squashfs_id(
+    def test_validate_image_properties_glance_image_missing_stage2_id(
             self, image_service_mock, boot_options_mock):
         inst_info = utils.get_image_instance_info(self.node)
         image_service_mock.return_value.show.return_value = {
diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py
index 430930780..81a72bc00 100644
--- a/ironic/tests/unit/drivers/modules/test_pxe.py
+++ b/ironic/tests/unit/drivers/modules/test_pxe.py
@@ -237,7 +237,7 @@ class PXEBootTestCase(db_base.DbTestCase):
                               task.driver.boot.validate_inspection, task)
 
     @mock.patch.object(image_service.GlanceImageService, 'show', autospec=True)
-    def test_validate_kickstart_has_squashfs_id(self, mock_glance):
+    def test_validate_kickstart_missing_stage2_id(self, mock_glance):
         mock_glance.return_value = {'properties': {'kernel_id': 'fake-kernel',
                                                    'ramdisk_id': 'fake-initr'}}
         self.node.deploy_interface = 'anaconda'
@@ -245,7 +245,7 @@ class PXEBootTestCase(db_base.DbTestCase):
         self.config(http_url='http://fake_url', group='deploy')
         with task_manager.acquire(self.context, self.node.uuid) as task:
             self.assertRaisesRegex(exception.MissingParameterValue,
-                                   'squashfs_id',
+                                   'stage2_id',
                                    task.driver.boot.validate, task)
 
     def test_validate_kickstart_fail_http_url_not_set(self):
@@ -1147,7 +1147,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)
@@ -1186,6 +1188,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)
@@ -1248,6 +1252,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.