KEP-3960: promote to beta in 1.30#4367
Conversation
|
/cc @thockin |
|
And a draft code in k/k: kubernetes/kubernetes#122456 |
|
kindly ping @thockin |
|
I am good with it - @kannon92 ? /lgtm |
|
Oh, right - this being sig-node I can't approve anyway. |
|
@AxeZhan please look over your PRR review. I think you are missing some forms. But otherwise, this LGTM. |
|
/lgtm /assign @mrunalp @SergeyKanzhelev @dchen1107 @klueska |
yes - will review later this week |
wojtek-t
left a comment
There was a problem hiding this comment.
Just one small comment - other than that LGTM
| Added in alpha: | ||
| Unit tests in `k8s.io/kubernetes/pkg/apis/core/validation_test` test the verification with feature-gate enabled. | ||
| The verification with feature-gate disabled is tested manually as it's hard to trigger `PrepareForCreate` strategy in unit tests. | ||
| E2e tests in `test/e2e/common/node/lifecycle_hook.go` test the behavior with feature-gate enabled. |
There was a problem hiding this comment.
Copying from the KEP template:
Additionally, for features that are introducing a new API field, unit tests that
are exercising the `switch` of feature gate itself (what happens if I disable a
feature gate after having objects written with the new field) are also critical.
You can take a look at one potential example of such test in:
https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3R246-R282
I'm actually more interested in sth like that - this question is not about testing the feature itself (testing code with FG enabled), but rather testing the transition between enabled and disabled (and vice versa).
There was a problem hiding this comment.
... Yea, I think we need a test like this for this feature. But I clearly misunderstood this and missed this in alpha🤦.
I'll bring this test in beta, I added some cases in Unit tests for beta.
cb25915 to
61ffb47
Compare
|
Thanks! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AxeZhan, mrunalp, thockin, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |
Promote KEP-3960 to beta in 1.30
Works will be done for beta