From 047bd6f651c50c1e28030cdae3115d3a39116b1f Mon Sep 17 00:00:00 2001
From: moe <moe@busyloop.net>
Date: Wed, 4 Feb 2015 05:56:19 +0100
Subject: [PATCH 1/2] Fixes #744.

The following cases work for me now:
- Create new ASG with tags
- Update tags on ASG (create/change/delete)

In short, the module should now work as expected
wrt tagging. The previous code did not work at all
with latest boto for me (serialization errors) and
the logic was buggy anyway; e.g. removed tags
would never get deleted from ec2.
---
 cloud/amazon/ec2_asg.py | 78 +++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 35 deletions(-)

diff --git a/cloud/amazon/ec2_asg.py b/cloud/amazon/ec2_asg.py
index 6e5d3508cb8..c20f2d500d8 100644
--- a/cloud/amazon/ec2_asg.py
+++ b/cloud/amazon/ec2_asg.py
@@ -219,6 +219,25 @@ def enforce_required_arguments(module):
 
 def get_properties(autoscaling_group):
     properties = dict((attr, getattr(autoscaling_group, attr)) for attr in ASG_ATTRIBUTES)
+
+    #
+    # Ansible output is serialized to JSON but our
+    # tag list is sometimes a list of Tag-objects
+    # (as received by Boto).
+    #
+    # Since we can not easily teach python to serialize
+    # such a list we replace it with a dict-representation
+    # in those cases.
+    #
+    # Yes, this is an ugly hack. But at least it works,
+    # unlike the even uglier hack that was here previously...
+    #
+    if 'tags' in properties and properties['tags'] is list:
+        serializable_tags = {}
+        for tag in properties['tags']:
+            serializable_tags[tag.key] = [tag.value, tag.propagate_at_launch]
+        properties['tags'] = serializable_tags
+
     properties['healthy_instances'] = 0
     properties['in_service_instances'] = 0
     properties['unhealthy_instances'] = 0
@@ -281,18 +300,12 @@ def create_autoscaling_group(connection, module):
 
     asg_tags = []
     for tag in set_tags:
-        if tag.has_key('key') and tag.has_key('value'):  # this block is to support depricated form
-            asg_tags.append(Tag(key=tag.get('key'),
-                value=tag.get('value'),
-                propagate_at_launch=bool(tag.get('propagate_at_launch', True)),
-                resource_id=group_name))
-        else:
-            for k,v in tag.iteritems():
-                if k !='propagate_at_launch':
-                    asg_tags.append(Tag(key=k,
-                         value=v,
-                         propagate_at_launch=bool(tag.get('propagate_at_launch', True)),
-                         resource_id=group_name))
+        for k,v in tag.iteritems():
+            if k !='propagate_at_launch':
+                asg_tags.append(Tag(key=k,
+                     value=v,
+                     propagate_at_launch=bool(tag.get('propagate_at_launch', True)),
+                     resource_id=group_name))
 
     if not as_groups:
         if not vpc_zone_identifier and not availability_zones:
@@ -344,30 +357,25 @@ def create_autoscaling_group(connection, module):
                     setattr(as_group, attr, module_attr)
 
         if len(set_tags) > 0:
-            existing_tags = as_group.tags
-            existing_tag_map = dict((tag.key, tag) for tag in existing_tags)
-            for tag in set_tags:
-                if tag.has_key('key') and tag.has_key('value'):  # this is to support deprecated method
-                    if 'key' not in tag:
-                        continue
-                    if ( not tag['key'] in existing_tag_map or
-                        existing_tag_map[tag['key']].value != tag['value'] or
-                        ('propagate_at_launch' in tag and
-                        existing_tag_map[tag['key']].propagate_at_launch != tag['propagate_at_launch']) ):
-                        changed = True
-                        continue
-                else:
-                    for k,v in tag.iteritems():
-                        if k !='propagate_at_launch':
-                            if ( not k in existing_tag_map or
-                                existing_tag_map[k].value != v or
-                                ('propagate_at_launch' in tag and
-                                existing_tag_map[k].propagate_at_launch != tag['propagate_at_launch']) ):
-                                changed = True
-                                continue
-            if changed:
+            have_tags = {}
+            want_tags = {}
+
+            for tag in asg_tags:
+                want_tags[tag.key] = [tag.value, tag.propagate_at_launch]
+
+            for tag in as_group.tags:
+                have_tags[tag.key] = [tag.value, tag.propagate_at_launch]
+                dead_tags = []
+                if not tag.key in want_tags:
+                    changed = True
+                    dead_tags.append(tag)
+
+                if dead_tags != []:
+                    connection.delete_tags(dead_tags)
+
+            if have_tags != want_tags:
+                changed = True
                 connection.create_or_update_tags(asg_tags)
-                as_group.tags = asg_tags
 
         # handle loadbalancers separately because None != []
         load_balancers = module.params.get('load_balancers') or []

From 0d0205ad54b805635ee32723f167e1b8123252ef Mon Sep 17 00:00:00 2001
From: moe <moe@busyloop.net>
Date: Wed, 4 Feb 2015 06:13:52 +0100
Subject: [PATCH 2/2] Woops, make collect/delete loop more efficient.

---
 cloud/amazon/ec2_asg.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cloud/amazon/ec2_asg.py b/cloud/amazon/ec2_asg.py
index c20f2d500d8..ac5e501bbee 100644
--- a/cloud/amazon/ec2_asg.py
+++ b/cloud/amazon/ec2_asg.py
@@ -363,15 +363,15 @@ def create_autoscaling_group(connection, module):
             for tag in asg_tags:
                 want_tags[tag.key] = [tag.value, tag.propagate_at_launch]
 
+            dead_tags = []
             for tag in as_group.tags:
                 have_tags[tag.key] = [tag.value, tag.propagate_at_launch]
-                dead_tags = []
                 if not tag.key in want_tags:
                     changed = True
                     dead_tags.append(tag)
 
-                if dead_tags != []:
-                    connection.delete_tags(dead_tags)
+            if dead_tags != []:
+                connection.delete_tags(dead_tags)
 
             if have_tags != want_tags:
                 changed = True