ipmitool driver could needlessly check power status even after power action fails

[Arun S A G]

It looks like the when someone wants to switch on a node. The driver does a 'ipmitool power on' command first and then runs 'ipmitool power status' to check if the power state changed to target power state. It looks like if the initial 'ipmitool power on' command fails for some reason (returns code > 0, or process execution error) The code still keeps doing 'power status' in a loop needlessly
https://github.com/openstack/ironic/blob/master/ironic/drivers/modules/ipmitool.py#L511

If the power action (on/off) itself fails, there is no need to check power status. We either have to retry the power action or error out.

[Jay Faulkner (jason-oldos)]

Thanks for the report.

[Julian Edwards (julian-edwards)]

https://review.openstack.org/#/c/449454/

[OpenStack Infra (hudson-openstack)]

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

commit ee5d4942a1c33736ffe05ec01619142be400c2f4
Author: Julian Edwards <email address hidden>
Date: Fri Mar 24 17:32:38 2017 +1000

    Don't retry power status if power action fails

    The old code blindly required power status even if the power action
    failed. Now, it will retry the power action only when it detects a
    retryable failure, and will only poll for power status if the power
    action is successful. This patch also moves the logic for handling
    waiting for power status into the conductor so that the logic is
    standardised between drivers.

    Change-Id: Ib48056e05d359848386ac057b58921f40b7bdd60
    Co-Authored-By: Sam Betts <email address hidden>
    Related-Bug: #1675529
    Closes-Bug: #1692895

[Ruby Loo (rloo)]

This is fixed by https://review.openstack.org/449454. (If not, please re-open.)

[John L. Villalovos (happycamp)]

The problem with this is that some BMC/IPMI systems give an error when setting the state to the already existing state.

See: https://bugs.launchpad.net/ironic/+bug/1718794