From 0fc99c393903f3f249df037e9d824a09ad0661f1 Mon Sep 17 00:00:00 2001
From: Jesse Pretorius <jesse.pretorius@rackspace.co.uk>
Date: Mon, 5 Nov 2018 12:50:21 +0000
Subject: [PATCH] Fix lvg module idempotency (#47620)

* Fix lvg module idempotency

In [1] changes were made to ensure that the physical
devices were appropriately filtered, but the dev_list
which is used to prepare the filter is modified from
the original arguments to resolve any symlinks. This
results in the existing devices given in the module
args to be left out of the filter, resulting
in the module trying to add the same device again
every time the task is executed.

In this PR we change dev_list to be a copy of the
module arguments so that we're able to add the given
pv list from the module arguments into the filter
as well, ensuring that there is idempotence when
running the task again.

[1] https://github.com/ansible/ansible/pull/38446

* Add lvg module idempotence test

To ensure that the lvg module is tested for idempotency,
we add a basic integration test.

Support for MacOS and FreeBSD are skipped because the
module does not currently support those platforms.
---
 lib/ansible/modules/system/lvg.py           |  4 +-
 test/integration/targets/lvg/aliases        |  5 +++
 test/integration/targets/lvg/tasks/main.yml | 48 +++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 test/integration/targets/lvg/aliases
 create mode 100644 test/integration/targets/lvg/tasks/main.yml

diff --git a/lib/ansible/modules/system/lvg.py b/lib/ansible/modules/system/lvg.py
index 8b52e48629..b4d449ea58 100644
--- a/lib/ansible/modules/system/lvg.py
+++ b/lib/ansible/modules/system/lvg.py
@@ -150,7 +150,7 @@ def main():
 
     dev_list = []
     if module.params['pvs']:
-        dev_list = module.params['pvs']
+        dev_list = list(module.params['pvs'])
     elif state == 'present':
         module.fail_json(msg="No physical volumes given.")
 
@@ -167,7 +167,7 @@ def main():
         # get pv list
         pvs_cmd = module.get_bin_path('pvs', True)
         if dev_list:
-            pvs_filter = ' || '. join(['pv_name = {0}'.format(x) for x in dev_list])
+            pvs_filter = ' || '. join(['pv_name = {0}'.format(x) for x in dev_list + module.params['pvs']])
             pvs_filter = "--select '%s'" % pvs_filter
         else:
             pvs_filter = ''
diff --git a/test/integration/targets/lvg/aliases b/test/integration/targets/lvg/aliases
new file mode 100644
index 0000000000..b1e9a5e724
--- /dev/null
+++ b/test/integration/targets/lvg/aliases
@@ -0,0 +1,5 @@
+destructive
+needs/privileged
+shippable/posix/group1
+skip/freebsd
+skip/osx
diff --git a/test/integration/targets/lvg/tasks/main.yml b/test/integration/targets/lvg/tasks/main.yml
new file mode 100644
index 0000000000..dbb5bc14e2
--- /dev/null
+++ b/test/integration/targets/lvg/tasks/main.yml
@@ -0,0 +1,48 @@
+- name: Install required packages (Linux)
+  package:
+    name: lvm2
+    state: present
+  when: ansible_system == 'Linux'
+
+- name: Test lvg module
+  block:
+    - name: Create file to use as a disk device
+      command: "dd if=/dev/zero of={{ ansible_user_dir }}/ansible_testing/img1 bs=1M count=10"
+
+    - name: Create loop device for file
+      command: "losetup --show -f {{ ansible_user_dir }}/ansible_testing/img1"
+      register: loop_device1
+
+    - name: Create volume group on disk device
+      lvg:
+        vg: testvg
+        pvs: "{{ loop_device1.stdout }}"
+
+    - name: Create the volume group again to verify idempotence
+      lvg:
+        vg: testvg
+        pvs: "{{ loop_device1.stdout }}"
+      register: repeat_vg_create
+
+    - name: Do all assertions to verify expected results
+      assert:
+        that:
+          - repeat_vg_create is not changed
+
+  always:
+    - name: Remove test volume group
+      lvg:
+        vg: testvg
+        state: absent
+
+    - name: Detach loop device
+      command: "losetup -d {{ loop_device1.stdout }}"
+      when:
+        - loop_device1 is defined
+        - loop_device1.stdout is defined
+        - loop_device1.stdout is match("/dev/.*")
+
+    - name: Remove the file
+      file:
+        path: "{{ ansible_user_dir }}/ansible_testing/img1"
+        state: absent