Issue 94: Fix for zk scale up problem #96
Conversation
Signed-off-by: pbelgundi <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #96 +/- ##
=========================================
- Coverage 49.26% 47.8% -1.46%
=========================================
Files 5 5
Lines 885 912 +27
=========================================
Hits 436 436
- Misses 421 448 +27
Partials 28 28
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
As mentioned inline, I'm of the opinion that the PVCs should be retained by default, and deleted explicitly. This matches the behavior of a StatefulSet, and, IMO, makes better sense.
Other than that small change, and a couple of small comments, this looks good!
Thanks!
Signed-off-by: pbelgundi <[email protected]>
| err = r.client.Delete(context.TODO(), pvcDelete) | ||
| if err != nil { | ||
| return err | ||
| if all { |
There was a problem hiding this comment.
A conditional switch statement can be used to clean up this nested if / else block.
There was a problem hiding this comment.
Could we instead have separate functions for clean all and clean orphan? I find the separation of functionality based on a flag error prone. We can avoid the duplication of code by having a common support function.
There was a problem hiding this comment.
Sure. Will do this.
| err = r.client.Delete(context.TODO(), pvcDelete) | ||
| if err != nil { | ||
| return err | ||
| if all { |
There was a problem hiding this comment.
Could we instead have separate functions for clean all and clean orphan? I find the separation of functionality based on a flag error prone. We can avoid the duplication of code by having a common support function.
Signed-off-by: pbelgundi <[email protected]>
Signed-off-by: pbelgundi <[email protected]>
Signed-off-by: pbelgundi <[email protected]>
* Issue 94: Fix for zk scale up problem Signed-off-by: pbelgundi <[email protected]> * code review comments Signed-off-by: pbelgundi <[email protected]> * code refactor for review comments Signed-off-by: pbelgundi <[email protected]> * Copyright notice fix Signed-off-by: pbelgundi <[email protected]> * fixed formatting Signed-off-by: pbelgundi <[email protected]>
Signed-off-by: pbelgundi [email protected]
Change log description
This issue occurs because on scale down PVCs attached to the scaled down servers are not deleted.
The fix is about deleting orphaned PVCs once a cluster has been scaled down.
For this fix to take effect
VolumeReclaimPolicyin zookeeperSpec needs to be set toDeletePurpose of the change
Fixes #94
How to verify it
Verified by scaling the cluster up and down 5 times after this fix and there were no issues with cluster scale and pod restarts.
Also PVCs attached to old pods were deleted after the scale down.