From 3e303bea4c305994addcd739e1848d9637112512 Mon Sep 17 00:00:00 2001 From: Julien Girardin Date: Thu, 20 Dec 2018 16:43:18 +0100 Subject: [PATCH] List correctly current PV in "lvg" module: fix lvg reduce (#49731) * Refactor integration test for lvg module to introduce grow/reduce test * List correctly current PV in lvg module: fix lvg reduce Previous behaviour was to only take into account PV passed in 'pvs' argument. This lead to reduce not working as expecting: * with state=present and list of wanted pvs, lvg found only the pvs to add or already present and ignored the pv to remove (obviously absent from the list of given PV) * with state=absent and a pv to remove, lvg found that the remaining pvs list is empty (ignoring possible other PV in the vg) and decides to remove the vg entirely (as supposely no PV are left anymore to store lvm metadata) * Add changelog fragment --- .../fix-lvg-reduce-by-better-listing-pv.yml | 2 + lib/ansible/modules/system/lvg.py | 9 +++- test/integration/targets/lvg/tasks/main.yml | 41 ++----------------- test/integration/targets/lvg/tasks/setup.yml | 13 ++++++ .../targets/lvg/tasks/teardown.yml | 17 ++++++++ .../targets/lvg/tasks/test_grow_reduce.yml | 33 +++++++++++++++ .../targets/lvg/tasks/test_indempotency.yml | 15 +++++++ 7 files changed, 91 insertions(+), 39 deletions(-) create mode 100644 changelogs/fragments/fix-lvg-reduce-by-better-listing-pv.yml create mode 100644 test/integration/targets/lvg/tasks/setup.yml create mode 100644 test/integration/targets/lvg/tasks/teardown.yml create mode 100644 test/integration/targets/lvg/tasks/test_grow_reduce.yml create mode 100644 test/integration/targets/lvg/tasks/test_indempotency.yml diff --git a/changelogs/fragments/fix-lvg-reduce-by-better-listing-pv.yml b/changelogs/fragments/fix-lvg-reduce-by-better-listing-pv.yml new file mode 100644 index 00000000000..6e7ec6c1326 --- /dev/null +++ b/changelogs/fragments/fix-lvg-reduce-by-better-listing-pv.yml @@ -0,0 +1,2 @@ +bugfixes: +- lvg - Take into account current PV in the VG to fix PV removal diff --git a/lib/ansible/modules/system/lvg.py b/lib/ansible/modules/system/lvg.py index b4d449ea581..ae16e92a790 100644 --- a/lib/ansible/modules/system/lvg.py +++ b/lib/ansible/modules/system/lvg.py @@ -86,6 +86,7 @@ EXAMPLES = ''' state: absent ''' +import itertools import os from ansible.module_utils.basic import AnsibleModule @@ -167,8 +168,12 @@ 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 + module.params['pvs']]) - pvs_filter = "--select '%s'" % pvs_filter + pvs_filter_pv_name = ' || '.join( + 'pv_name = {0}'.format(x) + for x in itertools.chain(dev_list, module.params['pvs']) + ) + pvs_filter_vg_name = 'vg_name = {0}'.format(vg) + pvs_filter = "--select '{0} || {1}' ".format(pvs_filter_pv_name, pvs_filter_vg_name) else: pvs_filter = '' rc, current_pvs, err = module.run_command("%s --noheadings -o pv_name,vg_name --separator ';' %s" % (pvs_cmd, pvs_filter)) diff --git a/test/integration/targets/lvg/tasks/main.yml b/test/integration/targets/lvg/tasks/main.yml index dbb5bc14e2f..a57f591bf02 100644 --- a/test/integration/targets/lvg/tasks/main.yml +++ b/test/integration/targets/lvg/tasks/main.yml @@ -6,43 +6,10 @@ - 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" + - import_tasks: setup.yml - - 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 + - import_tasks: test_indempotency.yml + - import_tasks: test_grow_reduce.yml 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 + - import_tasks: teardown.yml diff --git a/test/integration/targets/lvg/tasks/setup.yml b/test/integration/targets/lvg/tasks/setup.yml new file mode 100644 index 00000000000..29f29d6e9c4 --- /dev/null +++ b/test/integration/targets/lvg/tasks/setup.yml @@ -0,0 +1,13 @@ +- name: "Create files to use as a disk devices" + command: "dd if=/dev/zero of={{ ansible_user_dir }}/ansible_testing/img{{ item }} bs=1M count=10" + with_sequence: 'count=2' + +- name: "Create loop device for file" + command: "losetup --show -f {{ ansible_user_dir }}/ansible_testing/img{{ item }}" + with_sequence: 'count=2' + register: loop_devices + +- name: "Affect name on disk to work on" + set_fact: + loop_device1: "{{ loop_devices.results[0] }}" + loop_device2: "{{ loop_devices.results[1] }}" diff --git a/test/integration/targets/lvg/tasks/teardown.yml b/test/integration/targets/lvg/tasks/teardown.yml new file mode 100644 index 00000000000..71aa4e89075 --- /dev/null +++ b/test/integration/targets/lvg/tasks/teardown.yml @@ -0,0 +1,17 @@ +- name: Remove test volume group + lvg: + vg: testvg + state: absent + +- name: Detach loop device + command: "losetup -d {{ item.stdout }}" + loop: "{{ loop_devices.results|default([]) }}" + when: + - item.stdout is defined + - item.stdout is match("/dev/.*") + +- name: Remove device files + file: + path: "{{ ansible_user_dir }}/ansible_testing/img{{ item }}" + state: absent + with_sequence: 'count={{ loop_devices.results|length }}' diff --git a/test/integration/targets/lvg/tasks/test_grow_reduce.yml b/test/integration/targets/lvg/tasks/test_grow_reduce.yml new file mode 100644 index 00000000000..1e988045383 --- /dev/null +++ b/test/integration/targets/lvg/tasks/test_grow_reduce.yml @@ -0,0 +1,33 @@ +- name: "Create volume group on first disk" + lvg: + vg: testvg + pvs: "{{ loop_device1.stdout }}" + +- name: "get lvm facts" + setup: + +- debug: var=ansible_lvm + +- name: "Assert the testvg span only on first disk" + assert: + that: + - ansible_lvm.pvs[loop_device1.stdout].vg == "testvg" + - 'loop_device2.stdout not in ansible_lvm.pvs or + ansible_lvm.pvs[loop_device2.stdout].vg == ""' + +- name: "Extend to second disk AND reduce from the first disk" + lvg: + vg: testvg + pvs: "{{ loop_device2.stdout }}" + +- name: "get lvm facts" + setup: + +- debug: var=ansible_lvm + +- name: "Assert the testvg span only on first disk" + assert: + that: + - 'loop_device1.stdout not in ansible_lvm.pvs or + ansible_lvm.pvs[loop_device1.stdout].vg == ""' + - ansible_lvm.pvs[loop_device2.stdout].vg == "testvg" diff --git a/test/integration/targets/lvg/tasks/test_indempotency.yml b/test/integration/targets/lvg/tasks/test_indempotency.yml new file mode 100644 index 00000000000..5007e56a5b4 --- /dev/null +++ b/test/integration/targets/lvg/tasks/test_indempotency.yml @@ -0,0 +1,15 @@ +- 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