prepare_for_replace() is not protected by a lock

[Zane Bitter]

Since https://review.openstack.org/#/c/431347/ landed (in Pike) to cut down on DB bandwidth by doing resource locking as part of the state transitions, the Resource.prepare_for_replace() plugin API is called while we do not have a lock on the resource. This means that in convergence we could end up calling it at the same time as another traversal is trying to modify the resource in-place. That can only lead to very bad things.

In the case where UpdateReplace is raised from handle_update(), it seems clear that we should just call prepare_for_replace before releasing the lock.

In the case where UpdateReplace is raised before we actually take the lock, we should probably take the lock for the duration of the prepare_for_replace() call.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/516459
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=b0e18270b4af43452d6c2e229aa76db7ff120980
Submitter: Zuul
Branch: master

commit b0e18270b4af43452d6c2e229aa76db7ff120980
Author: Zane Bitter <email address hidden>
Date: Mon Oct 30 17:07:12 2017 -0400

    Protect prepare_update_replace() with resource lock

    When we consolidated resource locking with resource state transitions in
    a7376f7494b310e9367ebe5dcb43b432a4053023, prepare_update_replace() moved
    outside of the locked section, so that other update operations could
    potentially conflict with it. This change ensures that we call it while the
    lock is still held (if we have transitioned the resource to
    UPDATE_IN_PROGRESS, and thus acquired the lock), or that we acquire the
    lock before calling it (if UpdateReplace is raised before attempting to
    update the resource).

    To avoid unnecessary locking and unlocking, we only take the lock if the
    Resource plugin has overridden the default handler method (either
    restore_prev_rsrc() or prepare_for_replace()), since the defaults do
    nothing.

    Change-Id: Ie32836ed643e440143bde8b83aeb4d6159acd034
    Closes-Bug: #1727127

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/heat 10.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/535965
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=0519cd94fc3f95778e75f6fd917defa77f7d5afe
Submitter: Zuul
Branch: stable/pike

commit 0519cd94fc3f95778e75f6fd917defa77f7d5afe
Author: Zane Bitter <email address hidden>
Date: Mon Oct 30 17:07:12 2017 -0400

    Protect prepare_update_replace() with resource lock

    When we consolidated resource locking with resource state transitions in
    a7376f7494b310e9367ebe5dcb43b432a4053023, prepare_update_replace() moved
    outside of the locked section, so that other update operations could
    potentially conflict with it. This change ensures that we call it while the
    lock is still held (if we have transitioned the resource to
    UPDATE_IN_PROGRESS, and thus acquired the lock), or that we acquire the
    lock before calling it (if UpdateReplace is raised before attempting to
    update the resource).

    To avoid unnecessary locking and unlocking, we only take the lock if the
    Resource plugin has overridden the default handler method (either
    restore_prev_rsrc() or prepare_for_replace()), since the defaults do
    nothing.

     Conflicts:
     heat/engine/resource.py

    This differs from the patch on master because
    0e1b4908e55d6b1f11ec8cd190d719244194365c is not present in stable/pike,
    so there are different paths for the cases where there are or are not
    restricted actions in play.

    Change-Id: Ie32836ed643e440143bde8b83aeb4d6159acd034
    Closes-Bug: #1727127
    (cherry picked from commit b0e18270b4af43452d6c2e229aa76db7ff120980)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/heat 9.0.4 release.