Add AutoScaling Example With Scaling Rules and Notifications

[rcuza]

This pull requests creates an example troposphere script that creates the AutoScalingMultiAZWithNotifications.template.

In order to create a comment that was in the example CloudFormation template, I added a Comment class to troposphere/cloudformation.py. I am not sure if this is the right place. I was able to create the template when I added the same class to troposphere/__init__.py. I did not ultimately choose that location because I do not know if the Comment property is universal to all resources in CloudFormation templates.

Perhaps I should of done with the Comment property what I had to do with the NotificationConfigurations property in the AutoScalingGroup resource, namely edit the template to match the troposphere code. I could not create the NotificationConfigurations without passing it a list, which is supported by the AWS AutoScalingGroup documentation.

[rcuza]

To make it easier to see how I changed the original Amazon template, here is the diff:

(troposphere) $ cat tests/examples_output/AutoScalingMultiAZWithNotifications-orig.template | python -m json.tool  > tests/examples_output/AutoScalingMultiAZWithNotifications-orig-sorted.template
(troposphere) $ diff tests/examples_output/AutoScalingMultiAZWithNotifications-orig-sorted.template tests/examples_output/AutoScalingMultiAZWithNotifications.template
833,841c833,843
<                 "NotificationConfiguration": {
<                     "NotificationTypes": [
<                         "autoscaling:EC2_INSTANCE_LAUNCH",
<                         "autoscaling:EC2_INSTANCE_LAUNCH_ERROR",
<                         "autoscaling:EC2_INSTANCE_TERMINATE",
<                         "autoscaling:EC2_INSTANCE_TERMINATE_ERROR"
<                     ],
<                     "TopicARN": {
<                         "Ref": "NotificationTopic"
---
>                 "NotificationConfigurations": [
>                     {
>                         "NotificationTypes": [
>                             "autoscaling:EC2_INSTANCE_LAUNCH",
>                             "autoscaling:EC2_INSTANCE_LAUNCH_ERROR",
>                             "autoscaling:EC2_INSTANCE_TERMINATE",
>                             "autoscaling:EC2_INSTANCE_TERMINATE_ERROR"
>                         ],
>                         "TopicARN": {
>                             "Ref": "NotificationTopic"
>                         }
843c845
<                 }
---
>                 ]

[markpeek]

I like the example and think it is good to add it. The issue I'm wrestling with is whether adding Comment would lead to confusion since it will only work in a Metadata attribute and people might try using it in other objects. Let me ponder this one for a bit more.

[rcuza]

It looks like Metadata is an attribute for every resource (ref: Adding Comments inside AWS CloudFormation Templates and AWS Metadata Attribute doc). Metadata is a valid key at the top of a CF template, too.

As you pointed out, every Metadata attribute can have a Comment key, but Comment is not a attribute for every resource. What is the best way to put Comment inside of Metadata?

When I search the troposphere code base I find Metadata in __init__.py, autoscaling.py, and cloudformation.py. For the latter two the Metadata class is based on the AWSHelperFn class. For __init__.py, Metadata is just listed as an attribute. Why is there this difference? My first stab at the answer is that while all resources have a Metadata attribute, each resource has different keys within that it expects. A map resource with a Metadata attribute will not be able to do anything with a "AWS::CloudFormation::Init" key, but a LaunchConfig resource will be able to use it.

Instead of __init__.py declaring Metadata as a simple attribute of every BaseAWSObject, maybe it should be a class that could be extended for each resource if it needs it. The Comment key can be a method of this class or just an attribute. I'm not sure which would be easier to validate or more intuitive to use. Does this accomplish our goal? (yes, I am asking before I actually tried coding it. 🙀)

[markpeek]

As you pointed out Metadata is in some of the existing objects. I'll have to spend some time looking at git history to try to figure out why/when those were added. This is sometimes difficult given the lack of historical docs. Given that Metadata is really just a json blob we could remove Comment and just attach and arbitrary dict instead. Or just leave Comment as-is in your PR and see what happens. I'm leaning towards the latter but still thinking about it. I understand your comment about adding Metadata to every BaseAWSObject but wondering if the complexity is worth it for this issue.

[rcuza]

I tried adding the Comment using a variety of formats around commit 7a8026d, but it wouldn't build or pass tests (I can't remember which).

Whichever way you decide, let me know and I can adjust the PR. I might try a POC for making Metadata more robust.

[rcuza]

Doing some digging I found the following issues related to Metadata:

• Workaround for missing AWS resource types? Workaround for missing AWS resource types? #3
• updating InitConfig per AWS docs updating InitConfig per AWS docs #120
• Add of Metadata property to the template anatomy? Add of Metadata property to the template anatomy? #286
• generates an AttributeError Adding Metadata to an existing ec2.Instance instance generates an AttributeError #298
• No Example for ParameterGroups No Example for ParameterGroups #367
• LaunchConfig's Metadata missing Comment field LaunchConfig's Metadata missing Comment field #393 (This matches the problem I had)
• Configsets support Configsets support #424
• Metadata auth Metadata auth #155 (This PR adds the Metadata class to autoscaling)
• more metadata options more metadata options #153
• Commit 5e76bf7 adds the Metadata to cloudformation.py.
• Commit 33aedb8 handles CF templates to create Troposphere objects

Unfortunately, this exercise did not give me an opinion on which way to move forward. I'm sharing the links as it may be useful to you.

[rcuza]

Is this PR still relevant? If not, I'll close it out.

[rcuza]

The failure is in

FAIL: test_template_generator (tests.test_examples_template_generator.test_AutoScalingMultiAZWithNotifications)

If there is any interest in this let me know and I'll figure out how to update the tests to match the current code base. Otherwise I'll just close it.