Skip to content
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

fix: skip in-use loop devices during vgcreate/vgextend #113

Merged
merged 1 commit into from
Mar 1, 2022
Merged

fix: skip in-use loop devices during vgcreate/vgextend #113

merged 1 commit into from
Mar 1, 2022

Conversation

leelavg
Copy link
Contributor

@leelavg leelavg commented Feb 17, 2022

  • filter out loop devices based on Major number from lsblk
  • check whether the back-file is being used by k8s for mounting block
    devices and skip using that loop device in volume groups ops

Fixes: #97

Signed-off-by: Leela Venkaiah G [email protected]

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 17, 2022
@leelavg
Copy link
Contributor Author

leelavg commented Feb 17, 2022

  • well, the fix itself is working, just felt that FilterMap got kinda ugly and want to test some more cases, so made PR as draft
  • however PR is re-viewable, below is the log showing how devices are skipped if they are mounted by k8s
{"level":"info","ts":1645098272.500616,"logger":"controller.lvmvolumegroup.vg-manager","msg":"does not match filter","reconciler group":"lvm.topolvm.io","reconciler kind":"LVMVolumeGroup","name":"test","namespace":"lvm-operator-system","Device.Name":"loop1","filter.Name":"usableLoopDevice"}
{"level":"info","ts":1645098272.5372882,"logger":"controller.lvmvolumegroup.vg-manager","msg":"does not match filter","reconciler group":"lvm.topolvm.io","reconciler kind":"LVMVolumeGroup","name":"test","namespace":"lvm-operator-system","Device.Name":"loop2","filter.Name":"usableLoopDevice"}
{"level":"info","ts":1645098272.5835214,"logger":"controller.lvmvolumegroup.vg-manager","msg":"does not match filter","reconciler group":"lvm.topolvm.io","reconciler kind":"LVMVolumeGroup","name":"test","namespace":"lvm-operator-system","Device.Name":"loop3","filter.Name":"usableLoopDevice"}

@leelavg leelavg requested review from sp98 and nbalacha and removed request for sp98 February 17, 2022 11:57
@leelavg
Copy link
Contributor Author

leelavg commented Feb 22, 2022

  • tested locally
# before restart of vg-manager
sh-4.4# vgs
  VG   #PV #LV #SN Attr   VSize  VFree 
  test   3  10   0 wz--n- <4.37t <3.39t
sh-4.4# pvs
  PV           VG   Fmt  Attr PSize  PFree  
  /dev/nvme0n1 test lvm2 a--  <1.46t 490.41g
  /dev/nvme1n1 test lvm2 a--  <1.46t  <1.46t
  /dev/nvme2n1 test lvm2 a--  <1.46t  <1.46t

# after restart of vg-manager with 5 in-use loop devices and 1 usable loop devices and so vg is extended correctly
sh-4.4# pvs  
  PV           VG   Fmt  Attr PSize  PFree  
  /dev/loop5   test lvm2 a--  <5.00g  <5.00g
  /dev/nvme0n1 test lvm2 a--  <1.46t 490.41g
  /dev/nvme1n1 test lvm2 a--  <1.46t  <1.46t
  /dev/nvme2n1 test lvm2 a--  <1.46t  <1.46t
sh-4.4# vgs
  VG   #PV #LV #SN Attr   VSize VFree
  test   4  10   0 wz--n- 4.37t 3.39t

sh-4.4# lsblk
NAME                                            MAJ:MIN RM   SIZE RO TYPE MOUNTPOINT
loop0                                             7:0    0   100G  0 loop 
loop1                                             7:1    0   100G  0 loop 
loop2                                             7:2    0   100G  0 loop 
loop3                                             7:3    0   100G  0 loop 
loop4                                             7:4    0   100G  0 loop 
loop5                                             7:5    0     5G  0 loop 
sdb                                               8:16   0 893.8G  0 disk 
|-sdb1                                            8:17   0     1M  0 part 
|-sdb2                                            8:18   0   127M  0 part 
|-sdb3                                            8:19   0   384M  0 part /boot
`-sdb4                                            8:20   0 893.3G  0 part /sysroot
sr0                                              11:0    1   987M  0 rom  
nvme0n1                                         259:0    0   1.5T  0 disk 
|-test-6777f3ec--40de--4e8d--ac55--d32b71f7819a 253:0    0   100G  0 lvm  
|-test-ca957a21--0ae2--4b05--9e14--2896b38a0fd7 253:1    0   100G  0 lvm  
|-test-7901dd21--4942--4f1c--945c--7e5d23e49d01 253:2    0   100G  0 lvm  
|-test-32d72da7--580f--4c59--a9ef--5eed39ff139e 253:3    0   100G  0 lvm  
|-test-3e617cc8--c26f--40ac--9460--214421330e7a 253:4    0   100G  0 lvm  
|-test-6251c743--7326--4c96--9724--3eb34a667087 253:5    0   100G  0 lvm  
|-test-0681a68f--94bb--484d--add5--2f72e2859978 253:6    0   100G  0 lvm  
|-test-cfd826d7--cc8f--4ea8--9dbe--3b27904671c5 253:7    0   100G  0 lvm  
|-test-9d4aa183--72fc--441a--a577--59e4ff4c0f94 253:8    0   100G  0 lvm  
`-test-9cd06305--96a3--4b90--8080--01f45b0fa4e0 253:9    0   100G  0 lvm  
nvme1n1                                         259:1    0   1.5T  0 disk 
nvme2n1                                         259:2    0   1.5T  0 disk 

@leelavg leelavg marked this pull request as ready for review February 22, 2022 05:38
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 22, 2022
Copy link
Contributor

@sp98 sp98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2022
@leelavg
Copy link
Contributor Author

leelavg commented Feb 23, 2022

  • Tested again below scenarios
  1. An usable loop devices exists before vg-manager comes up and reboot vg-manager without creating block PVCs
  2. An usable and unusable loop devices exist after vg-manager reboot
  3. General, no loop devices before vg-manager comes up

@leelavg leelavg requested a review from nbalacha February 24, 2022 15:04
- filter out loop devices based on Major number from lsblk
- check whether the back-file is being used by k8s for mounting block
  devices and skip using that loop device in volume groups ops

Fixes: #97

Signed-off-by: Leela Venkaiah G <[email protected]>
@leelavg
Copy link
Contributor Author

leelavg commented Feb 28, 2022

  • btw, losetup already exists in the operator due to installation of util-linux

@leelavg leelavg requested a review from nbalacha February 28, 2022 08:47
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nbalacha, sp98

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 1, 2022
@nbalacha nbalacha merged commit 0cede65 into openshift:main Mar 1, 2022
@leelavg
Copy link
Contributor Author

leelavg commented Mar 1, 2022

/cherry-pick release-4.10

@openshift-cherrypick-robot

@leelavg: new pull request created: #122

In response to this:

/cherry-pick release-4.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do not use block devices created by topolvm in vg-manager
4 participants