-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support adding external finalizer for static provisioned volumes #129
Support adding external finalizer for static provisioned volumes #129
Conversation
controller/controller.go
Outdated
volumeFinalizers, modified = addFinalizer(volumeFinalizers, finalizerPV) | ||
} | ||
// For statically provisioned volumes remove the external finalizer if the volume is not in a Bound state | ||
if volume.Status.Phase != v1.VolumeBound { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(starting a separate "thread")
If you remove this check, then the code for dynamic and static provisioning will be very similar. I wonder if there is room for improvement and removing the special case. Like if ctrl.addFinalizer && reclaimPolicy == v1.PersistentVolumeReclaimDelete && volume.DeletionTimestamp == nil && (isDynamic || phase == Bound)
Signed-off-by: Deepak Kinni <[email protected]>
5182976
to
ee0d78c
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepakkinni, jsafrane The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Support adding the new finalizer
external-provisioner.volume.kubernetes.io/finalizer
on statically provisioned volumesThe finalizer will be added to the following volumes:
For (2) and (3), the finalizer will be added only if the volume is in a
Bound
state and removed if the volume changes to any other state apart fromBound
Signed-off-by: Deepak Kinni [email protected]