Skip to content

Commit ed6cfac

Browse files
committed
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 <rmeggins@redhat.com>
1 parent f10840d commit ed6cfac

8 files changed

Lines changed: 54 additions & 33 deletions
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
# Task file: clear_facts, run linux-system-roles.crypto_policies.
3+
# Include this with include_tasks or import_tasks
4+
# Input:
5+
# - __sr_tasks_from: tasks_from to run - same as tasks_from in include_role
6+
# - __sr_public: export private vars from role - same as public in include_role
7+
# - __sr_failed_when: set to false to ignore role errors - same as failed_when in include_role
8+
- name: Clear facts
9+
meta: clear_facts
10+
11+
# note that you can use failed_when with import_role but not with include_role
12+
# so this simulates the __sr_failed_when false case
13+
# Q: Why do we need a separate task to run the role normally? Why not just
14+
# run the role in the block and rethrow the error in the rescue block?
15+
# A: Because you cannot rethrow the error in exactly the same way as the role does.
16+
# It might be possible to exactly reconstruct ansible_failed_result but it's not worth the effort.
17+
- name: Run the role with __sr_failed_when false
18+
when:
19+
- __sr_failed_when is defined
20+
- not __sr_failed_when
21+
block:
22+
- name: Run the role
23+
include_role:
24+
name: linux-system-roles.crypto_policies
25+
tasks_from: "{{ __sr_tasks_from | default('main') }}"
26+
public: "{{ __sr_public | default(false) }}"
27+
rescue:
28+
- name: Ignore the failure when __sr_failed_when is false
29+
debug:
30+
msg: Ignoring failure when __sr_failed_when is false
31+
32+
- name: Run the role normally
33+
include_role:
34+
name: linux-system-roles.crypto_policies
35+
tasks_from: "{{ __sr_tasks_from | default('main') }}"
36+
public: "{{ __sr_public | default(false) }}"
37+
when: __sr_failed_when | d(true)

tests/tests_bootc_e2e.yml

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,13 @@
44
hosts: all
55
tags:
66
- tests::bootc-e2e
7-
gather_facts: false
87

98
tasks:
109
- name: Bootc image build preparation
1110
when: ansible_connection | d("") == "buildah"
1211
block:
1312
- name: Change crypto policy to FUTURE
14-
include_role:
15-
name: linux-system-roles.crypto_policies
13+
include_tasks: tasks/run_role_with_clear_facts.yml
1614
vars:
1715
crypto_policies_policy: FUTURE
1816

@@ -25,8 +23,7 @@
2523
- name: Restore policy to DEFAULT
2624
tags:
2725
- tests::cleanup
28-
include_role:
29-
name: linux-system-roles.crypto_policies
26+
include_tasks: tasks/run_role_with_clear_facts.yml
3027
vars:
3128
crypto_policies_policy: DEFAULT
3229

tests/tests_default.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22
---
33
- name: Ensure that the role runs with default parameters
44
hosts: all
5-
roles:
6-
- linux-system-roles.crypto_policies
7-
gather_facts: false
85
tasks:
6+
- name: Run linux-system-roles.crypto_policies
7+
include_tasks: tasks/run_role_with_clear_facts.yml
8+
99
- name: Verify the facts are correctly set
1010
block:
1111
- name: Flush handlers

tests/tests_include_vars_from_parent.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
---
22
- name: Test role include variable override
33
hosts: all
4-
gather_facts: true
54
tasks:
65
- name: Create var file in caller that can override the one in called role
76
delegate_to: localhost

tests/tests_no_package_update.yml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@
1313
no_log: true
1414

1515
- name: Include role
16-
include_role:
17-
name: linux-system-roles.crypto_policies
16+
include_tasks: tasks/run_role_with_clear_facts.yml
1817

1918
- name: Refresh package facts
2019
package_facts:

tests/tests_reboot.yml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
- name: Run test
1010
block:
1111
- name: Ensure we can set crypto policy including reboot of the system
12-
include_role:
13-
name: linux-system-roles.crypto_policies
12+
include_tasks: tasks/run_role_with_clear_facts.yml
1413
vars:
1514
crypto_policies_policy: FUTURE
1615
crypto_policies_reboot_ok: true
@@ -29,8 +28,7 @@
2928
- name: Restore policy to DEFAULT
3029
tags:
3130
- tests::cleanup
32-
include_role:
33-
name: linux-system-roles.crypto_policies
31+
include_tasks: tasks/run_role_with_clear_facts.yml
3432
vars:
3533
crypto_policies_policy: DEFAULT
3634
crypto_policies_reboot_ok: true

tests/tests_reload.yml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,15 @@
88
- name: Run test
99
block:
1010
- name: Run role to make sure all dependencies are installed
11-
include_role:
12-
name: linux-system-roles.crypto_policies
11+
include_tasks: tasks/run_role_with_clear_facts.yml
1312

1413
- name: Get SSHD pid before policy update
1514
slurp:
1615
src: /var/run/sshd.pid
1716
register: sshd_pid_before
1817

1918
- name: Change policy from DEFAULT TO LEGACY (disable reload)
20-
include_role:
21-
name: linux-system-roles.crypto_policies
19+
include_tasks: tasks/run_role_with_clear_facts.yml
2220
vars:
2321
crypto_policies_policy: LEGACY
2422
crypto_policies_reload: false
@@ -38,8 +36,7 @@
3836
- sshd_pid_before.content == sshd_pid_after_first.content
3937

4038
- name: Change policy from LEGACY to DEFAULT (reload by default)
41-
include_role:
42-
name: linux-system-roles.crypto_policies
39+
include_tasks: tasks/run_role_with_clear_facts.yml
4340
vars:
4441
crypto_policies_policy: DEFAULT
4542

@@ -62,8 +59,7 @@
6259
- name: Restore policy to DEFAULT
6360
tags:
6461
- tests::cleanup
65-
include_role:
66-
name: linux-system-roles.crypto_policies
62+
include_tasks: tasks/run_role_with_clear_facts.yml
6763
vars:
6864
crypto_policies_policy: DEFAULT
6965
- name: Check restoration

tests/tests_update.yml

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
- name: Run test
77
block:
88
- name: Set correct base policy
9-
include_role:
10-
name: linux-system-roles.crypto_policies
9+
include_tasks: tasks/run_role_with_clear_facts.yml
1110
vars:
1211
crypto_policies_policy: LEGACY
1312
crypto_policies_reload: false
@@ -26,8 +25,7 @@
2625
else 'DEFAULT:' ~ crypto_policies_available_subpolicies[0] }}"
2726

2827
- name: Set correct base policy and subpolicy
29-
include_role:
30-
name: linux-system-roles.crypto_policies
28+
include_tasks: tasks/run_role_with_clear_facts.yml
3129
vars:
3230
crypto_policies_policy: "{{ __policy_and_sub }}"
3331
crypto_policies_reload: false
@@ -40,8 +38,7 @@
4038
- name: Setting incorrect base policy should fail
4139
block:
4240
- name: Set incorrect base policy
43-
include_role:
44-
name: linux-system-roles.crypto_policies
41+
include_tasks: tasks/run_role_with_clear_facts.yml
4542
vars:
4643
crypto_policies_policy: NOEXIST
4744
crypto_policies_reload: false
@@ -59,8 +56,7 @@
5956
- name: Setting incorrect subpolicy should fail
6057
block:
6158
- name: Set incorrect subpolicy
62-
include_role:
63-
name: linux-system-roles.crypto_policies
59+
include_tasks: tasks/run_role_with_clear_facts.yml
6460
vars:
6561
crypto_policies_policy: DEFAULT:NOEXIST
6662
crypto_policies_reload: false
@@ -79,8 +75,7 @@
7975
- name: Restore policy to DEFAULT
8076
tags:
8177
- tests::cleanup
82-
include_role:
83-
name: linux-system-roles.crypto_policies
78+
include_tasks: tasks/run_role_with_clear_facts.yml
8479
vars:
8580
crypto_policies_policy: DEFAULT
8681
- name: Check restoration

0 commit comments

Comments
 (0)