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 <[email protected]>

Author: richm

tasks/main-blivet.yml

@@ -73,15 +73,10 @@
     extra_pkgs:
       - kpartx
 
-- name: Get service facts
-  service_facts:
-  when: storage_skip_checks is not defined or
-        not "service_facts" in storage_skip_checks
-
-# rejectattr required because the fix to service_facts is on Ansible > 2.12 only
-# https://github.com/ansible/ansible/pull/75326
-- name: Set storage_cryptsetup_services
-  set_fact:
+- name: Manage storage devices and check for errors
+  vars:
+    # rejectattr required because the fix to service_facts is on Ansible > 2.12 only
+    # https://github.com/ansible/ansible/pull/75326
     storage_cryptsetup_services: "{{
       ansible_facts.services.values() |
       selectattr('name', 'defined') |
@@ -91,10 +86,15 @@
       rejectattr('status', 'match', '^failed$') |
       map(attribute='name') |
       select('match', '^systemd-cryptsetup@') |
-      list }}"
-
-- name: Manage storage devices and check for errors
+      list
+      if 'services' in ansible_facts else [] }}"
   block:
+    - name: Get service facts
+      service_facts:
+      when:
+        - not "services" in ansible_facts
+        - not "cryptsetup_services" in storage_skip_checks | d([])
+
     - name: Mask the systemd cryptsetup services
       systemd:
         name: "{{ item }}"

tests/tasks/run_role_with_clear_facts.yml

@@ -0,0 +1,39 @@
+---
+# Task file: save facts, clear_facts, run linux-system-roles.storage, then restore facts.
+# Include this with include_tasks or import_tasks; ensure tests/library is in module search path.
+# 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
+# Output:
+# - ansible_facts: merged saved ansible_facts with ansible_facts modified by the role, if any
+- 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.storage
+        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.storage
+    tasks_from: "{{ __sr_tasks_from | default('main') }}"
+    public: "{{ __sr_public | default(false) }}"
+  when: __sr_failed_when | d(true)

tests/tests_change_disk_fs.yml

@@ -12,8 +12,7 @@
       else 'ext4' }}"
   tasks:
     - name: Run the role
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
 
     - name: Mark tasks to be skipped
       set_fact:
@@ -22,7 +21,6 @@
           - "{{ (lookup('env',
                         'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
                 ternary('packages_installed', '') }}"
-          - service_facts
 
     - name: Get unused disks
       include_tasks: get_unused_disk.yml
@@ -31,8 +29,7 @@
         max_return: 1
 
     - name: Create a disk device with the default file system type
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -44,8 +41,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Change the disk device file system type to {{ fs_type_after }}
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -58,8 +54,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Repeat the previous invocation to verify idempotence
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -72,8 +67,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Clean up
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1

tests/tests_change_disk_mount.yml

@@ -20,8 +20,7 @@
       changed_when: false
 
     - name: Run the role
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
 
     - name: Mark tasks to be skipped
       set_fact:
@@ -30,7 +29,6 @@
           - "{{ (lookup('env',
                         'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
                 ternary('packages_installed', '') }}"
-          - service_facts
 
     - name: Get unused disks
       include_tasks: get_unused_disk.yml
@@ -39,8 +37,7 @@
         max_return: 1
 
     - name: Create a disk device mounted at "{{ mount_location_before }}"
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -52,8 +49,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Change the disk device mount location to {{ mount_location_after }}
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -65,8 +61,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Repeat the previous invocation to verify idempotence
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1
@@ -78,8 +73,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Clean up
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_volumes:
           - name: test1

tests/tests_change_fs.yml

@@ -14,8 +14,7 @@
 
   tasks:
     - name: Run the role
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
 
     - name: Mark tasks to be skipped
       set_fact:
@@ -24,7 +23,6 @@
           - "{{ (lookup('env',
                         'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
                 ternary('packages_installed', '') }}"
-          - service_facts
 
     - name: Get unused disks
       include_tasks: get_unused_disk.yml
@@ -33,8 +31,7 @@
         max_return: 1
 
     - name: Create a LVM logical volume with default fs_type
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -48,8 +45,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Change the file system signature on the logical volume created above
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -64,8 +60,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Re-run the role on the same volume without specifying fs_type
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -86,8 +81,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Repeat the previous invocation to verify idempotence
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -102,8 +96,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Remove the FS
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -116,10 +109,8 @@
     - name: Verify role results - 5
       include_tasks: verify-role-results.yml
 
-
     - name: Clean up
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo

tests/tests_change_fs_use_partitions.yml

@@ -15,8 +15,7 @@
     - tests::lvm
   tasks:
     - name: Run the role
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
 
     - name: Mark tasks to be skipped
       set_fact:
@@ -25,7 +24,6 @@
           - "{{ (lookup('env',
                         'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
                 ternary('packages_installed', '') }}"
-          - service_facts
 
     - name: Get unused disks
       include_tasks: get_unused_disk.yml
@@ -34,8 +32,7 @@
         max_return: 1
 
     - name: Create an LVM partition with the default file system type
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: bar
@@ -49,8 +46,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Change the LVM partition file system type to {{ fs_type_after }}
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: bar
@@ -65,8 +61,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Repeat the previous invocation to verify idempotence
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: bar
@@ -81,8 +76,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Clean up
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: bar

tests/tests_change_mount.yml

@@ -11,8 +11,7 @@
 
   tasks:
     - name: Run the role
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
 
     - name: Mark tasks to be skipped
       set_fact:
@@ -21,7 +20,6 @@
           - "{{ (lookup('env',
                         'SYSTEM_ROLES_REMOVE_CLOUD_INIT') in ['', 'false']) |
                 ternary('packages_installed', '') }}"
-          - service_facts
 
     - name: Get unused disks
       include_tasks: get_unused_disk.yml
@@ -30,8 +28,7 @@
         max_return: 1
 
     - name: Create a LVM logical volume mounted at {{ mount_location_before }}
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -45,8 +42,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Change the mount location to {{ mount_location_after }}
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -60,8 +56,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Repeat the previous invocation to verify idempotence
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo
@@ -75,8 +70,7 @@
       include_tasks: verify-role-results.yml
 
     - name: Clean up
-      include_role:
-        name: linux-system-roles.storage
+      include_tasks: tasks/run_role_with_clear_facts.yml
       vars:
         storage_pools:
           - name: foo