-
Notifications
You must be signed in to change notification settings - Fork 250
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
Return an emulated etcd version in the status endpoint #316
Conversation
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
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.
LGTM, with a few nits
An additional question: if we fix the version reporting, Kubernetes will try to use watch progress notifications. However, Kubernetes also requires you to set the --experimental-watch-progress-notify-interval=5s
(instead of the default of 10m
) when using watch progress notifications. This is also true when using kine; it has the same default value for the --watch-progress-notify-interval
flag as upstream etcd.
Would you mind modifying the markdown files in the examples folder to call this out, and show the flag being set?
@brandond would you be open to changing the default value to 5s? |
Yeah, I can't see how it would hurt anything. K3s already sets it to 5 seconds, for both etcd and kine, so that things work right. I'm kinda curious why etcd didn't change the default as well, given that kubernetes will behave oddly if you don't. |
…upport Signed-off-by: Sambhav Kothari <skothari44@bloomberg.net>
@brandond all comments should be addressed. Could you also release a new version with these changes once the PR is merged? |
Note, I also added the following comment kubernetes/kubernetes#122805 (comment) |
Fixes #315
See /~https://github.com/kubernetes/kubernetes/blob/beb696c2c9467dbc44cbaf35c5a4a3daf0321db3/staging/src/k8s.io/apiserver/pkg/storage/feature/feature_support_checker.go#L157
Which expects that the etcd status endpoint return a version. This returns a default version allowing users to override it if necessary.