StatefulSet PersistentVolumeClaimDeletePolicy

[mattcary]

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]

@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]

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]

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]

@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]

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]

/retest

[mattcary]

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]

/retest

[mattcary]

/retest

[mattcary]

/retest

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

[mattcary]

@smarterclayton how does it look?

[smarterclayton]

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

[mattcary]

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]

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

ping? Thanks!

[alculquicondor]

Still lgtm for me

[smarterclayton]

/lgtm

Totally not monday....

[krmayankk]

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

[liggitt]

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

[lavalamp]

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]

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]

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

[palnabarun]

I see two possibilities here:

1. Revert this PR as implementation is not merged
2. 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]

Moving conversation over to issue #103727 to make tracking easier

[liggitt]

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