From 2648f48746fa01a5215e1fd9c79a0e7821414841 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Tue, 18 May 2021 14:51:44 +0100
Subject: [PATCH] Make setup module arguments configurable

Ansible facts can have a large impact on the performance of the Ansible
control host. This patch introduces some control over which facts are
gathered (kayobe_ansible_setup_gather_subset) and which facts are stored
(kayobe_ansible_setup_filter). By default we do not change the default
values of these arguments to the setup module. The flexibility of these
arguments is limited, but they do provide enough for a large performance
improvement in a typical moderate to large OpenStack cloud.

In particular, the large complex dict fact for each interface has a
large effect, and on an OpenStack controller or hypervisor there may be
many virtual interfaces. We can use the kayobe_ansible_setup_filter
variable to help:

    kayobe_ansible_setup_filter: 'ansible_[!qt]*'

This causes Ansible to collect but not store facts matching that
pattern, which includes the virtual interface facts. Currently we are
not referencing other facts matching the pattern within Kayobe.
Note that including the 'ansible_' prefix causes meta facts module_setup
and gather_subset to be filtered, but this seems to be the only way to
get a good match on the interface facts. To work around this, we use
ansible_facts rather than module_setup to detect whether facts exist in
the cache.

The exact improvement will vary, but has been reported to be as large as
18x on systems with many virtual interfaces.

This change also introduces a new command to gather facts for Kayobe &
Kolla Ansible on demand, 'kayobe overcloud facts gather'. This can be
used to populate a fact cache.

Depends-On: https://review.opendev.org/c/openstack/kolla-ansible/+/794610
Story: 2007993
Task: 42586

Change-Id: I5ce3c734433e1682ee942867505468c57440e689
---
 ansible/group_vars/all/globals                | 11 ++++
 ansible/kayobe-target-venv.yml                |  6 +-
 ansible/kolla-target-venv.yml                 |  4 +-
 ansible/overcloud-facts-gather.yml            | 10 +++
 ansible/roles/network-redhat/tasks/main.yml   |  2 +
 doc/source/administration/overcloud.rst       | 12 ++++
 .../configuration/reference/ansible.rst       | 62 +++++++++++++++++++
 etc/kayobe/globals.yml                        | 11 ++++
 kayobe/cli/commands.py                        | 16 +++++
 kayobe/tests/unit/cli/test_commands.py        | 37 +++++++++++
 .../setup-module-args-2c36e56bf78ab5f0.yaml   | 11 ++++
 setup.cfg                                     |  3 +
 12 files changed, 183 insertions(+), 2 deletions(-)
 create mode 100644 ansible/overcloud-facts-gather.yml
 create mode 100644 releasenotes/notes/setup-module-args-2c36e56bf78ab5f0.yaml

diff --git a/ansible/group_vars/all/globals b/ansible/group_vars/all/globals
index f6cad0af..3a7f362c 100644
--- a/ansible/group_vars/all/globals
+++ b/ansible/group_vars/all/globals
@@ -51,3 +51,14 @@ os_distribution: "centos"
 # OS release. Valid options are "8-stream" when os_distribution is "centos", or
 # "focal" when os_distribution is "ubuntu".
 os_release: "{{ '8-stream' if os_distribution == 'centos' else 'focal' }}"
+
+###############################################################################
+# Ansible configuration.
+
+# Filter to apply to the setup module when gathering facts. Default is to not
+# specify a filter.
+kayobe_ansible_setup_filter: "{{ omit }}"
+
+# Gather subset to apply to the setup module when gathering facts. Default is
+# to not specify a gather subset.
+kayobe_ansible_setup_gather_subset: "{{ omit }}"
diff --git a/ansible/kayobe-target-venv.yml b/ansible/kayobe-target-venv.yml
index b18acf9c..02bd526f 100644
--- a/ansible/kayobe-target-venv.yml
+++ b/ansible/kayobe-target-venv.yml
@@ -19,7 +19,9 @@
     - block:
         - name: Gather facts
           setup:
-          when: not ansible_facts.module_setup | default(false)
+            filter: "{{ kayobe_ansible_setup_filter }}"
+            gather_subset: "{{ kayobe_ansible_setup_gather_subset }}"
+          when: not ansible_facts
           register: gather_facts
 
         - name: Ensure the Python virtualenv package is installed
@@ -79,6 +81,8 @@
     # again using the interpreter from the virtual environment.
     - name: Gather facts
       setup:
+        filter: "{{ kayobe_ansible_setup_filter }}"
+        gather_subset: "{{ kayobe_ansible_setup_gather_subset }}"
       when:
         - virtualenv is defined
         - gather_facts is not skipped
diff --git a/ansible/kolla-target-venv.yml b/ansible/kolla-target-venv.yml
index 9a556b9a..7fd9e3dc 100644
--- a/ansible/kolla-target-venv.yml
+++ b/ansible/kolla-target-venv.yml
@@ -21,7 +21,9 @@
     - block:
         - name: Gather facts
           setup:
-          when: not ansible_facts.module_setup | default(false)
+            filter: "{{ kayobe_ansible_setup_filter }}"
+            gather_subset: "{{ kayobe_ansible_setup_gather_subset }}"
+          when: not ansible_facts
 
         - name: Ensure the Python virtualenv package is installed
           package:
diff --git a/ansible/overcloud-facts-gather.yml b/ansible/overcloud-facts-gather.yml
new file mode 100644
index 00000000..a8f101af
--- /dev/null
+++ b/ansible/overcloud-facts-gather.yml
@@ -0,0 +1,10 @@
+---
+- name: Gather facts
+  hosts: overcloud
+  gather_facts: false
+  tasks:
+    - name: Gather facts
+      setup:
+        filter: "{{ kayobe_ansible_setup_filter }}"
+        gather_subset: "{{ kayobe_ansible_setup_gather_subset }}"
+      when: not ansible_facts
diff --git a/ansible/roles/network-redhat/tasks/main.yml b/ansible/roles/network-redhat/tasks/main.yml
index 5f1f59d0..27aa92d7 100644
--- a/ansible/roles/network-redhat/tasks/main.yml
+++ b/ansible/roles/network-redhat/tasks/main.yml
@@ -36,6 +36,8 @@
          net_select_bonds |
          map('net_bond_obj') |
          list }}
+    interfaces_setup_filter: "{{ kayobe_ansible_setup_filter }}"
+    interfaces_setup_gather_subset: "{{ kayobe_ansible_setup_gather_subset }}"
 
 # Configure virtual ethernet patch links to connect the workload provision
 # and external network bridges to the Neutron OVS bridge.
diff --git a/doc/source/administration/overcloud.rst b/doc/source/administration/overcloud.rst
index 6a2aa60c..914e7270 100644
--- a/doc/source/administration/overcloud.rst
+++ b/doc/source/administration/overcloud.rst
@@ -249,3 +249,15 @@ Or to perform an incremental backup, run the following command:
 Further information on backing up and restoring the database is available in
 the :kolla-ansible-doc:`Kolla Ansible documentation
 <admin/mariadb-backup-and-restore.html>`.
+
+Gathering Facts
+===============
+
+The following command may be used to gather facts for all overcloud hosts, for
+both Kayobe and Kolla Ansible:
+
+.. code-block:: console
+
+   kayobe overcloud facts gather
+
+This may be useful to populate a fact cache in advance of other operations.
diff --git a/doc/source/configuration/reference/ansible.rst b/doc/source/configuration/reference/ansible.rst
index 55f2688f..49372428 100644
--- a/doc/source/configuration/reference/ansible.rst
+++ b/doc/source/configuration/reference/ansible.rst
@@ -84,3 +84,65 @@ caching using the `jsonfile cache plugin
 
 You may also wish to set the expiration timeout for the cache via ``[defaults]
 fact_caching_timeout``.
+
+Fact gathering
+==============
+
+Fact filtering
+--------------
+
+Filtering of facts can be used to speed up Ansible.  Environments with
+many network interfaces on the network and compute nodes can experience very
+slow processing with Kayobe and Kolla Ansible. This happens due to the
+processing of the large per-interface facts with each task.  To avoid storing
+certain facts, we can use the ``kayobe_ansible_setup_filter`` variable, which
+is used as the ``filter`` argument to the ``setup`` module.
+
+One case where this is particularly useful is to avoid collecting facts for
+virtual tap (beginning with t) and bridge (beginning with q) interfaces
+created by Neutron. These facts are large map values which can consume a lot
+of resources on the Ansible control host. Kayobe and Kolla Ansible typically
+do not need to reference them, so they may be filtered. For example, to
+avoid collecting facts beginning with q or t:
+
+.. code-block:: yaml
+   :caption: ``$KAYOBE_CONFIG_PATH/globals.yml``
+
+   kayobe_ansible_setup_filter: "ansible_[!qt]*"
+
+Similarly, for Kolla Ansible (notice the similar but different file names):
+
+.. code-block:: yaml
+   :caption: ``$KAYOBE_CONFIG_PATH/kolla/globals.yml``
+
+   kolla_ansible_setup_filter: "ansible_[!qt]*"
+
+This causes Ansible to collect but not store facts matching that pattern, which
+includes the virtual interface facts. Currently we are not referencing other
+facts matching the pattern within Kolla Ansible.  Note that including the
+'ansible_' prefix causes meta facts ``module_setup`` and ``gather_subset`` to
+be filtered, but this seems to be the only way to get a good match on the
+interface facts.
+
+The exact improvement will vary, but has been reported to be as large as 18x on
+systems with many virtual interfaces.
+
+Fact gathering subsets
+----------------------
+
+It is also possible to configure which subsets of facts are gathered, via
+``kayobe_ansible_setup_gather_subset``, which is used as the ``gather_subset``
+argument to the ``setup`` module. For example, if one wants to avoid collecting
+facts via facter:
+
+.. code-block:: yaml
+   :caption: ``$KAYOBE_CONFIG_PATH/globals.yml``
+
+   kayobe_ansible_setup_gather_subset: "all,!facter"
+
+Similarly, for Kolla Ansible (notice the similar but different file names):
+
+.. code-block:: yaml
+   :caption: ``$KAYOBE_CONFIG_PATH/kolla/globals.yml``
+
+   kolla_ansible_setup_gather_subset: "all,!facter"
diff --git a/etc/kayobe/globals.yml b/etc/kayobe/globals.yml
index 9efc114f..a4150d8e 100644
--- a/etc/kayobe/globals.yml
+++ b/etc/kayobe/globals.yml
@@ -53,6 +53,17 @@
 # "focal" when os_distribution is "ubuntu".
 #os_release:
 
+###############################################################################
+# Ansible configuration.
+
+# Filter to apply to the setup module when gathering facts. Default is to not
+# specify a filter.
+#kayobe_ansible_setup_filter:
+
+# Gather subset to apply to the setup module when gathering facts. Default is
+# to not specify a gather subset.
+#kayobe_ansible_setup_gather_subset:
+
 ###############################################################################
 # Dummy variable to allow Ansible to accept this file.
 workaround_ansible_issue_8743: yes
diff --git a/kayobe/cli/commands.py b/kayobe/cli/commands.py
index 38afdc6f..be5af166 100644
--- a/kayobe/cli/commands.py
+++ b/kayobe/cli/commands.py
@@ -932,6 +932,22 @@ class OvercloudDeprovision(KayobeAnsibleMixin, VaultMixin, Command):
         self.run_kayobe_playbooks(parsed_args, playbooks)
 
 
+class OvercloudFactsGather(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
+                           Command):
+    """Gather facts for Kayobe and Kolla Ansible."""
+
+    def take_action(self, parsed_args):
+        self.app.LOG.debug("Gathering overcloud host facts")
+
+        # Gather facts for Kayobe.
+        playbooks = _build_playbook_list("overcloud-facts-gather")
+        self.run_kayobe_playbooks(parsed_args, playbooks)
+
+        # Gather facts for Kolla Ansible.
+        self.generate_kolla_ansible_config(parsed_args, service_config=False)
+        self.run_kolla_ansible_overcloud(parsed_args, "gather-facts")
+
+
 class OvercloudHostConfigure(KollaAnsibleMixin, KayobeAnsibleMixin, VaultMixin,
                              Command):
     """Configure the overcloud host OS and services.
diff --git a/kayobe/tests/unit/cli/test_commands.py b/kayobe/tests/unit/cli/test_commands.py
index 4e2a0f55..b9205974 100644
--- a/kayobe/tests/unit/cli/test_commands.py
+++ b/kayobe/tests/unit/cli/test_commands.py
@@ -1008,6 +1008,43 @@ class TestCase(unittest.TestCase):
         ]
         self.assertEqual(expected_calls, mock_run.call_args_list)
 
+    @mock.patch.object(commands.KayobeAnsibleMixin,
+                       "run_kayobe_playbooks")
+    @mock.patch.object(commands.KollaAnsibleMixin,
+                       "run_kolla_ansible_overcloud")
+    def test_overcloud_facts_gather(self, mock_kolla_run, mock_run):
+        command = commands.OvercloudFactsGather(TestApp(), [])
+        parser = command.get_parser("test")
+        parsed_args = parser.parse_args([])
+
+        result = command.run(parsed_args)
+        self.assertEqual(0, result)
+
+        expected_calls = [
+            mock.call(
+                mock.ANY,
+                [
+                    utils.get_data_files_path(
+                        "ansible", "overcloud-facts-gather.yml"),
+                ],
+            ),
+            mock.call(
+                mock.ANY,
+                [utils.get_data_files_path("ansible", "kolla-ansible.yml")],
+                tags="config",
+                ignore_limit=True,
+            ),
+        ]
+        self.assertEqual(expected_calls, mock_run.call_args_list)
+
+        expected_calls = [
+            mock.call(
+                mock.ANY,
+                "gather-facts"
+            ),
+        ]
+        self.assertEqual(expected_calls, mock_kolla_run.call_args_list)
+
     @mock.patch.object(commands.KayobeAnsibleMixin,
                        "run_kayobe_playbooks")
     @mock.patch.object(commands.KollaAnsibleMixin,
diff --git a/releasenotes/notes/setup-module-args-2c36e56bf78ab5f0.yaml b/releasenotes/notes/setup-module-args-2c36e56bf78ab5f0.yaml
new file mode 100644
index 00000000..21aeb65b
--- /dev/null
+++ b/releasenotes/notes/setup-module-args-2c36e56bf78ab5f0.yaml
@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    Adds support for configuring the ``filter`` and ``gather_subset`` arguments
+    for the ``setup`` module via ``kayobe_ansible_setup_filter`` and
+    ``kayobe_ansible_setup_gather_subset`` respectively. These can be used to
+    reduce the number of facts, which can have a significant effect on
+    performance of Ansible.
+  - |
+    Adds a new command, ``kayobe overcloud facts gather``, to gather Ansible
+    facts for overcloud hosts. This may be useful for populating a fact cache.
diff --git a/setup.cfg b/setup.cfg
index 282c192e..708ea819 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -58,6 +58,7 @@ kayobe.cli=
     overcloud_database_recover = kayobe.cli.commands:OvercloudDatabaseRecover
     overcloud_deployment_image_build = kayobe.cli.commands:OvercloudDeploymentImageBuild
     overcloud_deprovision = kayobe.cli.commands:OvercloudDeprovision
+    overcloud_facts_gather = kayobe.cli.commands:OvercloudFactsGather
     overcloud_hardware_inspect = kayobe.cli.commands:OvercloudHardwareInspect
     overcloud_host_configure = kayobe.cli.commands:OvercloudHostConfigure
     overcloud_host_package_update = kayobe.cli.commands:OvercloudHostPackageUpdate
@@ -132,6 +133,8 @@ kayobe.cli.overcloud_deployment_image_build =
     hooks = kayobe.cli.commands:HookDispatcher
 kayobe.cli.overcloud_deprovision =
     hooks = kayobe.cli.commands:HookDispatcher
+kayobe.cli.overcloud_facts_gather =
+    hooks = kayobe.cli.commands:HookDispatcher
 kayobe.cli.overcloud_hardware_inspect =
     hooks = kayobe.cli.commands:HookDispatcher
 kayobe.cli.overcloud_host_configure =
-- 
GitLab