feat: PV resize support #438
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
japokorn
force-pushed
the
main-support_pvresize
branch
2 times, most recently
from
4c0455a
to
da3bbc8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #438 +/- ##
==========================================
- Coverage 16.54% 11.57% -4.98%
==========================================
Files 2 8 +6
Lines 284 1806 +1522
Branches 79 0 -79
==========================================
+ Hits 47 209 +162
- Misses 237 1597 +1360
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
japokorn
force-pushed
the
main-support_pvresize
branch
2 times, most recently
from
76af2bf
to
c1815c9
Compare
|
[citest] |
| scripts/does_library_support.py | ||
| blivet.formats.lvmpv.LVMPhysicalVolume.grow_to_fill | ||
| register: grow_supported | ||
| changed_when: false |
There was a problem hiding this comment.
Using script with python scripts is tricky because the python interpreter path is different on every platform. The snapshot role uses script and has quite a bit of logic to determine which python interpreter to use.
https://github.com/linux-system-roles/snapshot/blob/main/tasks/main.yml#L21
The default value for __snapshot_python is defined here: https://github.com/linux-system-roles/snapshot/blob/main/vars/main.yml#L6
override for el7: https://github.com/linux-system-roles/snapshot/blob/main/vars/RedHat_7.yml#L4 and https://github.com/linux-system-roles/snapshot/blob/main/vars/CentOS_7.yml#L4
override for el8: https://github.com/linux-system-roles/snapshot/blob/main/vars/CentOS_8.yml#L4 and https://github.com/linux-system-roles/snapshot/blob/main/vars/RedHat_8.yml#L4
You might ask - well, why doesn't script use the Ansible python interpreter on the remote system? If you can figure out how to do that, good - I could not figure out how to do that, which is why we implemented the snapshot role the way we did.
There was a problem hiding this comment.
I did a bit of testing and it seems that the value we are looking for is in ansible_python.executable.
I have added
args:
executable: "{{ ansible_python.executable }}"
to see how it fares.
tests/test-verify-pool-members.yml
Outdated
|
|
||
| - name: Verify that PVs fill the whole devices when they should | ||
| include_tasks: verify-pool-member-pvsize.yml | ||
| loop: "{{ _storage_test_pool_pvs | default([], true) }}" |
There was a problem hiding this comment.
You shouldn't need to use the form default([], true) here - just default([]) should work - if _storage_test_pool_pvs is defined, it should be a list - if it is an empty list (which evaluates to false in a boolean context, which is what the , true is for in the default filter), then nothing will be done - the loop will not be executed.
There was a problem hiding this comment.
Fixed.
(Removed unnecessary true)
|
|
||
| - name: Clean up test variables | ||
| set_fact: | ||
| actual_pv_size: null |
There was a problem hiding this comment.
| actual_pv_size: null | |
| actual_pv_size: null | |
| dev_size: null |
There was a problem hiding this comment.
Fixed.
(Added missing dev_size: null)
japokorn
force-pushed
the
main-support_pvresize
branch
from
c1815c9
to
6d02781
Compare
|
[citest] |
japokorn
force-pushed
the
main-support_pvresize
branch
from
6d02781
to
1609924
Compare
japokorn
force-pushed
the
main-support_pvresize
branch
from
1609924
to
419170a
Compare
|
[citest] |
|
Issues seem unrelated to this change since they are currently happening in the main branch as well. |
|
please rebase on top of the latest main branch to pick up test fixes |
japokorn
force-pushed
the
main-support_pvresize
branch
from
419170a
to
957534e
Compare
|
@japokorn please rebase on top of latest |
Storage role development often relies on blivet library and changes in it. Whether or not is specific feature supported by blivet is usually determined by its version. That is a cumbersome process, especially when the feature has not yet been added into blivet and the version has to be guessed. Added script verifies existence of the feature by using python introspection and asking for existence of specific item in the library (e.g. 'blivet.formats.lvmpv.LVMPhysicalVolume.grow_to_fill'). The script is supposed to be used for the tests only.
There is an usecase when the physical device size can change (e.g. on VM). We need to be able to change the size of the LVM PV to accomodate that. This adds a new pool parameter 'grow_to_fill'. When set, pool PVs will try to take all available space on their respective devices. Defaults to False. Requires blivet version that supports this feature. For tests this is checked by using 'does_library_support' script.
richm
force-pushed
the
main-support_pvresize
branch
from
19c6e82
to
0572eca
Compare
There is an usecase when the physical device size can change (e.g. on
VM). We need to be able to change the size of the LVM PV to accomodate
that. This adds a new pool parameter
grow_to_fill. When set, pool PVswill try to take all available space on their respective devices.
Defaults to
False.Requires blivet version that supports this feature.
For tests this is checked by using
does_library_supportscript.Storage role development often relies on blivet library and changes in
it. Whether or not is specific feature supported by blivet is usually determined
by blivet version. That is a cumbersome process, especially when the feature
has not yet been added into blivet and the version has to be guessed.
Added script
does_library_supportverifies existence of the feature by using pythonintrospection and asking for existence of specific item in the library
(e.g.
blivet.formats.lvmpv.LVMPhysicalVolume.grow_to_fill).The script is supposed to be used for the tests only.
Blivet PR: #1229
JIRA issue: STORAGECFG-743