-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
rafthttp: add 3.4.0,3.5.0 stream type #11274
Conversation
@YoyinZyc thanks for fixing! So the recently cut v3.4.2 missed the supportedStream, would you also please add there as well? etcd/etcdserver/api/rafthttp/stream.go Line 59 in bbe86b0
I will wait for a new 3.4 release then upgrade etcd to the new release: kubernetes/kubernetes#83987 |
I added it to 3.4 in another pr #11275 |
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. good catch!
@@ -57,6 +57,8 @@ var ( | |||
"3.1.0": {streamTypeMsgAppV2, streamTypeMessage}, | |||
"3.2.0": {streamTypeMsgAppV2, streamTypeMessage}, | |||
"3.3.0": {streamTypeMsgAppV2, streamTypeMessage}, | |||
"3.4.0": {streamTypeMsgAppV2, streamTypeMessage}, | |||
"3.5.0": {streamTypeMsgAppV2, streamTypeMessage}, |
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 write a test that fails if there is not entry in this map for the current etcd major version on master.
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.
That way it's impossible to forget to update this and have a passing build.
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.
SGTM
LGTM |
Test was added in #11279. I am merging this so the test can pass after rebase. |
Fix #11264
Basically, the bug is due to the failure of
checkStreamSupport
. We need to add stream support to 3.4 and 3.5.etcd/etcdserver/api/rafthttp/stream.go
Line 635 in 51b1677