From e924c99c522b03df4fbe1a9013a2659a11cae071 Mon Sep 17 00:00:00 2001
From: Mark Goddard <mark@stackhpc.com>
Date: Tue, 31 Mar 2020 14:52:56 +0100
Subject: [PATCH] Avoid unconditional fact gathering

One way to improve the performance of Ansible is through fact caching.
Rather than gather facts in every play, we can configure Ansible to
cache them in a persistent store. An example Ansible configuration for
doing this is as follows:

[defaults]
gathering = smart
fact_caching = jsonfile
fact_caching_connection = ./facts
fact_caching_timeout = 86400

While this mostly just works, there are a few places where we
unconditionally gather facts using the setup module. This change
modifies these to only gather facts when necessary.

We no longer execute the MichaelRigart.interfaces role using become:
true, since it may gather facts and we do not want it to do so as root.
The role uses become where necessary.

Change-Id: I9984a187fc6c0496ada489bb8eef36e44d695aac
Story: 2007492
Task: 39216
---
 ansible/baremetal-compute-serial-console.yml         |  1 +
 ansible/ip-allocation.yml                            |  1 +
 ansible/kayobe-ansible-user.yml                      |  1 +
 ansible/kayobe-target-venv.yml                       | 12 ++++++++++++
 ansible/kolla-target-venv.yml                        |  2 +-
 ansible/network.yml                                  |  1 -
 ansible/roles/swift-rings/tasks/main.yml             |  1 +
 ...nconditional-fact-gathering-4dfbf96e2111ad51.yaml |  7 +++++++
 8 files changed, 24 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/avoid-unconditional-fact-gathering-4dfbf96e2111ad51.yaml

diff --git a/ansible/baremetal-compute-serial-console.yml b/ansible/baremetal-compute-serial-console.yml
index 72a42bde..d36c42c6 100644
--- a/ansible/baremetal-compute-serial-console.yml
+++ b/ansible/baremetal-compute-serial-console.yml
@@ -47,6 +47,7 @@
            gather_subset: min
          delegate_to: localhost
          delegate_facts: true
+         when: not hostvars.localhost.module_setup | default(false)
 
        - name: Reserve TCP ports for ironic serial consoles
          include_role:
diff --git a/ansible/ip-allocation.yml b/ansible/ip-allocation.yml
index e5b446ff..4a6bb15c 100644
--- a/ansible/ip-allocation.yml
+++ b/ansible/ip-allocation.yml
@@ -15,6 +15,7 @@
       delegate_to: localhost
       delegate_facts: true
       run_once: true
+      when: not hostvars.localhost.module_setup | default(false)
 
 - name: Ensure IP addresses are allocated
   hosts: seed-hypervisor:seed:overcloud
diff --git a/ansible/kayobe-ansible-user.yml b/ansible/kayobe-ansible-user.yml
index 75ab7630..cf8d161f 100644
--- a/ansible/kayobe-ansible-user.yml
+++ b/ansible/kayobe-ansible-user.yml
@@ -36,6 +36,7 @@
 
 - name: Ensure the Kayobe Ansible user account exists
   hosts: kayobe_user_bootstrap_required_True
+  gather_facts: false
   tags:
     - kayobe-ansible-user
   vars:
diff --git a/ansible/kayobe-target-venv.yml b/ansible/kayobe-target-venv.yml
index b58ed634..aba2f4d7 100644
--- a/ansible/kayobe-target-venv.yml
+++ b/ansible/kayobe-target-venv.yml
@@ -19,6 +19,8 @@
     - block:
         - name: Gather facts
           setup:
+          when: not module_setup | default(false)
+          register: gather_facts
 
         - name: Ensure the Python virtualenv package is installed
           package:
@@ -82,6 +84,16 @@
         ansible_python_interpreter: /usr/libexec/platform-python
       when: virtualenv is defined
 
+    # If we gathered facts earlier it would have been with a different Python
+    # interpreter. For gathering modes that may use a fact cache, gather facts
+    # again using the interpreter from the virtual environment.
+    - name: Gather facts
+      setup:
+      when:
+        - virtualenv is defined
+        - gather_facts is not skipped
+        - lookup('config', 'DEFAULT_GATHERING') != 'implicit'
+
     - block:
         - name: Ensure Python setuptools and pip packages are installed
           vars:
diff --git a/ansible/kolla-target-venv.yml b/ansible/kolla-target-venv.yml
index e5d36742..c0c28529 100644
--- a/ansible/kolla-target-venv.yml
+++ b/ansible/kolla-target-venv.yml
@@ -21,7 +21,7 @@
     - block:
         - name: Gather facts
           setup:
-          when: ansible_python is not defined
+          when: not module_setup | default(false)
 
         - name: Ensure the Python virtualenv package is installed
           package:
diff --git a/ansible/network.yml b/ansible/network.yml
index 2db2ffe1..230a0cd0 100644
--- a/ansible/network.yml
+++ b/ansible/network.yml
@@ -67,7 +67,6 @@
         {{ bond_interfaces |
            map('net_bond_obj') |
            list }}
-      become: True
 
 # Configure virtual ethernet patch links to connect the workload provision
 # and external network bridges to the Neutron OVS bridge.
diff --git a/ansible/roles/swift-rings/tasks/main.yml b/ansible/roles/swift-rings/tasks/main.yml
index cb531268..7c0a64da 100644
--- a/ansible/roles/swift-rings/tasks/main.yml
+++ b/ansible/roles/swift-rings/tasks/main.yml
@@ -9,6 +9,7 @@
     # Facts required for ansible_user_uid and ansible_user_gid.
     - name: Gather facts for swift ring build host
       setup:
+      when: not module_setup | default(false)
 
     - name: Ensure Swift ring build directory exists
       file:
diff --git a/releasenotes/notes/avoid-unconditional-fact-gathering-4dfbf96e2111ad51.yaml b/releasenotes/notes/avoid-unconditional-fact-gathering-4dfbf96e2111ad51.yaml
new file mode 100644
index 00000000..b919e7af
--- /dev/null
+++ b/releasenotes/notes/avoid-unconditional-fact-gathering-4dfbf96e2111ad51.yaml
@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    Avoids unnecessary fact gathering using the ``setup`` module. This should
+    improve the performance of environments using fact caching and the Ansible
+    ``smart`` fact gathering policy. See `story 2007492
+    <https://storyboard.openstack.org/#!/story/2007492>`__ for details.
-- 
GitLab