From 09d8a94d5a9b3cabdf7fddc483821fd0df69e5ee Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Fri, 4 Feb 2022 22:14:45 +0000 Subject: [PATCH] Anaconda deploy handles configdrive correctly The anaconda deploy interface wasn't handling configdrive correctly. This fixes: - the code was incorrectly treating the config drive as a dict, whereas it could also be in iso6600 format, gzipped and base64-encoded. It is handled properly now. - kickstart commands that deal with the config drive were added to the end of the kickstart file. Which means that they are executed after an ironic API request is sent to ironic to indicate that the node has been provisioned and is ready to be rebooted. Which means that there is a possible race condition wrt these commands being completed before the node is powered off. This has been fixed by putting the API request at the end of the kickstart file. Also, a sync was added to ensure that all modifications are written to disk. - extra newlines ('\n') were incorrectly added to the user data content. This broke the content-type decoding and cloud-init was unable to proces them. The extra newlines have been removed. Change-Id: If00342a1bfa3e87b2094061b0e2b0b69de3a8a4f --- ironic/common/kickstart_utils.py | 14 +++++++++----- ironic/common/pxe_utils.py | 5 ++--- ironic/drivers/modules/ks.cfg.template | 11 ++++++++++- .../tests/unit/common/test_kickstart_utils.py | 2 +- ironic/tests/unit/common/test_pxe_utils.py | 1 + ...a-config-drive-fixes-5880884e34584549.yaml | 19 +++++++++++++++++++ 6 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/anaconda-config-drive-fixes-5880884e34584549.yaml diff --git a/ironic/common/kickstart_utils.py b/ironic/common/kickstart_utils.py index 519cb53264..433cf23907 100644 --- a/ironic/common/kickstart_utils.py +++ b/ironic/common/kickstart_utils.py @@ -55,7 +55,7 @@ def _get_config_drive_dict_from_iso( iso_path=iso_file_path, outfp=b_buf ) b_buf.seek(0) - content = b"\n".join(b_buf.readlines()).decode('utf-8') + content = b"".join(b_buf.readlines()).decode('utf-8') drive_dict[target_file_path] = content @@ -113,8 +113,7 @@ def _fetch_config_drive_from_url(url): "Can't download the configdrive content from '%(url)s'. " "Reason: %(reason)s" % {'url': url, 'reason': e}) - config_drive_iso = decode_and_extract_config_drive_iso(config_drive) - return read_iso9600_config_drive(config_drive_iso) + return config_drive def _write_config_drive_content(content, file_path): @@ -152,10 +151,15 @@ def prepare_config_drive(task, if not config_drive: return ks_config_drive - if not isinstance(config_drive, dict) and \ - ironic_utils.is_http_url(config_drive): + if ironic_utils.is_http_url(config_drive): config_drive = _fetch_config_drive_from_url(config_drive) + if not isinstance(config_drive, dict): + # The config drive is in iso6600 format, gzipped and base-64-encoded. + # Convert it to a dict. + config_drive_iso = decode_and_extract_config_drive_iso(config_drive) + config_drive = read_iso9600_config_drive(config_drive_iso) + for key in sorted(config_drive.keys()): target_path = os.path.join(config_drive_path, key) ks_config_drive += _write_config_drive_content( diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index 8192cd458a..c31eeed8c2 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -957,6 +957,7 @@ def build_kickstart_config_options(task): node.uuid ) params['heartbeat_url'] = heartbeat_url + params['config_drive'] = ks_utils.prepare_config_drive(task) return {'ks_options': params} @@ -1067,6 +1068,7 @@ def validate_kickstart_template(ks_template): """ ks_options = {'liveimg_url': 'fake_image_url', 'agent_token': 'fake_token', + 'config_drive': '', 'heartbeat_url': 'fake_heartbeat_url'} params = {'ks_options': ks_options} try: @@ -1185,9 +1187,6 @@ def prepare_instance_kickstart_config(task, image_info, anaconda_boot=False): ks_options = build_kickstart_config_options(task) kickstart_template = image_info['ks_template'][1] ks_cfg = utils.render_template(kickstart_template, ks_options) - ks_config_drive = ks_utils.prepare_config_drive(task) - if ks_config_drive: - ks_cfg = ks_cfg + ks_config_drive utils.write_to_file(image_info['ks_cfg'][1], ks_cfg, CONF.pxe.file_permission) diff --git a/ironic/drivers/modules/ks.cfg.template b/ironic/drivers/modules/ks.cfg.template index f7ef75e1cd..941d3c37d1 100644 --- a/ironic/drivers/modules/ks.cfg.template +++ b/ironic/drivers/modules/ks.cfg.template @@ -27,7 +27,16 @@ liveimg --url {{ ks_options.liveimg_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 -# Sending callback after the installation is mandatory +# Config-drive information, if any. +{{ ks_options.config_drive }} + +# Sending callback after the installation is mandatory. +# This ought to be the last thing done; otherwise the +# ironic-conductor could reboot the node before anaconda +# finishes executing everything in this file. +# The sync makes sure that the data is flushed out to disk, +# before rebooting. %post +sync /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/tests/unit/common/test_kickstart_utils.py b/ironic/tests/unit/common/test_kickstart_utils.py index fffacf7d46..0dd1ac5728 100644 --- a/ironic/tests/unit/common/test_kickstart_utils.py +++ b/ironic/tests/unit/common/test_kickstart_utils.py @@ -114,7 +114,7 @@ echo $CONTENT | /usr/bin/base64 --decode > {file_path}\n\ expected = self._get_expected_ks_config_drive(self.config_drive_dict) with task_manager.acquire(self.context, self.node.uuid) as task: i_info = task.node.instance_info - i_info['configdrive'] = self.config_drive_dict + i_info['configdrive'] = CONFIG_DRIVE task.node.instance_info = i_info task.node.save() self.assertEqual(expected, ks_utils.prepare_config_drive(task)) diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py index 52372f3952..5ba0033a7a 100644 --- a/ironic/tests/unit/common/test_pxe_utils.py +++ b/ironic/tests/unit/common/test_pxe_utils.py @@ -1514,6 +1514,7 @@ class PXEBuildKickstartConfigOptionsTestCase(db_base.DbTestCase): shared=True) as task: expected = {} expected['liveimg_url'] = task.node.instance_info['image_url'] + expected['config_drive'] = '' expected['heartbeat_url'] = ( 'http://ironic-api/v1/heartbeat/%s' % task.node.uuid ) diff --git a/releasenotes/notes/anaconda-config-drive-fixes-5880884e34584549.yaml b/releasenotes/notes/anaconda-config-drive-fixes-5880884e34584549.yaml new file mode 100644 index 0000000000..ca1a6a6e7d --- /dev/null +++ b/releasenotes/notes/anaconda-config-drive-fixes-5880884e34584549.yaml @@ -0,0 +1,19 @@ +--- +fixes: + - | + The anaconda deploy interface was treating the config drive + as a dict, whereas it could be a dict or in iso6600 format, + gzipped and base64-encoded. This has been fixed. + - | + The anaconda deploy interface was adding commands that deal with the + config drive, to the end of the kickstart config file. Which means + that they are handled after an ironic API request is sent (to the + conductor) to indicate that the node has been provisioned and is + ready to be rebooted. Which means that there is a possible race condition + wrt these commands being completed before the node is powered off. + A sync is added to ensure that all modifications have been written + to disk, before the API request is sent -- as the last thing. + - | + Extra newlines ('\n') were incorrectly added to the user data content. + This broke the content-type decoding and cloud-init was unable to + proces them. The extra newlines have been removed.