From a599afa384b4894cddbf8c0c7a1bbea181af6c08 Mon Sep 17 00:00:00 2001 From: Laszlo Szomor Date: Mon, 4 Dec 2023 05:58:55 +0100 Subject: [PATCH] lvol: Change ``pvs`` argument type to list (of str) (#7676) * lvol: Change ``pvs`` argument type to list (of str) * Add changelog fragment * Apply review suggestions --- .../fragments/7676-lvol-pvs-as-list.yml | 2 + plugins/modules/lvol.py | 13 ++-- tests/integration/targets/lvol/aliases | 12 ++++ tests/integration/targets/lvol/meta/main.yml | 8 +++ tests/integration/targets/lvol/tasks/main.yml | 24 +++++++ .../integration/targets/lvol/tasks/setup.yml | 57 +++++++++++++++++ .../targets/lvol/tasks/teardown.yml | 40 ++++++++++++ .../targets/lvol/tasks/test_pvs.yml | 64 +++++++++++++++++++ 8 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/7676-lvol-pvs-as-list.yml create mode 100644 tests/integration/targets/lvol/aliases create mode 100644 tests/integration/targets/lvol/meta/main.yml create mode 100644 tests/integration/targets/lvol/tasks/main.yml create mode 100644 tests/integration/targets/lvol/tasks/setup.yml create mode 100644 tests/integration/targets/lvol/tasks/teardown.yml create mode 100644 tests/integration/targets/lvol/tasks/test_pvs.yml diff --git a/changelogs/fragments/7676-lvol-pvs-as-list.yml b/changelogs/fragments/7676-lvol-pvs-as-list.yml new file mode 100644 index 0000000000..aa28fff59d --- /dev/null +++ b/changelogs/fragments/7676-lvol-pvs-as-list.yml @@ -0,0 +1,2 @@ +minor_changes: + - lvol - change ``pvs`` argument type to list of strings (https://github.com/ansible-collections/community.general/pull/7676, https://github.com/ansible-collections/community.general/issues/7504). diff --git a/plugins/modules/lvol.py b/plugins/modules/lvol.py index 8a029e7711..a2a870260a 100644 --- a/plugins/modules/lvol.py +++ b/plugins/modules/lvol.py @@ -75,9 +75,10 @@ options: description: - The name of a snapshot volume to be configured. When creating a snapshot volume, the O(lv) parameter specifies the origin volume. pvs: - type: str + type: list + elements: str description: - - Comma separated list of physical volumes (e.g. /dev/sda,/dev/sdb). + - List of physical volumes (for example V(/dev/sda, /dev/sdb)). thinpool: type: str description: @@ -110,7 +111,9 @@ EXAMPLES = ''' vg: firefly lv: test size: 512 - pvs: /dev/sda,/dev/sdb + pvs: + - /dev/sda + - /dev/sdb - name: Create cache pool logical volume community.general.lvol: @@ -299,7 +302,7 @@ def main(): shrink=dict(type='bool', default=True), active=dict(type='bool', default=True), snapshot=dict(type='str'), - pvs=dict(type='str'), + pvs=dict(type='list', elements='str'), resizefs=dict(type='bool', default=False), thinpool=dict(type='str'), ), @@ -340,7 +343,7 @@ def main(): if pvs is None: pvs = "" else: - pvs = pvs.replace(",", " ") + pvs = " ".join(pvs) if opts is None: opts = "" diff --git a/tests/integration/targets/lvol/aliases b/tests/integration/targets/lvol/aliases new file mode 100644 index 0000000000..3b92ba75c4 --- /dev/null +++ b/tests/integration/targets/lvol/aliases @@ -0,0 +1,12 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +azp/posix/1 +azp/posix/vm +destructive +needs/privileged +skip/aix +skip/freebsd +skip/osx +skip/macos diff --git a/tests/integration/targets/lvol/meta/main.yml b/tests/integration/targets/lvol/meta/main.yml new file mode 100644 index 0000000000..ca1915e05c --- /dev/null +++ b/tests/integration/targets/lvol/meta/main.yml @@ -0,0 +1,8 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +dependencies: + - setup_pkg_mgr + - setup_remote_tmp_dir diff --git a/tests/integration/targets/lvol/tasks/main.yml b/tests/integration/targets/lvol/tasks/main.yml new file mode 100644 index 0000000000..0e3bfa1a31 --- /dev/null +++ b/tests/integration/targets/lvol/tasks/main.yml @@ -0,0 +1,24 @@ +--- +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Install required packages (Linux) + package: + name: lvm2 + state: present + when: ansible_system == 'Linux' + +- name: Test lvol module + block: + - import_tasks: setup.yml + + - import_tasks: test_pvs.yml + + always: + - import_tasks: teardown.yml diff --git a/tests/integration/targets/lvol/tasks/setup.yml b/tests/integration/targets/lvol/tasks/setup.yml new file mode 100644 index 0000000000..195c50111b --- /dev/null +++ b/tests/integration/targets/lvol/tasks/setup.yml @@ -0,0 +1,57 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: "Create files to use as a disk devices" + command: "dd if=/dev/zero of={{ remote_tmp_dir }}/img{{ item }} bs=1M count=36" + with_sequence: 'count=4' + +- name: "Show next free loop device" + command: "losetup -f" + register: loop_device1 + +- name: "Create loop device for file" + command: "losetup -f {{ remote_tmp_dir }}/img1" + +- name: "Show next free loop device" + command: "losetup -f" + register: loop_device2 + +- name: "Create loop device for file" + command: "losetup -f {{ remote_tmp_dir }}/img2" + +- name: "Show next free loop device" + command: "losetup -f" + register: loop_device3 + +- name: "Create loop device for file" + command: "losetup -f {{ remote_tmp_dir }}/img3" + +- name: "Show next free loop device" + command: "losetup -f" + register: loop_device4 + +- name: "Create loop device for file" + command: "losetup -f {{ remote_tmp_dir }}/img4" + +- name: "Set loop device names" + set_fact: + loop_device1: "{{ loop_device1.stdout }}" + loop_device2: "{{ loop_device2.stdout }}" + loop_device3: "{{ loop_device3.stdout }}" + loop_device4: "{{ loop_device4.stdout }}" + +- name: Create testvg1 volume group on disk devices + lvg: + vg: testvg1 + pvs: + - "{{ loop_device1 }}" + - "{{ loop_device2 }}" + +- name: Create testvg2 volume group on disk devices + lvg: + vg: testvg2 + pvs: + - "{{ loop_device3 }}" + - "{{ loop_device4 }}" diff --git a/tests/integration/targets/lvol/tasks/teardown.yml b/tests/integration/targets/lvol/tasks/teardown.yml new file mode 100644 index 0000000000..a8080f874c --- /dev/null +++ b/tests/integration/targets/lvol/tasks/teardown.yml @@ -0,0 +1,40 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Remove test volume groups + loop: + - testvg1 + - testvg2 + lvg: + vg: "{{ item }}" + force: true + state: absent + +- name: Remove LVM devices + loop: + - "{{ loop_device1 | default('') }}" + - "{{ loop_device2 | default('') }}" + - "{{ loop_device3 | default('') }}" + - "{{ loop_device4 | default('') }}" + when: + - item|length > 0 + command: "lvmdevices --deldev {{ item }}" + ignore_errors: true + +- name: Detach loop devices + command: "losetup -d {{ item }}" + loop: + - "{{ loop_device1 | default('') }}" + - "{{ loop_device2 | default('') }}" + - "{{ loop_device3 | default('') }}" + - "{{ loop_device4 | default('') }}" + when: + - item != '' + +- name: Remove device files + file: + path: "{{ remote_tmp_dir }}/img{{ item }}" + state: absent + with_sequence: 'count=4' diff --git a/tests/integration/targets/lvol/tasks/test_pvs.yml b/tests/integration/targets/lvol/tasks/test_pvs.yml new file mode 100644 index 0000000000..c1cd3d1a86 --- /dev/null +++ b/tests/integration/targets/lvol/tasks/test_pvs.yml @@ -0,0 +1,64 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Create logical volume (testlv1) with pvs set as comma separated string + lvol: + vg: testvg1 + lv: testlv1 + size: 50%PVS + pvs: "{{ loop_device1 }},{{ loop_device2 }}" + register: css_pvs_create_testlv1_result + +- name: Assert logical volume (testlv1) created with pvs set as comma separated string + assert: + that: + - css_pvs_create_testlv1_result is success + - css_pvs_create_testlv1_result is changed + +- name: Create logical volume (testlv1) with pvs set as list + lvol: + vg: testvg1 + lv: testlv1 + size: 50%PVS + pvs: + - "{{ loop_device1 }}" + - "{{ loop_device2 }}" + register: list_pvs_create_testlv1_result + +- name: Assert logical volume (testlv1) creation idempotency with pvs set as list on second execution + assert: + that: + - list_pvs_create_testlv1_result is success + - list_pvs_create_testlv1_result is not changed + +- name: Create logical volume (testlv2) with pvs set as list + lvol: + vg: testvg2 + lv: testlv2 + size: 50%PVS + pvs: + - "{{ loop_device3 }}" + - "{{ loop_device4 }}" + register: list_pvs_create_testlv2_result + +- name: Assert logical volume (testlv2) created with pvs set as list + assert: + that: + - list_pvs_create_testlv2_result is success + - list_pvs_create_testlv2_result is changed + +- name: Create logical volume (testlv2) with pvs set as comma separated string + lvol: + vg: testvg2 + lv: testlv2 + size: 50%PVS + pvs: "{{ loop_device3 }},{{ loop_device4 }}" + register: css_pvs_create_testlv2_result + +- name: Assert logical volume (testlv2) creation idempotency with pvs set as comma separated string on second execution + assert: + that: + - css_pvs_create_testlv2_result is success + - css_pvs_create_testlv2_result is not changed