From 9c2565a60c79af9c36c7c613507ecc944efa7125 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Wed, 7 Jul 2021 12:39:55 +0000
Subject: [PATCH] Support Ansible diff mode

Adds a '--diff' argument to kayobe CLI commands. This is passed through
to ansible-playbook for Kayobe Ansible playbooks, and can be used with
the '--check' argument to see changes that would be made to files.

This change also passes through --check and --diff arguments to
kolla-ansible.

Story: 2009038
Task: 42794

Change-Id: I350795c328c0dc0a91a5cd500c252c5b7b1eafc1
---
 doc/source/usage.rst                              | 10 ++++++++++
 kayobe/ansible.py                                 | 15 +++++++++++----
 kayobe/kolla_ansible.py                           |  8 ++++++++
 kayobe/tests/unit/test_ansible.py                 |  8 +++++++-
 kayobe/tests/unit/test_kolla_ansible.py           | 10 ++++++++--
 .../notes/diff-mode-468e09bbb9185b50.yaml         | 11 +++++++++++
 6 files changed, 55 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/diff-mode-468e09bbb9185b50.yaml

diff --git a/doc/source/usage.rst b/doc/source/usage.rst
index 047480a9..5c64c17c 100644
--- a/doc/source/usage.rst
+++ b/doc/source/usage.rst
@@ -69,3 +69,13 @@ playbooks to be limited to matching plays and tasks.  The ``--kolla-tags
 limited to matching plays and tasks.  The ``--skip-tags <TAGS>`` and
 ``--kolla-skip-tags <TAGS>`` arguments allow for avoiding execution of matching
 plays and tasks.
+
+Check and diff mode
+-------------------
+
+Ansible supports `check and diff modes
+<https://docs.ansible.com/ansible/latest/user_guide/playbooks_checkmode.html>`_,
+which can be used to improve visibility into changes that would be made on
+target systems. The Kayobe CLI supports the ``--check`` argument, and since
+11.0.0, the ``--diff`` argument. Note that these modes are not always
+guaranteed to work, when some tasks are dependent on earlier ones.
diff --git a/kayobe/ansible.py b/kayobe/ansible.py
index 788c8a90..fee4191f 100644
--- a/kayobe/ansible.py
+++ b/kayobe/ansible.py
@@ -48,6 +48,10 @@ def add_args(parser):
                         help="path to Kayobe configuration. "
                              "(default=$%s or %s)" %
                              (CONFIG_PATH_ENV, DEFAULT_CONFIG_PATH))
+    parser.add_argument("-D", "--diff", action="store_true",
+                        help="when changing (small) files and templates, show "
+                             "the differences in those files; works great "
+                             "with --check")
     parser.add_argument("--environment", default=default_environment,
                         help="specify environment name (default=$%s or None)" %
                              ENVIRONMENT_ENV)
@@ -161,7 +165,7 @@ def _get_vars_files(vars_paths):
 
 def build_args(parsed_args, playbooks,
                extra_vars=None, limit=None, tags=None, verbose_level=None,
-               check=None, ignore_limit=False, list_tasks=None):
+               check=None, ignore_limit=False, list_tasks=None, diff=None):
     """Build arguments required for running Ansible playbooks."""
     cmd = ["ansible-playbook"]
     if verbose_level:
@@ -193,6 +197,8 @@ def build_args(parsed_args, playbooks,
         cmd += ["--become"]
     if check or (parsed_args.check and check is None):
         cmd += ["--check"]
+    if diff or (parsed_args.diff and diff is None):
+        cmd += ["--diff"]
     if not ignore_limit and (parsed_args.limit or limit):
         limit_arg = utils.intersect_limits(parsed_args.limit, limit)
         cmd += ["--limit", limit_arg]
@@ -227,13 +233,14 @@ def _get_environment(parsed_args):
 def run_playbooks(parsed_args, playbooks,
                   extra_vars=None, limit=None, tags=None, quiet=False,
                   check_output=False, verbose_level=None, check=None,
-                  ignore_limit=False, list_tasks=None):
+                  ignore_limit=False, list_tasks=None, diff=None):
     """Run a Kayobe Ansible playbook."""
     _validate_args(parsed_args, playbooks)
     cmd = build_args(parsed_args, playbooks,
                      extra_vars=extra_vars, limit=limit, tags=tags,
                      verbose_level=verbose_level, check=check,
-                     ignore_limit=ignore_limit, list_tasks=list_tasks)
+                     ignore_limit=ignore_limit, list_tasks=list_tasks,
+                     diff=diff)
     env = _get_environment(parsed_args)
     try:
         utils.run_command(cmd, check_output=check_output, quiet=quiet, env=env)
@@ -269,7 +276,7 @@ def config_dump(parsed_args, host=None, hosts=None, var_name=None,
         run_playbook(parsed_args, playbook_path,
                      extra_vars=extra_vars, tags=tags, check_output=True,
                      verbose_level=verbose_level, check=False,
-                     list_tasks=False)
+                     list_tasks=False, diff=False)
         hostvars = {}
         for path in os.listdir(dump_dir):
             LOG.debug("Found dump file %s", path)
diff --git a/kayobe/kolla_ansible.py b/kayobe/kolla_ansible.py
index 8954dae0..6eed8823 100644
--- a/kayobe/kolla_ansible.py
+++ b/kayobe/kolla_ansible.py
@@ -157,6 +157,14 @@ def _get_environment(parsed_args):
         ansible_cfg_path = os.path.join(parsed_args.config_path, "ansible.cfg")
         if utils.is_readable_file(ansible_cfg_path)["result"]:
             env.setdefault("ANSIBLE_CONFIG", ansible_cfg_path)
+    # kolla-ansible allows passing additional arguments to ansible-playbook via
+    # EXTRA_OPTS.
+    if parsed_args.check or parsed_args.diff:
+        extra_opts = env.setdefault("EXTRA_OPTS", "")
+        if parsed_args.check and "--check" not in extra_opts:
+            env["EXTRA_OPTS"] += " --check"
+        if parsed_args.diff and "--diff" not in extra_opts:
+            env["EXTRA_OPTS"] += " --diff"
     return env
 
 
diff --git a/kayobe/tests/unit/test_ansible.py b/kayobe/tests/unit/test_ansible.py
index de432429..779cc23a 100644
--- a/kayobe/tests/unit/test_ansible.py
+++ b/kayobe/tests/unit/test_ansible.py
@@ -68,6 +68,7 @@ class TestCase(unittest.TestCase):
             "-b",
             "-C",
             "--config-path", "/path/to/config",
+            "-D",
             "--environment", "test-env",
             "-e", "ev_name1=ev_value1",
             "-i", "/path/to/inventory",
@@ -88,6 +89,7 @@ class TestCase(unittest.TestCase):
             "-e", "ev_name1=ev_value1",
             "--become",
             "--check",
+            "--diff",
             "--limit", "group1:host",
             "--tags", "tag1,tag2",
             "playbook1.yml",
@@ -117,6 +119,7 @@ class TestCase(unittest.TestCase):
             "--become",
             "--check",
             "--config-path", "/path/to/config",
+            "--diff",
             "--environment", "test-env",
             "--extra-vars", "ev_name1=ev_value1",
             "--inventory", "/path/to/inventory",
@@ -138,6 +141,7 @@ class TestCase(unittest.TestCase):
             "-e", "ev_name1=ev_value1",
             "--become",
             "--check",
+            "--diff",
             "--limit", "group1:host1",
             "--skip-tags", "tag3,tag4",
             "--tags", "tag1,tag2",
@@ -249,6 +253,7 @@ class TestCase(unittest.TestCase):
             "tags": "tag3,tag4",
             "verbose_level": 0,
             "check": True,
+            "diff": True,
         }
         ansible.run_playbooks(parsed_args, ["playbook1.yml", "playbook2.yml"],
                               **kwargs)
@@ -260,6 +265,7 @@ class TestCase(unittest.TestCase):
             "-e", "ev_name1=ev_value1",
             "-e", "ev_name2='ev_value2'",
             "--check",
+            "--diff",
             "--limit", "group1:host1:&group2:host2",
             "--tags", "tag1,tag2,tag3,tag4",
             "playbook1.yml",
@@ -426,7 +432,7 @@ class TestCase(unittest.TestCase):
                                          },
                                          check_output=True, tags=None,
                                          verbose_level=None, check=False,
-                                         list_tasks=False)
+                                         list_tasks=False, diff=False)
         mock_rmtree.assert_called_once_with(dump_dir)
         mock_listdir.assert_any_call(dump_dir)
         mock_read.assert_has_calls([
diff --git a/kayobe/tests/unit/test_kolla_ansible.py b/kayobe/tests/unit/test_kolla_ansible.py
index 3eced948..6ab0dcd4 100644
--- a/kayobe/tests/unit/test_kolla_ansible.py
+++ b/kayobe/tests/unit/test_kolla_ansible.py
@@ -54,6 +54,8 @@ class TestCase(unittest.TestCase):
         kolla_ansible.add_args(parser)
         vault.add_args(parser)
         args = [
+            "-C",
+            "-D",
             "--kolla-config-path", "/path/to/config",
             "-ke", "ev_name1=ev_value1",
             "-ki", "/path/to/inventory",
@@ -73,8 +75,9 @@ class TestCase(unittest.TestCase):
             "--tags", "tag1,tag2",
         ]
         expected_cmd = " ".join(expected_cmd)
+        expected_env = {"EXTRA_OPTS": " --check --diff"}
         mock_run.assert_called_once_with(expected_cmd, shell=True, quiet=False,
-                                         env={})
+                                         env=expected_env)
 
     @mock.patch.object(utils, "run_command")
     @mock.patch.object(kolla_ansible, "_validate_args")
@@ -87,6 +90,8 @@ class TestCase(unittest.TestCase):
         mock_ask.return_value = "test-pass"
         args = [
             "--ask-vault-pass",
+            "--check",
+            "--diff",
             "--kolla-config-path", "/path/to/config",
             "--kolla-extra-vars", "ev_name1=ev_value1",
             "--kolla-inventory", "/path/to/inventory",
@@ -110,7 +115,8 @@ class TestCase(unittest.TestCase):
             "--tags", "tag1,tag2",
         ]
         expected_cmd = " ".join(expected_cmd)
-        expected_env = {"KAYOBE_VAULT_PASSWORD": "test-pass"}
+        expected_env = {"EXTRA_OPTS": " --check --diff",
+                        "KAYOBE_VAULT_PASSWORD": "test-pass"}
         expected_calls = [
             mock.call(["which", "kayobe-vault-password-helper"],
                       check_output=True, universal_newlines=True),
diff --git a/releasenotes/notes/diff-mode-468e09bbb9185b50.yaml b/releasenotes/notes/diff-mode-468e09bbb9185b50.yaml
new file mode 100644
index 00000000..a921d945
--- /dev/null
+++ b/releasenotes/notes/diff-mode-468e09bbb9185b50.yaml
@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    Adds a ``--diff`` argument to kayobe CLI commands. This is passed through
+    to ``ansible-playbook`` for Kayobe and Kolla Ansible playbooks, and can be
+    used with the ``--check`` argument to see changes that would be made to
+    files.
+upgrade:
+  - |
+    The ``--check`` argument to kayobe CLI commands is now passed through to
+    Kolla Ansible playbooks.
-- 
GitLab