Failed forced live migration does not rollback doubled up allocations in placement

Bug #1784022 reported by Matt Riedemann
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Invalid
High
Unassigned

Bug Description

***This is purely based on code inspection right now.***

With a forced host live migration, we bypass the scheduler and copy the instance's resource allocations from the source node to the dest node:

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/conductor/tasks/live_migrate.py#L109

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/scheduler/utils.py#L473

On successful post live migration, we remove the doubled up allocations (after logging a warning that we couldn't find allocations on the migration record):

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/manager.py#L6638L6669

However, for a failed live migration, we don't do anything like that in _rollback_live_migration. We'll call this _revert_allocation method:

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/manager.py#L6803

But it won't find allocations on the migration record and just return False:

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/compute/manager.py#L4130

Which means the instance will have doubled up allocations on both the source and dest nodes.

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

Looks like this was regressed in Queens:

https://review.openstack.org/#/c/507638/29/nova/compute/manager.py@a6289

And I even pointed it out on the review but we didn't think about the forced live migration case:

https://review.openstack.org/#/c/507638/25/nova/compute/manager.py@6252

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

This isn't an issue after all because we move the allocations on the source node from the instance to the migration *before* we do the copy:

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/conductor/tasks/live_migrate.py#L82

https://github.com/openstack/nova/blob/6be7f7248fb1c2bbb890a0a48a424e205e173c9c/nova/conductor/tasks/live_migrate.py#L109

I'll push up the functional test I wrote since it doesn't appear we have coverage for this.

no longer affects: nova/queens
Changed in nova:
status: Triaged → Invalid
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/586636

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

Reviewed: https://review.openstack.org/586636
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=11f473b170adbe0757c71f9e59dacab19e22d45c
Submitter: Zuul
Branch: master

commit 11f473b170adbe0757c71f9e59dacab19e22d45c
Author: Matt Riedemann <email address hidden>
Date: Fri Jul 27 14:31:22 2018 -0400

    Add functional test for forced live migration rollback allocs

    Per bug 1784022, it was not immediately clear that on a forced
    host live migration failure that the allocations on the dest
    host would be deleted and the original allocations for the instance
    would be back on the source host, but it is indeed working but
    as far as I could tell we didn't have a functional test for it,
    only the non-forced case. So this change adds the force=True
    wrinkle to the same existing functional test and also adds
    assertions on where the allocations exist for the source and dest
    nodes and the migration and instance records at the point of
    failure.

    Change-Id: Iaef9a4456c9c2bd9b2f4257b4ddd3efa3dea50f6
    Related-Bug: #1784022

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

Other bug subscribers

Remote bug watches

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