From 9111328e8a715d2dd5ff2a1e95d996d538300e82 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Mon, 21 Feb 2022 14:43:28 +0000 Subject: [PATCH] Use ansible_facts to reference facts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default, Ansible injects a variable for every fact, prefixed with ansible_. This can result in a large number of variables for each host, which at scale can incur a performance penalty. Ansible provides a configuration option [0] that can be set to False to prevent this injection of facts. In this case, facts should be referenced via ansible_facts.. This change updates all references to Ansible facts within Tenks from using individual fact variables to using the items in the ansible_facts dictionary. This allows users to disable fact variable injection in their Ansible configuration, which may provide some performance improvement. This change disables fact variable injection in the ansible configuration used in CI, to catch any attempts to use the injected variables. [0] https://docs.ansible.com/ansible/latest/reference_appendices/config.html#inject-facts-as-vars Change-Id: I4831769909c341c72bb178daf8df3309773a56ad Co-Authored-By: Radosław Piliszek --- .ansible-lint | 3 ++- ansible/group_vars/all | 4 ++-- ansible/group_vars/hypervisors | 2 +- ansible/group_vars/libvirt | 4 ++-- ansible/host_setup.yml | 4 ++-- ansible/hypervisor_setup.yml | 6 +++--- ansible/node_bmc.yml | 4 ++-- ansible/physical_network.yml | 8 ++++---- ansible/roles/ironic-enrolment/tasks/port.yml | 2 +- ansible/roles/virtualbmc-daemon/tasks/main.yml | 8 ++++---- playbooks/openvswitch.yml | 10 +++++----- playbooks/tenks-deploy-teardown/pre.yml | 16 ++++++++++++++-- .../templates/ansible.cfg.j2 | 7 +++++++ .../templates/tenks-overrides.yml.j2 | 2 +- playbooks/tenks-deploy-teardown/vars/common.yml | 12 ++++++------ roles/tenks-ci-prep/tasks/main.yml | 4 ++-- 16 files changed, 58 insertions(+), 38 deletions(-) create mode 100644 playbooks/tenks-deploy-teardown/templates/ansible.cfg.j2 diff --git a/.ansible-lint b/.ansible-lint index 92d88fd..2a503a9 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -2,5 +2,6 @@ skip_list: - '106' # Role name {} does not match ``^[a-z][a-z0-9_]+$`` pattern' warn_list: - - experimental - no-changed-when + - command-instead-of-shell # Use shell only when shell functionality is required + - experimental # all rules tagged as experimental diff --git a/ansible/group_vars/all b/ansible/group_vars/all index 9e971c6..3cd8c82 100644 --- a/ansible/group_vars/all +++ b/ansible/group_vars/all @@ -1,9 +1,9 @@ --- # Path to virtualenv used to install Python requirements. If a virtualenv does # not exist at this location, one will be created. -virtualenv_path: "{{ '/'.join([ansible_env['HOME'], 'tenks-venv']) }}" +virtualenv_path: "{{ '/'.join([ansible_facts.env['HOME'], 'tenks-venv']) }}" # The URL of the upper constraints file to pass to pip when installing Python # packages. python_upper_constraints_url: >- - https://releases.openstack.org/constraints/upper/{% if ansible_python.version.major == 2 %}train{% else %}master{% endif %} + https://releases.openstack.org/constraints/upper/{% if ansible_facts.python.version.major == 2 %}train{% else %}master{% endif %} diff --git a/ansible/group_vars/hypervisors b/ansible/group_vars/hypervisors index 61ec7a1..f5024c6 100644 --- a/ansible/group_vars/hypervisors +++ b/ansible/group_vars/hypervisors @@ -4,7 +4,7 @@ physnet_mappings: {} system_requirements: - - "python{% if ansible_python.version.major == 3 %}3{% endif %}-virtualenv" + - "python{% if ansible_facts.python.version.major == 3 %}3{% endif %}-virtualenv" # Tenks bridge type. Options are "openvswitch", "linuxbridge". Default is # "openvswitch". Note that this relates to bridges created by Tenks, not the diff --git a/ansible/group_vars/libvirt b/ansible/group_vars/libvirt index 304863c..0a171e3 100644 --- a/ansible/group_vars/libvirt +++ b/ansible/group_vars/libvirt @@ -5,8 +5,8 @@ libvirt_pool_type: dir # Capacity is irrelevant for directory-based pools. libvirt_pool_capacity: libvirt_pool_mode: 755 -libvirt_pool_owner: "{{ ansible_user_id }}" -libvirt_pool_group: "{{ ansible_user_id }}" +libvirt_pool_owner: "{{ ansible_facts.user_id }}" +libvirt_pool_group: "{{ ansible_facts.user_id }}" # By default, allow QEMU without hardware virtualisation since this is a # development tool. diff --git a/ansible/host_setup.yml b/ansible/host_setup.yml index 82b16f9..649ca8d 100644 --- a/ansible/host_setup.yml +++ b/ansible/host_setup.yml @@ -53,8 +53,8 @@ # NOTE(mgoddard): On CentOS 8 if SELinux is enabled, install # virtualbmc to the system rather than a virtualenv. SELinux # prevents systemd from accessing files in users' home directories. - selinux_enabled: "{{ ansible_selinux.status | default('disabled') == 'enabled' }}" - is_centos8: "{{ ansible_os_family == 'RedHat' and ansible_distribution_major_version | int == 8 }}" + selinux_enabled: "{{ ansible_facts.selinux.status | default('disabled') == 'enabled' }}" + is_centos8: "{{ ansible_facts.os_family == 'RedHat' and ansible_facts.distribution_major_version | int == 8 }}" vbmcd_virtualenv_path: "{{ '' if is_centos8 and selinux_enabled else virtualenv_path }}" vbmcd_python_upper_constraints_url: >- {{ python_upper_constraints_url }} diff --git a/ansible/hypervisor_setup.yml b/ansible/hypervisor_setup.yml index e2ceb2e..9ab891d 100644 --- a/ansible/hypervisor_setup.yml +++ b/ansible/hypervisor_setup.yml @@ -3,9 +3,9 @@ include_vars: "{{ item }}" with_first_found: - files: - - "{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml" - - "{{ ansible_distribution }}.yml" - - "{{ ansible_os_family }}.yml" + - "{{ ansible_facts.distribution }}-{{ ansible_facts.distribution_major_version }}.yml" + - "{{ ansible_facts.distribution }}.yml" + - "{{ ansible_facts.os_family }}.yml" skip: true tags: vars diff --git a/ansible/node_bmc.yml b/ansible/node_bmc.yml index b1e7483..a5abede 100644 --- a/ansible/node_bmc.yml +++ b/ansible/node_bmc.yml @@ -48,8 +48,8 @@ # NOTE(mgoddard): On CentOS 8 if SELinux is enabled, install virtualbmc # to the system rather than a virtualenv. SELinux prevents systemd from # accessing files in users' home directories. - selinux_enabled: "{{ ansible_selinux.status | default('disabled') == 'enabled' }}" - is_centos8: "{{ ansible_os_family == 'RedHat' and ansible_distribution_major_version | int == 8 }}" + selinux_enabled: "{{ ansible_facts.selinux.status | default('disabled') == 'enabled' }}" + is_centos8: "{{ ansible_facts.os_family == 'RedHat' and ansible_facts.distribution_major_version | int == 8 }}" vbmc_virtualenv_path: "{{ '' if is_centos8 and selinux_enabled else virtualenv_path }}" vbmc_log_directory: "{{ log_directory }}" vbmc_state: "{{ domain.get('state', 'present') }}" diff --git a/ansible/physical_network.yml b/ansible/physical_network.yml index c3ad261..c7ce898 100644 --- a/ansible/physical_network.yml +++ b/ansible/physical_network.yml @@ -4,9 +4,9 @@ include_vars: "{{ item }}" with_first_found: - files: - - "{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml" - - "{{ ansible_distribution }}.yml" - - "{{ ansible_os_family }}.yml" + - "{{ ansible_facts.distribution }}-{{ ansible_facts.distribution_major_version }}.yml" + - "{{ ansible_facts.distribution }}.yml" + - "{{ ansible_facts.os_family }}.yml" skip: true tags: vars @@ -15,7 +15,7 @@ msg: > The interface {{ source_interface }} specified for the physical network {{ network_name }} does not exist. - when: source_interface not in ansible_interfaces + when: source_interface not in ansible_facts.interfaces ### Firstly, some fact gathering. # Start off by assuming the source interface is direct, unless proven diff --git a/ansible/roles/ironic-enrolment/tasks/port.yml b/ansible/roles/ironic-enrolment/tasks/port.yml index 8377a48..e801a01 100644 --- a/ansible/roles/ironic-enrolment/tasks/port.yml +++ b/ansible/roles/ironic-enrolment/tasks/port.yml @@ -32,7 +32,7 @@ - name: Set Ironic port attributes vars: port_attributes: "{{ port_attributes_output.stdout | from_json }}" - switch_id: "{{ hostvars[ironic_hypervisor]['ansible_' + bridge].macaddress }}" + switch_id: "{{ hostvars[ironic_hypervisor].ansible_facts[bridge].macaddress }}" switch_info: "{{ bridge }}" port_id: >- {{ source_interface diff --git a/ansible/roles/virtualbmc-daemon/tasks/main.yml b/ansible/roles/virtualbmc-daemon/tasks/main.yml index 343294c..7eb8550 100644 --- a/ansible/roles/virtualbmc-daemon/tasks/main.yml +++ b/ansible/roles/virtualbmc-daemon/tasks/main.yml @@ -3,9 +3,9 @@ include_vars: "{{ item }}" with_first_found: - files: - - "{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml" - - "{{ ansible_distribution }}.yml" - - "{{ ansible_os_family }}.yml" + - "{{ ansible_facts.distribution }}-{{ ansible_facts.distribution_major_version }}.yml" + - "{{ ansible_facts.distribution }}.yml" + - "{{ ansible_facts.os_family }}.yml" skip: true tags: vars @@ -29,7 +29,7 @@ - name: Ensure Python requirements are installed pip: name: - - "virtualbmc>=1.4.0{% if ansible_python.version.major == 2 %},<2{% endif %}" + - "virtualbmc>=1.4.0{% if ansible_facts.python.version.major == 2 %},<2{% endif %}" # NOTE(priteau): Ignore PyYAML when installing system-wide to avoid the # following error: Cannot uninstall 'PyYAML'. It is a distutils installed # project and thus we cannot accurately determine which files belong to it diff --git a/playbooks/openvswitch.yml b/playbooks/openvswitch.yml index 117344f..3f3605a 100644 --- a/playbooks/openvswitch.yml +++ b/playbooks/openvswitch.yml @@ -29,8 +29,8 @@ include_role: name: fkautz.openvswitch-install when: - - ansible_os_family == "RedHat" - - ansible_distribution_major_version is version(8, '<') + - ansible_facts.os_family == "RedHat" + - ansible_facts.distribution_major_version is version(8, '<') - block: - name: Install the Delorean repositories @@ -50,8 +50,8 @@ name: openvswitch state: started when: - - ansible_os_family == "RedHat" - - ansible_distribution_major_version is version(8, '>=') + - ansible_facts.os_family == "RedHat" + - ansible_facts.distribution_major_version is version(8, '>=') - block: - name: Install packages @@ -68,5 +68,5 @@ service: name: openvswitch-switch state: started - when: ansible_os_family == "Debian" + when: ansible_facts.os_family == "Debian" diff --git a/playbooks/tenks-deploy-teardown/pre.yml b/playbooks/tenks-deploy-teardown/pre.yml index 1632fa0..d0d472b 100644 --- a/playbooks/tenks-deploy-teardown/pre.yml +++ b/playbooks/tenks-deploy-teardown/pre.yml @@ -23,13 +23,25 @@ become: true package: name: python3 - when: ansible_python.version.major == 3 + when: ansible_facts.python.version.major == 3 - name: Create virtualenv for tenks pip: requirements: "{{ tenks_src_dir }}/requirements.txt" virtualenv: "{{ tenks_venv }}" - virtualenv_python: "{{ ansible_python.executable }}" + virtualenv_python: "{{ ansible_facts.python.executable }}" + + - name: Ensure /etc/ansible exists + file: + path: /etc/ansible + state: directory + become: true + + - name: Template ansible.cfg + template: + src: "ansible.cfg.j2" + dest: /etc/ansible/ansible.cfg + become: true - name: Template requirements overrides template: diff --git a/playbooks/tenks-deploy-teardown/templates/ansible.cfg.j2 b/playbooks/tenks-deploy-teardown/templates/ansible.cfg.j2 new file mode 100644 index 0000000..35ac710 --- /dev/null +++ b/playbooks/tenks-deploy-teardown/templates/ansible.cfg.j2 @@ -0,0 +1,7 @@ +[defaults] +# Ensure that facts are referenced via ansible_facts.. +inject_facts_as_vars = False + +[ssh_connection] +pipelining = True +retries = 3 diff --git a/playbooks/tenks-deploy-teardown/templates/tenks-overrides.yml.j2 b/playbooks/tenks-deploy-teardown/templates/tenks-overrides.yml.j2 index 7e9dd4c..c32c1dc 100644 --- a/playbooks/tenks-deploy-teardown/templates/tenks-overrides.yml.j2 +++ b/playbooks/tenks-deploy-teardown/templates/tenks-overrides.yml.j2 @@ -1,6 +1,6 @@ --- # Use python3 on Ubuntu remote hosts. -ansible_python_interpreter: "{{ '/usr/bin/python3' if ansible_python.version.major == 3 else '/usr/bin/python2' }}" +ansible_python_interpreter: "{{ '/usr/bin/python3' if ansible_facts.python.version.major == 3 else '/usr/bin/python2' }}" # This file holds the config given to Tenks when running the zuul job, # tenks-deploy-teardown. It assumes the existence of the bridge `breth1`. diff --git a/playbooks/tenks-deploy-teardown/vars/common.yml b/playbooks/tenks-deploy-teardown/vars/common.yml index 84fbf3f..d05a365 100644 --- a/playbooks/tenks-deploy-teardown/vars/common.yml +++ b/playbooks/tenks-deploy-teardown/vars/common.yml @@ -2,14 +2,14 @@ # Variables shared between the playbooks -tenks_src_dir: "{{ ansible_env.HOME ~ '/' ~ zuul.projects['opendev.org/openstack/tenks'].src_dir }}" +tenks_src_dir: "{{ ansible_facts.env.HOME ~ '/' ~ zuul.projects['opendev.org/openstack/tenks'].src_dir }}" stackhpc_libvirt_host_src_dir: >- - {{ ansible_env.HOME ~ '/' ~ zuul.projects['github.com/stackhpc/ansible-role-libvirt-host'].src_dir }} + {{ ansible_facts.env.HOME ~ '/' ~ zuul.projects['github.com/stackhpc/ansible-role-libvirt-host'].src_dir }} stackhpc_libvirt_vm_src_dir: >- - {{ ansible_env.HOME ~ '/' ~ zuul.projects['github.com/stackhpc/ansible-role-libvirt-vm'].src_dir }} -upper_constraints_path: "{{ ansible_env.HOME ~ '/' ~ zuul.projects['opendev.org/openstack/requirements'].src_dir ~ '/upper-constraints.txt' }}" -tenks_venv: "{{ ansible_env.HOME ~ '/' ~ 'venv-tenks' }}" -config_dir: "{{ ansible_env.HOME ~ '/' ~ 'tenks-config' }}" + {{ ansible_facts.env.HOME ~ '/' ~ zuul.projects['github.com/stackhpc/ansible-role-libvirt-vm'].src_dir }} +upper_constraints_path: "{{ ansible_facts.env.HOME ~ '/' ~ zuul.projects['opendev.org/openstack/requirements'].src_dir ~ '/upper-constraints.txt' }}" +tenks_venv: "{{ ansible_facts.env.HOME ~ '/' ~ 'venv-tenks' }}" +config_dir: "{{ ansible_facts.env.HOME ~ '/' ~ 'tenks-config' }}" tenks_overrides_path: "{{ config_dir ~ '/' ~ 'tenks-overrides.yml' }}" tenks_requirements_overrides_path: "{{ tenks_src_dir }}/requirements-overrides.yml" logs_dir: "/tmp/logs" diff --git a/roles/tenks-ci-prep/tasks/main.yml b/roles/tenks-ci-prep/tasks/main.yml index c425596..70dceae 100644 --- a/roles/tenks-ci-prep/tasks/main.yml +++ b/roles/tenks-ci-prep/tasks/main.yml @@ -10,7 +10,7 @@ - name: Enable the EPEL yum repository command: yum-config-manager --enable epel - when: ansible_os_family == 'RedHat' + when: ansible_facts.os_family == 'RedHat' become: true - name: Install Python3 modules @@ -20,4 +20,4 @@ - python3-pip - python3-setuptools - python3-wheel - - "{% if ansible_os_family == 'Debian' %}virtualenv{% else %}python3-virtualenv{% endif %}" + - "{% if ansible_facts.os_family == 'Debian' %}virtualenv{% else %}python3-virtualenv{% endif %}"