From d9086ab46bb5cc60f3c80d684d9b34397bbd7d21 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Fri, 14 Aug 2015 07:14:42 +0530 Subject: [PATCH 1/5] Describe the groupname[x]/[x:y] syntax in more detail --- docsite/rst/intro_patterns.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docsite/rst/intro_patterns.rst b/docsite/rst/intro_patterns.rst index 07160b21828..8315c09b512 100644 --- a/docsite/rst/intro_patterns.rst +++ b/docsite/rst/intro_patterns.rst @@ -68,13 +68,19 @@ It's also ok to mix wildcard patterns and groups at the same time:: one*.com:dbservers -As an advanced usage, you can also select the numbered server in a group:: - - webservers[0] +You can select a host or subset of hosts from a group by their position. For example, given the following group:: -Or a range of servers in a group:: + [webservers] + cobweb + webbing + weber - webservers[0:25] +You can refer to hosts within the group by adding a subscript to the group name: + + webservers[0] # == cobweb + webservers[-1] # == weber + webservers[0:1] # == webservers[0]:webservers[1] + # == cobweb:webbing Most people don't specify patterns as regular expressions, but you can. Just start the pattern with a '~':: From 704c3815d35a3fe6f7379b379f77c12affe23e9b Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Sun, 23 Aug 2015 13:40:42 +0530 Subject: [PATCH 2/5] Reorder functions into a logical sequence based on usage There are no code changes; this is committed separately so as to make the subsequent "real" diffs easier to read. --- lib/ansible/inventory/__init__.py | 113 +++++++++++++++--------------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index e4ff5132d81..7d4263bdbcf 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -136,48 +136,6 @@ class Inventory(object): for host in self.get_hosts(): host.vars = combine_vars(host.vars, self.get_host_variables(host.name)) - - def _match(self, str, pattern_str): - try: - if pattern_str.startswith('~'): - return re.search(pattern_str[1:], str) - else: - return fnmatch.fnmatch(str, pattern_str) - except Exception, e: - raise AnsibleError('invalid host pattern: %s' % pattern_str) - - def _match_list(self, items, item_attr, pattern_str): - results = [] - try: - if not pattern_str.startswith('~'): - pattern = re.compile(fnmatch.translate(pattern_str)) - else: - pattern = re.compile(pattern_str[1:]) - except Exception, e: - raise AnsibleError('invalid host pattern: %s' % pattern_str) - - for item in items: - if pattern.match(getattr(item, item_attr)): - results.append(item) - return results - - def _split_pattern(self, pattern): - """ - takes e.g. "webservers[0:5]:dbservers:others" - and returns ["webservers[0:5]", "dbservers", "others"] - """ - - term = re.compile( - r'''(?: # We want to match something comprising: - [^:\[\]] # (anything other than ':', '[', or ']' - | # ...or... - \[[^\]]*\] # a single complete bracketed expression) - )* # repeated as many times as possible - ''', re.X - ) - - return [x for x in term.findall(pattern) if x] - def get_hosts(self, pattern="all"): """ Takes a pattern or list of patterns and returns a list of matching @@ -201,12 +159,29 @@ class Inventory(object): subset = self._evaluate_patterns(self._subset) hosts = [ h for h in hosts if h in subset ] - # exclude hosts mentioned in any restriction (ex: failed hosts) + # exclude hosts mentioned in any restriction if self._restriction is not None: hosts = [ h for h in hosts if h in self._restriction ] return hosts + def _split_pattern(self, pattern): + """ + takes e.g. "webservers[0:5]:dbservers:others" + and returns ["webservers[0:5]", "dbservers", "others"] + """ + + term = re.compile( + r'''(?: # We want to match something comprising: + [^:\[\]] # (anything other than ':', '[', or ']' + | # ...or... + \[[^\]]*\] # a single complete bracketed expression) + )* # repeated as many times as possible + ''', re.X + ) + + return [x for x in term.findall(pattern) if x] + def _evaluate_patterns(self, patterns): """ Takes a list of patterns and returns a list of matching host names, @@ -326,20 +301,6 @@ class Inventory(object): except IndexError: raise AnsibleError("no hosts matching the pattern '%s' were found" % pat) - def _create_implicit_localhost(self, pattern): - new_host = Host(pattern) - new_host.set_variable("ansible_python_interpreter", sys.executable) - new_host.set_variable("ansible_connection", "local") - new_host.ipv4_address = '127.0.0.1' - - ungrouped = self.get_group("ungrouped") - if ungrouped is None: - self.add_group(Group('ungrouped')) - ungrouped = self.get_group('ungrouped') - self.get_group('all').add_child_group(ungrouped) - ungrouped.add_host(new_host) - return new_host - def _hosts_in_unenumerated_pattern(self, pattern): """ Get all host names matching the pattern """ @@ -374,6 +335,44 @@ class Inventory(object): results.append(new_host) return results + def _match(self, str, pattern_str): + try: + if pattern_str.startswith('~'): + return re.search(pattern_str[1:], str) + else: + return fnmatch.fnmatch(str, pattern_str) + except Exception, e: + raise AnsibleError('invalid host pattern: %s' % pattern_str) + + def _match_list(self, items, item_attr, pattern_str): + results = [] + try: + if not pattern_str.startswith('~'): + pattern = re.compile(fnmatch.translate(pattern_str)) + else: + pattern = re.compile(pattern_str[1:]) + except Exception, e: + raise AnsibleError('invalid host pattern: %s' % pattern_str) + + for item in items: + if pattern.match(getattr(item, item_attr)): + results.append(item) + return results + + def _create_implicit_localhost(self, pattern): + new_host = Host(pattern) + new_host.set_variable("ansible_python_interpreter", sys.executable) + new_host.set_variable("ansible_connection", "local") + new_host.ipv4_address = '127.0.0.1' + + ungrouped = self.get_group("ungrouped") + if ungrouped is None: + self.add_group(Group('ungrouped')) + ungrouped = self.get_group('ungrouped') + self.get_group('all').add_child_group(ungrouped) + ungrouped.add_host(new_host) + return new_host + def clear_pattern_cache(self): ''' called exclusively by the add_host plugin to allow patterns to be recalculated ''' self._pattern_cache = {} From fa6ffa1dbdc83f5fa8aa5361d2d72a5cc63d8ee5 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Sun, 23 Aug 2015 13:54:22 +0530 Subject: [PATCH 3/5] Remove & and ! pattern prefixes as early as possible Now everything under _match_one_pattern can ignore them. This also means that we can use the cache to return the same results for 'foo' and '!foo'. --- lib/ansible/inventory/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 7d4263bdbcf..c105b7e5c04 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -229,10 +229,13 @@ class Inventory(object): def _match_one_pattern(self, pattern): """ Takes a single pattern (i.e., not "p1:p2") and returns a list of - matching hosts names. Does not take negatives or intersections + matching host names. Does not take negatives or intersections into account. """ + if pattern.startswith("&") or pattern.startswith("!"): + pattern = pattern[1:] + if pattern in self._pattern_cache: return self._pattern_cache[pattern] @@ -308,9 +311,6 @@ class Inventory(object): hosts = [] hostnames = set() - # ignore any negative checks here, this is handled elsewhere - pattern = pattern.replace("!","").replace("&", "") - def __append_host_to_results(host): if host.name not in hostnames: hostnames.add(host.name) From 73f10de386f1ec000028204328c2bd6c131fc7f4 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Sun, 23 Aug 2015 20:06:06 +0530 Subject: [PATCH 4/5] Document the behaviour of _match_one_pattern in some detail The possibilities are complicated enough that I didn't want to make changes without having a complete description of what it actually accepts/matches. Note that this text documents current behaviour, not necessarily the behaviour we want. Some of this is undocumented and may not be intended. --- lib/ansible/inventory/__init__.py | 37 ++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index c105b7e5c04..b9a69bb3d29 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -228,9 +228,40 @@ class Inventory(object): def _match_one_pattern(self, pattern): """ - Takes a single pattern (i.e., not "p1:p2") and returns a list of - matching host names. Does not take negatives or intersections - into account. + Takes a single pattern and returns a list of matching host names. + Ignores intersection (&) and exclusion (!) specifiers. + + The pattern may be: + + 1. A regex starting with ~, e.g. '~[abc]*' + 2. A shell glob pattern with ?/*/[chars]/[!chars], e.g. 'foo' + 3. An ordinary word that matches itself only, e.g. 'foo' + + The pattern is matched using the following rules: + + 1. If it's 'all', it matches all hosts in all groups. + 2. Otherwise, for each known group name: + (a) if it matches the group name, the results include all hosts + in the group or any of its children. + (b) otherwise, if it matches any hosts in the group, the results + include the matching hosts. + + This means that 'foo*' may match one or more groups (thus including all + hosts therein) but also hosts in other groups. + + The built-in groups 'all' and 'ungrouped' are special. No pattern can + match these group names (though 'all' behaves as though it matches, as + described above). The word 'ungrouped' can match a host of that name, + and patterns like 'ungr*' and 'al*' can match either hosts or groups + other than all and ungrouped. + + If the pattern matches one or more group names according to these rules, + it may have an optional range suffix to select a subset of the results. + This is allowed only if the pattern is not a regex, i.e. '~foo[1]' does + not work (the [1] is interpreted as part of the regex), but 'foo*[1]' + would work if 'foo*' matched the name of one or more groups. + + Duplicate matches are always eliminated from the results. """ if pattern.startswith("&") or pattern.startswith("!"): From 8bf0dbb7a956bda13c837ade8a31bf6a37032abd Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Mon, 24 Aug 2015 00:00:37 +0530 Subject: [PATCH 5/5] Use [x:y] host ranges instead of [x-y] This commit deprecates the earlier groupname[x-y] syntax in favour of the inclusive groupname[x:y] syntax. It also makes the subscripting code simpler and adds explanatory comments. One problem addressed by the cleanup is that _enumeration_info used to be called twice, and its results discarded the first time because of the convoluted control flow. --- lib/ansible/inventory/__init__.py | 111 ++++++++++++++++-------------- 1 file changed, 58 insertions(+), 53 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index b9a69bb3d29..03ac2ee05a8 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -267,76 +267,81 @@ class Inventory(object): if pattern.startswith("&") or pattern.startswith("!"): pattern = pattern[1:] - if pattern in self._pattern_cache: - return self._pattern_cache[pattern] - - (name, enumeration_details) = self._enumeration_info(pattern) - hpat = self._hosts_in_unenumerated_pattern(name) - result = self._apply_ranges(pattern, hpat) - self._pattern_cache[pattern] = result - return result - - def _enumeration_info(self, pattern): + if pattern not in self._pattern_cache: + (expr, slice) = self._split_subscript(pattern) + hosts = self._enumerate_matches(expr) + try: + hosts = self._apply_subscript(hosts, slice) + except IndexError: + raise AnsibleError("No hosts matched the subscripted pattern '%s'" % pattern) + self._pattern_cache[pattern] = hosts + + return self._pattern_cache[pattern] + + def _split_subscript(self, pattern): """ - returns (pattern, limits) taking a regular pattern and finding out - which parts of it correspond to start/stop offsets. limits is - a tuple of (start, stop) or None + Takes a pattern, checks if it has a subscript, and returns the pattern + without the subscript and a (start,end) tuple representing the given + subscript (or None if there is no subscript). + + Validates that the subscript is in the right syntax, but doesn't make + sure the actual indices make sense in context. """ # Do not parse regexes for enumeration info if pattern.startswith('~'): return (pattern, None) - # The regex used to match on the range, which can be [x] or [x-y]. - pattern_re = re.compile("^(.*)\[([-]?[0-9]+)(?:(?:-)([0-9]+))?\](.*)$") - m = pattern_re.match(pattern) + # We want a pattern followed by an integer or range subscript. + # (We can't be more restrictive about the expression because the + # fnmatch semantics permit [\[:\]] to occur.) + + pattern_with_subscript = re.compile( + r'''^ + (.+) # A pattern expression ending with... + \[(?: # A [subscript] expression comprising: + (-?[0-9]+) # A single positive or negative number + | # Or a numeric range + ([0-9]+)([:-])([0-9]+) + )\] + $ + ''', re.X + ) + + subscript = None + m = pattern_with_subscript.match(pattern) if m: - (target, first, last, rest) = m.groups() - first = int(first) - if last: - if first < 0: - raise AnsibleError("invalid range: negative indices cannot be used as the first item in a range") - last = int(last) + (pattern, idx, start, sep, end) = m.groups() + if idx: + subscript = (int(idx), None) else: - last = first - return (target, (first, last)) - else: - return (pattern, None) + subscript = (int(start), int(end)) + if sep == '-': + display.deprecated("Use [x:y] inclusive subscripts instead of [x-y]", version=2.0, removed=True) - def _apply_ranges(self, pat, hosts): + return (pattern, subscript) + + def _apply_subscript(self, hosts, subscript): """ - given a pattern like foo, that matches hosts, return all of hosts - given a pattern like foo[0:5], where foo matches hosts, return the first 6 hosts + Takes a list of hosts and a (start,end) tuple and returns the subset of + hosts based on the subscript (which may be None to return all hosts). """ - # If there are no hosts to select from, just return the - # empty set. This prevents trying to do selections on an empty set. - # issue#6258 - if not hosts: - return hosts - - (loose_pattern, limits) = self._enumeration_info(pat) - if not limits: + if not hosts or not subscript: return hosts - (left, right) = limits + (start, end) = subscript - if left == '': - left = 0 - if right == '': - right = 0 - left=int(left) - right=int(right) - try: - if left != right: - return hosts[left:right] - else: - return [ hosts[left] ] - except IndexError: - raise AnsibleError("no hosts matching the pattern '%s' were found" % pat) + if end: + return hosts[start:end+1] + else: + return [ hosts[start] ] - def _hosts_in_unenumerated_pattern(self, pattern): - """ Get all host names matching the pattern """ + def _enumerate_matches(self, pattern): + """ + Returns a list of host names matching the given pattern according to the + rules explained above in _match_one_pattern. + """ results = [] hosts = []