Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Postpone Deletion of a Persistent Volume Claim in case It Is Used by a Pod #55824

Merged
merged 5 commits into from
Nov 23, 2017
Merged

Postpone Deletion of a Persistent Volume Claim in case It Is Used by a Pod #55824

merged 5 commits into from
Nov 23, 2017

Conversation

pospispa
Copy link
Contributor

What this PR does / why we need it:
to fix #45143

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 #45143

Special notes for your reviewer:
Design: https://github.com/kubernetes/community/blob/master/contributors/design-proposals/storage/postpone-pvc-deletion-if-used-in-a-pod.md
@msau42 @jsafrane @gnufied PTAL

Release note:

PVC Finalizing Controller is introduced in order to prevent deletion of a PVC that is being used by a pod.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 15, 2017
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2017
@childsb childsb added this to the v1.9 milestone Nov 15, 2017
@dims
Copy link
Member

dims commented Nov 16, 2017

/sig storage

@pospispa can you please rebase?

@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Nov 16, 2017
@jsafrane
Copy link
Member

/kind feature
/priority important-soon

I'll take the PR from here and rebase & update it.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 16, 2017
@jsafrane
Copy link
Member

Approved 1.9 milestone.

@jsafrane jsafrane changed the title Postpone Deletion of a Persistent Volume Claim in case It Is Used by a Pod WIP: Postpone Deletion of a Persistent Volume Claim in case It Is Used by a Pod Nov 16, 2017
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2017
@jsafrane
Copy link
Member

Marking as WIP, this needs more than a rebase.

@jsafrane jsafrane force-pushed the 566-postpone-pvc-deletion-if-used-in-a-pod branch from 321ff39 to 71c986e Compare November 16, 2017 09:37
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 16, 2017
@jsafrane
Copy link
Member

created #55873 just with kubectl changes

@jsafrane jsafrane force-pushed the 566-postpone-pvc-deletion-if-used-in-a-pod branch from 71c986e to b83c4b4 Compare November 16, 2017 16:07
@jsafrane
Copy link
Member

  • removed scheduler predicate, will fill a separate PR
  • missing unit test in the last commit, the rest is reviewable

@jsafrane jsafrane force-pushed the 566-postpone-pvc-deletion-if-used-in-a-pod branch from b83c4b4 to 36b00cd Compare November 16, 2017 16:10
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2017
@jsafrane jsafrane force-pushed the 566-postpone-pvc-deletion-if-used-in-a-pod branch from 36b00cd to ed4b61b Compare November 17, 2017 13:37
@k8s-ci-robot k8s-ci-robot removed the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 17, 2017
@pospispa pospispa mentioned this pull request Dec 7, 2017
Merged
pospispa pushed a commit to pospispa/test-infra that referenced this pull request Dec 7, 2017
Test PVC Protection in ci-kubernetes-e2e-gci-gce-alpha-features
b883fc5 
PVC Protection alpha feature was merged in PRs:
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55873

PVC Protection E2E tests are in PR: kubernetes/kubernetes#56931

Adding the PVC Protection tests into the list of Alpha feature tests that are run.
@pospispa pospispa mentioned this pull request Dec 7, 2017
Merged
k8s-github-robot pushed a commit that referenced this pull request Dec 16, 2017
Merge pull request #56298 from pospispa/566-improvements-suggested-by…
3abbd6f 
…-thockin-during-review-of-PR55824

Automatic merge from submit-queue (batch tested with PRs 56401, 56506, 56551, 56298, 56581). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Addressing Comments from Code Review

**What this PR does / why we need it**: addressing comments from code review: #55824 (review)

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: N/A


**Special notes for your reviewer**:
@thockin @jsafrane @msau42 PTAL

**Release note**:

```release-note
NONE
```
pospispa pushed a commit to pospispa/kubernetes that referenced this pull request Dec 18, 2017
PVC Protection Alpha Feature E2E Tests
2e4c4b2 
PVC Protection alpha feature was introduced in PRs:
- kubernetes#55824
- kubernetes#55873

That's why E2E tests for this feature are added.
k8s-github-robot pushed a commit that referenced this pull request Dec 18, 2017
Merge pull request #56931 from pospispa/566-postpone-pvc-deletion-if-…
dcee5ce 
…used-in-a-pod-e2e-tests

Automatic merge from submit-queue (batch tested with PRs 57324, 56931, 57000, 57150, 56965). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

PVC Protection Alpha Feature E2E Tests

**What this PR does / why we need it**:
PVC Protection alpha feature was introduced in PRs:
- #55824
- #55873

This PR adds E2E tests for this feature.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
N/A

**Special notes for your reviewer**:

**Release note**:
```release-note
NONE
```
pospispa pushed a commit to pospispa/test-infra that referenced this pull request Dec 19, 2017
Test PVC Protection in ci-kubernetes-e2e-gci-gce-alpha-features
50d2264 
PVC Protection alpha feature was merged in PRs:
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55873

PVC Protection E2E tests are in PR: kubernetes/kubernetes#56931

Adding the PVC Protection tests into the list of Alpha feature tests that are run.
@dixudx dixudx mentioned this pull request Jan 3, 2018
Closed
@ncdc ncdc mentioned this pull request Jan 12, 2018
Closed
pospispa pushed a commit to pospispa/origin that referenced this pull request Jan 15, 2018
UPSTREAM: 55957: Scheduler Skips Pods that Use PVCs that Are Being De…
a8a1a1c 
…leted

PVC Protection feature consists of the below PRs:
- kubernetes/kubernetes#55873
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55957

The PRs #55873 and #55824 were merged into K8s 1.9, however, the PR #55957 was merged into K8s 1.10. That's why this PR is backported.

This commit modifies scheduler in such a way that it does not schedule a pod that uses a PVC that is being deleted.
pospispa pushed a commit to pospispa/origin that referenced this pull request Jan 15, 2018
UPSTREAM: 55957: Scheduler Skips Pods that Use PVCs that Are Being De…
6147226 
…leted

PVC Protection feature consists of the below PRs:
- kubernetes/kubernetes#55873
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55957

The PRs #55873 and #55824 were merged into K8s 1.9, however, the PR #55957 was merged into K8s 1.10. That's why this PR is backported.

This commit modifies scheduler in such a way that it does not schedule a pod that uses a PVC that is being deleted.
@pospispa pospispa mentioned this pull request Jan 16, 2018
Closed
pospispa pushed a commit to pospispa/origin that referenced this pull request Jan 17, 2018
UPSTREAM: 55957: Scheduler Skips Pods that Use PVCs that Are Being De…
eeb1dbb 
…leted

PVC Protection feature consists of the below PRs:
- kubernetes/kubernetes#55873
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55957

The PRs #55873 and #55824 were merged into K8s 1.9, however, the PR #55957 was merged into K8s 1.10. That's why this PR is backported.

This commit modifies scheduler in such a way that it does not schedule a pod that uses a PVC that is being deleted.
@saad-ali saad-ali mentioned this pull request Jan 23, 2018
Closed
@pospispa pospispa mentioned this pull request Feb 2, 2018
Closed
pospispa pushed a commit to pospispa/openshift-docs that referenced this pull request Feb 3, 2018
Persistent Volume Claim Protection
9270841 
PVC Protection alpha feature was added to K8s 1.9 in PRs:
- kubernetes/kubernetes#55824
- kubernetes/kubernetes#55873

and documented in K8s 1.9 in PR:
- kubernetes/website#6415

That's why the PVC Protection is a new alpha feature in OpenShift 3.9 and that's why PVC Protection documentation is added.
@smanpathak smanpathak mentioned this pull request Oct 12, 2018
Closed
@Sallyan
Copy link

Sallyan commented Jun 11, 2019

Hi,
@jsafrane @pospispa
I have one issue my PVCs are stuck in terminating status. The pods which use the PVCs are of completed status.
I use 'kubectl describe' to check the PVCs, PVCs are protected by Finalizers:[kubernetes.io/pvc-protection].
But actually now the PVCs are not in active use in Pods, I CANNOT delete the PVCs. Any suggestion?

Regards,
Hongyan

@msau42
Copy link
Member

msau42 commented Jun 11, 2019

Your pods have been completed but have they been deleted?

@Sallyan
Copy link

Sallyan commented Jun 12, 2019

Hi @msau42
My pods are of completed status and not been deleted. We are keeping around them to view the logs of completed pods to check for errors, warnings, or other diagnostic output.
So resources are still being consumed by these pods?
If i want to release the PVCs, the only way is to delete the pods?

@msau42
Copy link
Member

msau42 commented Jun 12, 2019

Yes, I believe the volumes are still mounted until you delete the Pod too.

@Sallyan
Copy link

Sallyan commented Jun 12, 2019

Well...and thanks so much for your reply.
Is it possible to unmount the volumes without pod deletion?

rikatz pushed a commit to rikatz/utils that referenced this pull request Apr 15, 2021
Addressing Comments from Code Review
1d87b4a 
Addressing comments from code review (kubernetes/kubernetes#55824 (review)) in order to simplify the code.
@jimmyjones2 jimmyjones2 mentioned this pull request Nov 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@thockin thockin thockin left review comments

@jsafrane jsafrane jsafrane left review comments

@smarterclayton smarterclayton smarterclayton left review comments

@liggitt liggitt liggitt left review comments

Assignees

@liggitt liggitt

@jsafrane jsafrane

@thockin thockin

@deads2k deads2k

@saad-ali saad-ali

@msau42 msau42

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.9
Development

Successfully merging this pull request may close these issues.

should prevent the deletion of a PVC that is referenced by an active pod