-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
change: check etcd cluster version when init_etcd #2233
change: check etcd cluster version when init_etcd #2233
Conversation
the etcd in CI is 3.2, so failed: https://github.com/apache/apisix/pull/2233/checks?check_run_id=1116987808#step:6:288 |
@moonming It seems the version selection for etcd is not specified in |
we used to be the default version installed by github actions, now it is the etcd 3.4 version installed manually in #2036 |
@moonming OK, let me rebase my branch. By the way, i think it's better to check out the CHANGELOG of etcd, AKAIK there are some bugs in 3.x series releases, but i'm not sure the exact version range. |
@moonming I found PR #2036 also checked the etcd version, but IMHO the way is wrong. Firstly, when we ask the version message from etcd, it returns a JSON string like this: {"etcdserver":"3.5.0-pre","etcdcluster":"3.5.0"} we should check the version of Secondly, the semantic version check method doesn't consider the |
nice! we can fix it in this PR :)
Thanks,
Ming Wen
Twitter: _WenMing
Alex Zhang <[email protected]> 于2020年9月16日周三 上午10:36写道:
… @moonming <https://github.com/moonming> I found PR #2036
<#2036> also checked the etcd
version, but IMHO the way is wrong.
Firstly, when we ask the version message from etcd, it returns a JSON
string like this:
{"etcdserver":"3.5.0-pre","etcdcluster":"3.5.0"}
we should check the version of etcdcluster, not the endpoint's server
version (etcdserver) that we requested, the cluster version is the
minimal server version of all endpoints in the cluster. So that's the one
we need to check.
Secondly, the semantic version check method doesn't consider the pre
release situation, although it's rare in prod, i think's better to
consider it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2233 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGJZBK5HWE4ZRBDK3NWH743SGAQERANCNFSM4RM3Y5IA>
.
|
8f43ad7
to
a4cf24a
Compare
Signed-off-by: tokers <[email protected]>
a4cf24a
to
da05a15
Compare
@membphis Fixed. |
@tokers Thank you for the fix! |
Signed-off-by: tokers [email protected]
What this PR does / why we need it:
Pre-submission checklist: