From 262ba0460a55e3b679dbb194e80eb9e10d4e86cd Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:23:34 +0100 Subject: [PATCH 01/20] inventory directory parser: add hosts to group non-recursively --- lib/ansible/inventory/dir.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index 46997d9be89..9b53bd17c65 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -71,7 +71,7 @@ class InventoryDirectory(object): # note: depth numbers on duplicates may be bogus for k, v in group.get_variables().iteritems(): self.groups[name].set_variable(k, v) - for host in group.get_hosts(): + for host in group.hosts: if host.name not in self.hosts: self.hosts[host.name] = host else: From 8b215149d4694ab2644df4f231790c3382462d97 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:24:43 +0100 Subject: [PATCH 02/20] inventory directory parser: add groups to parent_groups non-recursively --- lib/ansible/inventory/dir.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index 9b53bd17c65..37cc5390442 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -83,8 +83,8 @@ class InventoryDirectory(object): # This needs to be a second loop to ensure all the parent groups exist for name, group in parser.groups.iteritems(): - for ancestor in group.get_ancestors(): - self.groups[ancestor.name].add_child_group(self.groups[name]) + for parent in group.parent_groups: + self.groups[parent.name].add_child_group(self.groups[name]) def get_host_variables(self, host): """ Gets additional host variables from all inventories """ From 36f55d354932d5dd29f8ecf17a6d1338f11b13d7 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:26:26 +0100 Subject: [PATCH 03/20] inventory directory parser: add host to group only once --- lib/ansible/inventory/dir.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index 37cc5390442..39f45f0920b 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -79,7 +79,10 @@ class InventoryDirectory(object): # note: depth numbers on duplicates may be bogus for k, v in host.vars.iteritems(): self.hosts[host.name].set_variable(k, v) - self.groups[name].add_host(self.hosts[host.name]) + # host can already exist from other source (same name, + # different object): + if host.name not in [h.name for h in self.groups[name].hosts]: + self.groups[name].add_host(self.hosts[host.name]) # This needs to be a second loop to ensure all the parent groups exist for name, group in parser.groups.iteritems(): From 1c8690987566a62e43638831db7ce69f7580896d Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:37:33 +0100 Subject: [PATCH 04/20] inventory group: add groups to parent_groups only once --- lib/ansible/inventory/group.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index c5270ad554c..94ee8ecebaa 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -41,7 +41,12 @@ class Group(object): if not group in self.child_groups: self.child_groups.append(group) group.depth = max([self.depth+1, group.depth]) - group.parent_groups.append(self) + + # now add self to child's parent_groups list, but only if there + # isn't already a group with the same name + if not self.name in [g.name for g in group.parent_groups]: + group.parent_groups.append(self) + self.clear_hosts_cache() def add_host(self, host): From cc8efb4aabeb1aafd8f0ec470ce82d4395eec8bf Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:43:48 +0100 Subject: [PATCH 05/20] inventory groups: make sure group.depth is updated on all grandchildren --- lib/ansible/inventory/dir.py | 1 - lib/ansible/inventory/group.py | 11 +++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index 39f45f0920b..74a8eacba9d 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -68,7 +68,6 @@ class InventoryDirectory(object): self.groups[name] = group else: # group is already there, copy variables - # note: depth numbers on duplicates may be bogus for k, v in group.get_variables().iteritems(): self.groups[name].set_variable(k, v) for host in group.hosts: diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index 94ee8ecebaa..eb2eb45ea31 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -40,8 +40,13 @@ class Group(object): # don't add if it's already there if not group in self.child_groups: self.child_groups.append(group) + + # update the depth of the child group.depth = max([self.depth+1, group.depth]) + # update the depth of the grandchildren + group._check_children_depth() + # now add self to child's parent_groups list, but only if there # isn't already a group with the same name if not self.name in [g.name for g in group.parent_groups]: @@ -49,6 +54,12 @@ class Group(object): self.clear_hosts_cache() + def _check_children_depth(self): + + for group in self.child_groups: + group.depth = max([self.depth+1, group.depth]) + group._check_children_depth() + def add_host(self, host): self.hosts.append(host) From 0ceefbbf29872b3a3e749f099aa28db971fa39ee Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 5 Mar 2014 16:45:24 +0100 Subject: [PATCH 06/20] inventory ini parser: do not add all the groups to *all* group but only those with lowest depth, so we keep a proper tree structure --- lib/ansible/inventory/ini.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/ansible/inventory/ini.py b/lib/ansible/inventory/ini.py index 9863de17b8e..3848696006e 100644 --- a/lib/ansible/inventory/ini.py +++ b/lib/ansible/inventory/ini.py @@ -45,6 +45,7 @@ class InventoryParser(object): self._parse_base_groups() self._parse_group_children() + self._add_allgroup_children() self._parse_group_variables() return self.groups @@ -69,6 +70,13 @@ class InventoryParser(object): # gamma sudo=True user=root # delta asdf=jkl favcolor=red + def _add_allgroup_children(self): + + for group in self.groups.values(): + if group.depth == 0 and group.name != 'all': + self.groups['all'].add_child_group(group) + + def _parse_base_groups(self): # FIXME: refactor @@ -87,11 +95,9 @@ class InventoryParser(object): active_group_name = active_group_name.rsplit(":", 1)[0] if active_group_name not in self.groups: new_group = self.groups[active_group_name] = Group(name=active_group_name) - all.add_child_group(new_group) active_group_name = None elif active_group_name not in self.groups: new_group = self.groups[active_group_name] = Group(name=active_group_name) - all.add_child_group(new_group) elif line.startswith(";") or line == '': pass elif active_group_name: From be58808fe4aa6c2979511e4a25044bdb27923022 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Mon, 10 Mar 2014 12:49:50 +0100 Subject: [PATCH 07/20] inventory script parser: do not add all the groups to *all* group --- lib/ansible/inventory/script.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/ansible/inventory/script.py b/lib/ansible/inventory/script.py index 05348b68397..89a8c9d0693 100644 --- a/lib/ansible/inventory/script.py +++ b/lib/ansible/inventory/script.py @@ -46,6 +46,7 @@ class InventoryScript(object): self.host_vars_from_top = None self.groups = self._parse(stderr) + def _parse(self, err): all_hosts = {} @@ -60,7 +61,7 @@ class InventoryScript(object): raise errors.AnsibleError("failed to parse executable inventory script results: %s" % self.raw) for (group_name, data) in self.raw.items(): - + # in Ansible 1.3 and later, a "_meta" subelement may contain # a variable "hostvars" which contains a hash for each host # if this "hostvars" exists at all then do not call --host for each @@ -97,8 +98,6 @@ class InventoryScript(object): all.set_variable(k, v) else: group.set_variable(k, v) - if group.name != all.name: - all.add_child_group(group) # Separate loop to ensure all groups are defined for (group_name, data) in self.raw.items(): @@ -108,6 +107,11 @@ class InventoryScript(object): for child_name in data['children']: if child_name in groups: groups[group_name].add_child_group(groups[child_name]) + + for group in groups.values(): + if group.depth == 0 and group.name != 'all': + all.add_child_group(group) + return groups def get_host_variables(self, host): From 188375171e3790b7e2b478f5d64b6ad1711a6494 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Mon, 10 Mar 2014 13:06:04 +0100 Subject: [PATCH 08/20] Inventory: raise error when adding a group that already exists. The parsers check if a group already exists. --- lib/ansible/inventory/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index b6f644190f1..9f076a72cdc 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -16,7 +16,6 @@ # along with Ansible. If not, see . ############################################# - import fnmatch import os import sys @@ -376,8 +375,11 @@ class Inventory(object): return vars def add_group(self, group): - self.groups.append(group) - self._groups_list = None # invalidate internal cache + if group.name not in self.groups_list(): + self.groups.append(group) + self._groups_list = None # invalidate internal cache + else: + raise errors.AnsibleError("group already in inventory: %s" % group.name) def list_hosts(self, pattern="all"): From d3eaa1b79e76e4a3967d34c18f95ff3a14cca68c Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Thu, 20 Mar 2014 01:40:06 +0100 Subject: [PATCH 09/20] InventoryDir: refactor logic Make sure all hosts and groups are unique objects and that those are referenced uniquely everywhere. Also fixes test_dir_inventory unit tests which were broken after previous patches. modified: lib/ansible/inventory/dir.py --- lib/ansible/inventory/dir.py | 171 +++++++++++++++++++++++++++++------ test/units/TestInventory.py | 2 +- 2 files changed, 146 insertions(+), 27 deletions(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index 74a8eacba9d..f2c32a063e1 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -1,4 +1,5 @@ # (c) 2013, Daniel Hokka Zakrisson +# (c) 2014, Serge van Ginderachter # # This file is part of Ansible # @@ -36,9 +37,9 @@ class InventoryDirectory(object): self.parsers = [] self.hosts = {} self.groups = {} - + for i in self.names: - + if i.endswith("~") or i.endswith(".orig") or i.endswith(".bak"): continue if i.endswith(".ini"): @@ -62,31 +63,149 @@ class InventoryDirectory(object): else: parser = InventoryParser(filename=fullpath) self.parsers.append(parser) - # This takes a lot of code because we can't directly use any of the objects, as they have to blend - for name, group in parser.groups.iteritems(): - if name not in self.groups: - self.groups[name] = group - else: - # group is already there, copy variables - for k, v in group.get_variables().iteritems(): - self.groups[name].set_variable(k, v) + + # retrieve all groups and hosts form the parser and add them to + # self, don't look at group lists yet, to avoid + # recursion trouble, but just make sure all objects exist in self + newgroups = parser.groups.values() + for group in newgroups: for host in group.hosts: - if host.name not in self.hosts: - self.hosts[host.name] = host - else: - # host is already there, copy variables - # note: depth numbers on duplicates may be bogus - for k, v in host.vars.iteritems(): - self.hosts[host.name].set_variable(k, v) - # host can already exist from other source (same name, - # different object): - if host.name not in [h.name for h in self.groups[name].hosts]: - self.groups[name].add_host(self.hosts[host.name]) - - # This needs to be a second loop to ensure all the parent groups exist - for name, group in parser.groups.iteritems(): - for parent in group.parent_groups: - self.groups[parent.name].add_child_group(self.groups[name]) + self._add_host(host) + for group in newgroups: + self._add_group(group) + + # now check the objects lists so they contain only objects from + # self; membership data in groups is already fine (except all & + # ungrouped, see later), but might still reference objects not in self + for group in self.groups.values(): + # iterate on a copy of the lists, as those lists get changed in + # the loop + # list with group's child group objects: + for child in group.child_groups[:]: + if child != self.groups[child.name]: + group.child_groups.remove(child) + group.child_groups.append(self.groups[child.name]) + # list with group's parent group objects: + for parent in group.parent_groups[:]: + if parent != self.groups[parent.name]: + group.parent_groups.remove(parent) + group.parent_groups.append(self.groups[parent.name]) + # list with group's host objects: + for host in group.hosts[:]: + if host != self.hosts[host.name]: + group.hosts.remove(host) + group.hosts.append(self.hosts[host.name]) + # also check here that the group that contains host, is + # also contained in the host's group list + if group not in self.hosts[host.name].groups: + self.hosts[host.name].groups.append(group) + + # extra checks on special groups all and ungrouped + # remove hosts from 'ungrouped' if they became member of other groups + if 'ungrouped' in self.groups: + ungrouped = self.groups['ungrouped'] + # loop on a copy of ungrouped hosts, as we want to change that list + for host in ungrouped.hosts[:]: + if len(host.groups) > 1: + host.groups.remove(ungrouped) + ungrouped.hosts.remove(host) + + # remove hosts from 'all' if they became member of other groups + # all should only contain direct children, not grandchildren + # direct children should have dept == 1 + if 'all' in self.groups: + allgroup = self.groups['all' ] + # loop on a copy of all's child groups, as we want to change that list + for group in allgroup.child_groups[:]: + # groups might once have beeen added to all, and later be added + # to another group: we need to remove the link wit all then + if len(group.parent_groups) > 1: + # real children of all have just 1 parent, all + # this one has more, so not a direct child of all anymore + group.parent_groups.remove(allgroup) + allgroup.child_groups.remove(group) + elif allgroup not in group.parent_groups: + # this group was once added to all, but doesn't list it as + # a parent any more; the info in the group is the correct + # info + allgroup.child_groups.remove(group) + + + def _add_group(self, group): + """ Merge an existing group or add a new one; + Track parent and child groups, and hosts of the new one """ + + if group.name not in self.groups: + # it's brand new, add him! + self.groups[group.name] = group + if self.groups[group.name] != group: + # different object, merge + self._merge_groups(self.groups[group.name], group) + + def _add_host(self, host): + if host.name not in self.hosts: + # Papa's got a brand new host + self.hosts[host.name] = host + if self.hosts[host.name] != host: + # different object, merge + self._merge_hosts(self.hosts[host.name], host) + + def _merge_groups(self, group, newgroup): + """ Merge all of instance newgroup into group, + update parent/child relationships + group lists may still contain group objects that exist in self with + same name, but was instanciated as a different object in some other + inventory parser; these are handled later """ + + # name + if group.name != newgroup.name: + raise errors.AnsibleError("Cannot merge group %s with %s" % (group.name, newgroup.name)) + + # depth + group.depth = max([group.depth, newgroup.depth]) + + # hosts list (host objects are by now already added to self.hosts) + for host in newgroup.hosts: + grouphosts = dict([(h.name, h) for h in group.hosts]) + if host.name in grouphosts: + # same host name but different object, merge + self._merge_hosts(grouphosts[host.name], host) + else: + # new membership + group.add_host(self.hosts[host.name]) + + # group child membership relation + for newchild in newgroup.child_groups: + # dict with existing child groups: + childgroups = dict([(g.name, g) for g in group.child_groups]) + # check if child of new group is already known as a child + if newchild.name not in childgroups: + self.groups[group.name].add_child_group(newchild) + + # group parent membership relation + for newparent in newgroup.parent_groups: + # dict with existing parent groups: + parentgroups = dict([(g.name, g) for g in group.parent_groups]) + # check if parent of new group is already known as a parent + if newparent.name not in parentgroups: + if newparent.name not in self.groups: + # group does not exist yet in self, import him + self.groups[newparent.name] = newparent + # group now exists but not yet as a parent here + self.groups[newparent.name].add_child_group(group) + + # variables + group.vars = utils.combine_vars(group.vars, newgroup.vars) + + def _merge_hosts(self,host, newhost): + """ Merge all of instance newhost into host """ + + # name + if host.name != newhost.name: + raise errors.AnsibleError("Cannot merge host %s with %s" % (host.name, newhost.name)) + + # variables + host.vars = utils.combine_vars(host.vars, newhost.vars) def get_host_variables(self, host): """ Gets additional host variables from all inventories """ diff --git a/test/units/TestInventory.py b/test/units/TestInventory.py index 4aae739a233..7f20af6bb64 100644 --- a/test/units/TestInventory.py +++ b/test/units/TestInventory.py @@ -425,7 +425,7 @@ class TestInventory(unittest.TestCase): expected_vars = {'inventory_hostname': 'zeus', 'inventory_hostname_short': 'zeus', - 'group_names': ['greek', 'major-god', 'ungrouped'], + 'group_names': ['greek', 'major-god'], 'var_a': '3#4'} print "HOST VARS=%s" % host_vars From f6a55a35525a6ca0b56ad856f2ab11bbd226c1b4 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Mon, 17 Mar 2014 18:05:30 +0100 Subject: [PATCH 10/20] Refactor vars_plugins (host/group_vars) Split out parsing of vars files to per host and per group parsing, instead of reparsing all groups for each host. This enhances performance. Extend vars_plugins' API with two new methods: * get host variables: only parses host_vars * get group variables: only parses group_vars for specific group The initial run method is still used for backward compatibility. Parse all vars_plugins at inventory initialisation, instead of per host when touched first by runner. Here we can also loop through all groups once easily, then parse them. This also centralizes all parsing in the inventory constructor. modified: bin/ansible modified: bin/ansible-playbook modified: lib/ansible/inventory/__init__.py modified: lib/ansible/inventory/vars_plugins/group_vars.py --- bin/ansible | 2 +- bin/ansible-playbook | 10 ++-- lib/ansible/inventory/__init__.py | 45 +++++++++++++++--- .../inventory/vars_plugins/group_vars.py | 46 +++++++++++-------- 4 files changed, 70 insertions(+), 33 deletions(-) diff --git a/bin/ansible b/bin/ansible index 1e2540fafb7..7e767b2f7db 100755 --- a/bin/ansible +++ b/bin/ansible @@ -136,7 +136,7 @@ class Cli(object): if not options.ask_vault_pass: vault_pass = tmp_vault_pass - inventory_manager = inventory.Inventory(options.inventory) + inventory_manager = inventory.Inventory(options.inventory, vault_password=vault_pass) if options.subset: inventory_manager.subset(options.subset) hosts = inventory_manager.list_hosts(pattern) diff --git a/bin/ansible-playbook b/bin/ansible-playbook index f54c17d7aa7..d91e2d94847 100755 --- a/bin/ansible-playbook +++ b/bin/ansible-playbook @@ -97,11 +97,6 @@ def main(args): if (options.ask_vault_pass and options.vault_password_file): parser.error("--ask-vault-pass and --vault-password-file are mutually exclusive") - inventory = ansible.inventory.Inventory(options.inventory) - inventory.subset(options.subset) - if len(inventory.list_hosts()) == 0: - raise errors.AnsibleError("provided hosts list is empty") - sshpass = None sudopass = None su_pass = None @@ -155,6 +150,11 @@ def main(args): if not (os.path.isfile(playbook) or stat.S_ISFIFO(os.stat(playbook).st_mode)): raise errors.AnsibleError("the playbook: %s does not appear to be a file" % playbook) + inventory = ansible.inventory.Inventory(options.inventory, vault_password=vault_pass) + inventory.subset(options.subset) + if len(inventory.list_hosts()) == 0: + raise errors.AnsibleError("provided hosts list is empty") + # run all playbooks specified on the command line for playbook in args: diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index a8cca8faaf2..06acf87e484 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -39,13 +39,14 @@ class Inventory(object): __slots__ = [ 'host_list', 'groups', '_restriction', '_also_restriction', '_subset', 'parser', '_vars_per_host', '_vars_per_group', '_hosts_cache', '_groups_list', - '_pattern_cache', '_vars_plugins', '_playbook_basedir'] + '_pattern_cache', '_vault_password', '_vars_plugins', '_playbook_basedir'] - def __init__(self, host_list=C.DEFAULT_HOST_LIST): + def __init__(self, host_list=C.DEFAULT_HOST_LIST, vault_password=None): # the host file file, or script path, or list of hosts # if a list, inventory data will NOT be loaded self.host_list = host_list + self._vault_password=vault_password # caching to avoid repeated calculations, particularly with # external inventory scripts. @@ -140,6 +141,14 @@ class Inventory(object): self._vars_plugins = [ x for x in utils.plugins.vars_loader.all(self) ] + # get group vars from vars plugins + for group in self.groups: + group.vars = utils.combine_vars(group.vars, self.get_group_variables(group.name, self._vault_password)) + + # get host vars from vars plugins + for host in self.get_hosts(): + host.vars = utils.combine_vars(host.vars, self.get_variables(host.name, self._vault_password)) + def _match(self, str, pattern_str): if pattern_str.startswith('~'): @@ -370,16 +379,25 @@ class Inventory(object): return group return None - def get_group_variables(self, groupname): + def get_group_variables(self, groupname, vault_password=None): if groupname not in self._vars_per_group: - self._vars_per_group[groupname] = self._get_group_variables(groupname) + self._vars_per_group[groupname] = self._get_group_variables(groupname, vault_password=vault_password) return self._vars_per_group[groupname] - def _get_group_variables(self, groupname): + def _get_group_variables(self, groupname, vault_password=None): + group = self.get_group(groupname) if group is None: raise Exception("group not found: %s" % groupname) - return group.get_variables() + + vars = {} + vars_results = [ plugin.get_group_vars(group, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'get_group_vars')] + for updated in vars_results: + if updated is not None: + vars.update(updated) + + vars.update(group.get_variables()) + return vars def get_variables(self, hostname, vault_password=None): if hostname not in self._vars_per_host: @@ -393,14 +411,27 @@ class Inventory(object): raise errors.AnsibleError("host not found: %s" % hostname) vars = {} - vars_results = [ plugin.run(host, vault_password=vault_password) for plugin in self._vars_plugins ] + + # plugin.get_host_vars retrieves just vars for specific host + vars_results = [ plugin.get_host_vars(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'get_host_vars')] + for updated in vars_results: + if updated is not None: + vars = utils.combine_vars(vars, updated) + + # plugin.run retrieves all vars (also from groups) for host + vars_results = [ plugin.run(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'run')] for updated in vars_results: if updated is not None: vars = utils.combine_vars(vars, updated) vars = utils.combine_vars(vars, host.get_variables()) + + # still need to check InventoryParser per host vars + # which actually means InventoryScript per host, + # which is not performant if self.parser is not None: vars = utils.combine_vars(vars, self.parser.get_host_variables(host)) + return vars def add_group(self, group): diff --git a/lib/ansible/inventory/vars_plugins/group_vars.py b/lib/ansible/inventory/vars_plugins/group_vars.py index 93edceeecb5..2e8c7511220 100644 --- a/lib/ansible/inventory/vars_plugins/group_vars.py +++ b/lib/ansible/inventory/vars_plugins/group_vars.py @@ -143,28 +143,33 @@ class VarsModule(object): """ constructor """ self.inventory = inventory + self.inventory_basedir = inventory.basedir() + # There's no playbook initialized yet: + self.pb_basedir = None - def run(self, host, vault_password=None): - """ main body of the plugin, does actual loading """ + def get_host_vars(self, host, vault_password=None): + return self._get_vars(host=host, group=None, vault_password=vault_password) - inventory = self.inventory - basedir = inventory.playbook_basedir() - if basedir is not None: - basedir = os.path.abspath(basedir) - self.pb_basedir = basedir - # sort groups by depth so deepest groups can override the less deep ones - groupz = sorted(inventory.groups_for_host(host.name), key=lambda g: g.depth) - groups = [ g.name for g in groupz ] - inventory_basedir = inventory.basedir() + def get_group_vars(self, group, vault_password=None): + return self._get_vars(host=None, group=group, vault_password=vault_password) + + + def _get_vars(self, host=None, group=None, vault_password=None): + """ main body of the plugin, does actual loading""" + + if self.pb_basedir is None: + pb_basedir = self.inventory.playbook_basedir() + if pb_basedir is not None: + pb_basedir = os.path.abspath(pb_basedir) + self.pb_basedir = pb_basedir results = {} scan_pass = 0 # look in both the inventory base directory and the playbook base directory - for basedir in [ inventory_basedir, self.pb_basedir ]: - + for basedir in [self.inventory_basedir, self.pb_basedir ]: # this can happen from particular API usages, particularly if not run # from /usr/bin/ansible-playbook @@ -178,17 +183,18 @@ class VarsModule(object): continue # save work of second scan if the directories are the same - if inventory_basedir == self.pb_basedir and scan_pass != 1: + if self.inventory_basedir == self.pb_basedir and scan_pass != 1: continue - # load vars in dir/group_vars/name_of_group - for group in groups: - base_path = os.path.join(basedir, "group_vars/%s" % group) + if group and host is None: + # load vars in dir/group_vars/name_of_group + base_path = os.path.join(basedir, "group_vars/%s" % group.name) results = _load_vars(base_path, results, vault_password=vault_password) - # same for hostvars in dir/host_vars/name_of_host - base_path = os.path.join(basedir, "host_vars/%s" % host.name) - results = _load_vars(base_path, results, vault_password=vault_password) + elif host and group is None: + # same for hostvars in dir/host_vars/name_of_host + base_path = os.path.join(basedir, "host_vars/%s" % host.name) + results = _load_vars(base_path, results, vault_password=vault_password) # all done, results is a dictionary of variables for this particular host. return results From cc28fd891bbc2a9458c12654959b736019f5ca95 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Mon, 17 Mar 2014 18:20:20 +0100 Subject: [PATCH 11/20] Introduce noop vars plugin. In preparation to move the group_vars plugin code into core. new file: lib/ansible/inventory/vars_plugins/noop.py --- lib/ansible/inventory/vars_plugins/noop.py | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 lib/ansible/inventory/vars_plugins/noop.py diff --git a/lib/ansible/inventory/vars_plugins/noop.py b/lib/ansible/inventory/vars_plugins/noop.py new file mode 100644 index 00000000000..5d4b4b6658c --- /dev/null +++ b/lib/ansible/inventory/vars_plugins/noop.py @@ -0,0 +1,48 @@ +# (c) 2012-2014, Michael DeHaan +# (c) 2014, Serge van Ginderachter +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +class VarsModule(object): + + """ + Loads variables for groups and/or hosts + """ + + def __init__(self, inventory): + + """ constructor """ + + self.inventory = inventory + self.inventory_basedir = inventory.basedir() + + + def run(self, host, vault_password=None): + """ For backwards compatibility, when only vars per host were retrieved + This method should return both host specific vars as well as vars + calculated from groups it is a member of """ + return {} + + + def get_host_vars(self, host, vault_password=None): + """ Get host specific variables. """ + return {} + + + def get_group_vars(self, group, vault_password=None): + """ Get group specific variables. """ + return {} + From f8ea93c732df881fe051bcc2bc832af05c364fbb Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 19 Mar 2014 11:09:38 +0100 Subject: [PATCH 12/20] Move inventory.set_playbook_basedir from ansible-playbook to playbook constructor --- bin/ansible-playbook | 3 --- lib/ansible/inventory/__init__.py | 3 ++- lib/ansible/playbook/__init__.py | 4 ++++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/bin/ansible-playbook b/bin/ansible-playbook index d91e2d94847..4a8edad5eff 100755 --- a/bin/ansible-playbook +++ b/bin/ansible-playbook @@ -158,9 +158,6 @@ def main(args): # run all playbooks specified on the command line for playbook in args: - # let inventory know which playbooks are using so it can know the basedirs - inventory.set_playbook_basedir(os.path.dirname(playbook)) - stats = callbacks.AggregateStats() playbook_cb = callbacks.PlaybookCallbacks(verbose=utils.VERBOSITY) if options.step: diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 06acf87e484..53cc0af5d88 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -57,7 +57,8 @@ class Inventory(object): self._groups_list = {} self._pattern_cache = {} - # to be set by calling set_playbook_basedir by ansible-playbook + self._inventory_basedir = inventory.basedir() + # to be set by calling set_playbook_basedir by playbook code self._playbook_basedir = None # the inventory object holds a list of groups diff --git a/lib/ansible/playbook/__init__.py b/lib/ansible/playbook/__init__.py index f624be1b297..8b07a52c0ae 100644 --- a/lib/ansible/playbook/__init__.py +++ b/lib/ansible/playbook/__init__.py @@ -159,6 +159,10 @@ class PlayBook(object): self.basedir = os.path.dirname(playbook) or '.' utils.plugins.push_basedir(self.basedir) + + # let inventory know the playbook basedir so it can load more vars + self.inventory.set_playbook_basedir(self.basedir) + vars = extra_vars.copy() vars['playbook_dir'] = self.basedir if self.inventory.basedir() is not None: From d4634983f0382a5d75cff68c6c44b7b67475a2c9 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Fri, 21 Mar 2014 19:05:18 +0100 Subject: [PATCH 13/20] Move group/host_vars parsing into core inventory modified: lib/ansible/inventory/__init__.py deleted: lib/ansible/inventory/vars_plugins/group_vars.py modified: lib/ansible/utils/__init__.py --- lib/ansible/inventory/__init__.py | 107 ++++++++-- .../inventory/vars_plugins/group_vars.py | 201 ------------------ lib/ansible/utils/__init__.py | 110 +++++++++- 3 files changed, 199 insertions(+), 219 deletions(-) delete mode 100644 lib/ansible/inventory/vars_plugins/group_vars.py diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 53cc0af5d88..5089759081d 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -57,7 +57,6 @@ class Inventory(object): self._groups_list = {} self._pattern_cache = {} - self._inventory_basedir = inventory.basedir() # to be set by calling set_playbook_basedir by playbook code self._playbook_basedir = None @@ -142,11 +141,11 @@ class Inventory(object): self._vars_plugins = [ x for x in utils.plugins.vars_loader.all(self) ] - # get group vars from vars plugins + # get group vars from group_vars/ files and vars plugins for group in self.groups: group.vars = utils.combine_vars(group.vars, self.get_group_variables(group.name, self._vault_password)) - # get host vars from vars plugins + # get host vars from host_vars/ files and vars plugins for host in self.get_hosts(): host.vars = utils.combine_vars(host.vars, self.get_variables(host.name, self._vault_password)) @@ -380,8 +379,8 @@ class Inventory(object): return group return None - def get_group_variables(self, groupname, vault_password=None): - if groupname not in self._vars_per_group: + def get_group_variables(self, groupname, update_cached=False, vault_password=None): + if groupname not in self._vars_per_group or update_cached: self._vars_per_group[groupname] = self._get_group_variables(groupname, vault_password=vault_password) return self._vars_per_group[groupname] @@ -392,16 +391,23 @@ class Inventory(object): raise Exception("group not found: %s" % groupname) vars = {} + + # plugin.get_group_vars retrieves just vars for specific group vars_results = [ plugin.get_group_vars(group, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'get_group_vars')] for updated in vars_results: if updated is not None: - vars.update(updated) + vars = utils.combine_vars(vars, updated) + + # get group variables set by Inventory Parsers + vars = utils.combine_vars(vars, group.get_variables()) + + # Read group_vars/ files + vars = utils.combine_vars(vars, self.get_group_vars(group)) - vars.update(group.get_variables()) return vars - def get_variables(self, hostname, vault_password=None): - if hostname not in self._vars_per_host: + def get_variables(self, hostname, update_cached=False, vault_password=None): + if hostname not in self._vars_per_host or update_cached: self._vars_per_host[hostname] = self._get_variables(hostname, vault_password=vault_password) return self._vars_per_host[hostname] @@ -413,18 +419,19 @@ class Inventory(object): vars = {} - # plugin.get_host_vars retrieves just vars for specific host - vars_results = [ plugin.get_host_vars(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'get_host_vars')] + # plugin.run retrieves all vars (also from groups) for host + vars_results = [ plugin.run(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'run')] for updated in vars_results: if updated is not None: vars = utils.combine_vars(vars, updated) - # plugin.run retrieves all vars (also from groups) for host - vars_results = [ plugin.run(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'run')] + # plugin.get_host_vars retrieves just vars for specific host + vars_results = [ plugin.get_host_vars(host, vault_password=vault_password) for plugin in self._vars_plugins if hasattr(plugin, 'get_host_vars')] for updated in vars_results: if updated is not None: vars = utils.combine_vars(vars, updated) + # get host variables set by Inventory Parsers vars = utils.combine_vars(vars, host.get_variables()) # still need to check InventoryParser per host vars @@ -433,6 +440,9 @@ class Inventory(object): if self.parser is not None: vars = utils.combine_vars(vars, self.parser.get_host_variables(host)) + # Read host_vars/ files + vars = utils.combine_vars(vars, self.get_host_vars(host)) + return vars def add_group(self, group): @@ -532,10 +542,73 @@ class Inventory(object): return self._playbook_basedir def set_playbook_basedir(self, dir): - """ - sets the base directory of the playbook so inventory plugins can use it to find - variable files and other things. """ - self._playbook_basedir = dir + sets the base directory of the playbook so inventory can use it as a + basedir for host_ and group_vars, and other things. + """ + # Only update things if dir is a different playbook basedir + if dir != self._playbook_basedir: + self._playbook_basedir = dir + # get group vars from group_vars/ files + for group in self.groups: + group.vars = utils.combine_vars(group.vars, self.get_group_vars(group, new_pb_basedir=True)) + # get host vars from host_vars/ files + for host in self.get_hosts(): + host.vars = utils.combine_vars(host.vars, self.get_host_vars(host, new_pb_basedir=True)) + + def get_host_vars(self, host, new_pb_basedir=False): + """ Read host_vars/ files """ + return self._get_hostgroup_vars(host=host, group=None, new_pb_basedir=False) + + def get_group_vars(self, group, new_pb_basedir=False): + """ Read group_vars/ files """ + return self._get_hostgroup_vars(host=None, group=group, new_pb_basedir=False) + + def _get_hostgroup_vars(self, host=None, group=None, new_pb_basedir=False): + """ + Loads variables from group_vars/ and host_vars/ in directories parallel + to the inventory base directory or in the same directory as the playbook. Variables in the playbook + dir will win over the inventory dir if files are in both. + """ + + results = {} + scan_pass = 0 + _basedir = self.basedir() + + # look in both the inventory base directory and the playbook base directory + # unless we do an update for a new playbook base dir + if not new_pb_basedir: + basedirs = [_basedir, self._playbook_basedir] + else: + basedirs = [self._playbook_basedir] + for basedir in basedirs: + + # this can happen from particular API usages, particularly if not run + # from /usr/bin/ansible-playbook + if basedir is None: + continue + + scan_pass = scan_pass + 1 + + # it's not an eror if the directory does not exist, keep moving + if not os.path.exists(basedir): + continue + + # save work of second scan if the directories are the same + if _basedir == self._playbook_basedir and scan_pass != 1: + continue + + if group and host is None: + # load vars in dir/group_vars/name_of_group + base_path = os.path.join(basedir, "group_vars/%s" % group.name) + results = utils.load_vars(base_path, results, vault_password=self._vault_password) + + elif host and group is None: + # same for hostvars in dir/host_vars/name_of_host + base_path = os.path.join(basedir, "host_vars/%s" % host.name) + results = utils.load_vars(base_path, results, vault_password=self._vault_password) + + # all done, results is a dictionary of variables for this particular host. + return results diff --git a/lib/ansible/inventory/vars_plugins/group_vars.py b/lib/ansible/inventory/vars_plugins/group_vars.py deleted file mode 100644 index 2e8c7511220..00000000000 --- a/lib/ansible/inventory/vars_plugins/group_vars.py +++ /dev/null @@ -1,201 +0,0 @@ -# (c) 2012-2014, Michael DeHaan -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -import os -import stat -import errno - -from ansible import errors -from ansible import utils -import ansible.constants as C - -def _load_vars(basepath, results, vault_password=None): - """ - Load variables from any potential yaml filename combinations of basepath, - returning result. - """ - - paths_to_check = [ "".join([basepath, ext]) - for ext in C.YAML_FILENAME_EXTENSIONS ] - - found_paths = [] - - for path in paths_to_check: - found, results = _load_vars_from_path(path, results, vault_password=vault_password) - if found: - found_paths.append(path) - - - # disallow the potentially confusing situation that there are multiple - # variable files for the same name. For example if both group_vars/all.yml - # and group_vars/all.yaml - if len(found_paths) > 1: - raise errors.AnsibleError("Multiple variable files found. " - "There should only be one. %s" % ( found_paths, )) - - return results - -def _load_vars_from_path(path, results, vault_password=None): - """ - Robustly access the file at path and load variables, carefully reporting - errors in a friendly/informative way. - - Return the tuple (found, new_results, ) - """ - - try: - # in the case of a symbolic link, we want the stat of the link itself, - # not its target - pathstat = os.lstat(path) - except os.error, err: - # most common case is that nothing exists at that path. - if err.errno == errno.ENOENT: - return False, results - # otherwise this is a condition we should report to the user - raise errors.AnsibleError( - "%s is not accessible: %s." - " Please check its permissions." % ( path, err.strerror)) - - # symbolic link - if stat.S_ISLNK(pathstat.st_mode): - try: - target = os.path.realpath(path) - except os.error, err2: - raise errors.AnsibleError("The symbolic link at %s " - "is not readable: %s. Please check its permissions." - % (path, err2.strerror, )) - # follow symbolic link chains by recursing, so we repeat the same - # permissions checks above and provide useful errors. - return _load_vars_from_path(target, results) - - # directory - if stat.S_ISDIR(pathstat.st_mode): - - # support organizing variables across multiple files in a directory - return True, _load_vars_from_folder(path, results, vault_password=vault_password) - - # regular file - elif stat.S_ISREG(pathstat.st_mode): - data = utils.parse_yaml_from_file(path, vault_password=vault_password) - if type(data) != dict: - raise errors.AnsibleError( - "%s must be stored as a dictionary/hash" % path) - - # combine vars overrides by default but can be configured to do a - # hash merge in settings - results = utils.combine_vars(results, data) - return True, results - - # something else? could be a fifo, socket, device, etc. - else: - raise errors.AnsibleError("Expected a variable file or directory " - "but found a non-file object at path %s" % (path, )) - -def _load_vars_from_folder(folder_path, results, vault_password=None): - """ - Load all variables within a folder recursively. - """ - - # this function and _load_vars_from_path are mutually recursive - - try: - names = os.listdir(folder_path) - except os.error, err: - raise errors.AnsibleError( - "This folder cannot be listed: %s: %s." - % ( folder_path, err.strerror)) - - # evaluate files in a stable order rather than whatever order the - # filesystem lists them. - names.sort() - - # do not parse hidden files or dirs, e.g. .svn/ - paths = [os.path.join(folder_path, name) for name in names if not name.startswith('.')] - for path in paths: - _found, results = _load_vars_from_path(path, results, vault_password=vault_password) - return results - - -class VarsModule(object): - - """ - Loads variables from group_vars/ and host_vars/ in directories parallel - to the inventory base directory or in the same directory as the playbook. Variables in the playbook - dir will win over the inventory dir if files are in both. - """ - - def __init__(self, inventory): - - """ constructor """ - - self.inventory = inventory - self.inventory_basedir = inventory.basedir() - # There's no playbook initialized yet: - self.pb_basedir = None - - - def get_host_vars(self, host, vault_password=None): - return self._get_vars(host=host, group=None, vault_password=vault_password) - - - def get_group_vars(self, group, vault_password=None): - return self._get_vars(host=None, group=group, vault_password=vault_password) - - - def _get_vars(self, host=None, group=None, vault_password=None): - """ main body of the plugin, does actual loading""" - - if self.pb_basedir is None: - pb_basedir = self.inventory.playbook_basedir() - if pb_basedir is not None: - pb_basedir = os.path.abspath(pb_basedir) - self.pb_basedir = pb_basedir - - results = {} - scan_pass = 0 - - # look in both the inventory base directory and the playbook base directory - for basedir in [self.inventory_basedir, self.pb_basedir ]: - - # this can happen from particular API usages, particularly if not run - # from /usr/bin/ansible-playbook - if basedir is None: - continue - - scan_pass = scan_pass + 1 - - # it's not an eror if the directory does not exist, keep moving - if not os.path.exists(basedir): - continue - - # save work of second scan if the directories are the same - if self.inventory_basedir == self.pb_basedir and scan_pass != 1: - continue - - if group and host is None: - # load vars in dir/group_vars/name_of_group - base_path = os.path.join(basedir, "group_vars/%s" % group.name) - results = _load_vars(base_path, results, vault_password=vault_password) - - elif host and group is None: - # same for hostvars in dir/host_vars/name_of_host - base_path = os.path.join(basedir, "host_vars/%s" % host.name) - results = _load_vars(base_path, results, vault_password=vault_password) - - # all done, results is a dictionary of variables for this particular host. - return results - diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index 972089d6a3c..cbd33057468 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -1,4 +1,4 @@ -# (c) 2012, Michael DeHaan +# (c) 2012-2014, Michael DeHaan # # This file is part of Ansible # @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . +import errno import sys import re import os @@ -1108,5 +1109,112 @@ def before_comment(msg): msg = msg.replace("**NOT_A_COMMENT**","#") return msg +def load_vars(basepath, results, vault_password=None): + """ + Load variables from any potential yaml filename combinations of basepath, + returning result. + """ + + paths_to_check = [ "".join([basepath, ext]) + for ext in C.YAML_FILENAME_EXTENSIONS ] + + found_paths = [] + + for path in paths_to_check: + found, results = _load_vars_from_path(path, results, vault_password=vault_password) + if found: + found_paths.append(path) + + + # disallow the potentially confusing situation that there are multiple + # variable files for the same name. For example if both group_vars/all.yml + # and group_vars/all.yaml + if len(found_paths) > 1: + raise errors.AnsibleError("Multiple variable files found. " + "There should only be one. %s" % ( found_paths, )) + + return results + +## load variables from yaml files/dirs +# e.g. host/group_vars +# +def _load_vars_from_path(path, results, vault_password=None): + """ + Robustly access the file at path and load variables, carefully reporting + errors in a friendly/informative way. + Return the tuple (found, new_results, ) + """ + + try: + # in the case of a symbolic link, we want the stat of the link itself, + # not its target + pathstat = os.lstat(path) + except os.error, err: + # most common case is that nothing exists at that path. + if err.errno == errno.ENOENT: + return False, results + # otherwise this is a condition we should report to the user + raise errors.AnsibleError( + "%s is not accessible: %s." + " Please check its permissions." % ( path, err.strerror)) + + # symbolic link + if stat.S_ISLNK(pathstat.st_mode): + try: + target = os.path.realpath(path) + except os.error, err2: + raise errors.AnsibleError("The symbolic link at %s " + "is not readable: %s. Please check its permissions." + % (path, err2.strerror, )) + # follow symbolic link chains by recursing, so we repeat the same + # permissions checks above and provide useful errors. + return _load_vars_from_path(target, results) + + # directory + if stat.S_ISDIR(pathstat.st_mode): + + # support organizing variables across multiple files in a directory + return True, _load_vars_from_folder(path, results, vault_password=vault_password) + + # regular file + elif stat.S_ISREG(pathstat.st_mode): + data = parse_yaml_from_file(path, vault_password=vault_password) + if type(data) != dict: + raise errors.AnsibleError( + "%s must be stored as a dictionary/hash" % path) + + # combine vars overrides by default but can be configured to do a + # hash merge in settings + results = combine_vars(results, data) + return True, results + + # something else? could be a fifo, socket, device, etc. + else: + raise errors.AnsibleError("Expected a variable file or directory " + "but found a non-file object at path %s" % (path, )) + +def _load_vars_from_folder(folder_path, results, vault_password=None): + """ + Load all variables within a folder recursively. + """ + + # this function and _load_vars_from_path are mutually recursive + + try: + names = os.listdir(folder_path) + except os.error, err: + raise errors.AnsibleError( + "This folder cannot be listed: %s: %s." + % ( folder_path, err.strerror)) + + # evaluate files in a stable order rather than whatever order the + # filesystem lists them. + names.sort() + + # do not parse hidden files or dirs, e.g. .svn/ + paths = [os.path.join(folder_path, name) for name in names if not name.startswith('.')] + for path in paths: + _found, results = _load_vars_from_path(path, results, vault_password=vault_password) + return results From b0ff1ea425585ec16585ec6a7d2e676ed3c45430 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Tue, 25 Mar 2014 21:59:13 +0100 Subject: [PATCH 14/20] performance optimisation in hash merge logic rewrite deepcopy in util.merge_hash and just iterate on an inventory with 500 groups and 800 hosts this brings back the inventory initialisation from 13s to 3s (with hash_behaviour=merge) --- lib/ansible/utils/__init__.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/ansible/utils/__init__.py b/lib/ansible/utils/__init__.py index cbd33057468..9640ec8ed66 100644 --- a/lib/ansible/utils/__init__.py +++ b/lib/ansible/utils/__init__.py @@ -558,18 +558,19 @@ def merge_hash(a, b): ''' recursively merges hash b into a keys from b take precedence over keys from a ''' - result = copy.deepcopy(a) - - # next, iterate over b keys and values - for k, v in b.iteritems(): - # if there's already such key in a - # and that key contains dict - if k in result and isinstance(result[k], dict): - # merge those dicts recursively - result[k] = merge_hash(a[k], v) - else: - # otherwise, just copy a value from b to a - result[k] = v + result = {} + + for dicts in a, b: + # next, iterate over b keys and values + for k, v in dicts.iteritems(): + # if there's already such key in a + # and that key contains dict + if k in result and isinstance(result[k], dict): + # merge those dicts recursively + result[k] = merge_hash(a[k], v) + else: + # otherwise, just copy a value from b to a + result[k] = v return result From e36e2d38fee397a0df06c7b848a4a01426764e14 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Fri, 4 Apr 2014 09:27:44 +0200 Subject: [PATCH 15/20] InventoryDir: another fix for the host.groups list In some cases, where a host is mentioned in multiple groups, and those groups are referenced in multiple ini files, a group could still contain multiple instances of a group in its host,groups list, where only one of them is the right group, that exists in the inventory. --- lib/ansible/inventory/dir.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/ansible/inventory/dir.py b/lib/ansible/inventory/dir.py index f2c32a063e1..2f56181ee2b 100644 --- a/lib/ansible/inventory/dir.py +++ b/lib/ansible/inventory/dir.py @@ -171,8 +171,15 @@ class InventoryDirectory(object): # same host name but different object, merge self._merge_hosts(grouphosts[host.name], host) else: - # new membership + # new membership, add host to group from self + # group from self will also be added again to host.groups, but + # as different object group.add_host(self.hosts[host.name]) + # now remove this the old object for group in host.groups + for hostgroup in [g for g in host.groups]: + if hostgroup.name == group.name and hostgroup != self.groups[group.name]: + self.hosts[host.name].groups.remove(hostgroup) + # group child membership relation for newchild in newgroup.child_groups: @@ -204,6 +211,18 @@ class InventoryDirectory(object): if host.name != newhost.name: raise errors.AnsibleError("Cannot merge host %s with %s" % (host.name, newhost.name)) + # group membership relation + for newgroup in newhost.groups: + # dict with existing groups: + hostgroups = dict([(g.name, g) for g in host.groups]) + # check if new group is already known as a group + if newgroup.name not in hostgroups: + if newgroup.name not in self.groups: + # group does not exist yet in self, import him + self.groups[newgroup.name] = newgroup + # group now exists but doesn't have host yet + self.groups[newgroup.name].add_host(host) + # variables host.vars = utils.combine_vars(host.vars, newhost.vars) From 9ed7717634decad9dd66db6c04483c7ae7842542 Mon Sep 17 00:00:00 2001 From: Taylor Barstow Date: Thu, 10 Apr 2014 21:07:59 -0400 Subject: [PATCH 16/20] Adding unit tests for host groups with inventory dir --- test/units/TestInventory.py | 52 +++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/units/TestInventory.py b/test/units/TestInventory.py index 7f20af6bb64..e9fd8f7534e 100644 --- a/test/units/TestInventory.py +++ b/test/units/TestInventory.py @@ -439,3 +439,55 @@ class TestInventory(unittest.TestCase): actual_host_names = [host.name for host in group_greek] print "greek : %s " % actual_host_names assert actual_host_names == ['zeus', 'morpheus'] + + def test_dir_inventory_group_hosts(self): + inventory = self.dir_inventory() + expected_groups = {'all': ['morpheus', 'thor', 'zeus'], + 'major-god': ['thor', 'zeus'], + 'minor-god': ['morpheus'], + 'norse': ['thor'], + 'greek': ['morpheus', 'zeus'], + 'ungrouped': []} + + actual_groups = {} + for group in inventory.get_groups(): + actual_groups[group.name] = sorted([h.name for h in group.get_hosts()]) + print "INVENTORY groups[%s].hosts=%s" % (group.name, actual_groups[group.name]) + print "EXPECTED groups[%s].hosts=%s" % (group.name, expected_groups[group.name]) + + assert actual_groups == expected_groups + + def test_dir_inventory_groups_for_host(self): + inventory = self.dir_inventory() + expected_groups_for_host = {'morpheus': ['all', 'greek', 'minor-god'], + 'thor': ['all', 'norse', 'major-god'], + 'zeus': ['all', 'greek', 'major-god']} + + actual_groups_for_host = {} + for (host, expected) in expected_groups_for_host.iteritems(): + groups = inventory.groups_for_host(host) + names = sorted([g.name for g in groups]) + actual_groups_for_host[host] = names + print "INVENTORY groups_for_host(%s)=%s" % (host, names) + print "EXPECTED groups_for_host(%s)=%s" % (host, expected) + + assert actual_groups_for_host == expected_groups_for_host + + def test_dir_inventory_groups_list(self): + inventory = self.dir_inventory() + inventory_groups = inventory.groups_list() + + expected_groups = {'all': ['morpheus', 'thor', 'zeus'], + 'major-god': ['thor', 'zeus'], + 'minor-god': ['morpheus'], + 'norse': ['thor'], + 'greek': ['morpheus', 'zeus'], + 'ungrouped': []} + + for (name, expected_hosts) in expected_groups.iteritems(): + inventory_groups[name] = sorted(inventory_groups.get(name, [])) + print "INVENTORY groups_list['%s']=%s" % (name, inventory_groups[name]) + print "EXPECTED groups_list['%s']=%s" % (name, expected_hosts) + + assert inventory_groups == expected_groups + From 154055e9ffb0953c3345ef7e500381230c3a2a66 Mon Sep 17 00:00:00 2001 From: Taylor Barstow Date: Thu, 17 Apr 2014 16:41:49 -0400 Subject: [PATCH 17/20] Fixing expectations in test_dir_inventory_groups_for_host --- test/units/TestInventory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/units/TestInventory.py b/test/units/TestInventory.py index e9fd8f7534e..51d929c5a42 100644 --- a/test/units/TestInventory.py +++ b/test/units/TestInventory.py @@ -460,7 +460,7 @@ class TestInventory(unittest.TestCase): def test_dir_inventory_groups_for_host(self): inventory = self.dir_inventory() expected_groups_for_host = {'morpheus': ['all', 'greek', 'minor-god'], - 'thor': ['all', 'norse', 'major-god'], + 'thor': ['all', 'major-god', 'norse'], 'zeus': ['all', 'greek', 'major-god']} actual_groups_for_host = {} From 539426f612e2698b607e1179c020bf0ac95971d4 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Thu, 26 Jun 2014 22:40:31 -0500 Subject: [PATCH 18/20] Performance tuning inventory functions for large inventories --- lib/ansible/inventory/__init__.py | 40 +++++++++++++++++++++++-------- lib/ansible/inventory/group.py | 3 ++- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 830d74c01ef..bee8988c7b2 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -147,6 +147,17 @@ class Inventory(object): else: return fnmatch.fnmatch(str, pattern_str) + def _match_list(self, items, item_attr, pattern_str): + results = [] + if not pattern_str.startswith('~'): + pattern = re.compile(fnmatch.translate(pattern_str)) + else: + pattern = re.compile(pattern_str[1:]) + for item in items: + if pattern.search(getattr(item, item_attr)): + results.append(item) + return results + def get_hosts(self, pattern="all"): """ find all host names matching a pattern string, taking into account any inventory restrictions or @@ -297,20 +308,31 @@ class Inventory(object): def _hosts_in_unenumerated_pattern(self, pattern): """ Get all host names matching the pattern """ + results = [] hosts = [] hostnames = set() # ignore any negative checks here, this is handled elsewhere pattern = pattern.replace("!","").replace("&", "") - results = [] + def __append_host_to_results(host): + if host not in results and host.name not in hostnames: + hostnames.add(host.name) + results.append(host) + groups = self.get_groups() for group in groups: - for host in group.get_hosts(): - if pattern == 'all' or self._match(group.name, pattern) or self._match(host.name, pattern): - if host not in results and host.name not in hostnames: - results.append(host) - hostnames.add(host.name) + if pattern == 'all': + for host in group.get_hosts(): + __append_host_to_results(host) + else: + if self._match(group.name, pattern): + for host in group.get_hosts(): + __append_host_to_results(host) + else: + matching_hosts = self._match_list(group.get_hosts(), 'name', pattern) + for host in matching_hosts: + __append_host_to_results(host) if pattern in ["localhost", "127.0.0.1"] and len(results) == 0: new_host = self._create_implicit_localhost(pattern) @@ -325,10 +347,8 @@ class Inventory(object): results = [] groups = self.get_groups() for group in groups: - for hostn in group.get_hosts(): - if host == hostn.name: - results.append(group) - continue + if host in group.get_hosts(): + results.append(group) return results def groups_list(self): diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index c5270ad554c..e42ddc7fbfb 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -28,7 +28,8 @@ class Group(object): self.vars = {} self.child_groups = [] self.parent_groups = [] - self.clear_hosts_cache() + self._hosts_cache = None + #self.clear_hosts_cache() if self.name is None: raise Exception("group name is required") From ff4119adc040afe1dfdd3a554acca98ac80bbb9e Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Wed, 26 Mar 2014 16:24:54 +0100 Subject: [PATCH 19/20] Performance optimization in resolving host patterns Avoid resolving a pattern that is a plain host. When matching a hostname in the hosts_cache, just use the host object from there. When running a task on say 750 hosts, this yields a huge improvement. --- lib/ansible/inventory/__init__.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index bee8988c7b2..b2a3825c9c7 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -213,15 +213,18 @@ class Inventory(object): hosts = [] for p in patterns: - that = self.__get_hosts(p) - if p.startswith("!"): - hosts = [ h for h in hosts if h not in that ] - elif p.startswith("&"): - hosts = [ h for h in hosts if h in that ] + # avoid resolving a pattern that is a plain host + if p in self._hosts_cache: + hosts.append(self.get_host(p)) else: - to_append = [ h for h in that if h.name not in [ y.name for y in hosts ] ] - hosts.extend(to_append) - + that = self.__get_hosts(p) + if p.startswith("!"): + hosts = [ h for h in hosts if h not in that ] + elif p.startswith("&"): + hosts = [ h for h in hosts if h in that ] + else: + to_append = [ h for h in that if h.name not in [ y.name for y in hosts ] ] + hosts.extend(to_append) return hosts def __get_hosts(self, pattern): From aa261bdd14758f311951d96a7eaebaf3ad22f862 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Mon, 7 Jul 2014 21:08:39 -0500 Subject: [PATCH 20/20] Optimizing groups_for_host() lookup in inventory --- lib/ansible/inventory/__init__.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 3657ac81460..68060f3c0ef 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -206,7 +206,7 @@ class Inventory(object): pattern_exclude.append(p) elif p.startswith("&"): pattern_intersection.append(p) - else: + elif p: pattern_regular.append(p) # if no regular pattern was given, hence only exclude and/or intersection @@ -355,12 +355,10 @@ class Inventory(object): self._pattern_cache = {} def groups_for_host(self, host): - results = [] - groups = self.get_groups() - for group in groups: - if host in group.get_hosts(): - results.append(group) - return results + if host in self._hosts_cache: + return self._hosts_cache[host].get_groups() + else: + return [] def groups_list(self): if not self._groups_list: