Reverting migration-based allocations leaks allocations if the server is deleted

Note that we could have the same issue in the compute service, for example if the server is deleted during the resize claim and we get to this exception block handler:

https://github.com/openstack/nova/blob/1a226aaa9e8c969ddfdfe198c36f7966b1f692f3/nova/compute/manager.py#L4724

To revert the allocations here:

https://github.com/openstack/nova/blob/1a226aaa9e8c969ddfdfe198c36f7966b1f692f3/nova/compute/manager.py#L4574

Which calls move_allocations which has the same problem described above.

We could also have the same issue if the instance is gone during resize_instance on the source host:

https://github.com/openstack/nova/blob/1a226aaa9e8c969ddfdfe198c36f7966b1f692f3/nova/compute/manager.py#L4896

Or during finish_revert_resize I guess:

https://github.com/openstack/nova/blob/1a226aaa9e8c969ddfdfe198c36f7966b1f692f3/nova/compute/manager.py#L4454

I'm not sure about the fix yet, but we might want callers to optionally tell the move_allocations method if it should require the usage of the target consumer (instance in this case) generation so we don't use a .get() here:

https://github.com/openstack/nova/blob/1a226aaa9e8c969ddfdfe198c36f7966b1f692f3/nova/scheduler/client/report.py#L1886

Actually if the target consumer no longer exists in placement the 'consumer_generation' key won't exist in that allocations response and we'd have to handle it earlier, like this:

https://review.opendev.org/#/c/688832/2/nova/scheduler/client/report.py

I think we can claim this as a regression going back to migration-based allocations added in Queens. Looking at where a resize would fail during prep_resize in Pike, we call this method to remove the 'doubled up' allocations or the instance:

https://github.com/openstack/nova/blob/stable/pike/nova/scheduler/client/report.py#L1028

That's to remove the dest host provider from the allocations created by the scheduler.

In this case if the instance was deleted concurrently and had no allocations, we'd get here and return False:

https://github.com/openstack/nova/blob/stable/pike/nova/scheduler/client/report.py#L1062

And we'd log an error there and here in the resource tracker:

https://github.com/openstack/nova/blob/stable/pike/nova/compute/resource_tracker.py#L1320

But we wouldn't leak any allocations for the deleted instance which is the bug we have since Queens when reverting the allocation swap and the instance is gone.

Related fix proposed to branch: master
Review: https://review.opendev.org/688980

Related fix proposed to branch: master
Review: https://review.opendev.org/688994

Related fix proposed to branch: master
Review: https://review.opendev.org/689013

Fix proposed to branch: master
Review: https://review.opendev.org/689049

Reviewed: https://review.opendev.org/688980
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=24318f8cd4b71f1072c6f194a6f1e04a854acb06
Submitter: Zuul
Branch: master

commit 24318f8cd4b71f1072c6f194a6f1e04a854acb06
Author: Matt Riedemann <email address hidden>
Date: Wed Oct 16 13:08:08 2019 -0400

    Add functional recreate test for bug 1848343

    This adds a functional test to recreate a bug where the
    instance is deleted after conductor has swapped allocations
    to the migration consumer but before casting to compute. In
    this case, the scheduler fails due to NoValidHost which is
    entirely reasonable. The bug is that the conductor task rollback
    code re-creates the allocations on the source node for the
    now-deleted instance and as such those allocations get leaked.

    Note that we have similar exposures in the live migration
    task and reverting allocations when resize fails in the
    compute service. A TODO is left inline to add tests for those
    separately.

    Change-Id: Ifd156ac8789d3fc84d56d400cf1e160e2cd2fbee
    Related-Bug: #1848343

Reviewed: https://review.opendev.org/688994
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=252ee93086a4854c5a5922eec826ffb908f9b283
Submitter: Zuul
Branch: master

commit 252ee93086a4854c5a5922eec826ffb908f9b283
Author: Matt Riedemann <email address hidden>
Date: Wed Oct 16 14:03:59 2019 -0400

    Add live migration recreate test for bug 1848343

    This adds a live migration functional recreate test
    like Ifd156ac8789d3fc84d56d400cf1e160e2cd2fbee is
    for cold migrate/resize.

    Change-Id: I856db36d63779d521fe26b27ef5a12b7a4d3bd91
    Related-Bug: #1848343

Reviewed: https://review.opendev.org/689013
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=760ccb32dbbb0baf40ce4ea01977f9886d0d3a3b
Submitter: Zuul
Branch: master

commit 760ccb32dbbb0baf40ce4ea01977f9886d0d3a3b
Author: Matt Riedemann <email address hidden>
Date: Wed Oct 16 15:23:50 2019 -0400

    Add compute side revert allocation test for bug 1848343

    This adds a functional recreate test for a scenario where
    reverting migration-based allocations during resize failure
    in the compute service results in leaking allocations for a
    deleted server.

    Change-Id: Iac4dd9feebb1a405826c95cb6b046b82c61140a2
    Related-Bug: #1848343

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.opendev.org/689049
Reason: I'm not working on this.