Outputs aren't correctly validated

[Steven Hardy]

We've had bug(s) about this before IIRC, but it's still not working correctly, see reproducer below where an output references a wrong parameter - the stack goes CREATE_COMPLETE, but then resolving the outputs fails.

This is more of a problem than it seems at first glance, because although the stack-show error output is pretty clear, it's not reflected clearly at all when this happens in a nested template - some parent resource that references the nested stack output randomly fails and you have to manually inspect the nested stack to see the cause of the error - it'd be *much* clearer and easier to debug if the nested stack just failed.

If it's not possible to validate this during the validate stage due to dependencies on runtime data (I suspect this may be the case), we should special-case this for TemplateResource such that we resolve all attributes/outputs before declaring the parent resource COMPLETE.

[stack@instack openstack-tripleo-heat-templates]$ cat /tmp/reproducer.yaml
heat_template_version: 2016-04-08

parameters:
  SaharaWorkers:
    default: 0
    description: The number of workers for the sahara-api.
    type: number
outputs:
  role_data:
    description: Role data for the Sahara API role.
    value:
      config_settings:
        sahara::service::api::api_workers: {get_param: SaharaApiWorkers}

[stack@instack openstack-tripleo-heat-templates]$ heat stack-create r1 -f /tmp/reproducer.yaml
| 75d5a2bc-86e5-4551-8f88-4742d6a9f709 | r1 | CREATE_IN_PROGRESS | 2016-07-05T10:39:22 | None |

[stack@instack openstack-tripleo-heat-templates]$ heat stack-list
| 75d5a2bc-86e5-4551-8f88-4742d6a9f709 | r1 | CREATE_COMPLETE | 2016-07-05T10:39:22 | None |

[stack@instack openstack-tripleo-heat-templates]$ heat stack-show r1
...
| outputs | [ |
| | { |
| | "output_value": null, |
| | "output_error": "The Parameter (SaharaApiWorkers) was not provided.", |
| | "output_key": "role_data", |
| | "description": "Role data for the Sahara API role." |
| | }

[OpenStack Infra (hudson-openstack)]

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

[Lars Kellogg-Stedman (larsks)]

There is another problem that is not addressed by the current review.

Start with this in a file named `parent.yaml`:

    heat_template_version: 2016-04-08

    description: Fluentd logging configuration

    parameters:
      ParamA:
        type: string
        default: "this is a test"

    resources:
      TopLevelResource:
        type: ./child-1.yaml

    outputs:
      role_data:
        value:
          config_settings:
            map_merge:
              - get_attr: [TopLevelResource, role_data, config_settings]

Place this in `child-1.yaml`:

    heat_template_version: 2016-04-08

    description: Fluentd logging configuration

    parameters:
      ParamA:
        type: string
        default: "this is a test"

    resources:
      IntermediateResource:
        type: ./child-2.yaml

    outputs:
      role_data:
        value:
          config_settings:
            map_merge:
              - get_attr: [IntermediateResource, role_data, config_settings]

Place this in `child-2.yaml`:

    heat_template_version: 2016-04-08

    description: Fluentd logging configuration

    parameters:
      ParamA:
        type: string
        default: "this is a test"

Note that `child-2.yaml` stack has no outputs, although we ask for one
in the `child-1.yaml` stack.

Deploy a new stack from `parent.yaml`.

The stack will go to state `CREATE_COMPLETE`. Running `stack-show` on
this stack will reveal:

    [
      {
        "output_value": null,
        "output_error": "The Referenced Attribute (TopLevelResource role_data) is incorrect.",
        "output_key": "role_data",
        "description": "No description given"
      }
    ]

Note that the error is reported against `TopLevelResource`, even
though the actual problem is in the `child-2.yaml` stack. This makes
the error extremely hard to diagnose, especially in environments with
many nested stacks.

[Oleksii Chuprykov (ochuprykov)]

That patch is partial fix. It adds "static" validation, i.e. validation before starting creation of the stack. Some cases of invilid outputs can be determined only after creation of the stack. In your case you use 'get_attr' that resolves during creation/update and should be handled differently. I assumed one more patch for adding "dynamic" validation.

[Steven Hardy (shardy)]

Oleksii - are you still working on this? We keep getting folks confused due to this issue with TripleO templates, so if you're not planning to pick it up, can I take a look and revive your previous patch?

[OpenStack Infra (hudson-openstack)]

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

[Oleksii Chuprykov (ochuprykov)]

Hi, Steven. I'm going to work on this bug. But if you need this bug solved for "yesterday", feel free to take it. I uploaded a WIP patch few minutes ago, though. Welcome to review.

[Steven Hardy (shardy)]

Oleksii - thanks for the update - ideally I'd like to get this fixed for the final Newton release (or at least have a patch up we can backport to stable/newton soon after, I'll check out your patch and see if I can help speed things up with some reviews.

[Zane Bitter (zaneb)]

I think the real problem is that we raise InvalidTemplateAttribute in StackResource regardless of whether the error is the key not existing or an error resolving the output:

http://git.openstack.org/cgit/openstack/heat/tree/heat/engine/resources/stack_resource.py?h=stable%2Fnewton#n593

In the latter case that's clearly the wrong exception. (a) because InvalidTemplateAttribute means that you accessed an attribute that doesn't exist, so it tells the user the wrong thing, and (b) because InvalidTemplateAttribute doesn't allow you to specify a reason, so the error message gets lost. If we raised a useful exception containing the error message there then the user would get the right feedback without any change in behaviour.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit b90991e00b4e0029748c28c5258607f266f1bf85
Author: Zane Bitter <email address hidden>
Date: Mon Feb 20 15:12:49 2017 -0500

    Pass on outputs errors to parent stacks

    If getting an output from a child stack fails with an error, we didn't pass
    on the error message to the parent stack that was requesting it but instead
    reported essentially that the given output did not exist.

    Change-Id: I5653baf310a29dc4829ad570c769cf67ce12695e
    Partial-Bug: #1599114

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/441445
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=34d46d90a816f2811d203ddb5233ed3fd3d3a434
Submitter: Jenkins
Branch: stable/ocata

commit 34d46d90a816f2811d203ddb5233ed3fd3d3a434
Author: Zane Bitter <email address hidden>
Date: Mon Feb 20 15:12:49 2017 -0500

    Pass on outputs errors to parent stacks

    If getting an output from a child stack fails with an error, we didn't pass
    on the error message to the parent stack that was requesting it but instead
    reported essentially that the given output did not exist.

    Change-Id: I5653baf310a29dc4829ad570c769cf67ce12695e
    Partial-Bug: #1599114
    (cherry picked from commit b90991e00b4e0029748c28c5258607f266f1bf85)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/448589

[OpenStack Infra (hudson-openstack)]

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

commit e5cd34402572c73c701357deb2c69d72d589b781
Author: Zane Bitter <email address hidden>
Date: Mon Feb 20 15:12:49 2017 -0500

    Pass on outputs errors to parent stacks

    If getting an output from a child stack fails with an error, we didn't pass
    on the error message to the parent stack that was requesting it but instead
    reported essentially that the given output did not exist.

    Change-Id: I5653baf310a29dc4829ad570c769cf67ce12695e
    Partial-Bug: #1599114
    (cherry picked from commit b90991e00b4e0029748c28c5258607f266f1bf85)

[OpenStack Infra (hudson-openstack)]

Change abandoned by Rico Lin (<email address hidden>) on branch: master
Review: https://review.openstack.org/342124
Reason: Just a test patch, not going to be final solution

[OpenStack Infra (hudson-openstack)]

Change abandoned by Zane Bitter (<email address hidden>) on branch: master
Review: https://review.openstack.org/342124
Reason: Let's not do this.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Zane Bitter (<email address hidden>) on branch: master
Review: https://review.opendev.org/371704
Reason: Bug was fixed in I5653baf310a29dc4829ad570c769cf67ce12695e. Nobody has shown further interest in this approach, so abandoning.