From 7618fd8749c9d71493bb9f8a67c4231bc0c17bd1 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Sat, 14 May 2016 10:40:49 +0200 Subject: [PATCH] Fix check-mode incorrectly returning changed (#2220) The lvol module has a different logic in check-mode for knowing when a change is induced. And this logic is *only* based on a size check. However during a normal run, it is the lvreduce or lvextend tool that decides when a change is performed (or when the requested and existing sizes differ). So while in check-mode the module reports a change, in real run-mode it does not in fact changes anything an reports ok. One solution would be to implement the exact size-comparison logic that is implemented in lvextend and lvreduce, but we opted to use the `--test` option to each command to verify if a change is induced or not. In effect both check-mode and run-mode use the exact same logic and conclusion. --- system/lvol.py | 73 +++++++++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/system/lvol.py b/system/lvol.py index 71cb717c233..75d8c56ac9a 100644 --- a/system/lvol.py +++ b/system/lvol.py @@ -214,6 +214,12 @@ def main(): if opts is None: opts = "" + # Add --test option when running in check-mode + if module.check_mode: + test_opt = ' --test' + else: + test_opt = '' + if size: # LVCREATE(8) -l --extents option with percentage if '%' in size: @@ -297,28 +303,23 @@ def main(): if this_lv is None: if state == 'present': ### create LV - if module.check_mode: + lvcreate_cmd = module.get_bin_path("lvcreate", required=True) + if snapshot is not None: + cmd = "%s %s %s -%s %s%s -s -n %s %s %s/%s" % (lvcreate_cmd, test_opt, yesopt, size_opt, size, size_unit, snapshot, opts, vg, lv) + else: + cmd = "%s %s %s -n %s -%s %s%s %s %s %s" % (lvcreate_cmd, test_opt, yesopt, lv, size_opt, size, size_unit, opts, vg, pvs) + rc, _, err = module.run_command(cmd) + if rc == 0: changed = True else: - lvcreate_cmd = module.get_bin_path("lvcreate", required=True) - if snapshot is not None: - cmd = "%s %s -%s %s%s -s -n %s %s %s/%s" % (lvcreate_cmd, yesopt, size_opt, size, size_unit, snapshot, opts, vg, lv) - else: - cmd = "%s %s -n %s -%s %s%s %s %s %s" % (lvcreate_cmd, yesopt, lv, size_opt, size, size_unit, opts, vg, pvs) - rc, _, err = module.run_command(cmd) - if rc == 0: - changed = True - else: - module.fail_json(msg="Creating logical volume '%s' failed" % lv, rc=rc, err=err) + module.fail_json(msg="Creating logical volume '%s' failed" % lv, rc=rc, err=err) else: if state == 'absent': ### remove LV - if module.check_mode: - module.exit_json(changed=True) if not force: module.fail_json(msg="Sorry, no removal of logical volume %s without force=yes." % (this_lv['name'])) lvremove_cmd = module.get_bin_path("lvremove", required=True) - rc, _, err = module.run_command("%s --force %s/%s" % (lvremove_cmd, vg, this_lv['name'])) + rc, _, err = module.run_command("%s %s --force %s/%s" % (lvremove_cmd, test_opt, vg, this_lv['name'])) if rc == 0: module.exit_json(changed=True) else: @@ -349,20 +350,19 @@ def main(): tool = '%s %s' % (tool, '--force') if tool: - if module.check_mode: + cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs) + rc, out, err = module.run_command(cmd) + if "Reached maximum COW size" in out: + module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) + elif rc == 0: changed = True + msg="Volume %s resized to %s%s" % (this_lv['name'], size_requested, unit) + elif "matches existing size" in err: + module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size']) + elif "not larger than existing size" in err: + module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size'], msg="Original size is larger than requested size", err=err) else: - cmd = "%s -%s %s%s %s/%s %s" % (tool, size_opt, size, size_unit, vg, this_lv['name'], pvs) - rc, out, err = module.run_command(cmd) - if "Reached maximum COW size" in out: - module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) - elif rc == 0: - changed = True - msg="Volume %s resized to %s%s" % (this_lv['name'], size_requested, unit) - elif "matches existing size" in err: - module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size']) - else: - module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err) + module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err) else: ### resize LV based on absolute values @@ -379,19 +379,18 @@ def main(): tool = '%s %s' % (tool, '--force') if tool: - if module.check_mode: + cmd = "%s %s -%s %s%s %s/%s %s" % (tool, test_opt, size_opt, size, size_unit, vg, this_lv['name'], pvs) + rc, out, err = module.run_command(cmd) + if "Reached maximum COW size" in out: + module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) + elif rc == 0: changed = True + elif "matches existing size" in err: + module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size']) + elif "not larger than existing size" in err: + module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size'], msg="Original size is larger than requested size", err=err) else: - cmd = "%s -%s %s%s %s/%s %s" % (tool, size_opt, size, size_unit, vg, this_lv['name'], pvs) - rc, out, err = module.run_command(cmd) - if "Reached maximum COW size" in out: - module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err, out=out) - elif rc == 0: - changed = True - elif "matches existing size" in err: - module.exit_json(changed=False, vg=vg, lv=this_lv['name'], size=this_lv['size']) - else: - module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err) + module.fail_json(msg="Unable to resize %s to %s%s" % (lv, size, size_unit), rc=rc, err=err) module.exit_json(changed=changed, msg=msg)