Inconsistency deploy whole disk images in UEFI and BIOS mode

[Lucas Alvares Gomes]

When deploying whole disk images in UEFI mode there's a check that prevents the deployment unless the boot_option capability is explicitly set to "local" (for local boot)[0]. This is not checked for non-UEFI deployment and I don't see the difference, both could be deployed w/o any problems because ironic already assumes local boot for whole disk images [1].

That said, the iLO driver seems to add yet another check [2], I don't understand the specifics of iLO so I would suggest to move this check to inside the iLO driver and have it performed only for that driver.

[0] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe.py#L220-L229
[1] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L338-L341
[2] https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ilo/deploy.py#L338-L346

=== UPDATE ===

This seems to be triggered by an inconsistency between the agent_* drivers and the pxe_* drivers.

For agent_* drivers, when deploying a whole disk image without explicit setting the boot_option capability Ironic will assume booting from disk.

For pxe_* drivers, when deploying a whole disk image without explicit setting the boot_option capability Ironic will assume PXE boot + chainloading to the local disk.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/375481

[Shivanand Tendulker (stendulker)]

This was the reasoning behind these checks. Please let me know if I missed your point.

1. https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/pxe.py#L220-L229
This check is not required for iLO drivers. It was in place as UEFI bootloaders are not supporting chainloading.

2. https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ilo/deploy.py#L338-L346
This is extension to the above check in pxe_ilo driver as iLO drivers compute the boot mode if it is not specified in the node instance_info and properties/capabilities.

3. https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/agent.py#L338-L341
The chainloading limitation does not apply to agent driver.

[Lucas Alvares Gomes (lucasagomes)]

Thanks @Shivanand, I've updated the bug description to reflect that difference. Now, I'm not sure which behavior we should keep but it would be great if we could make both drivers consistent again.

I personally don't see much advantage in PXE boot + chainload to local disk, because it would require the local disk to contain a bootloader anyway. Why not just boot from it directly ?

What do you think ?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/410784

[OpenStack Infra (hudson-openstack)]

Change abandoned by Lucas Alvares Gomes (<email address hidden>) on branch: master
Review: https://review.openstack.org/375481
Reason: superseded by https://review.openstack.org/#/c/410784/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/410784
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=e63cd0834b0f05bd2b816d6e1f65cf2177358b55
Submitter: Jenkins
Branch: master

commit e63cd0834b0f05bd2b816d6e1f65cf2177358b55
Author: Lucas Alvares Gomes <email address hidden>
Date: Wed Dec 14 14:24:52 2016 +0000

    Remove check for UEFI + Whole disk images

    This patch is removing a conditional preventing whole disk images from
    being deployed in UEFI mode without local boot.

    This conditional is an inconsistency because whole disk images being
    deployed in non-UEFI mode don't need to explicitly set the boot_option
    from "local", also, in Ironic when deploying a whole disk image we
    already assume local boot by default.

    Change-Id: I678e3547397eac3199d8ff670fe281e20b2cd2c0
    Closes-Bug: #1627022

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/ironic 7.0.0 release.