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/12] 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 46997d9be8..9b53bd17c6 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/12] 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 9b53bd17c6..37cc539044 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/12] 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 37cc539044..39f45f0920 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/12] 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 c5270ad554..94ee8eceba 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/12] 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 39f45f0920..74a8eacba9 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 94ee8eceba..eb2eb45ea3 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/12] 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 9863de17b8..3848696006 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/12] 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 05348b6839..89a8c9d069 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/12] 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 b6f644190f..9f076a72cd 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/12] 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 74a8eacba9..f2c32a063e 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) - 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]) + # 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: + 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 4aae739a23..7f20af6bb6 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 e36e2d38fee397a0df06c7b848a4a01426764e14 Mon Sep 17 00:00:00 2001 From: Serge van Ginderachter Date: Fri, 4 Apr 2014 09:27:44 +0200 Subject: [PATCH 10/12] 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 f2c32a063e..2f56181ee2 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 11/12] 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 7f20af6bb6..e9fd8f7534 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 12/12] 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 e9fd8f7534..51d929c5a4 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 = {}