Merge branch 'amenonsen-hostpatterns' into devel

CHANGELOG.md

@@ -22,6 +22,10 @@ Major Changes:
 They will retain the value of `None`. To go back to the old behaviour, you can override
 the `null_representation` setting to an empty string in your config file or by setting the
 `ANSIBLE_NULL_REPRESENTATION` environment variable.
+* Use "pattern1,pattern2" to combine host matching patterns. The use of
+  ':' as a separator is deprecated (accepted with a warning) because it
+  conflicts with IPv6 addresses. The undocumented use of ';' as a
+  separator is no longer supported.
 * Backslashes used when specifying parameters in jinja2 expressions in YAML
 dicts sometimes needed to be escaped twice. This has been fixed so that
 escaping once works. Here's an example of how playbooks need to be modified:
@@ -253,8 +257,6 @@ Minor changes:
 
 * Many more tests. The new API makes things more testable and we took advantage of it.
 * big_ip modules now support turning off ssl certificate validation (use only for self-signed certificates).
-* Use "pattern1:pattern2" to combine host matching patterns. The undocumented
-use of semicolons or commas to combine patterns is no longer supported.
 * Use ``hosts: groupname[x:y]`` to select a subset of hosts in a group; the
 ``[x-y]`` range syntax is no longer supported. Note that ``[0:1]`` matches
 two hosts, i.e. the range is inclusive of its endpoints.

docsite/rst/intro_patterns.rst

@@ -27,7 +27,7 @@ The following patterns are equivalent and target all hosts in the inventory::
 It is also possible to address a specific host or set of hosts by name::
 
     one.example.com
-    one.example.com:two.example.com
+    one.example.com, two.example.com
     192.168.1.50
     192.168.1.*
 
@@ -35,20 +35,20 @@ The following patterns address one or more groups.  Groups separated by a colon
 This means the host may be in either one group or the other::
 
     webservers
-    webservers:dbservers
+    webservers,dbservers
 
 You can exclude groups as well, for instance, all machines must be in the group webservers but not in the group phoenix::
 
-    webservers:!phoenix
+    webservers,!phoenix
 
 You can also specify the intersection of two groups.  This would mean the hosts must be in the group webservers and
 the host must also be in the group staging::
 
-    webservers:&staging
+    webservers,&staging
 
 You can do combinations::
 
-    webservers:dbservers:&staging:!phoenix
+    webservers,dbservers,&staging,!phoenix
 
 The above configuration means "all machines in the groups 'webservers' and 'dbservers' are to be managed if they are in
 the group 'staging' also, but the machines are not to be managed if they are in the group 'phoenix' ... whew!
@@ -56,7 +56,7 @@ the group 'staging' also, but the machines are not to be managed if they are in
 You can also use variables if you want to pass some group specifiers via the "-e" argument to ansible-playbook, but this
 is uncommonly used::
 
-    webservers:!{{excluded}}:&{{required}}
+    webservers,!{{excluded}},&{{required}}
 
 You also don't have to manage by strictly defined groups.  Individual host names, IPs and groups, can also be referenced using
 wildcards::
@@ -66,7 +66,7 @@ wildcards::
 
 It's also ok to mix wildcard patterns and groups at the same time::
 
-    one*.com:dbservers
+    one*.com,dbservers
 
 You can select a host or subset of hosts from a group by their position. For example, given the following group::
 

lib/ansible/inventory/__init__.py

@@ -24,6 +24,7 @@ import os
 import sys
 import re
 import stat
+import itertools
 
 from ansible import constants as C
 from ansible.errors import AnsibleError
@@ -149,23 +150,6 @@ class Inventory(object):
                 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", ignore_limits_and_restrictions=False):
         """ 
         Takes a pattern or list of patterns and returns a list of matching
@@ -173,14 +157,6 @@ class Inventory(object):
         or applied subsets
         """
 
-        # Enumerate all hosts matching the given pattern (which may be
-        # either a list of patterns or a string like 'pat1:pat2').
-        if isinstance(pattern, list):
-            pattern = ':'.join(pattern)
-
-        if ';' in pattern or ',' in pattern:
-            display.deprecated("Use ':' instead of ',' or ';' to separate host patterns", version=2.0, removed=True)
-
         patterns = self._split_pattern(pattern)
         hosts = self._evaluate_patterns(patterns)
 
@@ -197,6 +173,57 @@ class Inventory(object):
 
         return hosts
 
+    def _split_pattern(self, pattern):
+        """
+        Takes a string containing host patterns separated by commas (or a list
+        thereof) and returns a list of single patterns (which may not contain
+        commas). Whitespace is ignored.
+
+        Also accepts ':' as a separator for backwards compatibility, but it is
+        not recommended due to the conflict with IPv6 addresses and host ranges.
+
+        Example: 'a,b[1], c[2:3] , d' -> ['a', 'b[1]', 'c[2:3]', 'd']
+        """
+
+        if isinstance(pattern, list):
+            return list(itertools.chain(*map(self._split_pattern, pattern)))
+
+        if ';' in pattern:
+            display.deprecated("Use ',' instead of ':' or ';' to separate host patterns", version=2.0, removed=True)
+
+        # If it's got commas in it, we'll treat it as a straightforward
+        # comma-separated list of patterns.
+
+        elif ',' in pattern:
+            patterns = re.split('\s*,\s*', pattern)
+
+        # If it doesn't, it could still be a single pattern. This accounts for
+        # non-separator uses of colons: IPv6 addresses and [x:y] host ranges.
+
+        else:
+            (base, port) = parse_address(pattern, allow_ranges=True)
+            if base:
+                patterns = [pattern]
+
+            # The only other case we accept is a ':'-separated list of patterns.
+            # This mishandles IPv6 addresses, and is retained only for backwards
+            # compatibility.
+
+            else:
+                patterns = re.findall(
+                    r'''(?:             # We want to match something comprising:
+                            [^\s:\[\]]  # (anything other than whitespace or ':[]'
+                            |           # ...or...
+                            \[[^\]]*\]  # a single complete bracketed expression)
+                        )+              # occurring once or more
+                    ''', pattern, re.X
+                )
+
+                if len(patterns) > 1:
+                    display.deprecated("Use ',' instead of ':' or ';' to separate host patterns", version=2.0)
+
+        return [p.strip() for p in patterns]
+
     def _evaluate_patterns(self, patterns):
         """
         Takes a list of patterns and returns a list of matching host names,
@@ -249,7 +276,7 @@ class Inventory(object):
         The pattern may be:
 
             1. A regex starting with ~, e.g. '~[abc]*'
-            2. A shell glob pattern with ?/*/[chars]/[!chars], e.g. 'foo'
+            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:

lib/ansible/parsing/utils/addresses.py

@@ -122,7 +122,7 @@ patterns = {
         r'''^
             (?:{0}:){{7}}{0}|           # uncompressed: 1:2:3:4:5:6:7:8
             (?:{0}:){{1,6}}:|           # compressed variants, which are all
-            (?:{0}:)(?:{0}){{1,6}}|     # a::b for various lengths of a,b
+            (?:{0}:)(?::{0}){{1,6}}|    # a::b for various lengths of a,b
             (?:{0}:){{2}}(?::{0}){{1,5}}|
             (?:{0}:){{3}}(?::{0}){{1,4}}|
             (?:{0}:){{4}}(?::{0}){{1,3}}|

test/units/inventory/__init__.py

@@ -0,0 +1,21 @@
+# (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
+#
+# 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 <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+

test/units/inventory/test_inventory.py

@@ -0,0 +1,69 @@
+# Copyright 2015 Abhijit Menon-Sen <ams@2ndQuadrant.com>
+#
+# 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 <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from ansible.compat.tests import unittest
+from ansible.compat.tests.mock import patch, MagicMock
+
+from ansible.errors import AnsibleError, AnsibleParserError
+from ansible.inventory import Inventory
+from ansible.vars import VariableManager
+
+from units.mock.loader import DictDataLoader
+
+class TestInventory(unittest.TestCase):
+
+    patterns = {
+        'a': ['a'],
+        'a, b': ['a', 'b'],
+        'a , b': ['a', 'b'],
+        ' a,b ,c[1:2] ': ['a', 'b', 'c[1:2]'],
+        '9a01:7f8:191:7701::9': ['9a01:7f8:191:7701::9'],
+        '9a01:7f8:191:7701::9,9a01:7f8:191:7701::9': ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9'],
+        '9a01:7f8:191:7701::9,9a01:7f8:191:7701::9,foo': ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9','foo'],
+        'foo[1:2]': ['foo[1:2]'],
+        'a::b': ['a::b'],
+        'a:b': ['a', 'b'],
+        ' a : b ': ['a', 'b'],
+        'foo:bar:baz[1:2]': ['foo', 'bar', 'baz[1:2]'],
+    }
+    pattern_lists = [
+        [['a'], ['a']],
+        [['a', 'b'], ['a', 'b']],
+        [['a, b'], ['a', 'b']],
+        [['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9,foo'],
+         ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9','foo']]
+    ]
+
+
+    def setUp(self):
+        v = VariableManager()
+        fake_loader = DictDataLoader({})
+
+        self.i = Inventory(loader=fake_loader, variable_manager=v, host_list='')
+
+    def test_split_patterns(self):
+
+        for p in self.patterns:
+            r = self.patterns[p]
+            self.assertEqual(r, self.i._split_pattern(p))
+
+        for p, r in self.pattern_lists:
+            self.assertEqual(r, self.i._split_pattern(p))