Issue 154: BK Operator should support multiple ledger PVCs, multiple journal PVCs and multiple index PVCs for 1 bookie #155
Conversation
Codecov Report
@@ Coverage Diff @@
## master #155 +/- ##
==========================================
+ Coverage 81.42% 82.00% +0.57%
==========================================
Files 9 9
Lines 1588 1639 +51
==========================================
+ Hits 1293 1344 +51
Misses 229 229
Partials 66 66
Continue to review full report at Codecov.
|
| var multiVolumesSetPerBookieEnabled bool | ||
| var ok bool | ||
|
|
||
| if _, ok = bk.Spec.Options["ledgerDirectories"]; ok { |
There was a problem hiding this comment.
Lines 407-428 might not be required, if we are making multivolume support to false . Could you please verify that?
There was a problem hiding this comment.
Lines 407-428 might not be required, if we are making multivolume support to
false. Could you please verify that?
OK. I will optimize this part according to your suggestion.
There was a problem hiding this comment.
@CraneShiEMC the changes look good.
I would however request you to increase code coverage and add documentation to explain how this option multiVolumesSetPerBookieEnabled should be configured to get the desired behaviour. Also please rebase your code (that would fix the end to end tests as well)
Could u also add a separate PR in the charts repo, inside the bookkeeper charts, wherein the option multiVolumesSetPerBookieEnabled has also been added inside the bookkeeper options field with its default value (false) ?
OK! I will change the code according to your comments |
|
Please, add a validation that |
Change log description
BK Operator should support multiple ledger PVCs, multiple journal PVCs and multiple index PVCs for 1 bookie.
Now, if we set flag option multiVolumesSetPerBookieEnabled to true and we provide multiple ledger directories in
ledgerDirectoriesoptions, for example,ledgerDirectories: "/bk/ledgers/l0,/bk/ledgers/l1,/bk/ledgers/l2,/bk/ledgers/l3", for each ledger directory such as/bk/ledgers/l0, bk operator will create 1 PVC with VolumeMount mount to this ledger directory. So doesjournalDirectoriesandindexDirectoriesoptions.By default, i.e. we don't set flag option multiVolumesSetPerBookieEnabled or we set this option to false, the bookkeeper operator will still have original behavior, i.e. 1 PVC with multiple volume mounts for multiple directories will be used.
Purpose of the change
Fixes #154
What the code does
Now, if we set flag option multiVolumesSetPerBookieEnabled to true and provide multiple ledger directories in
ledgerDirectoriesoptions, for example,ledgerDirectories: "/bk/ledgers/l0,/bk/ledgers/l1,/bk/ledgers/l2,/bk/ledgers/l3", for each ledger directory such as/bk/ledgers/l0, bk operator will create 1 PVC with VolumeMount mount to this ledger directory. So doesjournalDirectoriesandindexDirectoriesoptions.By default, i.e. we don't set flag option multiVolumesSetPerBookieEnabled or we set this option to false, the bookkeeper operator will still have original behavior, i.e. 1 PVC with multiple volume mounts for multiple directories will be used.
How to verify it
Provide multiple directories for
ledgerDirectories,journalDirectoriesorindexDirectoriesoptions and set flag option multiVolumesSetPerBookieEnabled to true while deploying bookies. Then, verify whether multiple PVCs for ledgers, journals or indices has been bound to each bookie as expected, one PVC mount to one directory.In our ObjectScale deployment with Pravega, bookie's multiple ledger PVCs, journal PVCs, index PVCs has been created and mount to multiple directories as expected. Also bookie has functioned normally and the IO can be balanced between multiple PVCs.
In addition, we have verified the negative test cases without multiVolumesSetPerBookieEnabled set and the bookie has original behavior as expected.