Resizing a pinned VM results in inconsistent state

So just by looking at the code - I can see that we probably want to call resource_tracker.update_usage() in the shelve_offload method so that we immediately unpin CPUs once that happens and not wait for the RT periodic task which in case of a speedy tempest test likely never happens.

This _might_ be the cause of the stack trace in the cleanup delete in case the spawn failed during unshelving for example.

Here's what would happen:

https://github.com/openstack/nova/blob/7616c88ad3a2769e9c9ee8a51ac55ddeed0bfd84/nova/compute/manager.py#L4368

When unshelving a shelve-offloaded instance, we would first run the claim_instance which does a new claim against the current state of the NumaTopology of the compute host (keep in mind that we never dropped the usage when we did shelve-offloading so we are actually leaking resources here). A successful claim here means the instance and compute node are update with the new pinning information (and we leak resources on the compute node until the next RT periodic update).

Now suppose the spawn on the next line fails for whatever reason that may or may not be related to a CPU pinning bug - this calls the __exit__ method of the claim, which in turn calls the abort() method of the claim unpinning the CPUs that were pinned during (see: https://github.com/openstack/nova/blob/7616c88ad3a2769e9c9ee8a51ac55ddeed0bfd84/nova/compute/claims.py#L121)

This will unpin the CPUs as tracked by the host NumaTopology, but it will not clear the mapping to host CPUs in the Instance object.

Finally - the test cleanup attempts to delete the instance, which attempts to unpin the already unpinned CPUs and fails.

In case the above makes sense - I think we need to do 2 things:

* Make sure offloading an instance also updates the resource_tracker in the same manner deleting it does immediately.
* Make sure that aborting the claim clears both the host field, and NUMA information of the Instance (host is problematic here as this is why the delete request ends up RPCed to the host instead of being done locally in the API, even though the instance is clearly not there since the claim failed and is being aborted).

I propose we start there and see if it fixes things.

PS - it is worth noting (if it was not clear from the text) that the above bugs impact mostly shelve functionality - nova spawn clears these things as part of the retry process so is not affected.

As someone with familiarity with the shelve/unshelve process that analysis makes sense to me. Updating resource usage on offload is something that should be done since resources are not in use, or "reserved", at that point. Clearing the host on a failed claim makes sense as well since as was pointed out there is nothing on the host at that point and therefore no real association with that host.

Something else to note is that shelving code leverages much of what resize did at the time it was written, so any issues discovered in that path may also affect resize. Though Tempest tests may not be resizing instances so shelving tests tend to find issues first.

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

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

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

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

commit a54997c62f1422c592dbabc28ea71d0a34e70b74
Author: Nikola Dipanov <email address hidden>
Date: Tue Feb 16 19:14:50 2016 +0000

    RT: Decrese usage for offloaded instances

    Allow for update_usage to consider SHELVED_OFFLOADED instances as
    removed and update the resource usage accordingly. This means we want to
    make sure that the stats class does the same.

    We can now make sure that RT is updated immediately once the instance
    has been shelved offloaded.

    Change-Id: Ia22963021995c71758a18b21070dfdf6a950da09
    Partial-bug: #1545675

Set to High because it seems to be impacting the NFV CI

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

commit 521decbe9f79e337fb14571367c6008969a7133a
Author: Nikola Dipanov <email address hidden>
Date: Wed Feb 17 19:24:06 2016 +0000

    objects: Allow instance to reset the NUMA topology

    This will be used when aborting claims, since the claim will (if
    successful) update the instance with the claimed topology,

    In case of an abort - we want to make sure it's clear for consistency
    sake. This will be done in the follow up patch.

    Change-Id: I2989446fdaa44a30d90ae2ab29fc27fb2ad03c4a
    Partial-bug: 1545675

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

commit c7a6673fd5621d1c121c20376634ec49644fae59
Author: Nikola Dipanov <email address hidden>
Date: Wed Feb 17 19:27:36 2016 +0000

    RT: aborting claims clears instance host and NUMA info

    When the claim is aborted, this information is no longer correct for the
    instance, so we clear it to avoid inconsistencies.

    Change-Id: I83a5f06adb22c21392d5fc867728181ea4b0454d
    Resolves-bug: 1545675

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

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

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

commit d969169022b32fc98a0a7af4891e75bc072269c6
Author: Stephen Finucane <email address hidden>
Date: Tue Mar 8 15:24:18 2016 +0000

    Address nits in Ia2296302

    Be consistent in which functions are called in tests to reinforce
    what's being checked.

    Change-Id: I54f0abda8db65d633189b31684467bae87016738
    Related-Bug: #1545675

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

commit 0469f71181c724480f3b3f12cb7eea97321ca79e
Author: Stephen Finucane <email address hidden>
Date: Mon Mar 7 13:40:21 2016 +0000

    Address nits in I83a5f06ad

    Make functions a little more coherent by ensuring they do one thing and
    one thing only. Similarly ensure we don't use "private" variables in
    other files.

    Change-Id: Ib916e01f655d59bcc7cd0932d909e56bb1d4af8a
    Related-Bug: #1545675

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

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

commit f1320a7c2debf127a93773046adffb80563fd20b
Author: Stephen Finucane <email address hidden>
Date: Mon May 30 16:03:35 2016 +0100

    Evaluate 'task_state' in resource (de)allocation

    There are two types of VM states associated with shelving. The first,
    'shelved' indicates that the VM has been powered off but the resources
    remain allocated on the hypervisor. The second, 'shelved_offloaded',
    indicates that the VM has been powered off and the resources freed.
    When "unshelving" VMs in the latter state, the VM state does not change
    from 'shelved_offloaded' until some time after the VM has been
    "unshelved".

    Change I83a5f06 introduced a change that allowed for deallocation of
    resources when they were set to the 'shelved_offloaded' state. However,
    the resource (de)allocation code path assumes any VM with a state of
    'shelved_offloaded' should have resources deallocated from it, rather
    than allocated to it. As the VM state has not changed when this code
    path is executed, resources are incorrectly deallocated from the
    instance twice.

    Enhance the aformentioned check to account for task state in addition to
    VM state. This ensures a VM that's still in 'shelved_offloaded' state,
    but is in fact being unshelved, does not trigger deallocation.

    Change-Id: Ie2e7b91937fc3d61bb1197fffc3549bebc65e8aa
    Signed-off-by: Stephen Finucane <email address hidden>
    Resolves-bug: #1587386
    Related-bug: #1545675

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

commit 59f55a14b5f5396e653d9eb1e5ded31db72f620b
Author: Stephen Finucane <email address hidden>
Date: Tue Jun 21 15:47:50 2016 +0100

    Don't immediately null host/node when shelving

    When offloading a shelved instance, resources should be freed. However,
    the ability to free resources is dependant on being able to find the
    resource tracker for an instance's node. At present, the instance node
    and host are nulled before attempting to update the resource tracker,
    meaning the resources are never actually freed. Fix this by nullifying
    these values *after* resources updates.

    Change-Id: I8f91367aacca0c7c673b28b3c844c70c0d12f0a5
    Related-bug: #1545675

Related fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/337107

Related fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/337108

Reviewed: https://review.openstack.org/337107
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=2703a3d80bcbd49eaafaae624289d00c521b5192
Submitter: Jenkins
Branch: stable/mitaka

commit 2703a3d80bcbd49eaafaae624289d00c521b5192
Author: Stephen Finucane <email address hidden>
Date: Mon May 30 16:03:35 2016 +0100

    Evaluate 'task_state' in resource (de)allocation

    There are two types of VM states associated with shelving. The first,
    'shelved' indicates that the VM has been powered off but the resources
    remain allocated on the hypervisor. The second, 'shelved_offloaded',
    indicates that the VM has been powered off and the resources freed.
    When "unshelving" VMs in the latter state, the VM state does not change
    from 'shelved_offloaded' until some time after the VM has been
    "unshelved".

    Change I83a5f06 introduced a change that allowed for deallocation of
    resources when they were set to the 'shelved_offloaded' state. However,
    the resource (de)allocation code path assumes any VM with a state of
    'shelved_offloaded' should have resources deallocated from it, rather
    than allocated to it. As the VM state has not changed when this code
    path is executed, resources are incorrectly deallocated from the
    instance twice.

    Enhance the aformentioned check to account for task state in addition to
    VM state. This ensures a VM that's still in 'shelved_offloaded' state,
    but is in fact being unshelved, does not trigger deallocation.

    Change-Id: Ie2e7b91937fc3d61bb1197fffc3549bebc65e8aa
    Signed-off-by: Stephen Finucane <email address hidden>
    Resolves-bug: #1587386
    Related-bug: #1545675
    (cherry picked from commit f1320a7c2debf127a93773046adffb80563fd20b)

Reviewed: https://review.openstack.org/337108
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=c455c82dac32f5a0313077ec3bd8aca1f7db1bcb
Submitter: Jenkins
Branch: stable/mitaka

commit c455c82dac32f5a0313077ec3bd8aca1f7db1bcb
Author: Stephen Finucane <email address hidden>
Date: Tue Jun 21 15:47:50 2016 +0100

    Don't immediately null host/node when shelving

    When offloading a shelved instance, resources should be freed. However,
    the ability to free resources is dependant on being able to find the
    resource tracker for an instance's node. At present, the instance node
    and host are nulled before attempting to update the resource tracker,
    meaning the resources are never actually freed. Fix this by nullifying
    these values *after* resources updates.

    Change-Id: I8f91367aacca0c7c673b28b3c844c70c0d12f0a5
    Related-bug: #1545675
    (cherry picked from commit 59f55a14b5f5396e653d9eb1e5ded31db72f620b)