Allocations are not managed properly in all evacuate scenarios

[Matt Riedemann]

Evacuate has some gaps with respect to dealing with resource allocations in Placement:

1. If the user specifies a host with evacuate, conductor bypasses the scheduler and we don't create allocations on the destination host:

https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/conductor/manager.py#L749

This could eventually lead to the claim failing on the destination compute:

https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/manager.py#L2795

This is similar to bug 1712008 where forcing a host during live migration bypasses the scheduler so allocations are not created in placement on the destination node. Before Pike this would be handled via the update_available_resource periodic task in the compute service which would 'heal' the allocations for instances tracked on a given node, but that is no longer happening once all computes are running Pike code due to this change: https://review.openstack.org/#/c/491012/

2. If the user does not specify a host with evacuate, conductor will ask the scheduler to pick a host, which will also create allocations for that host via the scheduler. If the claim (or rebuild) fails on the destination node, we don't cleanup the allocation on the destination node even if the instance isn't spawned on it:

https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/manager.py#L2795

^ is pretty obvious that we should cleanup because the claim for resources failed.

This generic exception handling is harder to know if we should cleanup though since we'd need to know if the guest was spawned on it:

https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/manager.py#L2812

But since we don't set the instance.host/node to the current host/node it won't be reported there anyway:

https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/manager.py#L2822-L2824

[Balazs Gibizer (balazs-gibizer)]

Is there a difference between providing setting the force flag in the API request? I guess in case of force=False the scheduler is involved somehow.

[Matt Riedemann (mriedem)]

Correct, it's like with the live migration API and it's force option:

https://developer.openstack.org/api-ref/compute/#id99

The force option tells conductor to bypass the scheduler when a host is provided.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/499678

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/499718

[Matt Riedemann (mriedem)]

Actually #2 might be OK if the move claim fails because of how ResourceTracker.drop_move_claim works. We should have a functional test to verify this though.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/499874

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/499877

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499678
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=f34b487db4864787ad92f5a7d6568f31812f1ddb
Submitter: Jenkins
Branch: master

commit f34b487db4864787ad92f5a7d6568f31812f1ddb
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 11:23:28 2017 -0400

    Add recreate test for forced host evacuate not setting dest allocations

    This adds a functional recreate test for bug 1713786 where
    allocations are not created against the forced destination host
    during an evacuation.

    This asserts the faulty behavior so that a later patch which fixes
    the bug can uncomment the assertions for correct behavior.

    Change-Id: I55bf64a95f6b1b781656920380dc38bda864bd14
    Related-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499718
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=34f8a351502418d30f137ad49e948802d6b2ab3c
Submitter: Jenkins
Branch: master

commit 34f8a351502418d30f137ad49e948802d6b2ab3c
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 12:30:15 2017 -0400

    Refactor out claim_resources_on_destination into a utility

    We need this method for a similar fix in evacuate, so this
    change pulls the method out of the LiveMigrationTask and moves
    it into scheduler utils along with unit tests.

    The only functional difference is that instead of raising
    MigrationPreCheckError, it now raises NoValidHost, which for
    live migration ends up being the same result when it's handled
    in ComputeTaskManager.

    Change-Id: Ie63a4798d420c39815e294843e02ab6473cfded2
    Related-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/pike
Review: https://review.openstack.org/503159

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/pike
Review: https://review.openstack.org/503160

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/503161

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/pike
Review: https://review.openstack.org/503162

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/pike
Review: https://review.openstack.org/503163

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/503165

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499399
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d564266a019cb009ece1a63e5d544698f2bf74d1
Submitter: Jenkins
Branch: master

commit d564266a019cb009ece1a63e5d544698f2bf74d1
Author: Matt Riedemann <email address hidden>
Date: Wed Aug 30 15:05:44 2017 -0400

    Create allocations against forced dest host during evacuate

    If a host is specified during an evacuate with the force=True
    flag, conductor bypasses the scheduler. Since the scheduler
    is what creates the "doubled up" allocation on the destination
    node, and the scheduler is bypassed in the force case, we have
    to create the allocations against the destination node in
    conductor directly.

    The unit tests cover the failure scenarios. The functional
    test covers the happy path.

    This is a short-term backportable fix. Long-term we'll likely
    want to call the scheduler even in the 'force' scenario but pass
    a flag to the scheduler to tell it to skip the filters but still
    create the allocation on the destination node so we don't have
    to duplicate that in conductor.

    Change-Id: I6590f0eda4ec4996543ad40d8c2640b83fc3dd9d
    Partial-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499874
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=6ed80ddcdd3f9e40f0ada581ac7bd524ec48135b
Submitter: Jenkins
Branch: master

commit 6ed80ddcdd3f9e40f0ada581ac7bd524ec48135b
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 22:47:07 2017 -0400

    Add recreate test for evacuate claim failure

    This adds a functional recreate test for when the MoveClaim
    fails on the destination node and the allocation on the
    destination node is not cleaned up. This is because the
    MoveClaim fails in it's constructor so it never exits the
    Claim context manager to call the drop_move_claim which
    would remove the destination node allocation.

    Eventually we'll have to manually remove the destination
    node allocation in the ComputeManager.rebuild_instance method.

    Change-Id: I8900ace4436c4837beb8b4eb1e1d05905efc6dce
    Related-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499877
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=30946f9a5eeea839631cdb1dba9c26d45f7a8d00
Submitter: Jenkins
Branch: master

commit 30946f9a5eeea839631cdb1dba9c26d45f7a8d00
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 22:56:59 2017 -0400

    Add a test to make sure failed evacuate cleans up dest allocation

    If we actually make the MoveClaim but the evacuation fails
    in the virt driver, the drop_move_claim via the MoveClaim.abort
    will remove the destination node allocation. This adds a functional
    test to show that works.

    Change-Id: Ib58c487e97a041b8498746e8a276efffee239c56
    Related-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/499878
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=5bc137f7ebc266f8a73d6febc7c10d8d648924e0
Submitter: Jenkins
Branch: master

commit 5bc137f7ebc266f8a73d6febc7c10d8d648924e0
Author: Matt Riedemann <email address hidden>
Date: Fri Sep 1 09:58:03 2017 -0400

    Remove dest node allocation if evacuate MoveClaim fails

    If the MoveClaim during an evacuate fails we need to
    remove the destination node allocation since the
    ResourceTracker isn't going to do it.

    This also fixes test_rebuild_server_exc which was faking
    a rebuild failure by raising an error that would not
    actually happen during a rebuild. ComputeResourcesUnavailable
    only happens if a claim fails, and for a rebuild, which
    is on the same host that the instance is already on, doesn't
    involve a claim attempt.

    Change-Id: I59ff47bf773c9831d0f44e1caf7877fe08c911c3
    Closes-Bug: #1713786

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503159
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=5ee7f9d5c8bd53394ef73a52171b4ded451be45e
Submitter: Jenkins
Branch: stable/pike

commit 5ee7f9d5c8bd53394ef73a52171b4ded451be45e
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 11:23:28 2017 -0400

    Add recreate test for forced host evacuate not setting dest allocations

    This adds a functional recreate test for bug 1713786 where
    allocations are not created against the forced destination host
    during an evacuation.

    This asserts the faulty behavior so that a later patch which fixes
    the bug can uncomment the assertions for correct behavior.

    Change-Id: I55bf64a95f6b1b781656920380dc38bda864bd14
    Related-Bug: #1713786
    (cherry picked from commit f34b487db4864787ad92f5a7d6568f31812f1ddb)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503160
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=b5ea0d1231009ba1f4a9ba87ecfc05e489c4017f
Submitter: Jenkins
Branch: stable/pike

commit b5ea0d1231009ba1f4a9ba87ecfc05e489c4017f
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 12:30:15 2017 -0400

    Refactor out claim_resources_on_destination into a utility

    We need this method for a similar fix in evacuate, so this
    change pulls the method out of the LiveMigrationTask and moves
    it into scheduler utils along with unit tests.

    The only functional difference is that instead of raising
    MigrationPreCheckError, it now raises NoValidHost, which for
    live migration ends up being the same result when it's handled
    in ComputeTaskManager.

    Change-Id: Ie63a4798d420c39815e294843e02ab6473cfded2
    Related-Bug: #1713786
    (cherry picked from commit 34f8a351502418d30f137ad49e948802d6b2ab3c)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503161
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=21150948246d52060a38e93f98982c2e5c9b347d
Submitter: Jenkins
Branch: stable/pike

commit 21150948246d52060a38e93f98982c2e5c9b347d
Author: Matt Riedemann <email address hidden>
Date: Wed Aug 30 15:05:44 2017 -0400

    Create allocations against forced dest host during evacuate

    If a host is specified during an evacuate with the force=True
    flag, conductor bypasses the scheduler. Since the scheduler
    is what creates the "doubled up" allocation on the destination
    node, and the scheduler is bypassed in the force case, we have
    to create the allocations against the destination node in
    conductor directly.

    The unit tests cover the failure scenarios. The functional
    test covers the happy path.

    This is a short-term backportable fix. Long-term we'll likely
    want to call the scheduler even in the 'force' scenario but pass
    a flag to the scheduler to tell it to skip the filters but still
    create the allocation on the destination node so we don't have
    to duplicate that in conductor.

    Change-Id: I6590f0eda4ec4996543ad40d8c2640b83fc3dd9d
    Partial-Bug: #1713786
    (cherry picked from commit d564266a019cb009ece1a63e5d544698f2bf74d1)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503162
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d0375c2f9e5cc4dac7f8805382c09dbe6224dcd9
Submitter: Jenkins
Branch: stable/pike

commit d0375c2f9e5cc4dac7f8805382c09dbe6224dcd9
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 22:47:07 2017 -0400

    Add recreate test for evacuate claim failure

    This adds a functional recreate test for when the MoveClaim
    fails on the destination node and the allocation on the
    destination node is not cleaned up. This is because the
    MoveClaim fails in it's constructor so it never exits the
    Claim context manager to call the drop_move_claim which
    would remove the destination node allocation.

    Eventually we'll have to manually remove the destination
    node allocation in the ComputeManager.rebuild_instance method.

    Change-Id: I8900ace4436c4837beb8b4eb1e1d05905efc6dce
    Related-Bug: #1713786
    (cherry picked from commit 6ed80ddcdd3f9e40f0ada581ac7bd524ec48135b)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503163
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=61ad70565e4d241f0a3f01d999e3aaf7a2eef3a3
Submitter: Jenkins
Branch: stable/pike

commit 61ad70565e4d241f0a3f01d999e3aaf7a2eef3a3
Author: Matt Riedemann <email address hidden>
Date: Thu Aug 31 22:56:59 2017 -0400

    Add a test to make sure failed evacuate cleans up dest allocation

    If we actually make the MoveClaim but the evacuation fails
    in the virt driver, the drop_move_claim via the MoveClaim.abort
    will remove the destination node allocation. This adds a functional
    test to show that works.

    Change-Id: Ib58c487e97a041b8498746e8a276efffee239c56
    Related-Bug: #1713786
    (cherry picked from commit 30946f9a5eeea839631cdb1dba9c26d45f7a8d00)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/503165
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=21d7f8b11f217b7e5ece6868a06df70a7a065b6b
Submitter: Jenkins
Branch: stable/pike

commit 21d7f8b11f217b7e5ece6868a06df70a7a065b6b
Author: Matt Riedemann <email address hidden>
Date: Fri Sep 1 09:58:03 2017 -0400

    Remove dest node allocation if evacuate MoveClaim fails

    If the MoveClaim during an evacuate fails we need to
    remove the destination node allocation since the
    ResourceTracker isn't going to do it.

    This also fixes test_rebuild_server_exc which was faking
    a rebuild failure by raising an error that would not
    actually happen during a rebuild. ComputeResourcesUnavailable
    only happens if a claim fails, and for a rebuild, which
    is on the same host that the instance is already on, doesn't
    involve a claim attempt.

    Change-Id: I59ff47bf773c9831d0f44e1caf7877fe08c911c3
    Closes-Bug: #1713786
    (cherry picked from commit 5bc137f7ebc266f8a73d6febc7c10d8d648924e0)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 17.0.0.0b1 development milestone.