From d09ad82e7159ea6dd2e694418bf32da9e87be4dc Mon Sep 17 00:00:00 2001 From: Tom Melendez Date: Thu, 29 Dec 2016 05:45:20 -0800 Subject: [PATCH] Add regex support to gce_tag module, add unit tests and update integration test. (#19087) The gce_tag module can support updating tags on multiple instances via an instance_pattern field. Full Python regex is supported in the instance_pattern field. 'instance_pattern' and 'instance_name' are mutually exclusive and one must be specified. The integration test for the gce_tag module has been updated to support the instance_pattern parameter. Unit tests have been added to test the list-manipulation functionality. Run the integration test with: TEST_FLAGS='--tags "test_gce_tag"' make gce Run the unit tests with: python test/units/modules/cloud/google/test_gce_tag.py --- lib/ansible/modules/cloud/google/gce_tag.py | 192 +++++++++--------- .../roles/test_gce_tag/tasks/test.yml | 86 +++++++- test/units/modules/cloud/google/__init__.py | 0 .../modules/cloud/google/test_gce_tag.py | 68 +++++++ 4 files changed, 250 insertions(+), 96 deletions(-) create mode 100644 test/units/modules/cloud/google/__init__.py create mode 100644 test/units/modules/cloud/google/test_gce_tag.py diff --git a/lib/ansible/modules/cloud/google/gce_tag.py b/lib/ansible/modules/cloud/google/gce_tag.py index 7122a2398a0..03afa025d0b 100644 --- a/lib/ansible/modules/cloud/google/gce_tag.py +++ b/lib/ansible/modules/cloud/google/gce_tag.py @@ -22,17 +22,25 @@ DOCUMENTATION = ''' --- module: gce_tag version_added: "2.0" -short_description: add or remove tag(s) to/from GCE instance +short_description: add or remove tag(s) to/from GCE instances description: - - This module can add or remove tags U(https://cloud.google.com/compute/docs/instances/#tags) - to/from GCE instance. + - This module can add or remove tags U(https://cloud.google.com/compute/docs/label-or-tag-resources#tags) + to/from GCE instances. Use 'instance_pattern' to update multiple instances in a specify zone options: instance_name: description: - - the name of the GCE instance to add/remove tags - required: true + - The name of the GCE instance to add/remove tags. Required if instance_pattern is not specified. + required: false default: null aliases: [] + instance_pattern: + description: + - The pattern of GCE instance names to match for adding/removing tags. Full-Python regex is supported. See U(https://docs.python.org/2/library/re.html) for details. + If instance_name is not specified, this field is required. + required: false + default: null + aliases: [] + version_added: "2.3" tags: description: - comma-separated list of tags to add or remove @@ -73,8 +81,12 @@ options: requirements: - "python >= 2.6" - - "apache-libcloud" -author: "Do Hoang Khiem (dohoangkhiem@gmail.com)" + - "apache-libcloud >= 0.17.0" +notes: + - Either I(instance_name) or I(instance_pattern) is required. +author: + - Do Hoang Khiem (dohoangkhiem@gmail.com) + - Tom Melendez (@supertom) ''' EXAMPLES = ''' @@ -91,7 +103,16 @@ EXAMPLES = ''' tags: foo,bar state: absent +# Add tags 'foo', 'bar' to instances in zone that match pattern +- gce_tag: + instance_pattern: test-server-* + tags: foo,bar + zone: us-central1-a + state: present + ''' +import re +import traceback try: from libcloud.compute.types import Provider @@ -107,38 +128,40 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.gce import gce_connect +def _union_items(baselist, comparelist): + """Combine two lists, removing duplicates.""" + return list(set(baselist) | set(comparelist)) -def add_tags(gce, module, instance_name, tags): - """Add tags to instance.""" - zone = module.params.get('zone') +def _intersect_items(baselist, comparelist): + """Return matching items in both lists.""" + return list(set(baselist) & set(comparelist)) - if not instance_name: - module.fail_json(msg='Must supply instance_name', changed=False) +def _get_changed_items(baselist, comparelist): + """Return changed items as they relate to baselist.""" + return list(set(baselist) & set(set(baselist) ^ set(comparelist))) - if not tags: - module.fail_json(msg='Must supply tags', changed=False) +def modify_tags(gce, module, node, tags, state='present'): + """Modify tags on an instance.""" + zone = node.extra['zone'].name + existing_tags = node.extra['tags'] tags = [x.lower() for x in tags] - - try: - node = gce.ex_get_node(instance_name, zone=zone) - except ResourceNotFoundError: - module.fail_json(msg='Instance %s not found in zone %s' % (instance_name, zone), changed=False) - except GoogleBaseError as e: - module.fail_json(msg=str(e), changed=False) - - node_tags = node.extra['tags'] - changed = False tags_changed = [] - for t in tags: - if t not in node_tags: - changed = True - node_tags.append(t) - tags_changed.append(t) - - if not changed: - return False, None + if state == 'absent': + # tags changed are any that intersect + tags_changed = _intersect_items(existing_tags, tags) + if not tags_changed: + return False, None + # update instance with tags in existing tags that weren't specified + node_tags = _get_changed_items(existing_tags, tags) + else: + # tags changed are any that in the new list that weren't in existing + tags_changed = _get_changed_items(tags, existing_tags) + if not tags_changed: + return False, None + # update instance with the combined list + node_tags = _union_items(existing_tags, tags) try: gce.ex_set_node_tags(node, node_tags) @@ -146,87 +169,70 @@ def add_tags(gce, module, instance_name, tags): except (GoogleBaseError, InvalidRequestError) as e: module.fail_json(msg=str(e), changed=False) - -def remove_tags(gce, module, instance_name, tags): - """Remove tags from instance.""" - zone = module.params.get('zone') - - if not instance_name: - module.fail_json(msg='Must supply instance_name', changed=False) - - if not tags: - module.fail_json(msg='Must supply tags', changed=False) - - tags = [x.lower() for x in tags] - - try: - node = gce.ex_get_node(instance_name, zone=zone) - except ResourceNotFoundError: - module.fail_json(msg='Instance %s not found in zone %s' % (instance_name, zone), changed=False) - except GoogleBaseError as e: - module.fail_json(msg=str(e), changed=False) - - node_tags = node.extra['tags'] - - changed = False - tags_changed = [] - - for t in tags: - if t in node_tags: - node_tags.remove(t) - changed = True - tags_changed.append(t) - - if not changed: - return False, None - - try: - gce.ex_set_node_tags(node, node_tags) - return True, tags_changed - except (GoogleBaseError, InvalidRequestError) as e: - module.fail_json(msg=str(e), changed=False) - - def main(): module = AnsibleModule( argument_spec=dict( - instance_name=dict(required=True), - tags=dict(type='list'), + instance_name=dict(required=False), + instance_pattern=dict(required=False), + tags=dict(type='list', required=True), state=dict(default='present', choices=['present', 'absent']), zone=dict(default='us-central1-a'), service_account_email=dict(), pem_file=dict(type='path'), project_id=dict(), - ) + ), + mutually_exclusive=[ + [ 'instance_name', 'instance_pattern' ] + ], + required_one_of=[ + [ 'instance_name', 'instance_pattern' ] + ] ) - if not HAS_LIBCLOUD: - module.fail_json(msg='libcloud with GCE support is required.') - instance_name = module.params.get('instance_name') + instance_pattern = module.params.get('instance_pattern') state = module.params.get('state') tags = module.params.get('tags') zone = module.params.get('zone') changed = False - if not zone: - module.fail_json(msg='Must specify "zone"', changed=False) - - if not tags: - module.fail_json(msg='Must specify "tags"', changed=False) + if not HAS_LIBCLOUD: + module.fail_json(msg='libcloud with GCE support (0.17.0+) required for this module') gce = gce_connect(module) - # add tags to instance. - if state == 'present': - changed, tags_changed = add_tags(gce, module, instance_name, tags) - - # remove tags from instance - if state == 'absent': - changed, tags_changed = remove_tags(gce, module, instance_name, tags) - - module.exit_json(changed=changed, instance_name=instance_name, tags=tags_changed, zone=zone) + # Create list of nodes to operate on + matching_nodes = [] + try: + if instance_pattern: + instances = gce.list_nodes(ex_zone=zone) + # no instances in zone + if not instances: + module.exit_json(changed=False, tags=tags, zone=zone, instances_updated=[]) + try: + # Python regex fully supported: https://docs.python.org/2/library/re.html + p = re.compile(instance_pattern) + matching_nodes = [i for i in instances if p.search(i.name) is not None] + except re.error as e: + module.fail_json(msg='Regex error for pattern %s: %s' % (instance_pattern, e), changed=False) + else: + matching_nodes = [gce.ex_get_node(instance_name, zone=zone)] + except ResourceNotFoundError: + module.fail_json(msg='Instance %s not found in zone %s' % (instance_name, zone), changed=False) + except GoogleBaseError as e: + module.fail_json(msg=str(e), changed=False, exception=traceback.format_exc()) + # Tag nodes + instance_pattern_matches = [] + tags_changed = [] + for node in matching_nodes: + changed, tags_changed = modify_tags(gce, module, node, tags, state) + if changed: + instance_pattern_matches.append({'instance_name': node.name, 'tags_changed': tags_changed}) + if instance_pattern: + module.exit_json(changed=changed, instance_pattern=instance_pattern, tags=tags_changed, zone=zone, instances_updated=instance_pattern_matches) + else: + module.exit_json(changed=changed, instance_name=instance_name, tags=tags_changed, zone=zone) if __name__ == '__main__': main() diff --git a/test/integration/roles/test_gce_tag/tasks/test.yml b/test/integration/roles/test_gce_tag/tasks/test.yml index a717f0f8211..4d475c07e72 100644 --- a/test/integration/roles/test_gce_tag/tasks/test.yml +++ b/test/integration/roles/test_gce_tag/tasks/test.yml @@ -2,11 +2,12 @@ ## Parameter checking tests ## # ============================================================ -- name: "test missing param: instance_name" +- name: "test missing param: instance_name or instance_pattern" gce_tag: service_account_email: "{{ service_account_email }}" pem_file: "{{ pem_file }}" project_id: "{{ project_id }}" + tags: foo,bar register: result ignore_errors: true tags: @@ -16,7 +17,7 @@ assert: that: - 'result.failed' - - 'result.msg == "missing required arguments: instance_name"' + - 'result.msg == "one of the following is required: instance_name,instance_pattern"' # ============================================================ @@ -36,8 +37,27 @@ assert: that: - 'result.failed' - - 'result.msg == "Must specify \"tags\""' + - 'result.msg == "missing required arguments: tags"' +# ============================================================ +- name: "test bad regex: instance_pattern" + gce_tag: + service_account_email: "{{ service_account_email }}" + pem_file: "{{ pem_file }}" + project_id: "{{ project_id }}" + zone: "{{ zone }}" + tags: foo,bar + instance_pattern: "&23424--[" + register: result + ignore_errors: true + tags: + - param-check + +- name: "assert failure when instance_pattern is invalid" + assert: + that: + - 'result.failed' + - 'result.msg == "Regex error for pattern &23424--[: unexpected end of regular expression"' ## Non-existant instance tests ## # # ============================================================ @@ -114,6 +134,45 @@ - 'result.changed == False' - 'result.tags == None' +# # ============================================================ +- name: "add tags using pattern (state==present)" + gce_tag: + service_account_email: "{{ service_account_email }}" + pem_file: "{{ pem_file }}" + project_id: "{{ project_id }}" + instance_pattern: "{{ instance_name }}" + zone: "{{ zone }}" + tags: instance-pattern-test + state: present + register: result + +- name: "assert tag using pattern successful" + assert: + that: + - 'result.changed == True' + - 'result.tags == ["instance-pattern-test"]' + - 'result.instances_updated[0].instance_name == "{{ instance_name }}"' + - 'result.instances_updated[0].tags_changed[0] == "instance-pattern-test"' + +# # ============================================================ +- name: "add existing tags with pattern, no change (state==present)" + gce_tag: + service_account_email: "{{ service_account_email }}" + pem_file: "{{ pem_file }}" + project_id: "{{ project_id }}" + instance_pattern: "{{ instance_name }}" + zone: "{{ zone }}" + tags: instance-pattern-test + state: present + register: result + +- name: "assert tag with pattern no change" + assert: + that: + - 'result.changed == False' + - 'result.tags == None' + - 'result.instances_updated|length == 0' + # # ============================================================ - name: "test tags removed from instance (state==absent)" gce_tag: @@ -131,3 +190,24 @@ that: - 'result.changed' - 'result.tags == ["foo", "bar"]' + + +# # ============================================================ +- name: "test tags removed with instance_pattern (state==absent)" + gce_tag: + service_account_email: "{{ service_account_email }}" + pem_file: "{{ pem_file }}" + project_id: "{{ project_id }}" + instance_pattern: "{{ instance_name }}" + zone: "{{ zone }}" + tags: instance-pattern-test + state: absent + register: result + +- name: "assert tags removed with instance_pattern" + assert: + that: + - 'result.changed' + - 'result.tags == ["instance-pattern-test"]' + - 'result.instances_updated[0].instance_name == "{{ instance_name }}"' + - 'result.instances_updated[0].tags_changed[0] == "instance-pattern-test"' diff --git a/test/units/modules/cloud/google/__init__.py b/test/units/modules/cloud/google/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/units/modules/cloud/google/test_gce_tag.py b/test/units/modules/cloud/google/test_gce_tag.py new file mode 100644 index 00000000000..6a4595e263d --- /dev/null +++ b/test/units/modules/cloud/google/test_gce_tag.py @@ -0,0 +1,68 @@ +#!/usr/bin/env python + +import unittest + +from ansible.modules.cloud.google.gce_tag import _get_changed_items, _intersect_items, _union_items + +class TestGCETag(unittest.TestCase): + """Unit tests for gce_tag module.""" + + def test_union_items(self): + """ + Combine items in both lists + removing duplicates. + """ + listA = [1, 2, 3, 4, 5, 8, 9] + listB = [1, 2, 3, 4, 5, 6, 7] + want = [1, 2, 3, 4, 5, 6, 7, 8, 9] + got = _union_items(listA, listB) + self.assertEqual(want, got) + + def test_intersect_items(self): + """ + All unique items from either list. + """ + listA = [1, 2, 3, 4, 5, 8, 9] + listB = [1, 2, 3, 4, 5, 6, 7] + want = [1, 2, 3, 4, 5] + got = _intersect_items(listA, listB) + self.assertEqual(want, got) + + # tags removed + new_tags = ['one', 'two'] + existing_tags = ['two'] + want = ['two'] # only remove the tag that was present + got = _intersect_items(existing_tags, new_tags) + self.assertEqual(want, got) + + def test_get_changed_items(self): + """ + All the items from left list that don't match + any item from the right list. + """ + listA = [1, 2, 3, 4, 5, 8, 9] + listB = [1, 2, 3, 4, 5, 6, 7] + want = [8, 9] + got = _get_changed_items(listA, listB) + self.assertEqual(want, got) + + # simulate new tags added + tags_to_add = ['one', 'two'] + existing_tags = ['two'] + want = ['one'] + got = _get_changed_items(tags_to_add, existing_tags) + self.assertEqual(want, got) + + # simulate removing tags + # specifying one tag on right that doesn't exist + tags_to_remove = ['one', 'two'] + existing_tags = ['two', 'three'] + want = ['three'] + got = _get_changed_items(existing_tags, tags_to_remove) + self.assertEqual(want, got) + +if __name__ == '__main__': + unittest.main() + + +