-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(growpart): add LVM resize support #6586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
edda542 to
6f2082b
Compare
|
I think there should be a configuration switch to control whether or not the LV is resized (the PV resize seems fine) - often when LVM is used there will be multiple LVs (i.e. for /home, for /var/log, etc) and so having the LV for rootfs ("/") be grown to use up the whole of the VG is likely to be undesirable. Also the growpart script (from cloud-utils) has some pvresize functionality (I forget any issues with it) and so it doesn't make sense to have LVM functionality in both cloud-util's growpart and in cloud-init's growpart module. |
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging blackboxsw, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag blackboxsw to reopen it.) |
|
@dermotbradley thanks a lot for your comment, let me check how to update the code. |
|
Hello @blackboxsw could you help me re-open it please? I will continue working on it. |
|
@dermotbradley So the solution for cloud-init could be, Or making a fix solution for cloud-utils, What's your opinion?
|
If we can have a more complete solution within cloud-init without depending on other external utilities, I think that might be more preferable. |
|
@holmanb @TheRealFalcon can you provide your inputs as well please? thanks. |
|
Another point is, with LVM we may not even need to grow an existing partition, we can add a new partition and use pvcreate+vgextend to add it to the existing volume group without any disruption. It should in theory be easier/safer than doing this without LVM. Why the current approach uses resize With LVM, we may not need to resize partitions because What do you think of it? Which solution is better? |
6f2082b to
4c3f8d1
Compare
9e773a2 to
59ee3b0
Compare
Fixes canonicalGH-3265 This change integrates LVM logical volume resizing into the existing growpart flow. When the target block device is an LVM LV, cloud-init now: - Performs lvs lookup to determine the volume group - Enumerates all PVs in the VG using new helper functions - Intelligently handles pvresize: * Skips pvresize for single-PV VGs when using growpart (growpart already handles it via maybe_lvm_resize) * Resizes all PVs for multi-PV VGs (growpart only resizes the partition's PV) - Conditionally extends LV based on `resize_lv` config option * When `resize_lv: true` (default): lvextend +100%FREE on the LV * When `resize_lv: false`: Only resize PVs, leaving LV unchanged (useful for multi-LV setups where free space should be preserved) The `resize_lv` option is particularly useful when multiple LVs exist in the same VG (e.g., separate LVs for /home, /var/log, etc.), allowing users to preserve free space for other LVs. New helper functions: - `_get_vg_for_lv()`: Returns VG name for a given LV device - `_get_pvs_for_vg()`: Returns list of PV device paths for a VG Filesystem resizing is intentionally omitted because resizefs is handled by a separate module. Unit tests have been added under test_cc_growpart.py to validate: - successful pvresize and lvextend calls - error propagation for failed operations - resize_lv=False behavior - Single-PV VG with growpart (skip pvresize) - Multi-PV VG with growpart (resize all PVs) This change does not affect existing non-LVM growpart behaviour. Signed-off-by: Amy Chen <xiachen@redhat.com>
59ee3b0 to
0c716f0
Compare
|
I updated the code, the main change is, Enhance the LVM resize functionality with:
The Update schema, documentation, and add comprehensive tests. |
Fixes GH-3265
This change integrates LVM logical volume resizing into the existing
growpart flow. When the target block device is an LVM LV, cloud-init
now:
- Performs lvs lookup to determine the volume group
- Enumerates all PVs in the VG using new helper functions
- Intelligently handles pvresize:
* Skips pvresize for single-PV VGs when using growpart (growpart
already handles it via maybe_lvm_resize)
* Resizes all PVs for multi-PV VGs (growpart only resizes the
partition's PV)
- Conditionally extends LV based on
resize_lvconfig option* When
resize_lv: true(default): lvextend +100%FREE on the LV* When
resize_lv: false: Only resize PVs, leaving LV unchanged(useful for multi-LV setups where free space should be preserved)
The
resize_lvoption is particularly useful when multiple LVs existin the same VG (e.g., separate LVs for /home, /var/log, etc.), allowing
users to preserve free space for other LVs.
New helper functions:
-
_get_vg_for_lv(): Returns VG name for a given LV device-
_get_pvs_for_vg(): Returns list of PV device paths for a VGFilesystem resizing is intentionally omitted because resizefs is handled
by a separate module.
Unit tests have been added under test_cc_growpart.py to validate:
- successful pvresize and lvextend calls
- error propagation for failed operations
- resize_lv=False behavior
- Single-PV VG with growpart (skip pvresize)
- Multi-PV VG with growpart (resize all PVs)
This change does not affect existing non-LVM growpart behaviour.
Redhat Jira ticket: [RFE][Azure]growpart fails to resize a root partition on LVM
https://issues.redhat.com/browse/RHEL-107485
Test Steps
I tested it on Azure and AWS with rhel image, for example,
Boot up a VM on Azure and change the os disk to bigger than 64G
Check the rootlv partition size, the root partition size is extended after VM boot up.
[Before changes]
The root partition size is not extended.
[After changes]
The root partition size is extended.
I’ve added three unit tests. Please let me know if further coverage is needed.
The integration test case is copied from #887, however I did not test it because that I failed to set up the local integration test environment, I will run it later.