OpenStack Compute (nova)

Allocations are not managed properly in all evacuate scenarios

  1. Series pike
  2. Bug #1713786
Bug #1713786 reported by Matt Riedemann
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Matt Riedemann
Pike
Fix Committed
High
Matt Riedemann

Bug Description

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

Revision history for this message
Balazs Gibizer (balazs-gibizer) wrote :

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.

Revision history for this message
Matt Riedemann (mriedem) wrote :

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.

Matt Riedemann (mriedem)
Changed in nova:
assignee: nobody → Matt Riedemann (mriedem)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Changed in nova:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
Matt Riedemann (mriedem) wrote :

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.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/pike)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/pike)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (stable/pike)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/pike)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/pike)

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)

tags: added: in-stable-pike
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/pike)

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/pike)

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/pike)

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 16.0.1

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 17.0.0.0b1

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

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

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.