From 817efe6e858dba8d615d8aa71648ddb5abd53d44 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Fri, 20 Mar 2026 07:21:09 -0600 Subject: [PATCH] test: ensure role gathers the facts it uses by having test clear_facts before include_role The role gathers the facts it uses. For example, if the user uses `ANSIBLE_GATHERING=explicit`, the role uses the `setup` module with the facts and subsets it requires. This change allows us to test this. Before every role invocation, the test will use `meta: clear_facts` so that the role starts with no facts. Create a task file tests/tasks/run_role_with_clear_facts.yml to do the tasks to clear the facts and run the role. Note that this means we don't need to use `gather_facts` for the tests. Some vars defined using `ansible_facts` have been changed to be defined with `set_fact` instead. This is because of the fact that `vars` are lazily evaluated - the var might be referenced when the facts have been cleared, and will issue an error like `ansible_facts["distribution"] is undefined`. This is typically done for blocks that have a `when` condition that uses `ansible_facts` and the block has a role invocation using run_role_with_clear_facts.yml These have been rewritten to define the `when` condition using `set_fact`. This is because the `when` condition is evaluated every time a task is invoked in the block, and if the facts are cleared, this will raise an undefined variable error. Ensure handlers are flushed so that services are in the correct state, restarted if necessary, and wait a couple of seconds for them to start. This allows the checks to pass consistently instead of potentially checking a service which is in an indeterminate state. This also clears up an issue on some platforms where a service triggered multiple times at the end of a test because the test should have flushed the handlers and restarted the services during the test. Signed-off-by: Rich Megginson --- tests/tasks/cleanup.yml | 3 +- tests/tasks/run_role_with_clear_facts.yml | 37 ++++++++++++++++++++ tests/tasks/setup.yml | 6 ++++ tests/tests_chrony.yml | 6 ++-- tests/tests_default.yml | 6 ++-- tests/tests_default_vars.yml | 6 ++-- tests/tests_ntp.yml | 41 +++++++++++------------ tests/tests_ntp_provider1.yml | 9 +++-- tests/tests_ntp_provider2.yml | 9 +++-- tests/tests_ntp_provider3.yml | 9 +++-- tests/tests_ntp_provider4.yml | 9 +++-- tests/tests_ntp_provider5.yml | 9 +++-- tests/tests_ntp_provider6.yml | 14 ++++---- tests/tests_ntp_ptp.yml | 20 +++++------ tests/tests_options.yml | 19 +++++------ tests/tests_ptp_multi.yml | 25 +++++++------- tests/tests_ptp_single.yml | 23 ++++++------- 17 files changed, 155 insertions(+), 96 deletions(-) create mode 100644 tests/tasks/run_role_with_clear_facts.yml diff --git a/tests/tasks/cleanup.yml b/tests/tasks/cleanup.yml index fc199c3f..48fdd7aa 100644 --- a/tests/tasks/cleanup.yml +++ b/tests/tasks/cleanup.yml @@ -1,7 +1,6 @@ --- - name: Reset settings and provider to os default - include_role: - name: linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml vars: timesync_ntp_provider: "{{ timesync_ntp_provider_os_default }}" timesync_ptp_domains: [] diff --git a/tests/tasks/run_role_with_clear_facts.yml b/tests/tasks/run_role_with_clear_facts.yml new file mode 100644 index 00000000..07f72e5e --- /dev/null +++ b/tests/tasks/run_role_with_clear_facts.yml @@ -0,0 +1,37 @@ +--- +# Task file: clear_facts, run linux-system-roles.timesync. +# Include this with include_tasks or import_tasks +# Input: +# - __sr_tasks_from: tasks_from to run - same as tasks_from in include_role +# - __sr_public: export private vars from role - same as public in include_role +# - __sr_failed_when: set to false to ignore role errors - same as failed_when in include_role +- name: Clear facts + meta: clear_facts + +# note that you can use failed_when with import_role but not with include_role +# so this simulates the __sr_failed_when false case +# Q: Why do we need a separate task to run the role normally? Why not just +# run the role in the block and rethrow the error in the rescue block? +# A: Because you cannot rethrow the error in exactly the same way as the role does. +# It might be possible to exactly reconstruct ansible_failed_result but it's not worth the effort. +- name: Run the role with __sr_failed_when false + when: + - __sr_failed_when is defined + - not __sr_failed_when + block: + - name: Run the role + include_role: + name: linux-system-roles.timesync + tasks_from: "{{ __sr_tasks_from | default('main') }}" + public: "{{ __sr_public | default(false) }}" + rescue: + - name: Ignore the failure when __sr_failed_when is false + debug: + msg: Ignoring failure when __sr_failed_when is false + +- name: Run the role normally + include_role: + name: linux-system-roles.timesync + tasks_from: "{{ __sr_tasks_from | default('main') }}" + public: "{{ __sr_public | default(false) }}" + when: __sr_failed_when | d(true) diff --git a/tests/tasks/setup.yml b/tests/tasks/setup.yml index c28be317..7202a750 100644 --- a/tests/tasks/setup.yml +++ b/tests/tasks/setup.yml @@ -28,6 +28,12 @@ __required_facts: "{{ __timesync_required_facts + ['default_ipv4'] }}" __required_facts_subsets: "{{ ['!all', '!min'] + __required_facts }}" +# This var is used in several tests and must be a fact not a var due +# to the clear_facts task. +- name: Set var for default_ipv4 interface fact + set_fact: + __timesync_default_ipv4_interface: "{{ ansible_facts['default_ipv4']['interface'] }}" + - name: Debug debug: msg: facts {{ ansible_facts | to_nice_json }} diff --git a/tests/tests_chrony.yml b/tests/tests_chrony.yml index c3a51923..4fefd239 100644 --- a/tests/tests_chrony.yml +++ b/tests/tests_chrony.yml @@ -22,9 +22,9 @@ tasks: - name: Run the role - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true when: not __bootc_validation | d(false) - name: Run the role only to get vars needed for validation diff --git a/tests/tests_default.yml b/tests/tests_default.yml index 1848014a..a28b8638 100644 --- a/tests/tests_default.yml +++ b/tests/tests_default.yml @@ -1,6 +1,6 @@ --- - name: Ensure that the role runs with default parameters hosts: all - gather_facts: false - roles: - - linux-system-roles.timesync + tasks: + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml diff --git a/tests/tests_default_vars.yml b/tests/tests_default_vars.yml index 1c22edb6..543a5d7c 100644 --- a/tests/tests_default_vars.yml +++ b/tests/tests_default_vars.yml @@ -3,9 +3,9 @@ hosts: all tasks: - name: Include the timesync role - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Check that public vars are defined assert: diff --git a/tests/tests_ntp.yml b/tests/tests_ntp.yml index 62b21cc9..b25a2da6 100644 --- a/tests/tests_ntp.yml +++ b/tests/tests_ntp.yml @@ -24,35 +24,34 @@ timesync_ntp_hwts_interfaces: ["*"] tasks: - name: Run the role - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Run test tags: tests::verify block: - - name: Flush handlers - meta: flush_handlers + - name: Ensure services are running properly if booted when: __timesync_is_booted + block: + - name: Flush handlers + meta: flush_handlers - - name: Wait for services to start - wait_for: - timeout: 2 - when: __timesync_is_booted + - name: Wait for services to start + wait_for: + timeout: 2 - - name: Get list of currently used time sources - shell: chronyc -n sources || ntpq -pn - register: sources - changed_when: false - when: __timesync_is_booted + - name: Get list of currently used time sources + shell: chronyc -n sources || ntpq -pn + register: sources + changed_when: false - - name: Check time sources - assert: - that: - - "'172.16.123.1' in sources.stdout" - - "'172.16.123.2' in sources.stdout" - - "'172.16.123.3' in sources.stdout" - when: __timesync_is_booted + - name: Check time sources + assert: + that: + - "'172.16.123.1' in sources.stdout" + - "'172.16.123.2' in sources.stdout" + - "'172.16.123.3' in sources.stdout" - name: Fetch chrony.conf file slurp: diff --git a/tests/tests_ntp_provider1.yml b/tests/tests_ntp_provider1.yml index 2d74bf2e..8c1cc957 100644 --- a/tests/tests_ntp_provider1.yml +++ b/tests/tests_ntp_provider1.yml @@ -4,8 +4,6 @@ vars: timesync_ntp_servers: - hostname: 172.16.123.1 - roles: - - linux-system-roles.timesync pre_tasks: - name: Common test setup tasks @@ -25,11 +23,18 @@ - chrony - ntp + tasks: + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml + post_tasks: - name: Verify provider is working tags: tests::verify when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_provider2.yml b/tests/tests_ntp_provider2.yml index cd174c9b..ead59e5b 100644 --- a/tests/tests_ntp_provider2.yml +++ b/tests/tests_ntp_provider2.yml @@ -4,8 +4,6 @@ vars: timesync_ntp_servers: - hostname: 172.16.123.1 - roles: - - linux-system-roles.timesync pre_tasks: - name: Common test setup tasks @@ -39,11 +37,18 @@ state: "{{ 'started' if __timesync_is_booted else omit }}" enabled: true + tasks: + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml + post_tasks: - name: Verify chronyd running tags: tests::verify when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_provider3.yml b/tests/tests_ntp_provider3.yml index 71bd3676..d9eb6e3f 100644 --- a/tests/tests_ntp_provider3.yml +++ b/tests/tests_ntp_provider3.yml @@ -4,8 +4,6 @@ vars: timesync_ntp_servers: - hostname: 172.16.123.1 - roles: - - linux-system-roles.timesync pre_tasks: - name: Common test setup tasks @@ -39,11 +37,18 @@ state: "{{ 'started' if __timesync_is_booted else omit }}" enabled: true + tasks: + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml + post_tasks: - name: Verify ntpd is running tags: tests::verify when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_provider4.yml b/tests/tests_ntp_provider4.yml index a0df3e03..dddc1d30 100644 --- a/tests/tests_ntp_provider4.yml +++ b/tests/tests_ntp_provider4.yml @@ -34,14 +34,17 @@ - name: Run the test block: - name: Run the role - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Verify chronyd is running tags: tests::verify when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_provider5.yml b/tests/tests_ntp_provider5.yml index 37ce7b90..314f3750 100644 --- a/tests/tests_ntp_provider5.yml +++ b/tests/tests_ntp_provider5.yml @@ -34,14 +34,17 @@ - name: Run test block: - name: Run the role - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Verify ntpd service tags: tests::verify when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_provider6.yml b/tests/tests_ntp_provider6.yml index dc11ec26..7b5ab014 100644 --- a/tests/tests_ntp_provider6.yml +++ b/tests/tests_ntp_provider6.yml @@ -3,7 +3,6 @@ Configure NTP with OS release non-default provider and then change it to the default provider hosts: all - gather_facts: true vars: is_ntp_default: "{{ ansible_facts['distribution'] in ['RedHat', 'CentOS'] and ansible_facts['distribution_version'] is version('7.0', '<') }}" @@ -52,10 +51,9 @@ - ntp - name: Call role to change provider - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true # ntp is the default choice for RedHat and CentOS # version < 7.0 - reverse it timesync_ntp_provider: "{{ 'chrony' if is_ntp_default else 'ntp' }}" @@ -87,14 +85,18 @@ changed_when: false - name: Call role to reset default provider - include_role: - name: linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true timesync_ntp_provider: "{{ timesync_ntp_provider_os_default }}" - name: Verify provider set correctly - 2 tags: tests::verify + when: __timesync_is_booted block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ntp_ptp.yml b/tests/tests_ntp_ptp.yml index 4c94fd71..e0a36d23 100644 --- a/tests/tests_ntp_ptp.yml +++ b/tests/tests_ntp_ptp.yml @@ -1,7 +1,6 @@ --- - name: Configure time synchronization with NTP servers and PTP domains hosts: all - gather_facts: true vars: timesync_ntp_servers: - hostname: 172.16.123.1 @@ -12,10 +11,10 @@ maxpoll: 2 timesync_ptp_domains: - number: 0 - interfaces: "{{ [ ansible_facts['default_ipv4']['interface'] ] }}" + interfaces: "{{ [ __timesync_default_ipv4_interface ] }}" delay: 0.0001 - number: 1 - interfaces: "{{ [ ansible_facts['default_ipv4']['interface'] ] }}" + interfaces: "{{ [ __timesync_default_ipv4_interface ] }}" delay: 0.0001 timesync_step_threshold: 0.001 timesync_dhcp_ntp_servers: false @@ -28,13 +27,9 @@ include_tasks: tasks/setup.yml - name: Run role - include_role: - name: linux-system-roles.timesync - public: true - - - name: Flush handlers - meta: flush_handlers - when: __timesync_is_booted + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Ensure ethtool is installed package: @@ -44,7 +39,7 @@ ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Check if SW/HW timestamping is supported - command: ethtool -T {{ ansible_facts['default_ipv4']['interface'] | quote }} + command: ethtool -T {{ __timesync_default_ipv4_interface | quote }} register: ethtool ignore_errors: true # noqa ignore-errors changed_when: false @@ -54,6 +49,9 @@ - __timesync_is_booted - "'ware-transmit' in ethtool.stdout" block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_options.yml b/tests/tests_options.yml index eb2dab11..a2e4027c 100644 --- a/tests/tests_options.yml +++ b/tests/tests_options.yml @@ -1,7 +1,6 @@ --- - name: Test setting OPTIONS hosts: all - gather_facts: true tasks: - name: Run tests block: @@ -34,9 +33,9 @@ register: __timesync_config_before - name: Run role with no arguments to get provider - include_role: - name: linux-system-roles.timesync - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Get OPTIONS after running command: grep ^OPTIONS= {{ timesync_chrony_sysconfig_path }} @@ -54,9 +53,9 @@ that: __timesync_config_before.stdout == __timesync_config_after.stdout - name: Try timesync_ntp_ip_family IPv4 - include_role: - name: linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true timesync_ntp_ip_family: IPv4 - name: Verify IPv4 setting @@ -64,9 +63,9 @@ changed_when: false - name: Try timesync_ntp_ip_family IPv6 - include_role: - name: linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true timesync_ntp_ip_family: IPv6 - name: Verify IPv6 setting @@ -75,9 +74,9 @@ always: - name: Reset OPTIONS - include_role: - name: linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true timesync_ntp_ip_family: all - name: Verify reset diff --git a/tests/tests_ptp_multi.yml b/tests/tests_ptp_multi.yml index f7666eff..d7317a1c 100644 --- a/tests/tests_ptp_multi.yml +++ b/tests/tests_ptp_multi.yml @@ -1,34 +1,30 @@ --- - name: Configure time synchronization with multiple PTP domains hosts: all - gather_facts: true vars: timesync_ptp_domains: - number: 0 - interfaces: "{{ [ ansible_facts['default_ipv4']['interface'] ] }}" + interfaces: "{{ [ __timesync_default_ipv4_interface ] }}" transport: L2 - number: 1 - interfaces: "{{ [ ansible_facts['default_ipv4']['interface'] ] }}" + interfaces: "{{ [ __timesync_default_ipv4_interface ] }}" delay: 0.001 transport: UDPv4 hybrid_e2e: true udp_ttl: 2 timesync_step_threshold: 0.0001 timesync_min_sources: 2 - roles: - - linux-system-roles.timesync tasks: + - name: Common test setup tasks + include_tasks: tasks/setup.yml + + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml + - name: Run test tags: tests::verify block: - - name: Common test setup tasks - include_tasks: tasks/setup.yml - - - name: Flush handlers - meta: flush_handlers - when: __timesync_is_booted - - name: Ensure ethtool is installed package: name: ethtool @@ -37,7 +33,7 @@ ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Check if SW/HW timestamping is supported - command: ethtool -T {{ ansible_facts['default_ipv4']['interface'] | quote }} + command: ethtool -T {{ __timesync_default_ipv4_interface | quote }} register: ethtool ignore_errors: true # noqa ignore-errors changed_when: false @@ -47,6 +43,9 @@ - __timesync_is_booted - "'ware-transmit' in ethtool.stdout" block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2 diff --git a/tests/tests_ptp_single.yml b/tests/tests_ptp_single.yml index 02ed80dc..5e417f42 100644 --- a/tests/tests_ptp_single.yml +++ b/tests/tests_ptp_single.yml @@ -1,25 +1,21 @@ --- - name: Configure time synchronization with single PTP domain hosts: all - gather_facts: true vars: timesync_ptp_domains: - number: 3 - interfaces: "{{ [ ansible_facts['default_ipv4']['interface'] ] }}" - roles: - - linux-system-roles.timesync + interfaces: "{{ [ __timesync_default_ipv4_interface ] }}" tasks: + - name: Common test setup tasks + include_tasks: tasks/setup.yml + + - name: Run linux-system-roles.timesync + include_tasks: tasks/run_role_with_clear_facts.yml + - name: Run test tags: tests::verify block: - - name: Common test setup tasks - include_tasks: tasks/setup.yml - - - name: Flush handlers - meta: flush_handlers - when: __timesync_is_booted - - name: Ensure ethtool is installed package: name: ethtool @@ -28,7 +24,7 @@ ternary('ansible.posix.rhel_rpm_ostree', omit) }}" - name: Check if SW/HW timestamping is supported - command: ethtool -T {{ ansible_facts['default_ipv4']['interface'] | quote }} + command: ethtool -T {{ __timesync_default_ipv4_interface | quote }} register: ethtool ignore_errors: true # noqa ignore-errors changed_when: false @@ -38,6 +34,9 @@ - __timesync_is_booted - "'ware-transmit' in ethtool.stdout" block: + - name: Flush handlers + meta: flush_handlers + - name: Wait for services to start wait_for: timeout: 2