Cleanup PVC finalizers added by cnsnodevmattachment #3837
Conversation
k8s-ci-robot
added
approved
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
SUCCESS --- Jenkins Build #845 |
|
Triggering CSI-TKG Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
SUCCESS --- Jenkins Build #793 |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chethanv28, xing-yang The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| for namespace := range orphanPVCs { | ||
| nodeAttachList := nodeattachv1a1.CnsNodeVmAttachmentList{} | ||
| err = cnsClient.List(ctx, &nodeAttachList, ctrlclient.InNamespace(namespace)) | ||
| if err != nil { |
There was a problem hiding this comment.
The code skips cleanup for an entire namespace if ANY CnsNodeVmAttachment CR exists. This means truly orphaned PVCs will never be cleaned up as long as there's at least one CR in the namespace.
| // map to hold all the PVCs that are orphaned by CnsNodeVmAttachment reconciler. | ||
| // structure of the map follows the logical grouping kubernetes uses. | ||
| // namespace -> pvc -> struct{}{} | ||
| orphanPVCs := make(map[string]map[string]struct{}) |
There was a problem hiding this comment.
Using map[string]map[string]struct{} loses the PVC object reference, which could be useful for additional validation.
| // All the PVCs that are remaining in the map are eligible orphans whose finalizer can be removed. | ||
| for namespace, pvcs := range orphanPVCs { | ||
| for name := range pvcs { | ||
| err := k8s.RemoveFinalizerFromPVC(ctx, c, name, namespace, cnsoperatortypes.CNSPvcFinalizer) |
There was a problem hiding this comment.
we need to make sure this routine is not having race with clean up routine in batchattach
divyenpatel
left a comment
divyenpatel
left a comment
There was a problem hiding this comment.
Can you address review comments?
5 participants
What this PR does / why we need it:
There have ben issues reported where, after a namespace deletion operation, Supervisor PVCs remain in terminating state due to CnsNodeVmAttachment CR getting deleted quickly & the PVC protection finalizer removal step is skipped. Although the occurrence of this issue is sporadic, adding a cleanup routine to remove finalizers from stale Supervisor PVCs reclaims space.
Hence, this PR is adding a routine to Cleanup PVC finalizers added by cnsnodevmattachment on Supervisor PVCs, by periodically checking for such orphan Supervisor PVCs which remain in terminating state.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Testing done:
WCP Pre-checkin pipeline : https://jenkins-vcf-csifvt.devops.broadcom.net/job/wcp-instapp-e2e-pre-checkin/845/
VKS Pre-checkin pipeline: https://jenkins-vcf-csifvt.devops.broadcom.net/job/vks-instapp-e2e-pre-checkin/793/
Manual testing details:
Created a Supervisor PVC & added the cns pvc protection finalizer.
Deleted the PVC and verified it remains in terminating state.
Wait for few minutes and verify that finalizer gets removed & PVC gets deleted successfully
Special notes for your reviewer:
Release note: