resource-list fails on stacks that were created before needed_by column was added

Bug #1703703 reported by Zane Bitter on 2017-07-11
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Heat
Fix Released
Medium
Zane Bitter
Newton
Fix Committed
Medium
Zane Bitter
Ocata
Fix Committed
Medium
Zane Bitter
Pike
Fix Released
Medium
Zane Bitter

Bug Description

We added the 'needed_by' column to the Resource table in the database back in Kilo. However, there is no data migration for that (or any other) column. Therefore, stacks that predate the point where we started setting the default to [] instead of None (in Liberty) will have null values for that column.

That's fine, but Heat is expecting that needed_by is always a list, and barfs when getting the resource list if it isn't:

Exception handling resource: argument of type 'NoneType' is not iterable
Traceback (most recent call last):

  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/server.py", line 133, in _process_incoming
    res = self.dispatcher.dispatch(message)

  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/dispatcher.py", line 150, in dispatch
    return self._do_dispatch(endpoint, method, ctxt, args)

  File "/usr/lib/python2.7/site-packages/oslo_messaging/rpc/dispatcher.py", line 121, in _do_dispatch
    result = func(ctxt, **new_args)

  File "/usr/lib/python2.7/site-packages/osprofiler/profiler.py", line 154, in wrapper
    return f(*args, **kwargs)

  File "/usr/lib/python2.7/site-packages/heat/common/context.py", line 424, in wrapped
    return func(self, ctx, *args, **kwargs)

  File "/usr/lib/python2.7/site-packages/heat/engine/service.py", line 1945, in list_stack_resources
    for resource in rsrcs]

  File "/usr/lib/python2.7/site-packages/heat/engine/api.py", line 345, in format_stack_resource
    rpc_api.RES_REQUIRED_BY: resource.required_by(),

  File "/usr/lib/python2.7/site-packages/heat/engine/resource.py", line 659, in required_by
    if r.id in self.needed_by]

(Traceback is from Newton, but the problem still exists AFAICT.)

Zane Bitter (zaneb) wrote :

Note that in theory we're not supposed to hit this path except in convergence, so although it would be simple to fix I'm not 100% sure that we'd be getting at the root cause.

Zane Bitter (zaneb) wrote :

It looks like the problem was introduced by https://review.openstack.org/#/c/278231/. We take the 'convergence' path in the case that the Resource object cannot be found in the Stack's dependency graph. The dependency graph is constructed from the Resource objects found in Stack.resources. However, the Resource objects we're listing are loaded from database objects independently of the Stack, and thus *never* appear in the dependencies.

So we always take the 'convergence' path, and display of dependencies is just broken for non-convergence-mode stacks.

Changed in heat:
status: New → Triaged
importance: Undecided → Medium
milestone: none → pike-3

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

Changed in heat:
status: Triaged → In Progress

Reviewed: https://review.openstack.org/483512
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=5ce238fb17e38702ceef85a3706e9d2236c43e0e
Submitter: Jenkins
Branch: master

commit 5ce238fb17e38702ceef85a3706e9d2236c43e0e
Author: Zane Bitter <email address hidden>
Date: Thu Jul 13 13:59:27 2017 -0400

    Don't load new copies of current resources

    The Stack._find_filtered_resources() method returns Resource objects for
    all resources associated with the Stack, regardless of whether they are
    current (present in the template; latest version in the case of
    convergence). To do this, it previously created a new Resource object
    for every resource found in the database.

    However, for those resources which *are* current this is unnecessary. We
    can access the Resource object simply through self.resources. It turns
    out this is necessary for obtaining the required_by list for legacy
    stacks, because only the Resources obtained from self.resources also
    appear in the Dependencies graph obtained from self.dependencies. The
    required_by list is read when listing or showing resources, which would
    either return an empty list or fail for legacy stacks.

    This patch also makes the Resource.required_by() method more robust in
    its error handling.

    Change-Id: Id438336e5c88dc7c2d168ba01ee703faa17e8b8e
    Closes-Bug: #1703703
    Related-Bug: #1523748

Changed in heat:
status: In Progress → Fix Released

Reviewed: https://review.openstack.org/485364
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=65630d4e8bbff2f3ae5c659f8e9396ec7a6f6ae9
Submitter: Jenkins
Branch: master

commit 65630d4e8bbff2f3ae5c659f8e9396ec7a6f6ae9
Author: Zane Bitter <email address hidden>
Date: Wed Jul 19 17:50:58 2017 -0400

    Handle non-existent resources in list during update

    When showing the resource list during a stack update, the patch
    5ce238fb17e38702ceef85a3706e9d2236c43e0e inadvertently raises a KeyError
    when a resource in the new template hasn't been created yet. This resolves
    the problem.

    Change-Id: If0784156211b21d8ef7047a7082d1b48e977252d
    Related-Bug: #1703703
    Closes-Bug: #1705170

Reviewed: https://review.openstack.org/483571
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=3fb8722012a991855fc760ba450ed74acd8b43d2
Submitter: Jenkins
Branch: stable/ocata

commit 3fb8722012a991855fc760ba450ed74acd8b43d2
Author: Zane Bitter <email address hidden>
Date: Thu Jul 13 13:59:27 2017 -0400

    Don't load new copies of current resources

    The Stack._find_filtered_resources() method returns Resource objects for
    all resources associated with the Stack, regardless of whether they are
    current (present in the template; latest version in the case of
    convergence). To do this, it previously created a new Resource object
    for every resource found in the database.

    However, for those resources which *are* current this is unnecessary. We
    can access the Resource object simply through self.resources. It turns
    out this is necessary for obtaining the required_by list for legacy
    stacks, because only the Resources obtained from self.resources also
    appear in the Dependencies graph obtained from self.dependencies. The
    required_by list is read when listing or showing resources, which would
    either return an empty list or fail for legacy stacks.

    This patch also makes the Resource.required_by() method more robust in
    its error handling.

    Change-Id: Id438336e5c88dc7c2d168ba01ee703faa17e8b8e
    Closes-Bug: #1703703
    Related-Bug: #1523748
    (cherry picked from commit 5ce238fb17e38702ceef85a3706e9d2236c43e0e
                           and 65630d4e8bbff2f3ae5c659f8e9396ec7a6f6ae9)

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

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

Reviewed: https://review.openstack.org/483574
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=4e09419701090997c43fff847bd75b91b0c1ace9
Submitter: Jenkins
Branch: stable/newton

commit 4e09419701090997c43fff847bd75b91b0c1ace9
Author: Zane Bitter <email address hidden>
Date: Thu Jul 13 13:59:27 2017 -0400

    Don't load new copies of current resources

    The Stack._find_filtered_resources() method returns Resource objects for
    all resources associated with the Stack, regardless of whether they are
    current (present in the template; latest version in the case of
    convergence). To do this, it previously created a new Resource object
    for every resource found in the database.

    However, for those resources which *are* current this is unnecessary. We
    can access the Resource object simply through self.resources. It turns
    out this is necessary for obtaining the required_by list for legacy
    stacks, because only the Resources obtained from self.resources also
    appear in the Dependencies graph obtained from self.dependencies. The
    required_by list is read when listing or showing resources, which would
    either return an empty list or fail for legacy stacks.

    This patch also makes the Resource.required_by() method more robust in
    its error handling.

     Conflicts:
     heat/engine/stack.py

    The backport is simpler than the patch on master because
    3967a93f5de2e147a6d9c3f4fef9fd1a8f5d3498 has not been backported to
    stable/newton.

    Change-Id: Id438336e5c88dc7c2d168ba01ee703faa17e8b8e
    Closes-Bug: #1703703
    Related-Bug: #1523748
    (cherry picked from commit 5ce238fb17e38702ceef85a3706e9d2236c43e0e
                           and 65630d4e8bbff2f3ae5c659f8e9396ec7a6f6ae9)

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

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

Duplicates of this bug

Other bug subscribers