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

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).

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 = ""

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

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

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

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 }}"

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'

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