Promote StatefulSetAutoDeletePVC to stable in 1.32 #128247
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
|
/sig apps |
k8s-ci-robot
added
sig/apps
mattcary
force-pushed
the
autodelete-ga
branch
from
e70bb3d to
d48ea39
Compare
k8s-ci-robot
added
size/L
|
/test |
|
@mattcary: The
The following commands are available to trigger optional jobs:
Use
In response to this:
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-sigs/prow repository. |
k8s-ci-robot
added
area/code-generation
area/test
kind/api-change
|
/test all |
mattcary
force-pushed
the
autodelete-ga
branch
2 times, most recently
from
1aa4264 to
e1cbf8d
Compare
k8s-ci-robot
removed
the
size/L
|
I did a quick scan of the changes and this all looks reasonable. cc @aaron-prindle @Jefftree for compatibility version sanity check. This will require approval from api-reviewers. I see the api-review label is already set. |
mattcary
force-pushed
the
autodelete-ga
branch
from
808fa5e to
2095d9e
Compare
|
Sorry all, looks like a I did something wrong with the rebase, there is a merge commit in here somehow. I'm in process of sorting it out. |
mattcary
force-pushed
the
autodelete-ga
branch
from
2095d9e to
d4b8926
Compare
k8s-ci-robot
removed
the
do-not-merge/contains-merge-commits
Okay, I think I sorted it out. Should be good to go. |
|
LGTM for the changes here with respect to Compatibility Versions. No user facing beta/GA APIs are modified/removed and bumping the feature gate is inline w/ Compatibility Versions guarantees. |
|
/test pull-kubernetes-unit That test is passing locally, hopefully it was a flake |
| @@ -597,7 +597,6 @@ func TestSetDefaultStatefulSet(t *testing.T) { | |||
| for _, test := range tests { | |||
| test := test | |||
| t.Run(test.name, func(t *testing.T) { | |||
| featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, test.enablePVCDeletionPolicy) | |||
There was a problem hiding this comment.
I'd probably leave that variable enablePVCDeletionPolicy as it was before, then you'd have here this:
if !test.enablePVCDeletionPolicy {
featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MustParse("1.31"))
}
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, test.enablePVCDeletionPolicy)That's what we've been using all over the places, and this will also mean less changes 😉
| if strings.Contains(name, enableStatefulSetAutoDeletePVC) { | ||
| featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, true) | ||
| } | ||
|
|
There was a problem hiding this comment.
just to confirm, do we want to remove the gating in the validation test or not?
|
/approve |
k8s-ci-robot
added
the
approved
There was a problem hiding this comment.
To summarize:
- validation_test.go - revert back the feature gate testing
- defaults_test.go (in all version) - about just using the pattern I've pointed in Promote StatefulSetAutoDeletePVC to stable in 1.32 #128247 (comment)
- v1/types.go - constants description
These are the remaining bits.
| if strings.Contains(name, enableStatefulSetAutoDeletePVC) { | ||
| featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, true) | ||
| } | ||
|
|
|
/milestone v1.32 |
Yup, @soltysh and I have been syncing on slack, there's a couple things we want that I should be able to do tonight and then this should be in tomorrow. |
|
The current state is:
Not needed, we're not using the
As discussed in that comment this is not needed.
This one still holds, so that's the only one left here to resolve. |
mattcary
force-pushed
the
autodelete-ga
branch
from
d4b8926 to
50362ac
Compare
Ack
Ack
Done! |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: da61249876b67fcdc336e75e4bf92b91ba265adc
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, 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:
Approvers can indicate their approval by writing |
|
@mattcary: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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-sigs/prow repository. I understand the commands that are listed here. |
|
/test pull-kubernetes-e2e-gce-cos-alpha-features seems to be a flake |
/kind feature
Promotes kubernetes/enhancements#1847 to stable.
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: