openstack/ironic
Code Issues Proposed changes

More fixes for anaconda deploy interface

Browse Source 
The anaconda deploy interface has a few issues that are
addressed here:

- fixes logic in get_instance_image_info() for anaconda. If the
  ironic node's instance_info doesn't have both 'stage2' and
  'ks_template' specified, we weren't using any values from the
  instance_info. This has been fixed to use values from
  instance_info if specified. Otherwise, they are set as follows:
  The 'stage2' value is taken from the image properties.
  We use the value for 'ks_template' if it is specified in the
  image properties. If not (since it is optional), we use the
  config option's '[anaconda]default_ks_template' value.
  setting.
- For anaconda's stage2 directory, we were incorrectly creating a
  directory using the full path of the stage2 file. It now
  correctly creates the right directory.
- The anaconda deploy interface expects the node's instance_info
  to be populated with the 'image_url'; added code to do that in
  PXEAnacondaDeploy's prepare() method.
- When the deploy is finished and the bm node is being rebooted,
  we incorrectly set the node's provision state to 'active'
  instead of doing it via the provisioning state machine mechanism.
- The code that was doing the validation of the kickstart file was
  incorrect and resulted in errors; this has been addressed.
- The '%traceback' section in the packaged 'ks.cfg.template' file
  is deprecated and fails validation, so it has been removed.

Change-Id: I953e948bcfa108d4c8e7b145da2f52b652e52a10
This commit is contained in:
Ruby Loo 2022-02-04 21:26:26 +00:00
parent a4a89d6b20
commit 06cc5d47dc
5 changed files with 93 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
ironic/common/pxe_utils.py
View File

@ -668,39 +668,49 @@ 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()
@ -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,
# 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'
)
)
'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')

6
ironic/drivers/modules/ks.cfg.template
View File

@ -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 }}

7
ironic/drivers/modules/pxe.py
View File

@ -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') %

10
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -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):

33
releasenotes/notes/anaconda-deploy-more-fixes-58d996c7031c8c4b.yaml Normal file
View File

@ -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.