-
Notifications
You must be signed in to change notification settings - Fork 40k
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
cherry-pick into K8s 1.10: Always Start pvc-protection-controller and pv-protection-controller #62938
Conversation
StorageObjectInUseProtection feature is enabled by default in K8s 1.10+. Assume K8s cluster is used with this feature enabled, i.e. finalizers are added to all PVs and PVCs. In case the K8s cluster admin disables the StorageObjectInUseProtection feature and a user deletes a PVC that is not in active use by a pod then the PVC is not removed from the system because of the finalizer. Therefore, the user will have to remove the finalizer manually in order to have the PVC removed from the system. Note: deleted PVs won't be removed from the system also because of finalizers. That's why pvc-protection-controller is always started because the pvc-protection-controller removes finalizers from PVCs automatically when a PVC is not in active use by a pod. Also the pv-protection-controller is always started to remove finalizers from PVs automatically when a PV is not Bound to a PVC. Related issue: kubernetes#60764 Related PRs: kubernetes#61370 kubernetes#61324
/sig storage |
/retest |
/lgtm |
@pospispa Is that identical to #61324 or was some custom change required for 1.10? If it's identical next time please use the hack/cherry_pick_pull.sh to create cherry-pick (no need to change this one). @deads2k Can you approve? Since there was some discussion on whether this should be cherry-picked on original PR I will defer with adding cherrypick-approved until it's approved. |
@MaciekPytel yes, this PR is identical to #61324 I created the cherry-pick manually because I didn't know about |
@deads2k Can you approve (or state that you don't approve)? This needs to merge by tomorrow EOD to make it to 1.10.3. |
/retest |
1 similar comment
/retest |
From sig-storage: /approve |
Assigning to 1.10 branch manager @MaciekPytel |
@deads2k Can you approve? |
/retest |
/approve |
/lgtm |
[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process @BenTheElder @MaciekPytel @msau42 @pospispa Pull Request Labels
|
update: it seems that @mikedanese is only an approver for this on master / 1.11 but the approvers mechanism checks against the target branch, so potentially we should cherry pick back approver changes... :| |
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, liggitt, mikedanese, msau42, pospispa, saad-ali 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 |
/test all [submit-queue is verifying that this PR is safe to merge] |
/retest |
Automatic merge from submit-queue. |
What this PR does / why we need it:
StorageObjectInUseProtection feature is enabled by default in K8s 1.10+. Assume K8s cluster is used with this feature enabled, i.e. finalizers are added to all PVs and PVCs. In case the K8s cluster admin disables the StorageObjectInUseProtection feature and a user deletes a PVC that is not in active use by a pod then the PVC is not removed from the system because of the finalizer. Therefore, the user will have to remove the finalizer manually in order to have the PVC removed from the system. Note: deleted PVs won't be removed from the system also because of finalizers.
This problem was fixed in K8s 1.9.6 in PR #61370
This problem is also fixed in K8s 1.11+ in PR #61324
However, this problem is not fixed in K8s 1.10, that's why I've cherry-picked the PR #61324 and proposing to merge it into K8s 1.10.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes
N/A
Related issue: #60764
Special notes for your reviewer:
Release note: