Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod #79164

cwdsuzhou · 2019-06-19T11:32:34Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Currently, statefulSet would not recreate the pvc when a pvc is deleted before a pod, which would lead to pod in pending status.

Which issue(s) this PR fixes:

Fixes #74374

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Bugfix: statefulSet would not recreate the pvc when a pvc is deleted before a pod

k8s-ci-robot · 2019-06-19T11:33:20Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cwdsuzhou
To complete the pull request process, please assign kow3ns
You can assign the PR to them by writing /assign @kow3ns in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

pkg/controller/statefulset/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cwdsuzhou · 2019-06-19T12:15:54Z

/test pull-kubernetes-integration

cwdsuzhou · 2019-06-19T12:39:22Z

/test pull-kubernetes-kubemark-e2e-gce-big

cwdsuzhou · 2019-06-19T12:53:32Z

/assign @msau42 @kow3ns

mucahitkurt · 2019-06-19T19:57:06Z

pkg/controller/statefulset/stateful_pod_control.go

@@ -192,6 +192,11 @@ func (spc *realStatefulPodControl) createPersistentVolumeClaims(set *apps.Statef
 		case err != nil:
 			errs = append(errs, fmt.Errorf("Failed to retrieve PVC %s: %s", claim.Name, err))
 			spc.recordClaimEvent("create", set, pod, &claim, err)
+		case err == nil:


I think there is a race condition with this solution as @msau42 already said, if the pvc can have DeletionTimeStamp after this method is run, and then pod will be pending state again?

Since this is not an atomic operation, I think creating statefulSet pod can not avoid the scene absolutely. However, it seldom(almost never) would happen.

kow3ns · 2019-07-08T22:26:41Z

Why not just add a watch on PVCs and check if a deleted PVC corresponds to a Pod in a StatefulSet that has not been deleted?

cwdsuzhou · 2019-08-02T07:09:57Z

Why not just add a watch on PVCs and check if a deleted PVC corresponds to a Pod in a StatefulSet that has not been deleted?

I am not sure if this would increase api-server load. So I would like to fix the issue by check DeleteTimeStamp

I have corrected the logic of this PR:

check PVC DeleteTimeStamp from cache
if not nil, get actual PVC from api server

getErr != nil && apierrors.IsNotFound(getErr) 
||realPvc != nil && acutualPvc.DeletionTimestamp != nil

we should also mark it as error.

PATL, thanks @kow3ns

cwdsuzhou · 2019-08-02T07:10:44Z

cc @janetkuo

PTAL, thanks.

cwdsuzhou · 2019-08-02T07:26:04Z

/priority important-longterm

cwdsuzhou · 2019-08-02T08:28:24Z

/retest

msau42 · 2019-10-04T01:54:30Z

/assign @janetkuo

cwdsuzhou · 2019-10-20T14:31:38Z

ping @janetkuo

PTAL thanks

tnozicka · 2019-11-11T09:15:56Z

pkg/controller/statefulset/stateful_pod_control.go

+				// if get pvc failed collect it;
+				// if not found or in terminating also collect error;
+				// if found pvc and not in terminating state, do not collect it.
+				actualPVC, getErr := spc.client.CoreV1().PersistentVolumeClaims(claim.Namespace).Get(claim.Name,


I don't see why rechecking the deletionTimestamp by live GET helps here. The same object may not transition back and you get race on detecting the recreate case where you can miss it so we'd need a reconciliation logic for that case anyways.

Watching if pods reference deleted PVC and reconciling it as @kow3ns suggested in #79164 (comment) sounds more reliable

I don't see why rechecking the deletionTimestamp by live GET helps here. The same object may not transition back and you get race on detecting the recreate case where you can miss it so we'd need a reconciliation logic for that case anyways.

Watching if pods reference deleted PVC and reconciling it as @kow3ns suggested in #79164 (comment) sounds more reliable

Thanks, I would do that later

fejta-bot · 2020-02-12T03:42:44Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2020-03-13T04:26:08Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

smileusd · 2020-03-30T07:15:12Z

Hi, How about this pr status?

fejta-bot · 2020-04-29T07:48:46Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2020-04-29T07:49:01Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 19, 2019

k8s-ci-robot requested review from kow3ns and krmayankk June 19, 2019 11:34

cwdsuzhou force-pushed the June/sts_delete_pvc branch from 95a77fd to 53d8d09 Compare June 19, 2019 11:34

cwdsuzhou mentioned this pull request Jun 19, 2019

Storage protection feature does not integrate well with StatefulSet PVC recreation #74374

Closed

cwdsuzhou changed the title ~~[WIP] Fix not recreate pvc in sts~~ Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod Jun 19, 2019

k8s-ci-robot assigned kow3ns and msau42 Jun 19, 2019

cwdsuzhou force-pushed the June/sts_delete_pvc branch from 53d8d09 to dd7c1ae Compare June 19, 2019 13:40

mucahitkurt reviewed Jun 19, 2019

View reviewed changes

k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Aug 2, 2019

k8s-ci-robot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Aug 2, 2019

Fix delete pvc before delete pod

a3f52d1

cwdsuzhou force-pushed the June/sts_delete_pvc branch from 908f749 to a3f52d1 Compare August 27, 2019 12:33

cwdsuzhou requested a review from mucahitkurt September 21, 2019 11:14

k8s-ci-robot assigned janetkuo Oct 4, 2019

cwdsuzhou mentioned this pull request Oct 20, 2019

Do not create StatefulSet pods when PVC is being deleted #83273

Closed

tnozicka reviewed Nov 11, 2019

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 12, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 13, 2020

smileusd mentioned this pull request Mar 30, 2020

skip statefulset create pod if pvc deleting #89643

Closed

k8s-ci-robot closed this Apr 29, 2020

ymmt2005 mentioned this pull request Jul 27, 2020

Do not create StatefulSet pods when PVC is being deleted #93457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod #79164

Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod #79164

cwdsuzhou commented Jun 19, 2019 •

edited

Loading

k8s-ci-robot commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

mucahitkurt Jun 19, 2019 •

edited

Loading

cwdsuzhou Jun 20, 2019

kow3ns commented Jul 8, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

msau42 commented Oct 4, 2019

cwdsuzhou commented Oct 20, 2019

tnozicka Nov 11, 2019

cwdsuzhou Nov 14, 2019

fejta-bot commented Feb 12, 2020

fejta-bot commented Mar 13, 2020

smileusd commented Mar 30, 2020

fejta-bot commented Apr 29, 2020

k8s-ci-robot commented Apr 29, 2020

Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod #79164

Bugfix:statefulSet would not recreate the pvc when a pvc is deleted before a pod #79164

Conversation

cwdsuzhou commented Jun 19, 2019 • edited Loading

k8s-ci-robot commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

cwdsuzhou commented Jun 19, 2019

mucahitkurt Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

cwdsuzhou Jun 20, 2019

Choose a reason for hiding this comment

kow3ns commented Jul 8, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

cwdsuzhou commented Aug 2, 2019

msau42 commented Oct 4, 2019

cwdsuzhou commented Oct 20, 2019

tnozicka Nov 11, 2019

Choose a reason for hiding this comment

cwdsuzhou Nov 14, 2019

Choose a reason for hiding this comment

fejta-bot commented Feb 12, 2020

fejta-bot commented Mar 13, 2020

smileusd commented Mar 30, 2020

fejta-bot commented Apr 29, 2020

k8s-ci-robot commented Apr 29, 2020

cwdsuzhou commented Jun 19, 2019 •

edited

Loading

mucahitkurt Jun 19, 2019 •

edited

Loading