-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update KEP 3157 (watch-list) for milestone 1.31 #4605
Update KEP 3157 (watch-list) for milestone 1.31 #4605
Conversation
p0lyn0mial
commented
Apr 30, 2024
- One-line PR description: Update KEP 3157 (watch-list) for milestone 1.31
- Issue link: Allow informers for getting a stream of data instead of chunking #3157
- Other comments: N/A
@p0lyn0mial - can we combine this PR with the other one updating criteria? |
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# The following PRR answers are required at beta release
metrics:
- my_feature_metric
I think this needs a tweak for PRR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, thx, interestingly i haven't found a kep that would provide info in this field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, my query was broken, there are KEPs that use this field, e.g.
b3d8812
to
21a7977
Compare
- [Switch](https://github.com/kubernetes/kubernetes/blob/a07b1aaa5b39b351ec8586de800baa5715304a3f/staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go#L416) | ||
the `storage/cacher` to use streaming directly from etcd | ||
(This will also allow us to [remove](https://github.com/kubernetes/kubernetes/blob/a07b1aaa5b39b351ec8586de800baa5715304a3f/staging/src/k8s.io/client-go/tools/cache/reflector.go#L110) the `reflector.UseWatchList` field). | ||
- Make **list** calls expensive in APF. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be post GA. clients need time to update. but I agree in principle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to capture it as a new section Post-GA
(will not match the template) or will it be okay to update the text ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add "Post-GA' section for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add "Post-GA' section for this
Yes. Spoke with Lucasz this morning and explained that we need to ensure the fallback list behavior works until all supported releases have streaming list GA and we have turned on "lock to on". After that we can increase the cost of regular list calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, thanks, pushed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: WatchListClient (the actual name might be different because it hasn't been added yet))
Is that still true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name is correct - https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/features/known_features.go#L39
In general, we are aware that this KEP is outdated and we are planing to change it to reflect the actual implementation.
21a7977
to
99ac308
Compare
99ac308
to
378db8e
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, p0lyn0mial 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 |