#2267 lead to behaviour regression

[jake2184]

Summary

I recently upgraded to 3.5.0, which is past the point #2267 was merged.

This PR broke some behaviour I have been using for a long time with the lvol module.

I manage a large number of VMs - when they're created from a template with variable disk size, I create an lvol data that fills all free space. If, further down the line, I simply expand the disk and run the same ansible config the data lvol is expanded to fill the new space. This has worked reliably well for a long time with the lvol module.

With #2267, a check was added to throw an error if an lvol didn't exist, but the size argument had an operator (+/-).

            if size_operator is not None:
                module.fail_json(msg="Bad size specification of '%s%s' for creating LV" % (size_operator, size))

Whilst I understand why this was added, the change of behaviour now means that I cannot use the single module call I use now.

The workaround I can now think of is:

• Get current lvol info
• If data doesn't exist, call lvol with 100%FREE
• If data does exist, call lvol with +100%FREE

This is turning a single module call into 3 module calls. Ansible is already slow enough, having to do this arduous work around for a long-running reasonable use case seems excessive.

Would a PR be accepted if I removed this clause? Other than 'user friendliness', is this safety check justified for the lack of functionality? The module now forces you to pass different arguments depending on if the resource exists or not, to get to the same end point. This doesn't seem very user friendly or idempotent.

Issue Type

Bug Report

Component Name

community/general/plugins/modules/lvol.py

Ansible Version

$ ansible --version
ansible [core 2.11.3] 
  config file = /home/jr/ansible/ansible.cfg
  configured module search path = ['/home/jrs/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/jr/ansible/venv/lib64/python3.8/site-packages/ansible
  ansible collection location = /home/jr/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/jr/ansible/venv/bin/ansible
  python version = 3.8.8 (default, Aug 25 2021, 16:13:02) [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
  jinja version = 3.0.1
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
3.5.0

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

- lvol:
    vg: system
    lv: data
    size: +100%FREE

Expected Results

An lvol to be created, or expanded, to fill all available space

Actual Results

failed: [hostname] (item={'vg': 'system', 'lv': 'data', 'size': '+100%FREE'}) => {
    "ansible_loop_var": "item",
    "changed": false,
    "item": {
        "lv": "data",
        "size": "+100%FREE",
        "vg": "system"
    }
}

MSG:

Bad size specification of '+100%FREE' for creating LV

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/system/lvol.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @abulimov @jhoekx @unkaputtbar112 @zigaSRC
click here for bot help

[felixfontein]

Would a PR be accepted if I removed this clause?

I'm not really familiar with the code, but it definitely should still fail if someone provided size_operator == '-'. So completely removing is not ok, but I guess that adjusting it to if size_operator != '-': should be fine.

@zigaSRC @unkaputtbar112 @russoz what do you think?

[zigaSRC]

That would not be ok, since you still have other relative options: VG, LV, PVS, ORIGIN.

If you're going to be replacing the check you should also take into account the option specified. For example, if the LV doesn't exist we should not allow the option of LV at all.