Adding validation checks for Bookkeeper Deployments #598
Conversation
Signed-off-by: Nishant Gupta <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #598 +/- ##
==========================================
+ Coverage 74.88% 75.03% +0.15%
==========================================
Files 16 16
Lines 4428 4479 +51
==========================================
+ Hits 3316 3361 +45
- Misses 978 984 +6
Partials 134 134
Continue to review full report at Codecov.
|
|
|
||
| if ensembleSizeInt < writeQuorumSizeInt { | ||
| if ensembleSize == "" { | ||
| return fmt.Errorf("The value provided for the option bookkeeper.write.quorum.size should be less than or equal to the default value of option bookkeeper.ensemble.size which is 3") |
There was a problem hiding this comment.
The message is a bit confusing (the user might have forgotten to set bookkeeper.ensemble.size altogether).
Maybe change it to something like this:
"The value provided for the option bookkeeper.write.quorum.size should be less than or equal to the value of option bookkeeper.ensemble.size (default is 3)"
Then the user would be aware of the need to either set the ensemble size explicitly or coordinate the quorum size with the default.
| if ackQuorumSize == "" { | ||
| return fmt.Errorf("The value provided for the option bookkeeper.write.quorum.size should be greater than or equal to the default value of bookkeeper.ack.quorum.size which is 3") | ||
| } | ||
| return fmt.Errorf("The value provided for the option bookkeeper.ack.quorum.size should less than or equal to the value of option bookkeeper.write.quorum.size") |
There was a problem hiding this comment.
(nit) should less than -> should be less than
Signed-off-by: Nishant Gupta <[email protected]>
| @@ -383,6 +383,21 @@ func (s *PravegaSpec) withDefaults() (changed bool) { | |||
| s.Options = map[string]string{} | |||
| } | |||
|
|
|||
| if s.Options["bookkeeper.ensemble.size"] == "" { | |||
There was a problem hiding this comment.
@nishant-yt since we are making all the 3 fields mandatory, will it enter this code flow
There was a problem hiding this comment.
@anishakj all the fields are not mandatory. If the user doesn't provide value for any of these options, we are setting it to 3 by default.
One of instances when it will enter the above mentioned code flow is when the user doesn't provide any value for bookkeeper.ensemble.size bookkeeper.write.quorum.size and bookkeeper.ack.quorum.size
Signed-off-by: Nishant Gupta [email protected]
Change log description
When deploying the pravega cluster, the options
bookkeeper.ensemble.sizebookkeeper.write.quorum.sizebookkeeper.ack.quorum.sizeshould be set such that the following condition is satisfied:bookkeeper.ensemble.size >= bookkeeper.write.quorum.size >= bookkeeper.ack.quorum.sizePurpose of the change
Fixes #587
What the code does
The code preforms the following checks :
bookkeeper.ensemble.size >= bookkeeper.write.quorum.size >= bookkeeper.ack.quorum.sizebookkeeper.ensemble.size == 1thenbookkeeper.write.quorum.racks.minimumCount.enableshould be set tofalseHow to verify it
Install a pravega cluster with different values of
bookkeeper.ensemble.sizebookkeeper.write.quorum.sizebookkeeper.ack.quorum.sizeand in case the above mentioned condition in not met , then an error is retuned by the validating webhook.In case the values adheres to the above condition say
bookkeeper.ensemble.size=4bookkeeper.write.quorum.size=3bookkeeper.ack.quorum.size=3then post installation of the pravega cluster if you go inside Controller/ Segment Store Pod then all these values should be available in the env variableJAVA_OPTSas