From 904dea11400ace95dc4370b978e1adc76f62bdc4 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Thu, 19 Mar 2026 08:01:28 -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. Signed-off-by: Rich Megginson --- tests/tasks/run_role_with_clear_facts.yml | 37 +++++++++++++++++++ tests/tests_basic_join.yml | 4 +- tests/tests_default.yml | 4 +- tests/tests_dyndns.yml | 9 ++--- tests/tests_full_integration.yml | 6 +-- tests/tests_full_integration_dc.yml | 6 +-- tests/tests_full_integration_dyndns.yml | 8 +--- tests/tests_full_integration_force_rejoin.yml | 9 ++--- tests/tests_include_vars_from_parent.yml | 1 - tests/tests_manage_packages.yml | 3 +- tests/tests_migrate_sssd_settings.yml | 4 +- tests/tests_preserve_authselect.yml | 4 +- tests/tests_sssd_custom_settings.yml | 10 ++--- tests/tests_sssd_settings.yml | 10 ++--- 14 files changed, 65 insertions(+), 50 deletions(-) create mode 100644 tests/tasks/run_role_with_clear_facts.yml 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..7f220a16 --- /dev/null +++ b/tests/tasks/run_role_with_clear_facts.yml @@ -0,0 +1,37 @@ +--- +# Task file: clear_facts, run linux-system-roles.ad_integration +# 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.ad_integration + 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.ad_integration + tasks_from: "{{ __sr_tasks_from | default('main') }}" + public: "{{ __sr_public | default(false) }}" + when: __sr_failed_when | d(true) diff --git a/tests/tests_basic_join.yml b/tests/tests_basic_join.yml index 47cf1e15..a6115377 100644 --- a/tests/tests_basic_join.yml +++ b/tests/tests_basic_join.yml @@ -2,7 +2,6 @@ --- - name: Ensure that the role runs with mandatory parameters populated hosts: all,!ad - gather_facts: false # test that role works in this case vars: # sample realm that will skip joining realm step ad_integration_realm: "{{ __ad_integration_sample_realm }}" @@ -14,8 +13,7 @@ __ad_integration_sample_realm: MiXeD.cAsE - name: Test - Run the system role - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml - name: Test - Check packages are present block: diff --git a/tests/tests_default.yml b/tests/tests_default.yml index 418b9f04..d6f1bbd3 100644 --- a/tests/tests_default.yml +++ b/tests/tests_default.yml @@ -2,13 +2,11 @@ --- - name: Ensure role behaviour with default parameters hosts: all,!ad - gather_facts: false # test that role works in this case tasks: - name: Verify that the role fails without ad_integration_realm variable block: - name: Include the role - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml rescue: - name: Assert that user is notified about missing variables diff --git a/tests/tests_dyndns.yml b/tests/tests_dyndns.yml index 38d76f3e..ce41425f 100644 --- a/tests/tests_dyndns.yml +++ b/tests/tests_dyndns.yml @@ -19,7 +19,6 @@ - name: Ensure that the role configures dynamic dns hosts: all,!ad - gather_facts: true vars: # if we don't have a real AD server, just verify the config # file is written properly @@ -38,10 +37,9 @@ when: __ad_integration_test_sssd_config_only - name: Test - Run the system role with bogus vars - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true ad_dyndns_iface: TESTING ad_dyndns_server: 127.0.0.1 ad_dyndns_auth: GSS-TSIG @@ -82,8 +80,7 @@ when: sssd_log.stdout | length > 0 - name: Test - Re-run the system role to remove vars - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_dyndns_iface: null ad_dyndns_server: null diff --git a/tests/tests_full_integration.yml b/tests/tests_full_integration.yml index 29953180..ce786872 100644 --- a/tests/tests_full_integration.yml +++ b/tests/tests_full_integration.yml @@ -62,9 +62,9 @@ block: - name: Test run the system role - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true register: role_run - name: Debug role_run diff --git a/tests/tests_full_integration_dc.yml b/tests/tests_full_integration_dc.yml index cfc1c6ff..d6cf607b 100644 --- a/tests/tests_full_integration_dc.yml +++ b/tests/tests_full_integration_dc.yml @@ -69,9 +69,9 @@ block: - name: Test run the system role - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true register: role_run - name: Debug role_run diff --git a/tests/tests_full_integration_dyndns.yml b/tests/tests_full_integration_dyndns.yml index 46e4bfbd..159af0e0 100644 --- a/tests/tests_full_integration_dyndns.yml +++ b/tests/tests_full_integration_dyndns.yml @@ -30,7 +30,6 @@ - name: Configure DynDNS on AD hosts: ad - gather_facts: true tasks: - name: Get AD address set_fact: @@ -62,7 +61,6 @@ - name: Ensure that the role configures dynamic dns hosts: client - gather_facts: true tasks: - name: Run the integration when: "'ad' in groups and groups['ad']" @@ -73,10 +71,9 @@ Interface {{ _ad_dyndns_iface | default(ansible_facts['default_ipv4']['interface']) }} and server : {{ hostvars[groups['ad'][0]].ansible_host }} - name: Run the system role with proper config - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml vars: + __sr_public: true ad_dyndns_server: "{{ hostvars[groups['ad'][0]].ansible_host }}" ad_dyndns_iface: "{{ _ad_dyndns_iface | default(ansible_facts['default_ipv4']['interface']) }}" ad_dyndns_auth: "none" @@ -126,7 +123,6 @@ - name: Check hosts on AD hosts: ad - gather_facts: true tasks: - name: Get network for zone set_fact: diff --git a/tests/tests_full_integration_force_rejoin.yml b/tests/tests_full_integration_force_rejoin.yml index b8aaae78..a79ca9c0 100644 --- a/tests/tests_full_integration_force_rejoin.yml +++ b/tests/tests_full_integration_force_rejoin.yml @@ -65,9 +65,9 @@ block: - name: Test run the system role - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true register: role_run - name: Debug role_run @@ -98,8 +98,7 @@ changed_when: false - name: Test run the system role with force rejoin - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_integration_force_rejoin: true register: role_run_rejoin diff --git a/tests/tests_include_vars_from_parent.yml b/tests/tests_include_vars_from_parent.yml index c25c76ea..0f24f633 100644 --- a/tests/tests_include_vars_from_parent.yml +++ b/tests/tests_include_vars_from_parent.yml @@ -1,7 +1,6 @@ --- - name: Test role inclusion variable overrides hosts: all,!ad - gather_facts: true tasks: - name: Create var file in caller that can override the one in called role delegate_to: localhost diff --git a/tests/tests_manage_packages.yml b/tests/tests_manage_packages.yml index 0cba75e4..0303f9ed 100644 --- a/tests/tests_manage_packages.yml +++ b/tests/tests_manage_packages.yml @@ -30,8 +30,7 @@ # role will fail if ad_integration_manage_packages is not handled correctly - name: Run the role with ad_integration_manage_packages - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_integration_manage_packages: false diff --git a/tests/tests_migrate_sssd_settings.yml b/tests/tests_migrate_sssd_settings.yml index 1bb7b422..85ce3d02 100644 --- a/tests/tests_migrate_sssd_settings.yml +++ b/tests/tests_migrate_sssd_settings.yml @@ -2,7 +2,6 @@ --- - name: Ensure that the role migrates sssd domain settings depending on case hosts: all,!ad - gather_facts: true vars: ad_integration_realm: MIGRATE.REALM.COM __ad_integration_test_sssd_config_only: true @@ -58,8 +57,7 @@ __ad_integration_realm_list: "domain-name: {{ ad_integration_realm }}" - name: Test - Run the system role merging duplicate sssd domain sections - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_integration_sssd_custom_settings: "{{ __ad_integration_sssd_custom_settings + __ad_integration_sssd_custom_settings2 + __ad_integration_sssd_custom_settings3 }}" diff --git a/tests/tests_preserve_authselect.yml b/tests/tests_preserve_authselect.yml index 2dbfbf29..7291f886 100644 --- a/tests/tests_preserve_authselect.yml +++ b/tests/tests_preserve_authselect.yml @@ -2,7 +2,6 @@ --- - name: Ensure that the role ad_integration_preserve_authselect_profile works hosts: all,!ad - gather_facts: false # test that role works in this case vars: # sample realm that will skip joining realm step ad_integration_realm: "{{ __ad_integration_sample_realm }}" @@ -11,8 +10,7 @@ tasks: - name: Test - Run the system role - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml - name: Test - Check that realmd config is present stat: diff --git a/tests/tests_sssd_custom_settings.yml b/tests/tests_sssd_custom_settings.yml index 041e2cdf..3fbfff5f 100644 --- a/tests/tests_sssd_custom_settings.yml +++ b/tests/tests_sssd_custom_settings.yml @@ -3,7 +3,6 @@ - name: Ensure that the role configures dynamic dns hosts: all,!ad - gather_facts: true vars: ad_integration_realm: sssd-custom-sample-realm.com __ad_integration_test_sssd_config_only: true @@ -22,9 +21,9 @@ __manage_fake_realm: setup - name: Test - Run the system role with bogus vars - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Check custom SSSD settings community.general.ini_file: @@ -59,8 +58,7 @@ map('combine', {'state': 'absent'}) | list }}" - name: Test - Re-run the system role to remove vars - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_integration_sssd_custom_settings: "{{ update_list }}" diff --git a/tests/tests_sssd_settings.yml b/tests/tests_sssd_settings.yml index dc936f04..8d8da299 100644 --- a/tests/tests_sssd_settings.yml +++ b/tests/tests_sssd_settings.yml @@ -3,7 +3,6 @@ - name: Ensure that the role configures dynamic dns hosts: all,!ad - gather_facts: false # test that role works in this case vars: ad_integration_realm: sample-realm.com __ad_integration_test_sssd_config_only: true @@ -15,9 +14,9 @@ tasks: - name: Test - Run the system role with bogus vars - include_role: - name: linux-system-roles.ad_integration - public: true + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_public: true - name: Check SSSD settings community.general.ini_file: @@ -52,8 +51,7 @@ map('combine', {'state': 'absent'}) | list }}" - name: Test - Re-run the system role to remove vars - include_role: - name: linux-system-roles.ad_integration + include_tasks: tasks/run_role_with_clear_facts.yml vars: ad_integration_sssd_settings: "{{ update_list }}"