virt_driver.detach_volume rollback calls in attach_volume failures uses wrong method signature

[Matt Riedemann]

Seen here:

https://github.com/openstack/nova/blob/64e503d82fe5966cb50b650b3309a7fc36c63cda/nova/virt/block_device.py#L473

and here:

https://github.com/openstack/nova/blob/64e503d82fe5966cb50b650b3309a7fc36c63cda/nova/virt/block_device.py#L570

Those are using the wrong method signature since 'context' was added to the driver detach_volume method signature in change:

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/563213
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=717d09149ebea4d6451efb262f2b300cdbcf2208
Submitter: Zuul
Branch: master

commit 717d09149ebea4d6451efb262f2b300cdbcf2208
Author: Matt Riedemann <email address hidden>
Date: Fri Apr 20 14:18:39 2018 -0400

    Fix detach_volume calls when rolling back a failed attach

    Change I751fcb7532679905c4279744919c6cce84a11eb4 modified
    the ComputeDriver.detach_volume method signature to pass
    the RequestContext as the first argument. This was missed
    in the volume attach rollback code where the driver.detach_volume
    method is called. It was missed because that is called
    in a try/except block which catches generic Exception, which
    also means that the unit test covering that flow, which
    raised an Exception, was not failing.

    This change fixes the code bug and re-writes the test to
    use mock rather than mox and explicitly assert the calls made.

    An alternative to this would be changing the generic
    Exception handling in the rollback code, however, the
    virt drivers raise things other than NovaException, like
    os-brick exceptions and libvirtError, so refactoring that
    error handling is non-trivial.

    Change-Id: I9bafd4c47489b61973302310720b11ec1b5e0374
    Closes-Bug: #1765742

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/567232
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0f95b6a898c238b29987001e72447943351d5a77
Submitter: Zuul
Branch: stable/queens

commit 0f95b6a898c238b29987001e72447943351d5a77
Author: Matt Riedemann <email address hidden>
Date: Fri Apr 20 14:18:39 2018 -0400

    Fix detach_volume calls when rolling back a failed attach

    Change I751fcb7532679905c4279744919c6cce84a11eb4 modified
    the ComputeDriver.detach_volume method signature to pass
    the RequestContext as the first argument. This was missed
    in the volume attach rollback code where the driver.detach_volume
    method is called. It was missed because that is called
    in a try/except block which catches generic Exception, which
    also means that the unit test covering that flow, which
    raised an Exception, was not failing.

    This change fixes the code bug and re-writes the test to
    use mock rather than mox and explicitly assert the calls made.

    An alternative to this would be changing the generic
    Exception handling in the rollback code, however, the
    virt drivers raise things other than NovaException, like
    os-brick exceptions and libvirtError, so refactoring that
    error handling is non-trivial.

    Change-Id: I9bafd4c47489b61973302310720b11ec1b5e0374
    Closes-Bug: #1765742
    (cherry picked from commit 717d09149ebea4d6451efb262f2b300cdbcf2208)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 17.0.4 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 18.0.0.0b2 development milestone.