Fix idempotent issue when using lvol with %VG or %PVS size options and VG is fully allocated

[JuddTracy-DAS]

SUMMARY

Changed the behavior when using %VG or %PVS to make the size_requested an even modulus with the VGs physical extents by rounding down. This makes the usage of %VG or %PVS idempotent when the calculated size_requested does not end on a physical extent boundary and the VG is fully allocated.

Fixes #290.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

lvol

ADDITIONAL INFORMATION

The VG looks like this:

vgs --noheadings --nosuffix -o vg_name,size,free,vg_extent_size --units 'm' --separator ';' mysql_vg

mysql_vg;3060.00;0;4.00

Based on the logic of the module the size requested looks like this:

size_requested = size_percent * this_vg['size'] / 100
2295.0 = 25 * 3060.00 / 100

When divided by the extent size we can see it is not whole number:

size_requested / 4 = 573.75

This is not an issue unless the VG is fully allocated. When that is the case you get the following error:

fatal: [mysql]: FAILED! => {"changed": false, "msg": "Logical Volume var_lib_mysql2 could not be extended. Not enough free space left (1.0m required / 0.0m available)"}

Example playbook:

    tasks:
    - lvol:
        vg: 'mysql_vg'
        lv: 'var_lib_mysql2'
        size: '25%VG'
        shrink: no
        state: 'present'

[ansibullbot]

cc @MorrisA @abulimov @bcoca @d-little @flynn1973 @gforster @jhoekx @kairoaraujo @marvin-sinister @mator @molekuul @ramooncamacho @wtcross
click here for bot help

[felixfontein]

Needs a changelog fragment.

[felixfontein]

@MorrisA @abulimov @bcoca @d-little @flynn1973 @gforster @jhoekx @kairoaraujo @marvin-sinister @mator @molekuul @ramooncamacho @wtcross since you are maintainers for this module, could you please check whether the code change is correct?

[Andersson007]

@JuddTracy-DAS any updates on this?

[Andersson007]

cc @jhoekx @abulimov
@JuddTracy-DAS could you also please email the authors directly? There are their emails in the module's header

[JuddTracy-DAS]

I merged the suggestions from felixfontein, is there something else you need done?

[Andersson007]

@JuddTracy-DAS not on my side but i don't use the module, better to have approvals from authors or users. Did you email them?

[Andersson007]

@JuddTracy-DAS if we merge this, could we add you to the module's maintainers? so you will receive notifications about related events (issues/PRs) and your shipit will be counted by the bot for fixes/minor-changes automerge

[felixfontein]

@MorrisA @abulimov @bcoca @d-little @flynn1973 @gforster @jhoekx @kairoaraujo @marvin-sinister @mator @molekuul @ramooncamacho @wtcross as the module maintainers, could you please take a look at this change?

[Andersson007]

@vburamdo @mator @danielmotaleite does anybody use this module? That needs to add only one line to check

[BenoitKnecht]

@Andersson007 I'm affected by this issue, and I can confirm that this PR fixes it.

Before applying this fix, running a playbook with lvol using size=8%VG:

1. First Ansible run creates the logical volume (19201523712 bytes in my case);
2. Second Ansible run reports the lvol result as changed, and actually extends the logical volume by a few bytes (for a total of 19205718016 bytes);
3. Thrid Ansible run onward reports no changes and doesn't resize the logical volume.

After applying the fix:

1. First Ansible run creates the logical volume (19201523712 bytes in my case);
2. Second Ansible run onward reports no changes and doesn't resize the logical volume.

which is the expected behavior.

[felixfontein]

If nobody complains, I'll merge this beginning of next week.

[Andersson007]

@BenoitKnecht great, thanks for the feedback!

[felixfontein]

@JuddTracy-DAS thanks for fixing this!
@Andersson007 @BenoitKnecht thanks for reviewing and testing this!

[BenoitKnecht]

Is this going to be automatically backported to ansible/ansible, or should a PR be manually submitted there?

[felixfontein]

This needs to be manually backported. If you want to do that, you need to create a PR there for the stable-2.9 branch, create exactly the same changes as here (with paths adjusted), and reference this PR.

[BenoitKnecht]

Thanks, done.