Errors in finish_revert_resize can leave migration.dest_compute pointing at source_compute

Bug #1818730 reported by Matt Riedemann on 2019-03-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Medium
Matt Riedemann
Rocky
Medium
Matt Riedemann
Stein
Medium
Matt Riedemann

Bug Description

Because of this code in finish_revert_resize:

https://github.com/openstack/nova/blob/8cdb8cc7c56b574382b9a9fff662cc95e78136a2/nova/compute/manager.py#L4121

And the @errors_out_migration decorator on the method, if something fails after that line we will save the migration object changes which would leave the dest_compute pointing at the source_compute, which could be very confusing when trying to debug.

The comment says the field is set temporarily but it's not really temporary if the migration changes are saved like in that decorator.

Matt Riedemann (mriedem) on 2019-03-05
Changed in nova:
status: New → Triaged
importance: Undecided → Medium
Matt Riedemann (mriedem) on 2019-03-05
Changed in nova:
assignee: nobody → Matt Riedemann (mriedem)

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

Changed in nova:
status: Triaged → In Progress

Reviewed: https://review.openstack.org/641137
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=855b5546564f0ed96243623b67b1322e9b891c8b
Submitter: Zuul
Branch: master

commit 855b5546564f0ed96243623b67b1322e9b891c8b
Author: Matt Riedemann <email address hidden>
Date: Tue Mar 5 16:39:51 2019 -0500

    Temporarily mutate migration object in finish_revert_resize

    As the comment in the code suggests, when calling migrate_instance_finish
    in finish_revert_resize we need to temporarily set the migration
    object dest_compute to the source_compute since we are moving port
    bindings from the dest back to the source. However, this is not
    really a temporary change to the object if something fails after
    this because the @errors_out_migration decorator will persist the
    change which could be confusing later if trying to debug this migration
    from the API and the dest_compute and source_compute have the same value.

    This fixes the issue by using the temporary_mutation utility.

    Also, the obj_to_primitive dance in here is removed since it is not
    necessary as migrate_instance_finish handles a Migration object.

    Change-Id: I312d61383345ea0ac1ab0c277b4c468e6aa94656
    Closes-Bug: #1818730

Changed in nova:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/648688
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=bcb42a43a9f12d929ec7025f29bd2e3d482cd406
Submitter: Zuul
Branch: stable/stein

commit bcb42a43a9f12d929ec7025f29bd2e3d482cd406
Author: Matt Riedemann <email address hidden>
Date: Tue Mar 5 16:39:51 2019 -0500

    Temporarily mutate migration object in finish_revert_resize

    As the comment in the code suggests, when calling migrate_instance_finish
    in finish_revert_resize we need to temporarily set the migration
    object dest_compute to the source_compute since we are moving port
    bindings from the dest back to the source. However, this is not
    really a temporary change to the object if something fails after
    this because the @errors_out_migration decorator will persist the
    change which could be confusing later if trying to debug this migration
    from the API and the dest_compute and source_compute have the same value.

    This fixes the issue by using the temporary_mutation utility.

    Also, the obj_to_primitive dance in here is removed since it is not
    necessary as migrate_instance_finish handles a Migration object.

    Change-Id: I312d61383345ea0ac1ab0c277b4c468e6aa94656
    Closes-Bug: #1818730
    (cherry picked from commit 855b5546564f0ed96243623b67b1322e9b891c8b)

Reviewed: https://review.openstack.org/648691
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=f1ac5183d43459614ba5a356c2c7b50251fcd94f
Submitter: Zuul
Branch: stable/rocky

commit f1ac5183d43459614ba5a356c2c7b50251fcd94f
Author: Matt Riedemann <email address hidden>
Date: Tue Mar 5 16:39:51 2019 -0500

    Temporarily mutate migration object in finish_revert_resize

    As the comment in the code suggests, when calling migrate_instance_finish
    in finish_revert_resize we need to temporarily set the migration
    object dest_compute to the source_compute since we are moving port
    bindings from the dest back to the source. However, this is not
    really a temporary change to the object if something fails after
    this because the @errors_out_migration decorator will persist the
    change which could be confusing later if trying to debug this migration
    from the API and the dest_compute and source_compute have the same value.

    This fixes the issue by using the temporary_mutation utility.

    Also, the obj_to_primitive dance in here is removed since it is not
    necessary as migrate_instance_finish handles a Migration object.

    Conflicts:
          nova/tests/unit/compute/test_compute_mgr.py

    NOTE(mriedem): The conflict is due to not having change
    I0851e2d54a1fdc82fe3291fb7e286e790f121e92 in Rocky.

    Change-Id: I312d61383345ea0ac1ab0c277b4c468e6aa94656
    Closes-Bug: #1818730
    (cherry picked from commit 855b5546564f0ed96243623b67b1322e9b891c8b)
    (cherry picked from commit bcb42a43a9f12d929ec7025f29bd2e3d482cd406)

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

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

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

Other bug subscribers