aws_ec2 - fix when hostnames bugs

Signed-off-by: Alina Buzachis <[email protected]>

plugins/inventory/aws_ec2.py

@@ -270,16 +270,6 @@
 from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code
 
 
-import logging
-logging.basicConfig(filename='/Users/alinabuzachis/dev/example.log', level=logging.DEBUG)
-logger = logging.getLogger("test")
-logger.setLevel(logging.DEBUG)
-ch = logging.StreamHandler()
-formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - " "%(message)s")
-ch.setFormatter(formatter)
-logger.addHandler(ch)
-
-
 # The mappings give an array of keys to get from the filter name to the value
 # returned by boto3's EC2 describe_instances method.
 
@@ -608,28 +598,30 @@ def _get_event_set_and_persistence(self, connection, instance_id, spot_instance)
         return host_vars
 
     def _get_tag_hostname(self, preference, instance):
-        logger.debug("preference.split('tag:', 1)")
-        logger.debug(preference.split('tag:', 1)[1])
         tag_hostnames = preference.split('tag:', 1)[1]
         if ',' in tag_hostnames:
             tag_hostnames = tag_hostnames.split(',')
         else:
             tag_hostnames = [tag_hostnames]
-        logger.debug("tag_hostnames")
-        logger.debug(tag_hostnames)
+
         tags = boto3_tag_list_to_ansible_dict(instance.get('Tags', []))
-        logger.debug("tags")
-        logger.debug(tags)
+        tag_values = []
         for v in tag_hostnames:
             if '=' in v:
                 tag_name, tag_value = v.split('=')
                 if tags.get(tag_name) == tag_value:
-                    return to_text(tag_name) + "_" + to_text(tag_value)
+                    tag_values.append(to_text(tag_name) + "_" + to_text(tag_value))
             else:
                 tag_value = tags.get(v)
                 if tag_value:
-                    return to_text(tag_value)
-        return None
+                    tag_values.append(to_text(tag_value))
+        return tag_values
+
+    def _sanitize_hostname(self, hostname):
+        if ':' in to_text(hostname):
+            return self._sanitize_group_name(to_text(hostname))
+        else:
+            return to_text(hostname)
 
     def _get_hostname(self, instance, hostnames):
         '''
@@ -641,53 +633,29 @@ def _get_hostname(self, instance, hostnames):
             hostnames = ['dns-name', 'private-dns-name']
 
         hostname = None
-        sanitized_hosts = []
+        hostname_list = []
         for preference in hostnames:
-            logger.debug("PREFERENCE")
-            logger.debug(preference)
             if isinstance(preference, dict):
                 if 'name' not in preference:
                     raise AnsibleError("A 'name' key must be defined in a hostnames dictionary.")
                 hostname = self._get_hostname(instance, [preference["name"]])
                 hostname_from_prefix = self._get_hostname(instance, [preference["prefix"]])
                 separator = preference.get("separator", "_")
                 if hostname and hostname_from_prefix and 'prefix' in preference:
-                    hostname = hostname_from_prefix + separator + hostname
+                    hostname = hostname_from_prefix[0] + separator + hostname[0]
             elif preference.startswith('tag:'):
                 hostname = self._get_tag_hostname(preference, instance)
-                logger.debug("HOSTNAMEHEREEEE")
-                logger.debug(hostname)
             else:
                 hostname = self._get_boto_attr_chain(preference, instance)
-            #if hostname:
-            #    break
+
             if hostname:
-                logger.debug("hostnamehere")
-                logger.debug(hostname)
-                if ':' in to_text(hostname):
-                    sanitized_hosts.append(self._sanitize_group_name(to_text(hostname)))
-                else:
-                    sanitized_hosts.append(hostname)
-        logger.debug("sanitized_hosts")
-        logger.debug(sanitized_hosts)
-        return sanitized_hosts
-            # for host in hostname:
-            #     logger.debug(host)
-            #     if ':' in to_text(host):
-            #         sanitizied_hostnames += self._sanitize_group_name(to_text(host))
-            #     else:
-            #         logger.debug("host inside")
-            #         logger.debug(host)
-            #         sanitizied_hostnames += to_text(host)
-            #     sanitizied_hostnames += ','
-            # logger.debug("sanitizied_hostnames")
-            # logger.debug(sanitizied_hostnames)
-            # return sanitizied_hostnames
-                # else:
-                #     logger.debug("to_text(hostname)")
-                #     logger.debug(to_text(hostname))
-                #     return to_text(hostname)
-
+                if isinstance(hostname, list):
+                    for host in hostname:
+                        hostname_list.append(self._sanitize_hostname(host))
+                elif isinstance(hostname, str):
+                    hostname_list.append(self._sanitize_hostname(hostname))
+
+        return hostname_list
 
     def _query(self, regions, include_filters, exclude_filters, strict_permissions):
         '''
@@ -720,10 +688,6 @@ def _query(self, regions, include_filters, exclude_filters, strict_permissions):
 
     def _populate(self, groups, hostnames):
         for group in groups:
-            logger.debug("GROUP")
-            logger.debug(group)
-            logger.debug("hostnames")
-            logger.debug(hostnames)
             group = self.inventory.add_group(group)
             self._add_hosts(hosts=groups[group], group=group, hostnames=hostnames)
             self.inventory.add_child('all', group)
@@ -735,33 +699,22 @@ def _add_hosts(self, hosts, group, hostnames):
             :param hostnames: a list of hostname destination variables in order of preference
         '''
         for host in hosts:
-            hostname = self._get_hostname(host, hostnames)
-            logger.debug("hostnamefine")
-            logger.debug(hostname)
+            hostname_list = self._get_hostname(host, hostnames)
 
             host = camel_dict_to_snake_dict(host, ignore_list=['Tags'])
             host['tags'] = boto3_tag_list_to_ansible_dict(host.get('tags', []))
 
-            logger.debug("self.get_option('use_contrib_script_compatible_ec2_tag_keys')")
-            logger.debug(self.get_option('use_contrib_script_compatible_ec2_tag_keys'))
-
             if self.get_option('use_contrib_script_compatible_ec2_tag_keys'):
                 for k, v in host['tags'].items():
-                    logger.debug("k, v")
-                    logger.debug(k)
-                    logger.debug(v)
                     host["ec2_tag_%s" % k] = v
-                logger.debug("host tags")
-                logger.debug(host)
+
             # Allow easier grouping by region
             host['placement']['region'] = host['placement']['availability_zone'][:-1]
 
-            if not hostname:
+            if not hostname_list:
                 continue
-            for h in hostname:
-                self.inventory.add_host(to_text(h), group=group)
-            logger.debug("\nself.inventory")
-            logger.debug(self.inventory.hosts)
+            for hostname in hostname_list:
+                self.inventory.add_host(to_text(hostname), group=group)
             hostvars_prefix = self.get_option("hostvars_prefix")
             hostvars_suffix = self.get_option("hostvars_suffix")
             new_vars = dict()
@@ -771,23 +724,23 @@ def _add_hosts(self, hosts, group, hostnames):
                 if hostvars_suffix:
                     hostvar = hostvar + hostvars_suffix
                 new_vars[hostvar] = hostval
-                for h in hostname:
-                    self.inventory.set_variable(to_text(h), hostvar, hostval)
+                for hostname in hostname_list:
+                    self.inventory.set_variable(to_text(hostname), hostvar, hostval)
             host.update(new_vars)
 
             # Use constructed if applicable
 
             strict = self.get_option('strict')
 
             # Composed variables
-            for h in hostname:
-                self._set_composite_vars(self.get_option('compose'), host, to_text(h), strict=strict)
+            for hostname in hostname_list:
+                self._set_composite_vars(self.get_option('compose'), host, to_text(hostname), strict=strict)
 
                 # Complex groups based on jinja2 conditionals, hosts that meet the conditional are added to group
-                self._add_host_to_composed_groups(self.get_option('groups'), host, to_text(h), strict=strict)
+                self._add_host_to_composed_groups(self.get_option('groups'), host, to_text(hostname), strict=strict)
 
                 # Create groups based on variable values and add the corresponding hosts to it
-                self._add_host_to_keyed_groups(self.get_option('keyed_groups'), host, to_text(h), strict=strict)
+                self._add_host_to_keyed_groups(self.get_option('keyed_groups'), host, to_text(hostname), strict=strict)
 
     def _set_credentials(self, loader):
         '''

tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames.yml

@@ -0,0 +1,59 @@
+---
+- hosts: 127.0.0.1
+  connection: local
+  gather_facts: false
+  environment: "{{ ansible_test.environment }}"
+  tasks:
+
+    - module_defaults:
+        group/aws:
+          aws_access_key: '{{ aws_access_key }}'
+          aws_secret_key: '{{ aws_secret_key }}'
+          security_token: '{{ security_token | default(omit) }}'
+          region: '{{ aws_region }}'
+      block:
+
+        # Create VPC, subnet, security group, and find image_id to create instance
+        - include_tasks: setup.yml
+
+        # Create new host
+        - name: create a new host
+          ec2_instance:
+            image_id: '{{ image_id }}'
+            name: '{{ resource_prefix }}'
+            tags:
+              Tag1: Test1
+              Tag2: Test2
+            instance_type: t2.micro
+            security_groups: '{{ sg_id }}'
+            vpc_subnet_id: '{{ subnet_id }}'
+            wait: false
+          register: setup_instance
+
+        # refresh inventory
+        - meta: refresh_inventory
+
+        - name: assert groups and hostvars were populated with inventory
+          assert:
+            that:
+              - "'aws_ec2' in groups"
+              - groups['aws_ec2'] | length == 2
+              - "'Test1' in groups['aws_ec2']"
+              - "'Test2' in groups['aws_ec2']"
+              - "'Test1' in hostvars"
+              - "'Test2' in hostvars"
+
+      always:
+
+        - name: remove ec2 instance
+          ec2_instance:
+            instance_type: t2.micro
+            instance_ids: '{{ setup_instance.instance_ids }}'
+            state: absent
+            name: '{{ resource_prefix }}'
+            security_groups: "{{ sg_id }}"
+            vpc_subnet_id: "{{ subnet_id }}"
+          ignore_errors: true
+          when: setup_instance is defined
+
+        - include_tasks: tear_down.yml

tests/integration/targets/inventory_aws_ec2/runme.sh

@@ -40,6 +40,8 @@ ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_w
 ansible-playbook playbooks/test_populating_inventory_with_concatenation.yml "$@"
 ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_literal_string.yml.j2'" "$@"
 ansible-playbook playbooks/test_populating_inventory_with_literal_string.yml "$@"
+ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_hostnames.yml.j2'" "$@"
+ansible-playbook playbooks/test_populating_inventory_with_hostnames.yml "$@"
 
 # generate inventory config with includes_entries_matching and prepare the tests
 ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_include_or_exclude_filters.yml.j2'" "$@"

tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames.yml.j2

@@ -0,0 +1,21 @@
+plugin: amazon.aws.aws_ec2
+aws_access_key_id: '{{ aws_access_key }}'
+aws_secret_access_key: '{{ aws_secret_key }}'
+{% if security_token | default(false) %}
+aws_security_token: '{{ security_token }}'
+{% endif %}
+regions:
+- '{{ aws_region }}'
+keyed_groups:
+- prefix: tag
+  key: tags
+hostnames:
+# can also be specified using
+# - tag:Tag1,Tag2
+# or
+# - tag:Tag1=Test1,Tag2=Test2
+# or
+- tag:Tag1
+- tag:Tag2
+compose:
+   ansible_host: private_ip_address

tests/unit/plugins/inventory/test_aws_ec2.py

@@ -145,19 +145,19 @@ def test_boto3_conn(inventory):
 
 def test_get_hostname_default(inventory):
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames=None) == "ec2-12-345-67-890.compute-1.amazonaws.com"
+    assert inventory._get_hostname(instance, hostnames=None)[0] == "ec2-12-345-67-890.compute-1.amazonaws.com"
 
 
 def test_get_hostname(inventory):
     hostnames = ['ip-address', 'dns-name']
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames) == "12.345.67.890"
+    assert inventory._get_hostname(instance, hostnames)[0] == "12.345.67.890"
 
 
 def test_get_hostname_dict(inventory):
     hostnames = [{'name': 'private-ip-address', 'separator': '_', 'prefix': 'tag:Name'}]
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames) == "aws_ec2_098.76.54.321"
+    assert inventory._get_hostname(instance, hostnames)[0] == "aws_ec2_098.76.54.321"
 
 
 def test_set_credentials(inventory):