better handling of diff key types in compose

also made separator configurable
fixed some exception types
better error msgs

lib/ansible/plugins/inventory/__init__.py

@@ -24,9 +24,9 @@ import os
 import re
 import string
 
-from collections import MutableMapping
+from collections import Mapping
 
-from ansible.errors import AnsibleError, AnsibleOptionsError, AnsibleParserError
+from ansible.errors import AnsibleError, AnsibleParserError
 from ansible.plugins import AnsiblePlugin
 from ansible.plugins.cache import InventoryFileCacheModule
 from ansible.module_utils._text import to_bytes, to_native
@@ -159,7 +159,7 @@ class BaseInventoryPlugin(AnsiblePlugin):
         return (os.path.exists(b_path) and os.access(b_path, os.R_OK))
 
     def _populate_host_vars(self, hosts, variables, group=None, port=None):
-        if not isinstance(variables, MutableMapping):
+        if not isinstance(variables, Mapping):
             raise AnsibleParserError("Invalid data from file, expected dictionary and got:\n\n%s" % to_native(variables))
 
         for host in hosts:
@@ -182,7 +182,7 @@ class BaseInventoryPlugin(AnsiblePlugin):
         elif config.get('plugin') != self.NAME:
             # this is not my config file
             raise AnsibleParserError("Incorrect plugin name in file: %s" % config.get('plugin', 'none found'))
-        elif not isinstance(config, MutableMapping):
+        elif not isinstance(config, Mapping):
             # configs are dictionaries
             raise AnsibleParserError('inventory source has invalid structure, it should be a dictionary, got: %s' % type(config))
 
@@ -253,7 +253,7 @@ class Constructable(object):
         ''' helper method for pluigns to compose variables for Ansible based on jinja2 expression and inventory vars'''
         t = self.templar
         t.set_available_variables(variables)
-        return t.do_template('%s%s%s' % (t.environment.variable_start_string, template, t.environment.variable_end_string), disable_lookups=True)
+        return t.template('%s%s%s' % (t.environment.variable_start_string, template, t.environment.variable_end_string), disable_lookups=True)
 
     def _set_composite_vars(self, compose, variables, host, strict=False):
         ''' loops over compose entries to create vars for hosts '''
@@ -263,7 +263,7 @@ class Constructable(object):
                     composite = self._compose(compose[varname], variables)
                 except Exception as e:
                     if strict:
-                        raise AnsibleOptionsError("Could set %s: %s" % (varname, to_native(e)))
+                        raise AnsibleError("Could not set %s: %s" % (varname, to_native(e)))
                     continue
                 self.inventory.set_variable(host, varname, composite)
 
@@ -278,8 +278,9 @@ class Constructable(object):
                     result = boolean(self.templar.template(conditional))
                 except Exception as e:
                     if strict:
-                        raise AnsibleOptionsError("Could not add to group %s: %s" % (group_name, to_native(e)))
+                        raise AnsibleParserError("Could not add host %s to group %s: %s" % (host, group_name, to_native(e)))
                     continue
+
                 if result:
                     # ensure group exists
                     self.inventory.add_group(group_name)
@@ -289,27 +290,40 @@ class Constructable(object):
     def _add_host_to_keyed_groups(self, keys, variables, host, strict=False):
         ''' helper to create groups for plugins based on variable values and add the corresponding hosts to it'''
         if keys and isinstance(keys, list):
+            groups = []
             for keyed in keys:
                 if keyed and isinstance(keyed, dict):
-                    prefix = keyed.get('prefix', '')
+
-                    key = keyed.get('key')
+                    try:
-                    if key is not None:
+                        key = self._compose(keyed.get('key'), variables)
-                        try:
+                    except Exception as e:
-                            groups = to_safe_group_name('%s_%s' % (prefix, self._compose(key, variables)))
+                        if strict:
-                        except Exception as e:
+                            raise AnsibleParserError("Could not generate group from %s entry: %s" % (keyed.get('key'), to_native(e)))
-                            if strict:
+                        continue
-                                raise AnsibleOptionsError("Could not generate group on %s: %s" % (key, to_native(e)))
+
-                            continue
+                    if key:
-                        if isinstance(groups, string_types):
+                        prefix = keyed.get('prefix', '')
-                            groups = [groups]
+                        sep = keyed.get('separator', '_')
-                        if isinstance(groups, list):
+
-                            for group_name in groups:
+                        if isinstance(key, string_types):
-                                if group_name not in self.inventory.groups:
+                            groups.append('%s%s%s' % (prefix, sep, key))
-                                    self.inventory.add_group(group_name)
+                        elif isinstance(key, list):
-                                self.inventory.add_child(group_name, host)
+                            for name in key:
+                                groups.append('%s%s%s' % (prefix, sep, name))
+                        elif isinstance(key, Mapping):
+                            for (gname, gval) in key.items():
+                                name = '%s%s%s' % (gname, sep, gval)
+                                groups.append('%s%s%s' % (prefix, sep, name))
                         else:
-                            raise AnsibleOptionsError("Invalid group name format, expected string or list of strings, got: %s" % type(groups))
+                            raise AnsibleParserError("Invalid group name format, expected a string or a list of them or dictionary, got: %s" % type(key))
                     else:
-                        raise AnsibleOptionsError("No key supplied, invalid entry")
+                        if strict:
+                            raise AnsibleParserError("No key or key resulted empty, invalid entry")
                 else:
-                    raise AnsibleOptionsError("Invalid keyed group entry, it must be a dictionary: %s " % keyed)
+                    raise AnsibleParserError("Invalid keyed group entry, it must be a dictionary: %s " % keyed)
+
+            # now actually add any groups
+            for group_name in groups:
+                gname = to_safe_group_name(group_name)
+                self.inventory.add_group(gname)
+                self.inventory.add_child(gname, host)

lib/ansible/plugins/inventory/constructed.py

@@ -43,7 +43,8 @@ EXAMPLES = '''
         multi_group: (group_names|intersection(['alpha', 'beta', 'omega']))|length >= 2
 
     keyed_groups:
-        # this creates a group per distro (distro_CentOS, distro_Debian) and assigns the hosts that have matching values to it
+        # this creates a group per distro (distro_CentOS, distro_Debian) and assigns the hosts that have matching values to it,
+        # using the default separator "_"
         - prefix: distro
           key: ansible_distribution