Fix ansible-lint issues in tests (#340)

* Fix ansible-lint issues in tests

All tasks must be named
Task names must begin with uppercase letter
Fix Jinja spacing issues
Fix long lines with appropriate wrapping
Do not use `{{ .... }}` in `when` and `that` conditionals
Some tests were using `safe_mode` instead of `storage_safe_mode`
LUKS tests were using `regex_search` incorrectly and always
passing - use `is search` instead and fix pattern to parse
luksdump output correctly.
Refactor tests that check for the role to raise an error - add
a new file verify-role-failed.yml which checks for ansible errors
and blivet errors

* check for exceptions in module_stdout

* fix errors

* lint

.ansible-lint

@@ -18,6 +18,5 @@ exclude_paths:
   - tests/roles/
   - .github/
   - examples/roles/
-  - tests/
 mock_roles:
   - linux-system-roles.storage

.yamllint.yml

@@ -3,5 +3,3 @@
 extends: default
 ignore: |
   /.tox/
-  /tests/
-  /.github/

tests/create-test-file.yml

@@ -1,4 +1,6 @@
-# Create a file to be checked that it still exists and no data loss has occurred.
+---
+# Create a file to be checked that it still exists and no data loss has
+# occurred.
 # To use:
 # - set testfile to a path under the mountpoint being tested
 # - include this file (create-test-file.yml) before executing the
@@ -7,7 +9,7 @@
 #   data in the filesystem where testfile is located
 # - include verify-data-preservation.yml
 
-- name: create a file
+- name: Create a file
   file:
     path: "{{ testfile }}"
     state: touch

tests/get_unused_disk.yml

@@ -15,7 +15,7 @@
 - name: Exit playbook when there's not enough unused disks in the system
   fail:
     msg: "Unable to find enough unused disks. Exiting playbook."
-  when: unused_disks is undefined or unused_disks|length < disks_needed|default(1)
+  when: unused_disks | d([]) | length < disks_needed | d(1)
 
 - name: Print unused disks
   debug:

tests/run_blivet.yml

@@ -1,9 +1,9 @@
 ---
-- name: test lvm and xfs package deps
+- name: Test lvm and xfs package deps
   blivet:
     packages_only: "{{ packages_only }}"
-    pools: "{{ storage_pools|default([]) }}"
-    volumes: "{{ storage_volumes|default([]) }}"
+    pools: "{{ storage_pools | default([]) }}"
+    volumes: "{{ storage_volumes | default([]) }}"
     pool_defaults: "{{ storage_pool_defaults }}"
     volume_defaults: "{{ storage_volume_defaults }}"
   register: blivet_output

tests/scripts/generate_tests.py

@@ -23,15 +23,17 @@
 TEST_FILE_CONTENTS = """\
 ---
 # This file was generated by generate_tests.py
-- hosts: all
+- name: Run test %s for %s
+  hosts: all
   tags:
     - tests::%s
   tasks:
-    - name: set disk interface for test
+    - name: Set disk interface for test
       set_fact:
         storage_test_use_interface: "%s"
 
-- import_playbook: %s
+- name: Import playbook
+  import_playbook: %s
   tags:
     - tests::%s
 """
@@ -51,7 +53,7 @@ def generate_file(org_filename, iface):
     gen_path_file = path.join(TARGET_PATH, generate_filename(org_filename, iface))
 
     with open(gen_path_file, "w") as f:
-        f.write(TEST_FILE_CONTENTS % (iface, iface, org_filename, iface))
+        f.write(TEST_FILE_CONTENTS % (org_filename, iface, iface, iface, org_filename, iface))
 
 
 # will new test be generated based on this item?

tests/test-verify-pool-members.yml

@@ -1,10 +1,13 @@
-- set_fact:
+---
+- name: Set test variables
+  set_fact:
     _storage_test_pool_pvs_lvm: "{{ ansible_lvm.pvs | dict2items |
       selectattr('value.vg', 'match', '^' ~ storage_test_pool.name ~ '$') |
       map(attribute='key') | list }}"
     _storage_test_expected_pv_count: "{{ 0
       if storage_test_pool.state == 'absent'
-      else (storage_test_pool.raid_level | ternary(1, storage_test_pool.disks|length)) }}"
+      else (storage_test_pool.raid_level |
+      ternary(1, storage_test_pool.disks | length)) }}"
   when: storage_test_pool.type == 'lvm'
 
 - name: Get the canonical device path for each member device
@@ -16,38 +19,52 @@
   register: pv_paths
   when: storage_test_pool.type == 'lvm'
 
-- set_fact:
-    __pvs_lvm_len: "{{ _storage_test_pool_pvs_lvm|length }}"
+- name: Set pvs lvm length
+  set_fact:
+    __pvs_lvm_len: "{{ _storage_test_pool_pvs_lvm | length }}"
   when: storage_test_pool.type == 'lvm'
 
-- set_fact:
-    _storage_test_pool_pvs: "{{ pv_paths.results[0:(__pvs_lvm_len|int)] | map(attribute='device') | list }}"
+- name: Set pool pvs
+  set_fact:
+    _storage_test_pool_pvs: "{{ pv_paths.results[0 : (__pvs_lvm_len | int)] |
+      map(attribute='device') | list }}"
   when: storage_test_pool.type == 'lvm'
 
 - name: Verify PV count
   assert:
-    that: "{{ __pvs_lvm_len|int == _storage_test_expected_pv_count|int }}"
-    msg: "Unexpected PV count for pool {{ storage_test_pool.name }}
-          (has: {{ __pvs_lvm_len|int }} expected: {{ _storage_test_expected_pv_count|int }})"
+    that: __pvs_lvm_len | int == _storage_test_expected_pv_count | int
+    msg: >-
+      Unexpected PV count for pool {{ storage_test_pool.name }}
+      (has: {{ __pvs_lvm_len | int }}
+       expected: {{ _storage_test_expected_pv_count | int }})
   when: storage_test_pool.type == 'lvm'
 
-- set_fact:
-    _storage_test_expected_pv_type: "{{ 'crypt' if storage_test_pool.encryption else 'disk' }}"
+- name: Set expected pv type
+  set_fact:
+    _storage_test_expected_pv_type: "{{ 'crypt'
+      if storage_test_pool.encryption else 'disk' }}"
   when: storage_test_pool.type == 'lvm'
 
-- set_fact:
-    _storage_test_expected_pv_type: "{{ 'partition' if storage_use_partitions|default(false) else 'disk' }}"
+- name: Set expected pv type
+  set_fact:
+    _storage_test_expected_pv_type: "{{ 'partition'
+      if storage_use_partitions | d(false) else 'disk' }}"
   when: storage_test_pool.type == 'lvm' and not storage_test_pool.encryption
 
-- set_fact:
+- name: Set expected pv type
+  set_fact:
     _storage_test_expected_pv_type: "{{ storage_test_pool.raid_level }}"
   when: storage_test_pool.type == 'lvm' and storage_test_pool.raid_level
 
 - name: Check the type of each PV
   assert:
-    that: "{{ storage_test_blkinfo.info[pv]['type'] == _storage_test_expected_pv_type }}"
-    msg: "Incorrect type for PV {{ pv }} (has: '{{ storage_test_blkinfo.info[pv]['type'] }}'
-          expected: '{{ _storage_test_expected_pv_type }})' in pool '{{ storage_test_pool.name }}'"
+    that: storage_test_blkinfo.info[pv]['type'] ==
+      _storage_test_expected_pv_type
+    msg: >-
+      Incorrect type for PV {{ pv }}
+      (has: '{{ storage_test_blkinfo.info[pv]['type'] }}'
+      expected: '{{ _storage_test_expected_pv_type }})'
+      in pool '{{ storage_test_pool.name }}'
   loop: "{{ _storage_test_pool_pvs }}"
   loop_control:
     loop_var: pv
@@ -68,7 +85,8 @@
 - name: Check VDO
   include_tasks: verify-pool-members-vdo.yml
 
-- set_fact:
+- name: Clean up test variables
+  set_fact:
     _storage_test_expected_pv_type: null
     _storage_test_expected_pv_count: null
     _storage_test_pool_pvs_lvm: []

tests/test-verify-pool-volumes.yml

@@ -1,6 +1,6 @@
 ---
 
-- name: verify the volumes
+- name: Verify the volumes
   include_tasks: "test-verify-volume.yml"
   loop: "{{ storage_test_pool.volumes }}"
   loop_control:

tests/test-verify-pool.yml

@@ -15,7 +15,7 @@
     #     compression
     #     deduplication
 
-- name:
+- name: Verify pool subset
   include_tasks: "test-verify-pool-{{ storage_test_pool_subset }}.yml"
   loop: "{{ _storage_pool_tests }}"
   loop_control:

tests/test-verify-volume-cache.yml

@@ -1,8 +1,7 @@
 ---
-
-- name: check cache options
+- name: Check cache options
+  when: storage_test_volume.type == 'lvm' and _storage_test_volume_present
   block:
-
     - name: Get information about the LV
       command: >
           lvs --noheadings --nameprefixes --units=b --nosuffix --unquoted
@@ -11,35 +10,49 @@
       register: lvs
       changed_when: false
 
-    - set_fact:
-        storage_test_lv_segtype: "{{ lvs.stdout|regex_search('LVM2_SEGTYPE=(\\S+)',  '\\1') }}"
+    - name: Set LV segment type
+      set_fact:
+        storage_test_lv_segtype: "{{ lvs.stdout |
+          regex_search('LVM2_SEGTYPE=(\\S+)', '\\1') }}"
 
-    - name: check segment type
+    - name: Check segment type
       assert:
-        that: storage_test_volume.cached | ternary(storage_test_lv_segtype[0] == 'cache', storage_test_lv_segtype[0] != 'cache')
-        msg: Unexpected segtype {{ storage_test_lv_segtype }} for {{ storage_test_volume.name }}
-
-    - set_fact:
-        storage_test_lv_cache_size: "{{ lvs.stdout|regex_search('LVM2_CACHE_TOTAL_BLOCKS=([0-9]*) LVM2_CHUNK_SIZE=([0-9]+)',  '\\1', '\\2') }}"
-      when: storage_test_volume.cached|bool
-
-    - name: parse the requested cache size
+        that: storage_test_volume.cached |
+          ternary(storage_test_lv_segtype[0] == 'cache',
+                  storage_test_lv_segtype[0] != 'cache')
+        msg: >-
+          Unexpected segtype {{ storage_test_lv_segtype }}
+          for {{ storage_test_volume.name }}
+
+    - name: Set LV cache size
+      set_fact:
+        storage_test_lv_cache_size: "{{ lvs.stdout |
+          regex_search(__pattern, '\\1', '\\2') | map('int') | list }}"
+      vars:
+        __pattern: LVM2_CACHE_TOTAL_BLOCKS=([0-9]*) LVM2_CHUNK_SIZE=([0-9]+)
+      when: storage_test_volume.cached | bool
+
+    - name: Parse the requested cache size
       bsize:
         size: "{{ storage_test_volume.cache_size }}"
       register: storage_test_requested_cache_size
-      when: storage_test_volume.cached|bool
+      when: storage_test_volume.cached | bool
 
-    - set_fact:
-        storage_test_expected_cache_size: "{{ storage_test_requested_cache_size.bytes }}"
-      when: storage_test_volume.cached|bool
+    - name: Set expected cache size
+      set_fact:
+        storage_test_expected_cache_size: "{{
+          storage_test_requested_cache_size.bytes }}"
+      when: storage_test_volume.cached | bool
 
     - name: Check cache size
       assert:
-        that: (storage_test_expected_cache_size|int -
-               storage_test_lv_cache_size[0]|int * storage_test_lv_cache_size[1]|int)|abs / storage_test_expected_cache_size|int < 0.01
+        that: (storage_test_expected_cache_size | int - __actual_size | int) |
+          abs / storage_test_expected_cache_size | int < 0.01
         msg: >
-            Unexpected cache size, expected {{ storage_test_expected_cache_size }},
-            got {{ storage_test_lv_cache_size[0]|int * storage_test_lv_cache_size[1]|int }}
-      when: storage_test_volume.cached|bool
-
-  when: storage_test_volume.type == 'lvm' and _storage_test_volume_present
+            Unexpected cache size, expected
+            {{ storage_test_expected_cache_size }},
+            got {{ __actual_size }}
+      vars:
+        __actual_size: "{{ storage_test_lv_cache_size[0] *
+          storage_test_lv_cache_size[1] }}"
+      when: storage_test_volume.cached | bool

tests/test-verify-volume-device.yml

@@ -1,38 +1,45 @@
 ---
-
 # name/path (use raw_device; encryption layer is validated separately)
 - name: See whether the device node is present
   stat:
     path: "{{ storage_test_volume._raw_device }}"
-    follow: yes
+    follow: true
   register: storage_test_dev
 
 - name: Verify the presence/absence of the device node
   assert:
-    that: "{{ storage_test_dev.stat.exists and storage_test_dev.stat.isblk
-              if _storage_test_volume_present or storage_test_volume.type == 'disk'
-              else
-              not storage_test_dev.stat.exists }}"
-    msg: "Incorrect device node presence for volume {{ storage_test_volume.name }}"
+    that: storage_test_dev.stat.exists and storage_test_dev.stat.isblk
+    msg: >-
+      Incorrect device node presence for volume {{ storage_test_volume.name }}
+  when: _storage_test_volume_present or storage_test_volume.type == 'disk'
+
+- name: Verify the presence/absence of the device node
+  assert:
+    that: not storage_test_dev.stat.exists
+    msg: >-
+      Incorrect device node presence for volume {{ storage_test_volume.name }}
+  when: not (_storage_test_volume_present or
+             storage_test_volume.type == 'disk')
 
 - name: Make sure we got info about this volume
   assert:
-    that: "{{ storage_test_volume._raw_device in storage_test_blkinfo.info }}"
-    msg: "Failed to gather info about volume '{{ storage_test_volume.name }}'"
+    that: storage_test_volume._raw_device in storage_test_blkinfo.info
+    msg: Failed to gather info about volume '{{ storage_test_volume.name }}'
   when: _storage_test_volume_present
 
-- name: (1/2) Process volume type (set initial value)
+- name: Process volume type (set initial value) (1/2)
   set_fact:
     st_volume_type: "{{ storage_test_volume.type }}"
 
-- name: (2/2) Process volume type (get RAID value)
+- name: Process volume type (get RAID value) (2/2)
   set_fact:
     st_volume_type: "{{ storage_test_volume.raid_level }}"
   when: storage_test_volume.type == "raid"
 
 - name: Verify the volume's device type
   assert:
-    that: "{{ storage_test_blkinfo.info[storage_test_volume._raw_device].type == st_volume_type }}"
+    that: storage_test_blkinfo.info[storage_test_volume._raw_device].type ==
+      st_volume_type
   when: _storage_test_volume_present
 
 # disks