Resource type substitution is destructive in convergence

[Zane Bitter]

We're supposed to be able to replace a deprecated resource type like OS::Heat::SoftwareDeployments with its designated substitute (OS::Heat::SoftwareDeploymentGroup in that case) without replacing the underlying physical resource. That works for the legacy path, but is broken in convergence.

The likely cause is https://review.openstack.org/#/c/420971/ - meaning that both Pike and Ocata are likely effected. Worse still, that patch was backported to Newton.

The test that was supposed to guard against this is checking completely the wrong resource - the server (which isn't even modified) instead of the software deployment group (which is getting replaced when it shouldn't be): https://review.openstack.org/#/c/516759/2

[OpenStack Infra (hudson-openstack)]

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

commit 93b4551d9a1ff9194b30a079f02db691129c69b0
Author: Zane Bitter <email address hidden>
Date: Wed Nov 1 18:03:20 2017 -0400

    Fix non-destructive upgrade for deprecated res types in convergence

    When a user updates from a deprecated resource type to an
    equivalent-but-differently-named one (e.g. from
    OS::Heat::SoftwareDeployments to OS::Heat::SoftwareDeploymentGroup), Heat
    is supposed to change the type without replacing the resource as it would
    normally do when a resource type changes. This was broken in convergence,
    because since 45073226752c58d640ea5a59b7e532c022a4939b the new Resource
    object we create during the check_resource operation (using the new type's
    plugin) is no longer automatically initialised with data from the database
    as resources in the legacy path are.

    Move the substitution checking to the Resource.load() method, so that it
    now returns an instance of the new plugin where allowed. In the actual
    update_convergence() method then we need only check that the resource class
    is the one we'd expect from the new template, and replace the resource if
    not.

    We do have a test that is designed to check that this is working, but in it
    we didn't compare the physical IDs of the resource that is potentially
    getting replaced, but rather the physical IDs of some other resource that
    can't possibly get modified (surprise! it doesn't change).

    Change-Id: I75778abc303525a71d0a918f7192f00a43c21284
    Closes-Bug: #1729439

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/526803

[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/526802
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=0d005ffcb69b8c5be3228957f7dfb8809b811742
Submitter: Zuul
Branch: stable/pike

commit 0d005ffcb69b8c5be3228957f7dfb8809b811742
Author: Zane Bitter <email address hidden>
Date: Wed Nov 1 18:03:20 2017 -0400

    Fix non-destructive upgrade for deprecated res types in convergence

    When a user updates from a deprecated resource type to an
    equivalent-but-differently-named one (e.g. from
    OS::Heat::SoftwareDeployments to OS::Heat::SoftwareDeploymentGroup), Heat
    is supposed to change the type without replacing the resource as it would
    normally do when a resource type changes. This was broken in convergence,
    because since 45073226752c58d640ea5a59b7e532c022a4939b the new Resource
    object we create during the check_resource operation (using the new type's
    plugin) is no longer automatically initialised with data from the database
    as resources in the legacy path are.

    Move the substitution checking to the Resource.load() method, so that it
    now returns an instance of the new plugin where allowed. In the actual
    update_convergence() method then we need only check that the resource class
    is the one we'd expect from the new template, and replace the resource if
    not.

    We do have a test that is designed to check that this is working, but in it
    we didn't compare the physical IDs of the resource that is potentially
    getting replaced, but rather the physical IDs of some other resource that
    can't possibly get modified (surprise! it doesn't change).

    Conflicts:
     heat/engine/resource.py

    This patch is modified with respect to the one on master, since the
    branch does not contain 960f626c2455b77e654aea1d79597fadb91dc894. Thus,
    resource_owning_stack.defn is substituted for initial_stk_defn and
    curr_stack.defn for latest_stk_defn.

    Change-Id: I75778abc303525a71d0a918f7192f00a43c21284
    Closes-Bug: #1729439
    (cherry picked from commit 93b4551d9a1ff9194b30a079f02db691129c69b0)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/526803
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=9ef06ccead321fcc224ba4c33b168d2e7e746de1
Submitter: Zuul
Branch: stable/ocata

commit 9ef06ccead321fcc224ba4c33b168d2e7e746de1
Author: Zane Bitter <email address hidden>
Date: Wed Nov 1 18:03:20 2017 -0400

    Fix non-destructive upgrade for deprecated res types in convergence

    When a user updates from a deprecated resource type to an
    equivalent-but-differently-named one (e.g. from
    OS::Heat::SoftwareDeployments to OS::Heat::SoftwareDeploymentGroup), Heat
    is supposed to change the type without replacing the resource as it would
    normally do when a resource type changes. This was broken in convergence,
    because since 45073226752c58d640ea5a59b7e532c022a4939b the new Resource
    object we create during the check_resource operation (using the new type's
    plugin) is no longer automatically initialised with data from the database
    as resources in the legacy path are.

    Move the substitution checking to the Resource.load() method, so that it
    now returns an instance of the new plugin where allowed. In the actual
    update_convergence() method then we need only check that the resource class
    is the one we'd expect from the new template, and replace the resource if
    not.

    We do have a test that is designed to check that this is working, but in it
    we didn't compare the physical IDs of the resource that is potentially
    getting replaced, but rather the physical IDs of some other resource that
    can't possibly get modified (surprise! it doesn't change).

    Conflicts:
     heat/engine/resource.py

    This patch is modified with respect to the one on master, since the
    branch does not contain 960f626c2455b77e654aea1d79597fadb91dc894. Thus,
    resource_owning_stack.defn is substituted for initial_stk_defn and
    curr_stack.defn for latest_stk_defn.

    It is also modified with respect to the one on stable/pike, since the
    branch does not contain 764b8fb25132ac60c7760f134b78d600269ff516 or
    a7376f7494b310e9367ebe5dcb43b432a4053023. Thus, resource definitions are
    obtained from the Template rather than the StackDefinition.

    Change-Id: I75778abc303525a71d0a918f7192f00a43c21284
    Closes-Bug: #1729439
    (cherry picked from commit 93b4551d9a1ff9194b30a079f02db691129c69b0)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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