Skip to content

Commit

Permalink
refactor: centralize test setup/cleanup - add cleanup debugging
Browse files Browse the repository at this point in the history 
We are getting some test flakes which I believe are related to
incomplete cleanup.  Rather than add the same code to every test,
I created a tasks/cleanup.yml to do this, and variables for the
storage pools and other parameters to cleanup.  Then, it made
sense to do the same for setup - tasks/setup.yml

Signed-off-by: Rich Megginson <[email protected]>
  • Loading branch information
@richm
richm committed Feb 16, 2024
1 parent 769a0e7 commit 5cbc04d
Show file tree
Hide file tree
Showing 33 changed files with 871 additions and 1,841 deletions.
27 changes: 22 additions & 5 deletions tests/get_unused_disk.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,28 @@
unused_disks: "{{ unused_disks_return.disks }}"
when: "'Unable to find unused disk' not in unused_disks_return.disks"

- 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 | d([]) | length < disks_needed | d(1)

- name: Print unused disks
debug:
var: unused_disks
verbosity: 2

- name: Try to find out why there are not enough unused disks
when: unused_disks | d([]) | length < disks_needed | d(1)
block:
- name: Print info from find_unused_disk
debug:
var: unused_disks_return.info
when: unused_disks_return.info | d([]) | length > 0

- name: Show disk information
shell: |
set -euxo pipefail
exec 1>&2
lvs --all
pvs --all
vgs --all
changed_when: false

- name: Exit playbook when there's not enough unused disks in the system
fail:
msg: "Unable to find enough unused disks. Exiting playbook."
19 changes: 19 additions & 0 deletions tests/library/find_unused_disk.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,37 +185,56 @@ def run_module():
module = AnsibleModule(argument_spec=module_args, supports_check_mode=True)

max_size = Size(module.params["max_size"])
info = []
for path, attrs in get_disks(module).items():
if is_ignored(path):
info.append("Disk [%s] attrs [%s] is ignored" % (path, attrs))
continue

interface = module.params["with_interface"]

if interface is not None and not is_device_interface(module, path, interface):
info.append(
"Disk [%s] attrs [%s] is not an interface [%s]"
% (path, attrs, interface)
)
continue

if attrs["fstype"]:
info.append("Disk [%s] attrs [%s] has fstype" % (path, attrs))
continue

if Size(attrs["size"]).bytes < Size(module.params["min_size"]).bytes:
info.append(
"Disk [%s] attrs [%s] size is less than requested" % (path, attrs)
)
continue

if max_size.bytes > 0 and Size(attrs["size"]).bytes > max_size.bytes:
info.append(
"Disk [%s] attrs [%s] size is greater than requested" % (path, attrs)
)
continue

if get_partitions(path):
info.append("Disk [%s] attrs [%s] has partitions" % (path, attrs))
continue

if not no_holders(get_sys_name(path)):
info.append("Disk [%s] attrs [%s] has holders" % (path, attrs))
continue

if not can_open(path):
info.append(
"Disk [%s] attrs [%s] cannot be opened exclusively" % (path, attrs)
)
continue

result["disks"].append(os.path.basename(path))
if len(result["disks"]) >= module.params["max_return"]:
break

result["info"] = info
if not result["disks"]:
result["disks"] = "Unable to find unused disk"
else:
Expand Down
51 changes: 51 additions & 0 deletions tests/tasks/cleanup.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# SPDX-License-Identifier: MIT
# Input:
# - test_disk_min_size e.g. "1g"
# - test_disk_count e.g. 10
# - test_storage_pools - the list of pools & volumes to create
---
- name: Remove storage volumes
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_safe_mode: false
storage_pools: |
{% set cleanup_pools = [] %}
{% for pool in test_storage_pools %}
{% set clean_pool = {"name": pool["name"],
"disks": pool["disks"], "state": "absent"} %}
{% set vols = [] %}
{% for vol in pool.get("volumes", []) %}
{% set clean_vol = {"name": vol["name"], "state": "absent"} %}
{% set _ = vols.append(clean_vol) %}
{% endfor %}
{% if vols %}
{% set _ = clean_pool.__setitem__("volumes", vols) %}
{% endif %}
{% set _ = cleanup_pools.append(clean_pool) %}
{% endfor %}
{{ cleanup_pools }}
- name: Save unused_disk_return before verify
set_fact:
unused_disks_before: "{{ unused_disks }}"

- name: Verify that pools/volumes used in test are removed
include_tasks: get_unused_disk.yml
vars:
min_size: "{{ test_disk_min_size }}"
min_return: "{{ test_disk_count }}"

- name: Debug why list of unused disks has changed
shell: |
set -euxo pipefail
exec 1>&2
mount -f -l
df -H
lvs --all
pvs --all
vgs --all
cat /tmp/snapshot_role.log || :
changed_when: false
when: unused_disks_before != unused_disks_return.disks
failed_when: unused_disks_before != unused_disks_return.disks
34 changes: 34 additions & 0 deletions tests/tasks/setup.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# SPDX-License-Identifier: MIT
# Input:
# - test_disk_min_size e.g. "1g"
# - test_disk_count e.g. 10
# - test_storage_pools - the list of pools & volumes to create
# Output:
# - unused_disks e.g. ["sda", "sdb", ..]
# - test_mnt_parent e.g. /mnt or /var/mnt
---
- name: Check if system is ostree
stat:
path: /run/ostree-booted
register: __ostree_booted_stat

- name: Set mount parent
set_fact:
test_mnt_parent: "{{ __ostree_booted_stat.stat.exists |
ternary('/var/mnt', '/mnt') }}"

- name: Run the storage role install base packages
include_role:
name: fedora.linux_system_roles.storage

- name: Get unused disks
include_tasks: get_unused_disk.yml
vars:
min_size: "{{ test_disk_min_size }}"
min_return: "{{ test_disk_count }}"

- name: Create LVM logical volumes under volume groups
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_pools: "{{ test_storage_pools }}"
117 changes: 34 additions & 83 deletions tests/tests_basic.yml
View file
Original file line number Diff line number Diff line change
@@ -1,58 +1,40 @@
---
- name: Basic snapshot test
hosts: all
vars:
test_disk_min_size: "1g"
test_disk_count: 10
test_storage_pools:
- name: test_vg1
disks: "{{ range(0, 3) | map('extract', unused_disks) | list }}"
volumes:
- name: lv1
size: "15%"
- name: lv2
size: "50%"
- name: test_vg2
disks: "{{ range(3, 6) | map('extract', unused_disks) | list }}"
volumes:
- name: lv3
size: "10%"
- name: lv4
size: "20%"
- name: test_vg3
disks: "{{ range(6, 10) | map('extract', unused_disks) | list }}"
volumes:
- name: lv5
size: "30%"
- name: lv6
size: "25%"
- name: lv7
size: "10%"
- name: lv8
size: "10%"
tasks:
- name: Run tests
block:
- name: Run the storage role to create test LVs
include_role:
name: fedora.linux_system_roles.storage

- name: Get unused disks
include_tasks: get_unused_disk.yml
vars:
min_size: "1g"
min_return: 10

- name: Set disk lists
set_fact:
disk_list_1: "{{ range(0, 3) | map('extract', unused_disks) |
list }}"
disk_list_2: "{{ range(3, 6) | map('extract', unused_disks) |
list }}"
disk_list_3: "{{ range(6, 10) | map('extract', unused_disks) |
list }}"

- name: Create LVM logical volumes under volume groups
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_pools:
- name: test_vg1
disks: "{{ disk_list_1 }}"
volumes:
- name: lv1
size: "15%"
- name: lv2
size: "50%"
- name: test_vg2
disks: "{{ disk_list_2 }}"
volumes:
- name: lv3
size: "10%"
- name: lv4
size: "20%"
- name: test_vg3
disks: "{{ disk_list_3 }}"
volumes:
- name: lv5
size: "30%"
- name: lv6
size: "25%"
- name: lv7
size: "10%"
- name: lv8
size: "10%"
- name: Setup
include_tasks: tasks/setup.yml

- name: Run the snapshot role to create snapshot LVs
include_role:
Expand Down Expand Up @@ -87,37 +69,6 @@
snapshot_lvm_verify_only: true
snapshot_lvm_action: remove
always:
- name: Remove storage volumes
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_safe_mode: false
storage_pools:
- name: test_vg1
disks: "{{ disk_list_1 }}"
state: absent
volumes:
- name: lv1
state: absent
- name: lv2
state: absent
- name: test_vg2
disks: "{{ disk_list_2 }}"
state: absent
volumes:
- name: lv3
state: absent
- name: lv4
state: absent
- name: test_vg3
disks: "{{ disk_list_3 }}"
state: absent
volumes:
- name: lv5
state: absent
- name: lv6
state: absent
- name: lv7
state: absent
- name: lv8
state: absent
- name: Cleanup
include_tasks: tasks/cleanup.yml
tags: tests::cleanup
51 changes: 14 additions & 37 deletions tests/tests_check_no_lv_fail.yml
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,20 @@
---
- name: Verify the check commmand fails when source LV doesn't exist
hosts: all
vars:
test_disk_min_size: "1g"
test_disk_count: 10
test_storage_pools:
- name: test_vg1
disks: "{{ range(0, 3) | map('extract', unused_disks) | list }}"
volumes:
- name: lv1
size: "50%"
tasks:
- name: Run tests
block:
- name: Run the storage role to create test LVs
include_role:
name: fedora.linux_system_roles.storage

- name: Get unused disks
include_tasks: get_unused_disk.yml
vars:
min_size: "1g"
min_return: 10

- name: Set disk list
set_fact:
disk_list_1: "{{ range(0, 3) | map('extract', unused_disks) |
list }}"

- name: Create LVM logical volumes under volume groups
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_pools:
- name: test_vg1
disks: "{{ disk_list_1 }}"
volumes:
- name: lv1
size: "50%"
- name: Setup
include_tasks: tasks/setup.yml

- name: Test failure of check with verify only set for incorrect LV
include_tasks: verify-role-failed.yml
Expand All @@ -43,15 +29,6 @@
snapshot_lvm_verify_only: true
snapshot_lvm_action: check
always:
- name: Remove storage volumes
include_role:
name: fedora.linux_system_roles.storage
vars:
storage_safe_mode: false
storage_pools:
- name: test_vg1
disks: "{{ disk_list_1 }}"
state: absent
volumes:
- name: lv1
state: absent
- name: Cleanup
include_tasks: tasks/cleanup.yml
tags: tests::cleanup
Loading

0 comments on commit 5cbc04d

Please sign in to comment.