StatefulSet PersistentVolumeClaimDeletePolicy #99378

mattcary · 2021-02-23T21:18:26Z

API change for kubernetes/enhancements#1847.

/kind feature
/kind api-change

What this PR does / why we need it:

This adds a PersistentVolumeClaimDeletePolicy field to the StatefulSet spec according to kubernetes/enhancements#1847 which adds options for autodelete of PVCs created for StatefulSet pods.

Add PersistentVolumeClaimDeletePoilcy to StatefulSet API.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[KEP]: Auto remove PVCs created by StatefulSet enhancements#1847 / https://github.com/kubernetes/enhancements/tree/master/keps/sig-apps/1847-autoremove-statefulset-pvcs

/assign @kow3ns
/assign @janetkuo
/cc @kk-src

k8s-ci-robot · 2021-02-23T21:18:28Z

@mattcary: GitHub didn't allow me to request PR reviews from the following users: kk-src.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

API change for kubernetes/enhancements#1847.

/kind feature
/kind api-change

What this PR does / why we need it:

This adds a PersistentVolumeClaimDeletePolicy field to the StatefulSet spec according to kubernetes/enhancements#1847 which adds options for autodelete of PVCs created for StatefulSet pods.
Add PersistentVolumeClaimDeletePoilcy to StatefulSet API.
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2021-02-23T21:18:29Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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. I understand the commands that are listed here.

mattcary · 2021-02-23T21:21:02Z

Note that I'm still working on getting the fuzzer test to pass (k8s.io/kubernetes/pkg/api/testing). In spite of my change to fuzzer.go it still seems to be trying to fuzz the field. Any hints would be appreciated!

/assign @kow3ns
/assign @janetkuo
/cc @kk-src
/sig apps

k8s-ci-robot · 2021-02-23T21:21:04Z

@mattcary: GitHub didn't allow me to request PR reviews from the following users: kk-src.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Note that I'm still working on getting the fuzzer test to pass (k8s.io/kubernetes/pkg/api/testing). In spite of my change to fuzzer.go it still seems to be trying to fuzz the field. Any hints would be appreciated!

/assign @kow3ns
/assign @janetkuo
/cc @kk-src
/sig apps

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.

fejta-bot · 2021-02-23T23:10:43Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

mattcary · 2021-02-25T00:21:07Z

/retest

mattcary · 2021-02-25T00:48:02Z

Hi! This should be ready for review now.

I figured out how to get the api tests to pass, although I'm a little suspicious of what I did in apps/fuzzer.go so please let me know if I did it wrong.

It appears the presubmit tests are currently failing due to something unrelated to my change.

mattcary · 2021-02-25T21:53:43Z

/retest

pkg/apis/apps/types.go

mattcary · 2021-06-23T21:28:28Z

/retest

mattcary · 2021-06-24T00:04:11Z

/retest

Still looks like a flake (only an NFS storage test failed)

mattcary · 2021-06-25T00:03:32Z

@smarterclayton how does it look?

smarterclayton · 2021-06-25T20:11:27Z

any other comments from the other reviewers? if no, on monday eod i’ll tag this as lgtm

mattcary · 2021-06-25T20:15:28Z

Not that I know of. Thanks!

…

On Fri, Jun 25, 2021 at 1:11 PM Clayton Coleman ***@***.***> wrote: any other comments from the other reviewers? if no, on monday eod i’ll tag this as lgtm — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#99378 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJCBAHM6PBX4KFQ3XSBL2TTUTPHXANCNFSM4YDHMOPQ> .

mattcary · 2021-06-29T16:17:29Z

any other comments from the other reviewers? if no, on monday eod i’ll tag this as lgtm

ping? Thanks!

alculquicondor · 2021-06-29T17:18:20Z

Still lgtm for me

smarterclayton · 2021-06-30T17:03:59Z

/lgtm

Totally not monday....

Here is a second batch for feature gate updates in 1.22. - EndpointSliceProxying kubernetes/kubernetes#103451 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 - LogarithmicScaleDown kubernetes/kubernetes#101767 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - DynamicKubeletConfig kubernetes/kubernetes#102966 - NodeSwapEnabled kubernetes/kubernetes#102823 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - EndpointSliceProxying kubernetes/kubernetes#103451 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - DynamicKubeletConfig kubernetes/kubernetes#102966 - NodeSwapEnabled kubernetes/kubernetes#102823 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - NodeSwap kubernetes/kubernetes#102823, kubernetes/kubernetes#103553 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

krmayankk · 2021-07-13T06:46:11Z

@lavalamp @liggitt @smarterclayton i thought we never merged API without the implementation , or has the policy changed around this ?

liggitt · 2021-07-13T13:22:06Z

I would not have expected this to merge until #99728 was ready to merge

lavalamp · 2021-07-13T16:08:23Z

Agree, we need to back this out if the implementation isn't going to make the release :/

I see a reference to alpha in the implementation PR comments but this seems to change a beta API

mattcary · 2021-07-13T16:22:26Z

I see a reference to alpha in the implementation PR comments but this seems to change a beta API

The feature gate in question is alpha. The feature adds a field to the StatefulSet spec; there are 3 StatefulSet apis currently (v1, v1beta1 and v1beta2); I added the new field to all of them.

The implementation PR has an approval so is near ready to be merged, is there any chance of that being a possibility?

lavalamp · 2021-07-13T16:40:17Z

You need to get an exception from the release team at this point.

palnabarun · 2021-07-14T15:16:24Z

I see two possibilities here:

Revert this PR as implementation is not merged
File an exception request

For Case 2,
a. If the request is accepted, #99728 can be merged.
b. If the request is denied, this PR needs to be reverted.

Now, wearing the hat of a previous release lead, I feel that it is highly likely that the exception request would be denied as it doesn't sound like a release-blocking feature and we are almost a week past code freeze.

mattcary · 2021-07-16T00:12:02Z

Moving conversation over to issue #103727 to make tracking easier

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - NodeSwap kubernetes/kubernetes#102823, kubernetes/kubernetes#103553 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

Revert #99378

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

liggitt · 2021-08-05T14:56:58Z

Marking as API review completed since the API bits were approved. Can reference this review when reintroducing in 1.23 along with the implementation

k8s-ci-robot assigned janetkuo Feb 23, 2021

k8s-ci-robot assigned kow3ns Feb 23, 2021

mattcary force-pushed the api branch from 753d7a0 to 5708689 Compare February 23, 2021 22:06

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Feb 23, 2021

mattcary force-pushed the api branch 2 times, most recently from a30b95a to 402138d Compare February 25, 2021 00:17

mattcary force-pushed the api branch from 402138d to cc5d161 Compare February 25, 2021 00:40

mattcary force-pushed the api branch 2 times, most recently from b25f982 to e7c6b15 Compare February 25, 2021 20:47

mattcary force-pushed the api branch from e7c6b15 to f67d9d8 Compare February 28, 2021 22:16

kk-src reviewed Mar 1, 2021

View reviewed changes

pkg/apis/apps/types.go Outdated Show resolved Hide resolved

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 30, 2021

k8s-ci-robot merged commit 98d20f5 into kubernetes:master Jun 30, 2021

tengqm mentioned this pull request Jul 8, 2021

Batch update feature gates for 1.22 kubernetes/website#28849

Merged

mattcary mentioned this pull request Jul 16, 2021

Revert #99378, StatefulSet autodelete API changes #103727

Closed

kikisdeliveryservice mentioned this pull request Jul 16, 2021

Revert #99378 #103747

Merged

k8s-ci-robot added a commit that referenced this pull request Jul 20, 2021

Merge pull request #103747 from mattcary/noapi

7303b2c

Revert #99378

alculquicondor mentioned this pull request Aug 30, 2021

Add alculquicondor to sig-apps-reviewers #104663

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StatefulSet PersistentVolumeClaimDeletePolicy #99378

StatefulSet PersistentVolumeClaimDeletePolicy #99378

mattcary commented Feb 23, 2021 •

edited

Loading

k8s-ci-robot commented Feb 23, 2021

What this PR does / why we need it:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Feb 23, 2021

mattcary commented Feb 23, 2021

k8s-ci-robot commented Feb 23, 2021

fejta-bot commented Feb 23, 2021

mattcary commented Feb 25, 2021

mattcary commented Feb 25, 2021

mattcary commented Feb 25, 2021

mattcary commented Jun 23, 2021

mattcary commented Jun 24, 2021

mattcary commented Jun 25, 2021

smarterclayton commented Jun 25, 2021

mattcary commented Jun 25, 2021 via email

mattcary commented Jun 29, 2021

alculquicondor commented Jun 29, 2021

smarterclayton commented Jun 30, 2021

krmayankk commented Jul 13, 2021

liggitt commented Jul 13, 2021

lavalamp commented Jul 13, 2021

mattcary commented Jul 13, 2021

lavalamp commented Jul 13, 2021

palnabarun commented Jul 14, 2021 •

edited

Loading

mattcary commented Jul 16, 2021

liggitt commented Aug 5, 2021

StatefulSet PersistentVolumeClaimDeletePolicy #99378

StatefulSet PersistentVolumeClaimDeletePolicy #99378

Conversation

mattcary commented Feb 23, 2021 • edited Loading

What this PR does / why we need it:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Feb 23, 2021

What this PR does / why we need it:

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Feb 23, 2021

mattcary commented Feb 23, 2021

k8s-ci-robot commented Feb 23, 2021

fejta-bot commented Feb 23, 2021

mattcary commented Feb 25, 2021

mattcary commented Feb 25, 2021

mattcary commented Feb 25, 2021

mattcary commented Jun 23, 2021

mattcary commented Jun 24, 2021

mattcary commented Jun 25, 2021

smarterclayton commented Jun 25, 2021

mattcary commented Jun 25, 2021 via email

mattcary commented Jun 29, 2021

alculquicondor commented Jun 29, 2021

smarterclayton commented Jun 30, 2021

krmayankk commented Jul 13, 2021

liggitt commented Jul 13, 2021

lavalamp commented Jul 13, 2021

mattcary commented Jul 13, 2021

lavalamp commented Jul 13, 2021

palnabarun commented Jul 14, 2021 • edited Loading

mattcary commented Jul 16, 2021

liggitt commented Aug 5, 2021

mattcary commented Feb 23, 2021 •

edited

Loading

palnabarun commented Jul 14, 2021 •

edited

Loading