feature: support etcd v3 by Yiyiyimu · Pull Request #1943 · apache/apisix

Yiyiyimu · 2020-07-30T09:26:46Z

What this PR does / why we need it:

fix #1767

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible?

save save

membphis · 2020-07-30T09:46:21Z

@Yiyiyimu love you, nice PR

moonming · 2020-07-31T04:13:20Z

    - "http://127.0.0.1:2379"     # multiple etcd address
  prefix: "/apisix"               # apisix configurations prefix
  timeout: 30                     # 3 seconds
+  version: "v3"                   # etcd version


I think we can only support etcd v3

In my plan, I'll implement for etcd v3 only at first, and then go to multi-version etcd, if necessary. And of course I'll be glad if we support v3 only.

moonming · 2020-07-31T04:15:38Z

@Yiyiyimu do we need to update lua-resty-etcd first?

Yiyiyimu · 2020-07-31T05:15:24Z

@Yiyiyimu do we need to update lua-resty-etcd first?

Hi @moonming I think v1.0 would be enough for now.

I'm trying to make all the changes on the APISIX side but there might be some modifications on lua-resty-etcd later, for example, compact(to avoid revision accumulation) has not been fully implemented yet. So I think it would be better to release newer version of lua-resty-etcd sometime later when everything on this side is settled down.

moonming · 2020-07-31T05:49:53Z

the same feature as #1941?

chelintsien · 2020-07-31T10:24:36Z

the same feature as #1941?

yes

Yiyiyimu · 2020-07-31T14:42:33Z

Hi @chelintsien @moonming supporting etcd v3 is a task of ISCAS summer plan (#1542)

nic-chen · 2020-07-31T16:26:54Z

Hi, @chelintsien, Thank you for your contribution!
As @Yiyiyimu said, etcd v3 support is a task of summer 2020. He spent a lot of time studying and researching and working with APISIX and lua-resty-etcd. And judging from the code submitted by this pr, I think he can do this job well. We should give him the opportunity to complete this task. You can help review the code, which is also a great contribution. What do you think?

@Yiyiyimu etcd v3 support is an urgent need, we should hurry up and complete it as soon as possible.

chelintsien · 2020-07-31T16:28:31Z

ok！ nic-chen <[email protected]>于2020年8月1日周六上午12:27写道：

Hi, @chelintsien <https://github.com/ChelinTsien>, Thank you for your contribution! As @Yiyiyimu <https://github.com/Yiyiyimu> said, etcd v3 support is a task of summer 2020. He spent a lot of time studying and researching and working with APISIX and lua-resty-etcd. And judging from the code submitted by this pr, I think he can do this job well. We should give him the opportunity to complete this task. You can help review the code, which is also a great contribution. What do you think? @Yiyiyimu <https://github.com/Yiyiyimu> etcd v3 support is an urgent need, we should hurry up and complete it as soon as possible. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1943 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMYPQ25LGZR5MPVO2PAE73R6LWGBANCNFSM4PNJOLEA> .

-- Best regards

chelintsien · 2020-07-31T16:39:11Z

Since the production environment uses apisix and apisix doesn't support etcd v3 at the moment so it took almost a day to deal with this issue, sorry for not seeing this assignment, agree with you! @nic-chen

nic-chen · 2020-07-31T16:45:32Z

@chelintsien Thank you for your hard work and the understanding. ：）

Yiyiyimu · 2020-07-31T17:56:09Z

@Yiyiyimu etcd v3 support is an urgent need, we should hurry up and complete it as soon as possible.

Sure I'll try to provide the minimum working PR ASAP

Yiyiyimu · 2020-08-01T14:36:46Z

Feature implemented in this commit:

Change etcd version directly in config.yaml and core.lua would do the job to select etcd.lua and config_etcd.lua of a certain version. Thanks to @nic-chen's idea.
Trying to make the output of v2&v3 stay the same, so we don't need to make a copy of test files just a little bit different from each other. The main modification is inside test_admin.t for now. And I'm still in the middle of trying to modify each test file to make it compatible with both v2 and v3 output. I'm not so sure if there would be a more elegant way to implement this feature

And another question hasn't been solved here is do we need multi-version of etcd? @moonming

Yiyiyimu · 2020-08-01T14:39:49Z

+        end
+        res_data = res_data.node and res_data.node or res_data
+        res_data = res_data.nodes and res_data.nodes or res_data
+        res_data = #res_data == 1 and res_data[1] or res_data


The main modification to make the output of v2&v3 stay the same

Yiyiyimu · 2020-08-01T14:52:54Z

Some difference of v2/v3 being solved:

--	v2	v3
`delete` key not exist	return 404	return 200, but response body.deleted(num) would become nil
implicitly `get` dir	the same of `readdir`(don't know why)	only return single k-v
response of `set`	return key and value	only return headers
normal response	k-v is under body/node	k-v is under body/kvs[1]
readdir response	k-v is under body/node/nodes[1..n]	k-v is under body/kvs[1..n]

membphis · 2020-08-02T01:41:24Z


 local config_center = local_conf.apisix and local_conf.apisix.config_center
                      or "etcd"
+local etcd_version = local_conf.etcd.version == "v3" and "_v3" or ""


all code in one line is not easy to read.

split it into multiple lines is better for this case.

Sure, just which one would be better, if-else, or like config_center to split or case into next line

chelintsien

need to support etcd v3 in file : bin/apisix (init_etcd function)

Yiyiyimu · 2020-08-02T09:36:35Z

need to support etcd v3 in file : bin/apisix (init_etcd function)

Thank you and your work! I've overlooked this part before

membphis · 2020-08-09T04:29:51Z

                        "uri": "/index.html"
                }]],
                [[{
-                    "node": {


we'd better keep the old response structure.

Hi @membphis! Response of etcd v3 is quite different from v2. For example like this, v3 would not return action, and flatten "node-nodes" structure into one layer "kvs". Thus I try to only return content inside node/nodes and kvs, to develop compatibility of different etcd version,

But modifying all test files is quite a big work. So I just submit a PR to mock etcd v2 response in #2036, keeping the old structure if we need.
But I still believe it could be better to get closer to v3 structure, since v2 could be deprecated one day, and there are some new functions of v3 that could not been mocked into v2.

Yiyiyimu · 2020-09-16T13:59:38Z

Close since #2036 is merged

Yiyiyimu added 7 commits July 19, 2020 17:36

init work environment

b2db39f

pseudo-push; v3 auth

c0e8cc5

config_etcd v3 compatibility

f566be2

save save

0727

818d4ec

demo

af04246

polish

bdaaf17

etcd v3.4

32357b9

moonming reviewed Jul 31, 2020

View reviewed changes

chelintsien mentioned this pull request Jul 31, 2020

etcd v3 api support #1941

Closed

Yiyiyimu added 3 commits August 1, 2020 14:02

rebase

a9cfa26

merge master

7799466

[WIP] v2/v3 output compatibility

ed38401

Yiyiyimu commented Aug 1, 2020

View reviewed changes

membphis requested changes Aug 2, 2020

View reviewed changes

chelintsien reviewed Aug 2, 2020

View reviewed changes

[WIP] admin

56e62b6

membphis reviewed Aug 9, 2020

View reviewed changes

Yiyiyimu mentioned this pull request Aug 10, 2020

feature: support etcd v3, by mocking v2 API #2036

Merged

4 tasks

Yiyiyimu closed this Sep 16, 2020

Uh oh!

Conversation

Yiyiyimu commented Jul 30, 2020

What this PR does / why we need it:

Pre-submission checklist:

Uh oh!

membphis commented Jul 30, 2020

Uh oh!

moonming Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu Jul 31, 2020

Choose a reason for hiding this comment

Uh oh!

moonming commented Jul 31, 2020

Uh oh!

Yiyiyimu commented Jul 31, 2020

Uh oh!

moonming commented Jul 31, 2020

Uh oh!

chelintsien commented Jul 31, 2020

Uh oh!

Yiyiyimu commented Jul 31, 2020

Uh oh!

nic-chen commented Jul 31, 2020

Uh oh!

chelintsien commented Jul 31, 2020 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chelintsien commented Jul 31, 2020

Uh oh!

nic-chen commented Jul 31, 2020

Uh oh!

Yiyiyimu commented Jul 31, 2020

Uh oh!

Yiyiyimu commented Aug 1, 2020

Uh oh!

Yiyiyimu Aug 1, 2020

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu commented Aug 1, 2020

Uh oh!

membphis Aug 2, 2020

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu Aug 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chelintsien left a comment

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu commented Aug 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

membphis Aug 9, 2020

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu Aug 10, 2020

Choose a reason for hiding this comment

Uh oh!

Yiyiyimu commented Sep 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

chelintsien commented Jul 31, 2020 via email •

edited

Loading

Yiyiyimu commented Aug 2, 2020 •

edited

Loading