From 2c0f705e7acab5691591c215e40dea707d12da15 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Thu, 12 Jan 2023 14:15:11 +0000
Subject: [PATCH] Generate local Kolla Ansible config in check mode

If running a command in check mode such as

    kayobe overcloud service deploy --check

Kayobe does not generate the local configuration for Kolla Ansible. This
can lead to an inaccurate result when comparing with the remote
configuration, if there are changes in kayobe-config.

For example:

* Run kayobe overcloud service deploy
* Change a file in etc/kayobe/kolla
* Run kayobe overcloud service deploy --check --diff

We would expect that the changed file results in a diff against the
remote config. However there is no diff displayed.

This change fixes the issue by always generating the local Kolla Ansible
config, even in check mode.

Change-Id: Ic1dd075076ea186b0928bba1a235605c0cd2ec71
Story: 2010526
Task: 47132
---
 kayobe/cli/commands.py                        | 10 +++---
 kayobe/tests/unit/cli/test_commands.py        | 33 +++++++++++++++++--
 ...ck-mode-local-config-3f8a4ba231a32c1f.yaml |  7 ++++
 3 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 releasenotes/notes/check-mode-local-config-3f8a4ba231a32c1f.yaml

diff --git a/kayobe/cli/commands.py b/kayobe/cli/commands.py
index 717f2bdfb..05c8458b5 100644
--- a/kayobe/cli/commands.py
+++ b/kayobe/cli/commands.py
@@ -105,15 +105,15 @@ class KayobeAnsibleMixin(object):
         tags = None if install else "config"
         playbooks = _build_playbook_list("kolla-ansible")
         self.run_kayobe_playbooks(parsed_args, playbooks, tags=tags,
-                                  ignore_limit=True)
+                                  ignore_limit=True, check=False)
         if service_config:
             playbooks = _build_playbook_list("kolla-openstack")
             self.run_kayobe_playbooks(parsed_args, playbooks,
-                                      ignore_limit=True)
+                                      ignore_limit=True, check=False)
         if bifrost_config:
             playbooks = _build_playbook_list("kolla-bifrost")
             self.run_kayobe_playbooks(parsed_args, playbooks,
-                                      ignore_limit=True)
+                                      ignore_limit=True, check=False)
 
 
 class KollaAnsibleMixin(object):
@@ -261,7 +261,7 @@ class ControlHostBootstrap(KayobeAnsibleMixin, KollaAnsibleMixin, VaultMixin,
             ka_tags = "install"
         playbooks = _build_playbook_list("kolla-ansible")
         self.run_kayobe_playbooks(parsed_args, playbooks, tags=ka_tags,
-                                  ignore_limit=True)
+                                  ignore_limit=True, check=False)
 
         if passwords_exist:
             # If we are bootstrapping a control host for an existing
@@ -301,7 +301,7 @@ class ControlHostUpgrade(KayobeAnsibleMixin, VaultMixin, Command):
         self.run_kayobe_playbooks(parsed_args, playbooks, ignore_limit=True)
         playbooks = _build_playbook_list("kolla-ansible")
         self.run_kayobe_playbooks(parsed_args, playbooks, tags="install",
-                                  ignore_limit=True)
+                                  ignore_limit=True, check=False)
 
 
 class ConfigurationDump(KayobeAnsibleMixin, VaultMixin, Command):
diff --git a/kayobe/tests/unit/cli/test_commands.py b/kayobe/tests/unit/cli/test_commands.py
index 066f0ee4d..75f552ff2 100644
--- a/kayobe/tests/unit/cli/test_commands.py
+++ b/kayobe/tests/unit/cli/test_commands.py
@@ -62,7 +62,8 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="install",
-                ignore_limit=True
+                ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -95,7 +96,8 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags=None,
-                ignore_limit=True
+                ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -140,7 +142,8 @@ class TestCase(unittest.TestCase):
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="install",
-                ignore_limit=True
+                ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -811,11 +814,13 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-bifrost.yml")],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -861,11 +866,13 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
                 [utils.get_data_files_path("ansible", "kolla-bifrost.yml")],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1120,6 +1127,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1216,6 +1224,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1426,6 +1435,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1457,6 +1467,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1487,6 +1498,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1495,6 +1507,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1526,6 +1539,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 tags="config",
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1534,6 +1548,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1587,6 +1602,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags="config",
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1595,6 +1611,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1652,6 +1669,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags="config",
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1660,6 +1678,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1705,6 +1724,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags="config",
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1713,6 +1733,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
         ]
         self.assertListEqual(expected_calls, mock_run.call_args_list)
@@ -1743,6 +1764,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags="config",
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1751,6 +1773,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1807,6 +1830,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags="config",
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1815,6 +1839,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1871,6 +1896,7 @@ class TestCase(unittest.TestCase):
                 [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
                 ignore_limit=True,
                 tags=None,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
@@ -1879,6 +1905,7 @@ class TestCase(unittest.TestCase):
                                               "kolla-openstack.yml"),
                 ],
                 ignore_limit=True,
+                check=False,
             ),
             mock.call(
                 mock.ANY,
diff --git a/releasenotes/notes/check-mode-local-config-3f8a4ba231a32c1f.yaml b/releasenotes/notes/check-mode-local-config-3f8a4ba231a32c1f.yaml
new file mode 100644
index 000000000..fba1aa222
--- /dev/null
+++ b/releasenotes/notes/check-mode-local-config-3f8a4ba231a32c1f.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue where local configuration generation would be skipped when
+    running in check mode. This would lead to Kolla Ansible checking with stale
+    configuration. See `story 2010526
+    <https://storyboard.openstack.org/#!/story/2010526>`__ for details.