Delay removal of orphanPVC to avoid the removal of PVC in use #67
Conversation
|
@gurjotkaur20 this is great ! The only thing which i might want to do is add a finalizer instead of labels ( cc @avtarOPS ) Regardless, ill take look at it. |
controllers/druid/handler.go
Outdated
| return err | ||
|
|
||
| if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { | ||
| deletionTS := pvc.GetLabels()["deletionTimestamp"] |
There was a problem hiding this comment.
we can change the label name as the same label is used by the finalizer.
| err := writers.Delete(ctx, sdk, drd, pvcList[i], emitEvents, &client.DeleteAllOfOptions{}) | ||
| if err != nil { | ||
| return err | ||
|
|
There was a problem hiding this comment.
This became way too big, we should refactor this, separating it into functions
controllers/druid/handler.go
Outdated
| if err != nil { | ||
| return err | ||
|
|
||
| if _, ok := pvc.GetLabels()["toBeDeleted"]; ok { |
There was a problem hiding this comment.
This and the below should be consts
|
I have refactored the code and updated the label name. |
Fine by me :) |
controllers/druid/handler.go
Outdated
| } else { | ||
| // do nothing | ||
| } |
There was a problem hiding this comment.
why do we need this block ?
controllers/druid/handler.go
Outdated
| } else { | ||
| // do nothing | ||
| } |
There was a problem hiding this comment.
same, why do we need this block ? can return nil ?
There was a problem hiding this comment.
same, why do we need this block ? can return nil ?
Done changes for both else blocks.
gurjotkaur20
force-pushed
the
master
branch
2 times, most recently
from
d6a50d2
to
7212552
Compare
Fixes #32
Description
Possibly a race condition is causing removal of PVC in use.
The solution in the PR: Added two labels
toBeDeletedanddeletionTimestampto mark the pvc for deletion. Do not delete the pvc right away, wait for 60s to delete it and avoid the race condition.This PR has:
Key changed/added files in this PR
controllers/druid/handler.gocontrollers/druid/util.go