feature: support etcd v3, by mocking v2 API#2036
Conversation
|
TODO:
|
|
It seems travis ci passed in my personal repo, but github CI failed quite early. I'm a bit confused here. UPDATE: |
|
Currently implemented multi-version support (auto change etcd prefix) in t/APISIX.pm, worked for most tests. But for test files like t/admin/stream-routes-disable.t, which change the content of config.yaml before test would not get the prefix change in APISIX.pm. So right now mannually set etcd prefix to "/v3alpha" to try to pass the github ci |
|
Fixed by iterating through watch response.
Currently there is only one error in test file, which is the last test of t/plugin/key-auth.t. Normally in etcd v2, it would add 20 consumers and find the 13th. But in current implementation of etcd v3, the test would add 20 consumers but only get first three of them, so it could not get the 13th. However, if I rerun the test, the test would get all 20 consumers and passed. I think it might related to the implementation of waitdir between two versions, but I could not find the way to debug. I print |
Use v3 or v2 protocol, their test results should be the same. Because |
I think your guess is correct, it should be a |
Is there any reason that could cause the difference? Or the default benchmark test is not suitable for this change |
I think you need to check the error log first for confirming some detail. If you need more help, you need to provide your benchmark step one by one. |
|
Note: Running on Linux is ok. |
we can fix this in a new PR, here is the related issue: #2227 |
| # limitations under the License. | ||
| # | ||
|
|
||
| wget https://github.com/etcd-io/etcd/releases/download/v3.4.0/etcd-v3.4.0-linux-amd64.tar.gz |
There was a problem hiding this comment.
Just a suggestion, you can refer to the way of docker: https://github.com/apache/apisix/pull/2225/files#diff-65e6a3c4290328a0a57797b4cf3de4d2R39.
we can not modify it in this PR
| end | ||
|
|
||
|
|
||
| function _M.watch_format(v3res) |
There was a problem hiding this comment.
Need a more meaningful function name
|
|
||
|
|
||
| function _M.get_format(res, realkey) | ||
| if res.body.error == "etcdserver: user name is empty" then |
There was a problem hiding this comment.
does etcd has error code with msg?
There was a problem hiding this comment.
Some does and some don't. For example in this case, ngx.status is set to 400.
{
body = {
code = 3,
error = "etcdserver: user name is empty",
message = "etcdserver: user name is empty"
},
body_reader = <function 1>,
has_body = true,
headers = {...},
read_body = <function 4>,
read_trailers = <function 5>,
reason = "Bad Request",
status = 400,
trailer_reader = <function 6>
}
|
|
||
| function _M.get_format(res, realkey) | ||
| if res.body.error == "etcdserver: user name is empty" then | ||
| return nil, "insufficient credentials code: 401" |
There was a problem hiding this comment.
why not return res.body.error?
There was a problem hiding this comment.
and do we need to deal with other error msg in res.body.error?
There was a problem hiding this comment.
- The error resulted from
getwhen etcd auth failed in v3 is different from v2, so I tried to keep it the same - That's the only difference I know between v2 and v3. We could return other error directly.
|
Currently the log produced by lua-resty-etcd would show message without base64 decode. For example Shall we do the decode on etcd side, or just remove these logs like v2 did? |
we could optimize it in another pr |
@nic-chen I think we need to release a newer version of lua-resty-etcd, since the current version would output TONS of logs. |
|
@Yiyiyimu you can create a new Github issue if you find some other things. this PR has been merged, we should use the new issue to resolve the problem. |
|
TODO: we need to add doc for etcd migration |
6 participants
What this PR does / why we need it:
A simplified way to support etcd v3, comparing to #1943, by mocking both requests and responses of v2 API.
#1767
Pre-submission checklist:
The discuss thread in mailing list: https://lists.apache.org/thread.html/r9a6dd85cc5388547ce1c20446d18366ed6f11844cacbb7bdd6be6005%40%3Cdev.apisix.apache.org%3E