WaitCondition FnGetAtt fails if handle doesn't exist yet

[Steve Baker]

If WaitCondition Data is referred to in the Resources section of the template, the template will fail to load due to FnGetAtt being called before the handle exists.

[OpenStack Infra (hudson-openstack)]

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

[Zane Bitter (zaneb)]

Do you have an example of this? It seems like we ought to add dependencies for "Fn::GetAtt" references in much the same way we do for "Ref".

[Zane Bitter (zaneb)]

An audit of the heat-templates repo reveals exactly one place that this creates a circular dependency: in EC2WithEBSSample.template, we use Fn::GetAtt to get the availability zone of the instance as a property of a Volume.

At first glance, this seems like an obvious use case to want to support (use default method of selecting AZ for Instance, put the Volume in the same one). But on reflection, there really is no way to know which AZ the instance will be in until it is created. It seems that the intent was that when using EBS you must supply the AZ explicitly. So I think the template is wrong.

[Clint Byrum (clint-fewbar)]

I don't believe this is a bug at all. Fn::GetAtt does not need to establish dependencies, DependsOn and Ref do that just fine.

[Steve Baker (steve-stevebaker)]

The original description lacked some context.

Here is a template snippet:
...
        "brokerWaitHandle": {
            "Type": "AWS::CloudFormation::WaitConditionHandle"
        },
        "brokerWaitCondition": {
            "Type": "AWS::CloudFormation::WaitCondition",
            "DependsOn": "BrokerInstance",
            "Properties": {
                "Handle": {
                    "Ref": "brokerWaitHandle"
                },
                "Timeout": "6000"
            }
        },
        "NodeInstance": {
            "Type": "AWS::EC2::Instance",
            "DependsOn": "brokerWaitCondition",
...
                "UserData": {
                    "Fn::Base64": {
                        "Fn::Join": [
                            "",
                            [
...
                                "export DNS_SEC_KEY=\"`python -c 'print ",{ "Fn::GetAtt": [ "brokerWaitCondition", "Data" ] },"[\"00000\"]'`\"\n",
...

Here is the error as logged:
2013-05-29 16:18:49.439 11266 INFO heat.engine.resource [-] Validating WaitCondition "brokerWaitCondition"
2013-05-29 16:18:49.439 11266 INFO heat.engine.resource [-] Validating Instance "NodeInstance"
2013-05-29 16:18:49.440 11266 ERROR heat.engine.parser [-] Property error : "brokerWaitHandle" is not a valid URL
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser Traceback (most recent call last):
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/engine/parser.py
", line 248, in validate
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser result = res.validate()
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/engine/resources
/instance.py", line 407, in validate
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser res = super(Instance, self).validate()
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/engine/resource.
py", line 412, in validate
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser return self.properties.validate()
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser File "/home/steveb/dev/localstack/heat/heat/engine/propertie
s.py", line 182, in validate
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser raise exception.StackValidationFailed(message=msg)
2013-05-29 16:18:49.440 11266 TRACE heat.engine.parser StackValidationFailed: Property error : "brokerWaitHandle" is not a valid URL
2013-05-29 16:18:49.462 11266 ERROR heat.openstack.common.rpc.amqp [-] Exception during message handling
2013-05-29 16:18:49.462 11266 TRACE heat.openstack.common.rpc.amqp Traceback (most recent call last):
2013-05-29 16:18:49.462 11266 TRACE heat.openstack.common.rpc.amqp File "/home/steveb/dev/localstack/heat/heat/openstack/common/rpc/amqp.py", line 434, in _process_data
2013-05-29 16:18:49.462 11266 TRACE heat.openstack.common.rpc.amqp **args)
2013-05-29 16:18:49.462 11266 TRACE heat.openstack.common.rpc.amqp File "/home/steveb/dev/localstack/heat/heat/openstack/common/rpc/dispatcher.py", line 172, in dispatch
2013-05-29 16:18:49.462 11266 TRACE heat.openstack.common.rpc.amqp result = getattr(pr...

[Zane Bitter (zaneb)]

Ah, OK, thanks for the detail. I don't think it's the case that it can be called *anywhere* in the life cycle, but it's definitely the case that it gets called during validation.

Maybe we need to come up with a method of validation that doesn't involve calling FnGetRefId and FnGetAtt. Then we wouldn't need hacks in every single resource subclass to deal with them.

I still think that Fn::GetAtt should create a dependency so I'll submit that patch separately, even though it doesn't resolve this bug.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/31607
Committed: http://github.com/openstack/heat/commit/85b363342c0d3a25155c500b42810f9ef9d82a22
Submitter: Jenkins
Branch: master

commit 85b363342c0d3a25155c500b42810f9ef9d82a22
Author: Steve Baker <email address hidden>
Date: Tue Jun 4 17:25:42 2013 +1200

    For Fn::Join, replace None items with an empty string.

    Currently, attempts to join lists with None items results in an error.

    This change is required for the next attempt to fix bug: #1184794

    Change-Id: Ib599eeb21b7325ff797768100f1556a424a8bde9

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/31608
Committed: http://github.com/openstack/heat/commit/c19a3deb649b6e4c5ea933e7c0ad157f4049d185
Submitter: Jenkins
Branch: master

commit c19a3deb649b6e4c5ea933e7c0ad157f4049d185
Author: Steve Baker <email address hidden>
Date: Tue Jun 4 16:40:12 2013 +1200

    Only call FnGetAtt if resource is in acceptable state.

    This will give resource type authors some certainty on what error
    checking their FnGetAtt methods will require.

    Fixes bug: #1184794

    Change-Id: Iec456075c14cd609cf9607f07bce23e4c6b33c0c

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/grizzly
Review: https://review.openstack.org/35120

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/grizzly
Review: https://review.openstack.org/35121

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/35121
Committed: http://github.com/openstack/heat/commit/a9a8ff6dbd0c92cd37300150e4cadd82b40d42f3
Submitter: Jenkins
Branch: stable/grizzly

commit a9a8ff6dbd0c92cd37300150e4cadd82b40d42f3
Author: Steve Baker <email address hidden>
Date: Tue Jun 4 16:40:12 2013 +1200

    Only call FnGetAtt if resource is in acceptable state.

    This will give resource type authors some certainty on what error
    checking their FnGetAtt methods will require.

    Fixes bug: #1184794

    Change-Id: Iec456075c14cd609cf9607f07bce23e4c6b33c0c
    (cherry picked from commit c19a3deb649b6e4c5ea933e7c0ad157f4049d185
     couple of test fixups required for grizzly backport, notably the
     duplicate create from Id90edd3a09ec15d3814a322ac0b7ec75fd0be54a
     in test_loadbalancer.py is required or the test fails)