From 017b092df70706177c542c3a3cab3f9d0a75e5c2 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 15 Oct 2020 16:35:51 +0100 Subject: [PATCH] Fix --limit with commas Kayobe allows specifying a --limit argument, which is passed through to Ansible. In some cases we wish to add an intersection with a group. This allows us to reuse playbooks for the seed, overcloud etc. For example, the lvm.yml playbook specifies a host list of seed-hypervisor:seed:overcloud. When executed as part of a kayobe overcloud host configure command, Kayobe passes a limit of overcloud. If the user specifies a --limit argument, this gets intersected with the overcloud limit: host1:&overcloud. The problem happens if the user specifies multiple parts to the host pattern in their limit using a comma, e.g. host1,host2. This results in host1,host2:&overcloud. Ansible ignores the colon, and treats this as host1 or host2:&overcloud. The solution is to use a comma to join the patterns if the user has used a comma: host1,host2,&overcloud Change-Id: Ibe42fa372c6fa0c539d2c2b0e238601286dc213d Story: 2008255 Task: 41111 --- kayobe/ansible.py | 4 +- kayobe/kolla_ansible.py | 4 +- kayobe/tests/unit/test_utils.py | 37 +++++++++++++++++++ kayobe/utils.py | 27 ++++++++++++++ ...ix-limit-with-commas-04a357b0b7ef0371.yaml | 6 +++ 5 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/fix-limit-with-commas-04a357b0b7ef0371.yaml diff --git a/kayobe/ansible.py b/kayobe/ansible.py index 656c891b5..6e907a3d2 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 1c50d1405..4171bbaac 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 b41c07543..79eaafa1c 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 2ad91cc33..40828ac5e 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 000000000..dc1286794 --- /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 + `__ for details.