AgentRescue validation fails to get the signature of PXEBoot.prepare_ramdisk

Bug #1746730 reported by Hironori Shiina
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
High
Jim Rollenhagen
ironic-lib
Fix Released
High
John L. Villalovos

Bug Description

With agent rescue interface and pxe boot interface, node driver validation fails for rescue interface as follows.

$ openstack baremetal node validate node-0
+------------+--------+---------------------------------------------------------------------------------------------------------------------------------------------+
| Interface | Result | Reason |
+------------+--------+---------------------------------------------------------------------------------------------------------------------------------------------+
<snip>
| rescue | False | boot interface implementation 'of 'prepare_ramdisk' and/or 'clean_up_ramdisk' with 'mode' argument' is not supported by hardware type ipmi. |
| storage | True | |
+------------+--------+---------------------------------------------------------------------------------------------------------------------------------------------+

This validation sees the signature of "prepare_ramdisk" of boot interface. "reflection.get_signature" uses "funcsigs.signature" for python2[3]. "funcsigs.signature" expects __wrapped__ when a function is wrapped by a decorator[4]. It seems that six.wraps is required to set __wrapped__[5]. However, METRICS.timer dosn't use it[6].

[1] https://github.com/openstack/ironic/blob/4624c572e21f136db5df8ebcd0ddae03eed71a59/ironic/drivers/modules/agent.py#L790
[2] https://github.com/openstack/ironic/blob/4624c572e21f136db5df8ebcd0ddae03eed71a59/ironic/drivers/modules/pxe.py#L472
[3] https://github.com/openstack/oslo.utils/blob/master/oslo_utils/reflection.py#L38
[4] https://github.com/aliles/funcsigs/blob/master/funcsigs/__init__.py#L87
[5] https://pythonhosted.org/six/
[6] https://github.com/openstack/ironic-lib/blob/master/ironic_lib/metrics.py#L58

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ironic (master)

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

Changed in ironic:
assignee: nobody → Jim Rollenhagen (jim-rollenhagen)
status: New → In Progress
Ruby Loo (rloo)
Changed in ironic:
importance: Undecided → High
Changed in ironic-lib:
assignee: nobody → John L. Villalovos (happycamp)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic (master)

Reviewed: https://review.openstack.org/539961
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=954a23482082f3dcfb57cfbb71e5d61f05c7fbce
Submitter: Zuul
Branch: master

commit 954a23482082f3dcfb57cfbb71e5d61f05c7fbce
Author: Jim Rollenhagen <email address hidden>
Date: Thu Feb 1 09:00:16 2018 -0500

    Remove mode argument from boot.(prepare|clean_up)_ramdisk

    Ideally, the boot interface shouldn't care if it's booting an image for
    deploy or rescue. The first step to unwinding this is not passing the
    mode argument into the boot interface - for now, we infer it from the
    state. Also stop validating whether the boot interface methods have a
    mode argument, as this is totally broken anyway (due to the decorator on
    the method). The rest of the boot interface's knowledge about deploy vs
    rescue can be eliminated in the future, as we shouldn't rework too much
    of that during feature freeze.

    We fix the bug in validation this way for now, for two reasons:

    * This argument really doesn't belong, and it's only been on master for
      six days. Get it out before people use it.
    * Library updates are currently frozen; fixing the decorator isn't an
      option right now.

    Change-Id: Icdeb870e9fd9cf836ff7ab97624189c8436adaba
    Closes-Bug: #1746730

Changed in ironic:
status: In Progress → Fix Released
Revision history for this message
Dmitry Tantsur (divius) wrote :
Changed in ironic-lib:
importance: Undecided → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic-lib (master)

Reviewed: https://review.openstack.org/540031
Committed: https://git.openstack.org/cgit/openstack/ironic-lib/commit/?id=cdea1086d64d54b07884caa63103258fbf3047ed
Submitter: Zuul
Branch: master

commit cdea1086d64d54b07884caa63103258fbf3047ed
Author: John L. Villalovos <email address hidden>
Date: Thu Feb 1 08:38:26 2018 -0800

    Use six.wraps() for Metrics so decorated methods can be inspected

    Previously it was using functools.wraps() to create the decorators.
    The problem is that we can't use the
    oslo_utils.reflection.get_signature() function to get the correct
    signature.

    Change it to use six.wraps() which will add the '__wrapped__'
    attribute which will be used when calling get_signature() and return
    the signature of the decorated function.

    Added unit tests to show we are able to see the signature of a wrapped
    function.

    Closes-Bug: #1746730
    Change-Id: I75428e948b64b3b6758d31678a80d158a11c6beb

Changed in ironic-lib:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ironic-lib (stable/queens)

Fix proposed to branch: stable/queens
Review: https://review.openstack.org/540985

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/ironic 10.1.0

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ironic-lib (stable/queens)

Reviewed: https://review.openstack.org/540985
Committed: https://git.openstack.org/cgit/openstack/ironic-lib/commit/?id=59e08fb16f12628a59e29da1b3bd932a5bfc00dd
Submitter: Zuul
Branch: stable/queens

commit 59e08fb16f12628a59e29da1b3bd932a5bfc00dd
Author: John L. Villalovos <email address hidden>
Date: Thu Feb 1 08:38:26 2018 -0800

    Use six.wraps() for Metrics so decorated methods can be inspected

    Previously it was using functools.wraps() to create the decorators.
    The problem is that we can't use the
    oslo_utils.reflection.get_signature() function to get the correct
    signature.

    Change it to use six.wraps() which will add the '__wrapped__'
    attribute which will be used when calling get_signature() and return
    the signature of the decorated function.

    Added unit tests to show we are able to see the signature of a wrapped
    function.

    Closes-Bug: #1746730
    Change-Id: I75428e948b64b3b6758d31678a80d158a11c6beb
    (cherry picked from commit cdea1086d64d54b07884caa63103258fbf3047ed)

tags: added: in-stable-queens
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/ironic-lib 2.12.1

This issue was fixed in the openstack/ironic-lib 2.12.1 release.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/ironic-lib 2.13.0

This issue was fixed in the openstack/ironic-lib 2.13.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.