fix: Fix PV grow_to_fill feature

[vojtechtrefny]

The issues with the PV grow feature: the code doesn't correctly set the grow_to_fill property for blivet and uses VG size instead of the underlying block device size for the target size for the resize action. I've fixed these and adjusted tests to actually check the PV size after resize without allowing for difference bigger than 4 MiB (one physical extent for metadata and padding) and to cover the customer issue (resizing both the PV format and LV).

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 10.73%. Comparing base (59fd1c6) to head (f085afd).
Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
library/blivet.py	0.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (59fd1c6) and HEAD (f085afd). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (59fd1c6)	HEAD (f085afd)
sanity	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
- Coverage   16.54%   10.73%   -5.81%     
==========================================
  Files           2        8       +6     
  Lines         284     1946    +1662     
  Branches       79        0      -79     
==========================================
+ Hits           47      209     +162     
- Misses        237     1737    +1500     

Flag	Coverage Δ
sanity	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[vojtechtrefny]

[citest]

[vojtechtrefny]

[citest]

[richm]

You'll also need to make this change:

--- a/tests/test-verify-pool-members.yml
+++ b/tests/test-verify-pool-members.yml
@@ -78,6 +78,7 @@
     executable: "{{ ansible_python.executable }}"
   register: grow_supported
   changed_when: false
+  failed_when: grow_supported.rc not in [0, 1]
 
 - name: Verify that PVs fill the whole devices when they should
   include_tasks: verify-pool-member-pvsize.yml

because when the script returns rc 1, Ansible will think the script failed, and will exit. This is probably why Jan coded it with a string return value and exit with 0 every time. You could also revert back to the way Jan coded it.

[richm]

@guazhangRH Can you try this out so we can see if this is going to cause problems with some of the devices and large disk sizes you use that typically show problems with this sort of check?

[guazhangRH]

test pass with tests_lvm_pool_pv_grow_nvme_generated.yml with 1T nvme test server.

ansible-playbook -i host  -vv tests_lvm_pool_pv_grow_nvme_generated.yml 

[root@smicro-s110p-01 ~]# lsblk
NAME                            MAJ:MIN RM   SIZE RO TYPE MOUNTPOINTS
sda                               8:0    0 447.1G  0 disk 
├─sda1                            8:1    0   600M  0 part /boot/efi
├─sda2                            8:2    0     1G  0 part /boot
└─sda3                            8:3    0 445.5G  0 part 
  ├─rhel_smicro--s110p--01-root 253:0    0    70G  0 lvm  /
  ├─rhel_smicro--s110p--01-swap 253:1    0  31.3G  0 lvm  [SWAP]
  └─rhel_smicro--s110p--01-home 253:2    0 344.3G  0 lvm  /home
nvme4n1                         259:0    0 931.5G  0 disk 
nvme0n1                         259:1    0 931.5G  0 disk 
nvme5n1                         259:5    0 931.5G  0 disk 
nvme3n1                         259:6    0 931.5G  0 disk 
nvme1n1                         259:7    0 931.5G  0 disk 
nvme2n1                         259:8    0 931.5G  0 disk 
[root@smicro-s110p-01 ~]# 

[vojtechtrefny]

Thank you for testing. Using the PE start value should be safe here -- that's the part of the VG metadata size which is affected by hardware specific things things like optimal IO size.

[vojtechtrefny]

You'll also need to make this change:

--- a/tests/test-verify-pool-members.yml
+++ b/tests/test-verify-pool-members.yml
@@ -78,6 +78,7 @@
     executable: "{{ ansible_python.executable }}"
   register: grow_supported
   changed_when: false
+  failed_when: grow_supported.rc not in [0, 1]
 
 - name: Verify that PVs fill the whole devices when they should
   include_tasks: verify-pool-member-pvsize.yml

because when the script returns rc 1, Ansible will think the script failed, and will exit. This is probably why Jan coded it with a string return value and exit with 0 every time. You could also revert back to the way Jan coded it.

Thanks, I was wondering why it was written to check the output, I should've realized that a non-zero return code will simply fail the entire execution. Unfortunately on both C9S and C10S the test is now skipped because of some noise on stdout (I was surprised that Python deperecation warnings are printed to stdout and not stderr).

[vojtechtrefny]

[citest]