cleanup: replace ENABLE_CLIENT_GO_WATCH_LIST_ALPHA with WatchListClient gate#122791
Conversation
|
/hold until we resolve https://github.com/kubernetes/perf-tests/blob/master/util-images/watch-list/main.go#L51 |
| } | ||
| } | ||
|
|
||
| type enabledWatchListClientFeatureGateRegistry struct { |
There was a problem hiding this comment.
i'm going to remove this struct once the WatchListClient is enabled for KCM.
|
/test pull-kubernetes-e2e-gce-cos-alpha-features |
|
I think that |
Actually, |
…nv var with `KUBE_FEATURE_WatchListClient` The previous environment variable is being replaced in kubernetes/kubernetes#122791
|
created kubernetes/perf-tests#2506 The watchlist perf tests are run once a day. We can merge both pull requests roughly at the same time. In addition to that, no one actually monitors the results of the watchlist perf tests, so even if the tests temporarily fail, it is okay. |
|
|
||
| func (r *enabledWatchListClientFeatureGateRegistry) Enabled(feature clientfeatures.Feature) bool { | ||
| if feature == clientfeatures.WatchListClient { | ||
| return true |
There was a problem hiding this comment.
Given that you force the gate here, why do we need to wait for an env var change? It appears the test used to force the feature to "on" and that after this change it is still force "on" regardless of the state of the env var being specified.
There was a problem hiding this comment.
sorry, I linked a wrong PR. We need #122571 (already merged).
|
verify failure. looks fine otherwise |
|
/retest |
|
failing |
|
@wojtek-t: Reopened this PR. DetailsIn 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/test-infra repository. |
|
Please note that we're already in Test Freeze for the Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Wed Apr 3 02:18:49 UTC 2024. |
189dcce to
700f43a
Compare
|
/retest |
82f382e to
ae08142
Compare
| // regression test for https://issues.k8s.io/117258 | ||
| func TestAPIServerTransportMetrics(t *testing.T) { | ||
| featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, "AllAlpha", true) | ||
| featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.WatchList, false) |
There was a problem hiding this comment.
For tests that enable all alpha and beta flags, we need to disable the WatchList FG until we can manually request progress notification from the server.
This is not an ideal solution because in the meantime someone might add a new test.
There was a problem hiding this comment.
We already can request progress notifications manually if you switch on ConsistentListFromCache feature
- this basically already works: https://github.com/kubernetes/kubernetes/pull/123513/files
So this shouldn't be needed - if it is it suggests something isn't working somewhere and we should debug it.
There was a problem hiding this comment.
ah, okay, I will have a look, thanks.
ae08142 to
d71fc28
Compare
|
@p0lyn0mial - can you please revert this PR to what it was supposed to do - it should work fine now. |
d71fc28 to
9248ccc
Compare
|
/test pull-kubernetes-e2e-gce-cos-alpha-features |
done, let's wait until |
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: 59d6f1f68fdf40df40d8646e5677303bda1f47e5 |
|
/hold cancel |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, p0lyn0mial, 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 |
|
/test pull-kubernetes-node-e2e-containerd |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
replace
ENABLE_CLIENT_GO_WATCH_LIST_ALPHAwithclient-go.WatchListClientgaterequires:
#122571
kubernetes/perf-tests#2506
#122721
xref: kubernetes/enhancements#3157
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
this change needs to be coordinated with updating https://github.com/kubernetes/perf-tests/blob/master/util-images/watch-list/main.go#L51 otherwise some performance test will show inaccurate results.
I used
https://grep.app/search?q=ENABLE_CLIENT_GO_WATCH_LIST_ALPHA&filter[repo.pattern][0]=kubernetes to find usages of the env var across k/* repository.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: