Skip PVCs with key rotation annotation when SC is changed #637
Conversation
Signed-off-by: Niraj Yadav <[email protected]>
mergify
bot
requested review from
iPraveenParihar,
Madhu-1,
nixpanic,
Rakshith-R and
yati1998
| @@ -317,6 +318,9 @@ func (r *PersistentVolumeClaimReconciler) storageClassEventHandler() handler.Eve | |||
| if _, ok := pvc.GetAnnotations()[rsCronJobScheduleTimeAnnotation]; ok { | |||
| continue | |||
| } | |||
| if _, ok := pvc.GetAnnotations()[krcJobScheduleTimeAnnotation]; ok { | |||
There was a problem hiding this comment.
What if
- A storageclass already has rsCronJob annotation
- PVCs already have rsCronJob annotation
- krcJob annotation is added to that same storageclass
I think we need a more elaborate check here which also considers annotations on chnaged storageclass.
|
I think the following does not result in the expected annotation:
It would be expected that the PVC in step (4) will get the updated annotation too. You could add an annotation like |
|
Looking at the edge cases, what if we do not skip the reconcile here and delegate the decision to the predicate functions? We have predicates in place for both SC and PVCs that only reconcile when the value of the relevant annotations has been updated. @iPraveenParihar WDYT? |
I've been working on a similar approach for ReclaimSpace #626. I've been occupied with other priority tasks, but I'll get back to it as soon as I have some time.
@black-dragon74, from SC predicate you can't enqueue PV reconcile requests. |
Yes. I get that. But predicate is run before the reconcile happens, no? |
|
@black-dragon74 Is this still required? |
|
Closing this one, I will follow this up in another PR with a different approach. |
Existing PVCs with keyoration annotation are enqueued when a SC object is modified.
This patch skips reconcile of such PVCs
Closes: #636