Errors in finish_revert_resize can leave migration.dest_compute pointing at source_compute

[Matt Riedemann]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/stein
Review: https://review.openstack.org/648688

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.openstack.org/648691

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.