diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py index e87f731855..c2d97df62f 100644 --- a/ironic/common/pxe_utils.py +++ b/ironic/common/pxe_utils.py @@ -668,42 +668,52 @@ def get_instance_image_info(task, ipxe_enabled=False): return image_info labels = ('kernel', 'ramdisk') + image_properties = None d_info = deploy_utils.get_image_instance_info(node) if not (i_info.get('kernel') and i_info.get('ramdisk')): + # NOTE(rloo): If both are not specified in instance_info + # we won't use any of them. We'll use the values specified + # with the image, which we assume have been set. glance_service = service.GlanceImageService(context=ctx) - iproperties = glance_service.show(d_info['image_source'])['properties'] + image_properties = glance_service.show( + d_info['image_source'])['properties'] for label in labels: - i_info[label] = str(iproperties[label + '_id']) + i_info[label] = str(image_properties[label + '_id']) node.instance_info = i_info node.save() anaconda_labels = () if deploy_utils.get_boot_option(node) == 'kickstart': - # stage2 - Installer stage2 squashfs image - # ks_template - Anaconda kickstart template + # stage2: installer stage2 squashfs image + # ks_template: anaconda kickstart template # ks_cfg - rendered ks_template anaconda_labels = ('stage2', 'ks_template', 'ks_cfg') - if not (i_info.get('stage2') and i_info.get('ks_template')): - iproperties = glance_service.show( - d_info['image_source'] - )['properties'] - for label in anaconda_labels: + if not i_info.get('stage2') or not i_info.get('ks_template'): + if not image_properties: + glance_service = service.GlanceImageService(context=ctx) + image_properties = glance_service.show( + d_info['image_source'])['properties'] + if not i_info.get('ks_template'): # ks_template is an optional property on the image - if (label == 'ks_template' - and not iproperties.get('ks_template')): - i_info[label] = CONF.anaconda.default_ks_template - elif label == 'ks_cfg': - i_info[label] = '' - elif label == 'stage2' and 'stage2_id' not in iproperties: - msg = ("stage2_id property missing on the image. " - "The anaconda deploy interface requires stage2_id " - "property to be associated with the os image. ") + if 'ks_template' not in image_properties: + i_info['ks_template'] = CONF.anaconda.default_ks_template + else: + i_info['ks_template'] = str( + image_properties['ks_template']) + if not i_info.get('stage2'): + if 'stage2_id' not in image_properties: + msg = ("'stage2_id' property is missing from the OS image " + "%s. The anaconda deploy interface requires this " + "to be set with the OS image or in instance_info. " + % d_info['image_source']) raise exception.ImageUnacceptable(msg) else: - i_info[label] = str(iproperties['stage2_id']) + i_info['stage2'] = str(image_properties['stage2_id']) + # NOTE(rloo): This is internally generated; cannot be specified. + i_info['ks_cfg'] = '' - node.instance_info = i_info - node.save() + node.instance_info = i_info + node.save() for label in labels + anaconda_labels: image_info[label] = ( @@ -1090,16 +1100,17 @@ def validate_kickstart_file(ks_cfg): return with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.cfg') as ks_file: - ks_file.writelines(ks_cfg) + dir=CONF.tempdir, suffix='.cfg', mode='wt') as ks_file: + ks_file.write(ks_cfg) + ks_file.flush() try: - result = utils.execute( + utils.execute( 'ksvalidator', ks_file.name, check_on_exit=[0], attempts=1 ) - except processutils.ProcessExecutionError: + except processutils.ProcessExecutionError as e: msg = _(("The kickstart file generated does not pass validation. " - "The ksvalidator tool returned following error(s): %s") % - (result)) + "The ksvalidator tool returned the following error: %s") % + (e)) raise exception.InvalidKickstartFile(msg) @@ -1197,17 +1208,14 @@ def cache_ramdisk_kernel(task, pxe_info, ipxe_enabled=False): else: path = os.path.join(CONF.pxe.tftp_root, node.uuid) ensure_tree(path) - # anconda deploy will have 'stage2' as one of the labels in pxe_info dict + # anaconda deploy will have 'stage2' as one of the labels in pxe_info dict if 'stage2' in pxe_info.keys(): - # stage2 will be stored in ipxe http directory. So make sure they - # exist. - ensure_tree( - get_file_path_from_label( - node.uuid, - CONF.deploy.http_root, - 'stage2' - ) - ) + # stage2 will be stored in ipxe http directory so make sure the + # directory exists. + file_path = get_file_path_from_label(node.uuid, + CONF.deploy.http_root, + 'stage2') + ensure_tree(os.path.dirname(file_path)) # ks_cfg is rendered later by the driver using ks_template. It cannot # be fetched and cached. t_pxe_info.pop('ks_cfg') diff --git a/ironic/drivers/modules/ks.cfg.template b/ironic/drivers/modules/ks.cfg.template index 40552377be..f7ef75e1cd 100644 --- a/ironic/drivers/modules/ks.cfg.template +++ b/ironic/drivers/modules/ks.cfg.template @@ -18,7 +18,7 @@ autopart # Downloading and installing OS image using liveimg section is mandatory liveimg --url {{ ks_options.liveimg_url }} -# Following %pre, %onerror and %trackback sections are mandatory +# Following %pre and %onerror sections are mandatory %pre /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 @@ -27,10 +27,6 @@ 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 -%traceback -/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 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 }} diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 50d962fcf6..6eccc1938c 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -85,6 +85,9 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin, # NOTE(TheJulia): If this was any other interface, we would # unconfigure tenant networks, add provisioning networks, etc. task.driver.storage.attach_volumes(task) + node.instance_info = deploy_utils.build_instance_info_for_deploy( + task) + node.save() if node.provision_state in (states.ACTIVE, states.UNRESCUING): # In the event of takeover or unrescue. task.driver.boot.prepare_instance(task) @@ -123,13 +126,13 @@ class PXEAnacondaDeploy(agent_base.AgentBaseMixin, agent_base.HeartbeatMixin, agent_base.log_and_raise_deployment_error(task, msg) try: + task.process_event('resume') self.clean_up(task) manager_utils.node_power_action(task, states.POWER_OFF) task.driver.network.remove_provisioning_network(task) task.driver.network.configure_tenant_networks(task) manager_utils.node_power_action(task, states.POWER_ON) - node.provision_state = states.ACTIVE - node.save() + task.process_event('done') except Exception as e: msg = (_('Error rebooting node %(node)s after deploy. ' 'Error: %(error)s') % diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index e1559b6f69..d999a8f7ae 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/ironic/tests/unit/drivers/modules/test_pxe.py @@ -1022,15 +1022,23 @@ class PXEAnacondaDeployTestCase(db_base.DbTestCase): mock_prepare_ks_config.assert_called_once_with(task, image_info, anaconda_boot=True) + @mock.patch.object(deploy_utils, 'build_instance_info_for_deploy', + autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) - def test_prepare(self, mock_prepare_instance): + def test_prepare(self, mock_prepare_instance, mock_build_instance): + node = self.node node.provision_state = states.DEPLOYING node.instance_info = {} node.save() + updated_instance_info = {'image_url': 'foo'} + mock_build_instance.return_value = updated_instance_info with task_manager.acquire(self.context, node.uuid) as task: task.driver.deploy.prepare(task) self.assertFalse(mock_prepare_instance.called) + mock_build_instance.assert_called_once_with(task) + node.refresh() + self.assertEqual(updated_instance_info, node.instance_info) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) def test_prepare_active(self, mock_prepare_instance): diff --git a/releasenotes/notes/anaconda-deploy-more-fixes-58d996c7031c8c4b.yaml b/releasenotes/notes/anaconda-deploy-more-fixes-58d996c7031c8c4b.yaml new file mode 100644 index 0000000000..9298c30363 --- /dev/null +++ b/releasenotes/notes/anaconda-deploy-more-fixes-58d996c7031c8c4b.yaml @@ -0,0 +1,33 @@ +--- +fixes: + - | + Fixes the logic for the anaconda deploy interface. If the + ironic node's instance_info doesn't have both 'stage2' and + 'ks_template' specified, we weren't using the instance_info + at all. This has been fixed to use the instance_info if it + was specified. Otherwise, 'stage2' is taken from the + image's properties (assumed that it is set there). + 'ks_template' value is from the image properties if specified + there (since it is optional); else we use the config setting + '[anaconda] default_ks_template'. + - | + For the anaconda deploy interface, the 'stage2' directory was + incorrectly being created using the full path of the stage2 file; + this has been fixed. + - | + The anaconda deploy interface expects the node's instance_info + to be populated with the 'image_url'; this is now populated + (via PXEAnacondaDeploy's prepare() method). + - | + For the anaconda deploy interface, when the deploy was finished + and the bm node was being rebooted, the node's provision state was + incorrectly being set to 'active' -- the provisioning state-machine + mechanism now handles that. + - | + For the anaconda deploy interface, the code that was doing the + validation of the kickstart file was incorrect and resulted in + errors; this has been addressed. + - | + For the anaconda deploy interface, the '%traceback' section in the + packaged 'ks.cfg.template' file is deprecated and fails validation, + so it has been removed.