diff --git a/kayobe/ansible.py b/kayobe/ansible.py index 656c891b55896272462134c1f9a340ee2da058eb..6e907a3d2c7705f226856c0baed3ecd3688be5dd 100644 --- a/kayobe/ansible.py +++ b/kayobe/ansible.py @@ -146,8 +146,8 @@ def build_args(parsed_args, playbooks, if check or (parsed_args.check and check is None): cmd += ["--check"] if not ignore_limit and (parsed_args.limit or limit): - limits = [l for l in [parsed_args.limit, limit] if l] - cmd += ["--limit", ":&".join(limits)] + limit_arg = utils.intersect_limits(parsed_args.limit, limit) + cmd += ["--limit", limit_arg] if parsed_args.skip_tags: cmd += ["--skip-tags", parsed_args.skip_tags] if parsed_args.tags or tags: diff --git a/kayobe/kolla_ansible.py b/kayobe/kolla_ansible.py index 1c50d140522cf440a7809eeabd9cda98cd353fa6..4171bbaac9b348da028342b9881871a635f9b9f0 100644 --- a/kayobe/kolla_ansible.py +++ b/kayobe/kolla_ansible.py @@ -131,8 +131,8 @@ def build_args(parsed_args, command, inventory_filename, extra_vars=None, extra_var_value = utils.quote_and_escape(extra_var_value) cmd += ["-e", "%s=%s" % (extra_var_name, extra_var_value)] if parsed_args.kolla_limit or limit: - limits = [l for l in [parsed_args.kolla_limit, limit] if l] - cmd += ["--limit", ":&".join(limits)] + limit_arg = utils.intersect_limits(parsed_args.kolla_limit, limit) + cmd += ["--limit", limit_arg] if parsed_args.kolla_skip_tags: cmd += ["--skip-tags", parsed_args.kolla_skip_tags] if parsed_args.kolla_tags or tags: diff --git a/kayobe/tests/unit/test_utils.py b/kayobe/tests/unit/test_utils.py index b41c07543bc3ff987d481f1239cd3fb3063e50e0..79eaafa1c2322da80ce443eaa6eea3677a5b36e9 100644 --- a/kayobe/tests/unit/test_utils.py +++ b/kayobe/tests/unit/test_utils.py @@ -19,6 +19,7 @@ from unittest import mock import yaml +from kayobe import exception from kayobe import utils @@ -127,3 +128,39 @@ key2: value2 expected = os.path.normpath("/tmp/test/local/") result = utils._detect_install_prefix(path) self.assertEqual(expected, os.path.normpath(result)) + + def test_intersect_limits_no_arg_no_cli(self): + result = utils.intersect_limits(None, None) + self.assertEqual("", result) + + def test_intersect_limits_arg_no_cli(self): + result = utils.intersect_limits("foo", None) + self.assertEqual("foo", result) + + def test_intersect_limits_arg_no_cli_comma(self): + result = utils.intersect_limits("foo,bar", None) + self.assertEqual("foo,bar", result) + + def test_intersect_limits_arg_no_cli_colon(self): + result = utils.intersect_limits("foo:bar", None) + self.assertEqual("foo:bar", result) + + def test_intersect_limits_arg_no_cli_colon_and_comma(self): + self.assertRaises(exception.Error, + utils.intersect_limits, "foo:bar,baz", None) + + def test_intersect_limits_no_arg_cli(self): + result = utils.intersect_limits(None, "foo") + self.assertEqual("foo", result) + + def test_intersect_limits_arg_and_cli(self): + result = utils.intersect_limits("foo", "bar") + self.assertEqual("foo:&bar", result) + + def test_intersect_limits_arg_and_cli_comma(self): + result = utils.intersect_limits("foo,bar", "baz") + self.assertEqual("foo,bar,&baz", result) + + def test_intersect_limits_arg_and_cli_colon(self): + result = utils.intersect_limits("foo:bar", "baz") + self.assertEqual("foo:bar:&baz", result) diff --git a/kayobe/utils.py b/kayobe/utils.py index 2ad91cc330b795189fe60684f250190ca17bbd96..40828ac5ecb112d8ca6380d1db35ad7e5ba02c62 100644 --- a/kayobe/utils.py +++ b/kayobe/utils.py @@ -22,6 +22,8 @@ import sys import yaml +from kayobe import exception + LOG = logging.getLogger(__name__) @@ -195,3 +197,28 @@ def escape_jinja(string): b64_value = base64.b64encode(string.encode()) return ''.join(('{{', "'", b64_value.decode(), "' | b64decode ", '}}')) + + +def intersect_limits(args_limit, cli_limit): + """Create an Ansible host pattern of the intersection of two patterns. + + :param args_limit: user-specified limit, or None. + :param cli_limit: limit originating from this CLI, or None. + :returns: a string representing an intersection of the two patterns. + """ + # NOTE(mgoddard): Ansible uses either commas (,) or colons (:) to separate + # parts of a host pattern. An intersection is specified using a separator + # followed by an ampersand (&). If a mix of comma and colon separators is + # used, Ansible picks one and treats the other as part of the host pattern. + # This leads to hard to diagnose errors. Try to determine which separator + # the user has specified, and be consistent. Error if both are used. + if args_limit and ',' in args_limit: + if ':' in args_limit: + raise exception.Error("Invalid format for host limit argument. " + "Cannot mix commas and colons to separate " + "hosts") + separator = ',&' + else: + separator = ':&' + limits = [l for l in [args_limit, cli_limit] if l] + return separator.join(limits) diff --git a/releasenotes/notes/fix-limit-with-commas-04a357b0b7ef0371.yaml b/releasenotes/notes/fix-limit-with-commas-04a357b0b7ef0371.yaml new file mode 100644 index 0000000000000000000000000000000000000000..dc12867940fa5b08c5db7beca3f1727010f0dc19 --- /dev/null +++ b/releasenotes/notes/fix-limit-with-commas-04a357b0b7ef0371.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue when using the ``--limit`` argument with a host pattern + including commas. See `story 2008255 + <https://storyboard.openstack.org/#!/story/2008255>`__ for details.