KEP-1847: Update beta milestone to 1.25 #3193

mattcary · 2022-01-27T17:40:04Z

One-line PR description: We will not be able to push this to beta in 1.24, therefore moving to 1.25.
Issue link: Auto remove PVCs created by StatefulSet #1847

k8s-triage-robot · 2022-04-27T18:25:09Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2022-05-27T18:59:43Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

mattcary · 2022-05-27T19:11:51Z

/remove-lifecycle rotten

Am looking at pushing this to beta but need more work on interaction with existing applications/helm charts, etc.

soltysh · 2022-06-08T17:26:48Z

Am looking at pushing this to beta but need more work on interaction with existing applications/helm charts, etc.

I was actually meant to reach out to you @mattcary if this is still something you're planning to pick up, feel free to ping me on slack for faster review turnaround

soltysh · 2022-06-13T10:47:11Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

+
+#### Unit tests
+
+In `pkg/controller/statefulset/`,


Please update following the template from https://github.com/kubernetes/enhancements/blob/master/keps/NNNN-kep-template/README.md?plain=1#L257-L328, ie. here you should have:

<package>: <date> - <current test coverage>

soltysh · 2022-06-13T10:47:44Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

+
+##### e2e/integration tests
+
+Tests added to `test/e2e/apps/statefulset.go`.


Same as above, although here:

- <test>: <link to test coverage>

Also e2e and integration should be two distinct paragraphs.

Done.

I wasn't aware of the integration tests, as noted these need to be added. Or should I just record the plan so that the KEP doesn't have to be updated when I add these (as I plan to do in the next month)?

soltysh · 2022-06-13T10:49:42Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/kep.yaml

@@ -22,12 +22,12 @@ approvers:

 stage: alpha

-latest-milestone: "v1.23"
+latest-milestone: "v1.25"


You're missing https://github.com/kubernetes/enhancements/blob/master/keps/prod-readiness/sig-apps/1847.yaml updated for PRR review.

I updated 1847.yaml with the beta tag, is that sufficient? Also is it okay to leave Wojtek on there?

soltysh · 2022-06-14T11:33:21Z

keps/prod-readiness/sig-apps/1847.yaml

@@ -1,3 +1,3 @@
 kep-number: 1847
-alpha:
+beta:


Don't change the tag, but add another one. @wojtek-t will you have some time to check this one out?

done, kept the reviewer as Wojtek but please LMK if I should switch it to another person.

soltysh · 2022-06-14T11:34:40Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

+  - `stateful_set_control_test`
+     - Added `runTestOverPVCRetentionPolicies` to augment existing tests with
+    coverage for retention policies.
+     - `checkClaimInvarients` extended to test new feature.


You're still not following the template: https://github.com/kubernetes/enhancements/blob/master/keps/NNNN-kep-template/README.md?plain=1#L257-L328, ie. here you should have:

<package>: <date> - <current test coverage>

Sorry, I guess I don't understand what the template is trying to do. What is the date referring to?

Something like this: https://github.com/kubernetes/enhancements/tree/master/keps/sig-apps/3140-TimeZone-support-in-CronJob#unit-tests

These are updated now as best as I could figure.

soltysh · 2022-06-14T11:35:46Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

+##### Integration tests
+
+- `test/integration/statefulset`
+  - TBD, add same tests as in e2e, below, so they can be tested while the


The fact that we're missing these tests might be a blocker for beta promotion.

Sure, do I have to have these tests written by the KEP freeze or the code freeze?

This is listing existing tests, which are required for beta promotion.

Okay, I've created kubernetes/kubernetes#110612 to add the integration tests immediately.

As an aside, none of the integration tests seem to be appearing in the triage dashboard suggested in the template, as noted in my PR.

I tagged that PR.

Thanks, that's merged now.

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

soltysh · 2022-06-14T11:42:19Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/kep.yaml

 # metrics:
-# Currently no metrics is planned for alpha release.
+# No new metrics are planned (the stateful set controller does not export any metrics).


I'd encourage to add metrics, especially if none exist, yet

Is this a hard requirement? Since nothing exists at this point, does this mean we're not promoting to beta in 1.25, or if I list metrics here that get added before the 1.25 freeze is that enough?

Thanks for the feedback!

I've added some hopefully obvious metrics, and updated the appropriate discussions in the readme on how I see that they would be used.

mattcary · 2022-06-22T23:45:53Z

ping?

thx!

soltysh

/lgtm
/approve
from sig-apps pov

mattcary · 2022-06-23T16:15:16Z

/assign @wojtek-t

mattcary · 2022-06-23T16:23:19Z

/assign @johnbelamaric

Wojceich is OOO, I've contacted John offline & he's kindly agreed to look at this.

johnbelamaric

Troubleshooting section is not filled out, can you quickly do that? I know time is short...

johnbelamaric · 2022-06-23T20:02:25Z

keps/sig-apps/1847-autoremove-statefulset-pvcs/README.md

-      - Impact of its outage on the feature:
-      - Impact of its degraded performance or high-error rates on the feature:
+
+  No, outside of dependin on the scheduler, the garbage collector and volume


s/dependin/depending/

troubleshooting done, and typo fixed, ptal

Thanks!

johnbelamaric · 2022-06-23T21:08:08Z

/approve

k8s-ci-robot · 2022-06-23T21:08:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric, mattcary, soltysh

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~keps/prod-readiness/OWNERS~~ [johnbelamaric]
~~keps/sig-apps/OWNERS~~ [johnbelamaric,soltysh]

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

johnbelamaric · 2022-06-23T21:58:20Z

/lgtm

wojtek-t · 2022-07-15T14:34:21Z

FWIW - this LGTM too :)

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 27, 2022

k8s-ci-robot requested a review from kow3ns January 27, 2022 17:40

k8s-ci-robot added the kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory label Jan 27, 2022

k8s-ci-robot requested a review from soltysh January 27, 2022 17:40

k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Jan 27, 2022

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

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 May 27, 2022

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label May 27, 2022

reylejano mentioned this pull request Jun 8, 2022

Auto remove PVCs created by StatefulSet #1847

Open

4 tasks

mattcary force-pushed the ss124 branch 2 times, most recently from 88971c1 to be3526d Compare June 9, 2022 15:53

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 9, 2022

mattcary force-pushed the ss124 branch from be3526d to 72794c1 Compare June 9, 2022 15:55

soltysh requested changes Jun 13, 2022

View reviewed changes

mattcary force-pushed the ss124 branch from 72794c1 to f1e5170 Compare June 13, 2022 18:20

soltysh reviewed Jun 14, 2022

View reviewed changes

mattcary force-pushed the ss124 branch 2 times, most recently from cd9331e to 4078039 Compare June 14, 2022 20:59

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 14, 2022

mattcary force-pushed the ss124 branch from 4078039 to 319bfa0 Compare June 15, 2022 20:33

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 15, 2022

mattcary mentioned this pull request Jun 15, 2022

Add TestAutodeleteOwnerRefs statefulset integration test kubernetes/kubernetes#110612

Merged

mattcary force-pushed the ss124 branch 2 times, most recently from ed83119 to e0668a8 Compare June 22, 2022 00:00

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 22, 2022

soltysh approved these changes Jun 23, 2022

View reviewed changes

k8s-ci-robot assigned soltysh Jun 23, 2022

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

k8s-ci-robot assigned wojtek-t Jun 23, 2022

k8s-ci-robot assigned johnbelamaric Jun 23, 2022

johnbelamaric reviewed Jun 23, 2022

View reviewed changes

mattcary force-pushed the ss124 branch from e0668a8 to 6ccfd70 Compare June 23, 2022 20:16

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 23, 2022

Update statefulset autodelete beta to 1.25

289d18c

mattcary force-pushed the ss124 branch from 6ccfd70 to 289d18c Compare June 23, 2022 20:17

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 23, 2022

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

k8s-ci-robot merged commit 8c6d57e into kubernetes:master Jun 23, 2022

k8s-ci-robot added this to the v1.25 milestone Jun 23, 2022


		##### e2e/integration tests

		Tests added to `test/e2e/apps/statefulset.go`.

KEP-1847: Update beta milestone to 1.25 #3193

KEP-1847: Update beta milestone to 1.25 #3193

Conversation

mattcary commented Jan 27, 2022

k8s-triage-robot commented Apr 27, 2022

k8s-triage-robot commented May 27, 2022

mattcary commented May 27, 2022

soltysh commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattcary commented Jun 22, 2022

soltysh left a comment

Choose a reason for hiding this comment

mattcary commented Jun 23, 2022

mattcary commented Jun 23, 2022

johnbelamaric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnbelamaric commented Jun 23, 2022

k8s-ci-robot commented Jun 23, 2022

johnbelamaric commented Jun 23, 2022

wojtek-t commented Jul 15, 2022