Backport pv-protection-controller Finalizer Removal Part#61316
Closed
pospispa wants to merge 2 commits intokubernetes:release-1.9from
Closed
Backport pv-protection-controller Finalizer Removal Part#61316pospispa wants to merge 2 commits intokubernetes:release-1.9from
pospispa wants to merge 2 commits intokubernetes:release-1.9from
Conversation
After K8s 1.9 is upgraded to K8s 1.10 finalizer [kubernetes.io/pvc-protection] is added to PVCs because StorageObjectInUseProtection feature is enabled by default in K8s 1.10. However, when K8s 1.10 is downgraded to K8s 1.9 the finalizers remain in the PVCs and as pvc-protection-controller is not started by default in K8s 1.9 finalizers are not removed automatically from deleted PVCs and that's why deleted PVC are not removed but remain in Terminating phase. 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. Related issue: kubernetes#60764
After K8s 1.9 is upgraded to K8s 1.10 finalizer [kubernetes.io/pv-protection] is added to PVs because StorageObjectInUseProtection feature is enabled by default in K8s 1.10. However, when K8s 1.10 is downgraded to K8s 1.9 the finalizers remain in the PVs and as pv-protection-controller does not exist in K8s 1.9 PV finalizers are not removed automatically from deleted PVs and that's why deleted PV remain in the system. That's why the finalizer removing part of the pv-protection-controller is backported from K8s 1.10 in order to remove finalizers automatically when a PV is deleted and is not Bound to a PVC. Related issue: kubernetes#60764 Related pv-protection-controller PR: kubernetes#58743
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pospispa Assign the PR to them by writing 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 |
This was referenced Mar 17, 2018
gnufied
reviewed
Mar 19, 2018
| // it's not. | ||
| isUsed := c.isBeingUsed(pv) | ||
| if !isUsed { | ||
| return c.removeFinalizer(pv) |
Member
There was a problem hiding this comment.
Same as before - if the feature flag is disabled then isUsed by default should be true right?
gnufied
reviewed
Mar 19, 2018
| ctx.InformerFactory.Core().V1().Pods(), | ||
| ctx.ClientBuilder.ClientOrDie("pvc-protection-controller"), | ||
| ).Run(1, ctx.Stop) | ||
| return true, nil |
Member
There was a problem hiding this comment.
Did this bit come from #61282 ? Shouldn't this PR be just about PV protection?
gnufied
reviewed
Mar 19, 2018
| glog.V(4).Infof("Finished processing PV %s (%v)", pvName, time.Now().Sub(startTime)) | ||
| }() | ||
|
|
||
| pv, err := c.pvLister.Get(pvName) |
Member
There was a problem hiding this comment.
So this controller behaves different from pvc protection controller. While pvc controller adds the finalizer during update to old PVCs (just to be sure), this one doesn't. Is that expected?
Contributor
|
/hold |
|
This PR is not for the master branch but does not have the |
Contributor
Author
|
Replaced by PR #61370 so I'm closing this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
After K8s 1.9 is upgraded to K8s 1.10 finalizer
[kubernetes.io/pv-protection]is added to PVs becauseStorageObjectInUseProtectionfeature is enabled by default in K8s 1.10. However, when K8s 1.10 is downgraded to K8s 1.9 the finalizers remain in the PVs and aspv-protection-controllerdoes not exist in K8s 1.9 PV finalizers are not removed automatically from deleted PVs and that's why deleted PVs remain in the system.That's why the finalizer removing part of the
pv-protection-controlleris backported from K8s 1.10 in order to remove finalizers automatically when a PV is deleted and is not Bound to a PVC.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 #60764
Special notes for your reviewer:
The PVC finalizer removal part of the #60764 is addressed in the PR #61282
This PR is based on the PR #61282
This is backport of the PV finalizer removal part of the PR #58743
Release note: