[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r489141555
##
File path: bin/apisix
##
@@ -879,35 +887,35 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
+local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
+-- check if etcd version above 3.4
+cmd = "curl " .. host .. "/version 2>&1"
local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
+local op_ver = str_split(res, "\"")[4]
+local need_ver = "3.4.0"
+if not check_version(op_ver, need_ver) then
+io.stderr:write("etcd version must >=", need_ver, " current ",
op_ver, "\n")
return
end
-end
-
-local etcd_ok = false
-for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
"/proto"}) do
-local cmd = "curl " .. uri .. dir_name
-.. "?prev_exist=false -X PUT -d dir=true "
-.. "--connect-timeout " .. timeout
+local key = (etcd_conf.prefix or "") .. dir_name .. "/"
+
+local base64_encode = require("base64").encode
+local uri = host .. "/v3/kv/put"
+local post_json = '{"value":"' .. base64_encode("init_dir") ..
'", "key":"' .. base64_encode(key) .. '"}'
+cmd = "curl " .. uri .. " -X POST -d '" .. post_json
+.. "' --connect-timeout " .. timeout
.. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
local res = excute_cmd(cmd)
-if not res:find("index", 1, true)
-and not res:find("createdIndex", 1, true) then
+if (etcd_version == "v3" and not res:find("OK", 1, true)) then
Review comment:
you can create a new issue about this
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488331969
##
File path: bin/apisix
##
@@ -873,35 +873,26 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
-for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
-local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
-return
-end
-end
-
local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
"/proto"}) do
-local cmd = "curl " .. uri .. dir_name
-.. "?prev_exist=false -X PUT -d dir=true "
-.. "--connect-timeout " .. timeout
+local key = (etcd_conf.prefix or "") .. dir_name .. "/"
+
+local base64_encode = require("base64").encode
Review comment:
I checked the source code of `base64`:
https://github.com/iskolbin/lbase64/blob/master/base64.lua
It is pure Lua source code, confirm it works fine with `Lua 5.1+` and
`LuaJIT`, and it under MIT license.
We can use this library.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488328057
##
File path: t/core/etcd.t
##
@@ -0,0 +1,335 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: delete test data if exists
+--- config
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /delete
+--- no_error_log
+[error]
+--- ignore_response
+
+
+
+=== TEST 2: (add + update + delete) *2 (same uri)
+--- config
+location /add {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /update {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 2
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- pipelined_requests eval
+["GET /add", "GET /hello", "GET /update", "GET /hello", "GET /delete", "GET
/hello",
Review comment:
got it, keep the current way ^_^
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488306915
##
File path: rockspec/apisix-master-0.rockspec
##
@@ -52,6 +52,7 @@ dependencies = {
"lua-resty-kafka = 0.07",
"lua-resty-logger-socket = 2.0-0",
"skywalking-nginx-lua-plugin = 1.0-0",
+"base64 = 1.5-2"
Review comment:
same as https://github.com/apache/apisix/pull/2036/files#r487795007
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488305432
##
File path: apisix/core/etcd.lua
##
@@ -44,24 +49,144 @@ end
_M.new = new
+local function kvs_to_node(kvs)
+local node = {}
+node.key = kvs.key
+node.value = kvs.value
+node.createdIndex = tonumber(kvs.create_revision)
+node.modifiedIndex = tonumber(kvs.mod_revision)
+return node
+end
+
+local function kvs_to_nodes(res, start_index)
+res.body.node.dir = true
+res.body.node.nodes = {}
+for i=start_index, #res.body.kvs do
+if start_index == 1 then
+res.body.node.nodes[i] = kvs_to_node(res.body.kvs[i])
+else
+res.body.node.nodes[i-1] = kvs_to_node(res.body.kvs[i])
Review comment:
many thx for your explain
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036: URL: https://github.com/apache/apisix/pull/2036#discussion_r488305164 ## File path: README.md ## @@ -166,10 +166,9 @@ There are several ways to install the Apache Release version of APISIX: apisix start ``` -**Note**: Apache APISIX does not yet support the v3 protocol of etcd, so you need to enable v2 protocol when starting etcd. -We are doing support for etcd v3 protocol. +**Note**: Apache APISIX would not support the v2 protocol of etcd anymore since APISIX v2.0, so you need to enable v3 protocol when starting etcd, if etcd version is below v3.4. Review comment: got it, thx This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488305087
##
File path: apisix/core/config_etcd.lua
##
@@ -92,17 +105,29 @@ local function waitdir(etcd_cli, key, modified_index,
timeout)
return nil, nil, "not inited"
end
-local res, err = etcd_cli:waitdir(key, modified_index, timeout)
+local opts = {}
+opts.start_revision = modified_index
+opts.timeout = timeout
+local res_fun, fun_err = etcd_cli:watchdir(key, opts)
+if not res_fun then
+return nil, fun_err
+end
+
+-- try twice to skip create info
Review comment:
got it. thx
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r488008439
##
File path: t/core/etcd.t
##
@@ -0,0 +1,335 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: delete test data if exists
+--- config
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /delete
+--- no_error_log
+[error]
+--- ignore_response
+
+
+
+=== TEST 2: (add + update + delete) *2 (same uri)
+--- config
+location /add {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /update {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 2
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- pipelined_requests eval
+["GET /add", "GET /hello", "GET /update", "GET /hello", "GET /delete", "GET
/hello",
Review comment:
I rewrite `=== TEST 2: (add + update + delete) *2 (same uri)` right now,
please confirm if it is acceptable.
@Yiyiyimu @nic-chen
```
=== TEST 2: create route(id: 1)
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"host": "foo.com",
"uri": "/hello"
}]],
nil
)
ngx.status = code
ngx.say(body)
}
}
--- request
GET /t
--- error_code: 201
--- no_error_log
[error]
=== TEST 3: missing route(no host)
--- request
GET /hello
--- error_code: 404
--- no_error_log
[error]
=== TEST 4: hit routes
--- request
GET /hello
--- more_headers
Host: foo.com
--- response_body
hello world
--- no_error_log
[error]
=== TEST 5: update route(host: foo2.com)
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r487990382
##
File path: t/core/etcd.t
##
@@ -0,0 +1,335 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: delete test data if exists
+--- config
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /delete
+--- no_error_log
+[error]
+--- ignore_response
+
+
+
+=== TEST 2: (add + update + delete) *2 (same uri)
+--- config
+location /add {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /update {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1',
+ngx.HTTP_PUT,
+[[{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 2
+},
+"type": "roundrobin"
+},
+"host": "foo.com",
+"uri": "/hello"
+}]],
+nil
+)
+ngx.status = code
+ngx.say(body)
+}
+}
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(body)
+}
+}
+--- pipelined_requests eval
+["GET /add", "GET /hello", "GET /update", "GET /hello", "GET /delete", "GET
/hello",
Review comment:
let me make a try ^_^
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r487989714
##
File path: apisix/core/etcd.lua
##
@@ -44,24 +49,144 @@ end
_M.new = new
+local function kvs_to_node(kvs)
+local node = {}
+node.key = kvs.key
+node.value = kvs.value
+node.createdIndex = tonumber(kvs.create_revision)
Review comment:
got it. many thx
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r487773165
##
File path: apisix/core/config_etcd.lua
##
@@ -92,17 +105,29 @@ local function waitdir(etcd_cli, key, modified_index,
timeout)
return nil, nil, "not inited"
end
-local res, err = etcd_cli:waitdir(key, modified_index, timeout)
+local opts = {}
+opts.start_revision = modified_index
+opts.timeout = timeout
+local res_fun, fun_err = etcd_cli:watchdir(key, opts)
+if not res_fun then
+return nil, fun_err
+end
+
+-- try twice to skip create info
Review comment:
need more comments, it makes me confused
##
File path: README.md
##
@@ -166,10 +166,9 @@ There are several ways to install the Apache Release
version of APISIX:
apisix start
```
-**Note**: Apache APISIX does not yet support the v3 protocol of etcd, so you
need to enable v2 protocol when starting etcd.
-We are doing support for etcd v3 protocol.
+**Note**: Apache APISIX would not support the v2 protocol of etcd anymore
since APISIX v2.0, so you need to enable v3 protocol when starting etcd, if
etcd version is below v3.4.
Review comment:
we still not release `v2.0` now. We need to add a comment here
##
File path: apisix/core/etcd.lua
##
@@ -44,24 +49,144 @@ end
_M.new = new
+local function kvs_to_node(kvs)
+local node = {}
+node.key = kvs.key
+node.value = kvs.value
+node.createdIndex = tonumber(kvs.create_revision)
Review comment:
is it possible the `kvs.create_revision` is non-numeric value?
##
File path: apisix/core/etcd.lua
##
@@ -44,24 +49,144 @@ end
_M.new = new
+local function kvs_to_node(kvs)
+local node = {}
+node.key = kvs.key
+node.value = kvs.value
+node.createdIndex = tonumber(kvs.create_revision)
+node.modifiedIndex = tonumber(kvs.mod_revision)
+return node
+end
+
+local function kvs_to_nodes(res, start_index)
+res.body.node.dir = true
+res.body.node.nodes = {}
+for i=start_index, #res.body.kvs do
+if start_index == 1 then
+res.body.node.nodes[i] = kvs_to_node(res.body.kvs[i])
+else
+res.body.node.nodes[i-1] = kvs_to_node(res.body.kvs[i])
+end
+end
+return res
+end
+
+
+local function not_found(res)
+res.body.message = "Key not found"
+res.reason = "Not found"
+res.status = 404
+return res
+end
+
+
+function _M.get_format(res, realkey)
+if res.body.error == "etcdserver: user name is empty" then
+return nil, "insufficient credentials code: 401"
+end
+
+res.headers["X-Etcd-Index"] = res.body.header.revision
+
+if not res.body.kvs then
+return not_found(res)
+end
+res.body.action = "get"
+
+res.body.node = kvs_to_node(res.body.kvs[1])
+-- kvs.value = nil, so key is root
+if type(res.body.kvs[1].value) == "userdata" or not res.body.kvs[1].value
then
+-- remove last "/" when necesary
+if string.sub(res.body.node.key, -1, -1) == "/" then
+res.body.node.key = string.sub(res.body.node.key, 1,
#res.body.node.key-1)
+end
+res = kvs_to_nodes(res, 2)
+-- key not match, so realkey is root
+elseif res.body.kvs[1].key ~= realkey then
+res.body.node.key = realkey
+res = kvs_to_nodes(res, 1)
+-- first is root (in v2, root not contains value), others are nodes
+elseif #res.body.kvs > 1 then
+res = kvs_to_nodes(res, 2)
+end
+
+res.body.kvs = nil
+return res, nil
Review comment:
`, nil` useless, we can remove it
##
File path: t/core/etcd.t
##
@@ -0,0 +1,335 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+log_level("info");
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: delete test data if exists
+--- config
+location /delete {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/routes/1', ngx.HTTP_DELETE)
+ngx.status = code
+ngx.say(bo
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r487755488
##
File path: .travis/linux_tengine_runner.sh
##
@@ -259,6 +259,12 @@ do_install() {
sudo luarocks install luacheck > build.log 2>&1 || (cat build.log && exit
1)
+wget
https://github.com/etcd-io/etcd/releases/download/v3.4.0/etcd-v3.4.0-linux-amd64.tar.gz
+tar xf etcd-v3.4.0-linux-amd64.tar.gz
+sudo cp etcd-v3.4.0-linux-amd64/etcd /usr/local/bin/
+sudo cp etcd-v3.4.0-linux-amd64/etcdctl /usr/local/bin/
+rm -rf etcd-v3.4.0-linux-amd64
Review comment:
Duplication is acceptable here so that each use case script is
independent.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r479726294
##
File path: rockspec/apisix-master-0.rockspec
##
@@ -52,6 +52,7 @@ dependencies = {
"lua-resty-kafka = 0.07",
"lua-resty-logger-socket = 2.0-0",
"skywalking-nginx-lua-plugin = 1.0-0",
+"luaposix = 35.0-1",
Review comment:
as I said, we do not need `init_etcd` in v3 protocol.
I think we can remove the function `init_etcd` totally.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r477402880
##
File path: bin/apisix
##
@@ -870,35 +870,21 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
-for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
-local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
-return
-end
-end
-
local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
Review comment:
we can remove those `for` code in etcd v3 now, it is useless
##
File path: apisix/core/etcd.lua
##
@@ -15,11 +15,38 @@
-- limitations under the License.
--
local fetch_local_conf = require("apisix.core.config_local").local_conf
-local etcd = require("resty.etcd")
-local clone_tab = require("table.clone")
+local etcd = require("resty.etcd")
+local clone_tab= require("table.clone")
+local io = io
+local type = type
+local ipairs = ipairs
+local string = string
+local tonumber = tonumber
local _M = {version = 0.1}
+local prefix_v3 = {
+["3.5"] = "/v3",
+["3.4"] = "/v3",
+["3.3"] = "/v3beta",
+["3.2"] = "/v3alpha",
+}
+
+
+-- TODO: Default lua-resty-etcd version auto-detection is broken, so directly
get version from cmd
+-- we don't need to call this so many times, need to save it in some
place
+local function etcd_version_from_cmd()
+local cmd = "export ETCDCTL_API=3 && etcdctl version"
Review comment:
here is an example code:
https://github.com/api7/lua-resty-etcd/blob/master/lib/resty/etcd.lua#L17
##
File path: apisix/core/etcd.lua
##
@@ -15,11 +15,38 @@
-- limitations under the License.
--
local fetch_local_conf = require("apisix.core.config_local").local_conf
-local etcd = require("resty.etcd")
-local clone_tab = require("table.clone")
+local etcd = require("resty.etcd")
+local clone_tab= require("table.clone")
+local io = io
+local type = type
+local ipairs = ipairs
+local string = string
+local tonumber = tonumber
local _M = {version = 0.1}
+local prefix_v3 = {
+["3.5"] = "/v3",
+["3.4"] = "/v3",
+["3.3"] = "/v3beta",
+["3.2"] = "/v3alpha",
+}
+
+
+-- TODO: Default lua-resty-etcd version auto-detection is broken, so directly
get version from cmd
+-- we don't need to call this so many times, need to save it in some
place
+local function etcd_version_from_cmd()
+local cmd = "export ETCDCTL_API=3 && etcdctl version"
Review comment:
If we only fetch the etcd version, we can use `etcd` v2 library, it
should work fine.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r475212996
##
File path: rockspec/apisix-master-0.rockspec
##
@@ -52,6 +52,7 @@ dependencies = {
"lua-resty-kafka = 0.07",
"lua-resty-logger-socket = 2.0-0",
"skywalking-nginx-lua-plugin = 1.0-0",
+"luaposix = 35.0-1",
Review comment:
where we use this module `luaposix`? please send the code link which
used this library.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r475192884
##
File path: apisix/core/config_etcd.lua
##
@@ -268,86 +289,90 @@ local function sync_data(self)
return false, err
end
-local key = short_key(self, res.key)
-if res.value and type(res.value) ~= "table" then
-self:upgrade_version(res.modifiedIndex)
-return false, "invalid item data of [" .. self.key .. "/" .. key
- .. "], val: " .. tostring(res.value)
- .. ", it shoud be a object"
-end
-
-if res.value and self.item_schema then
-local ok, err = check_schema(self.item_schema, res.value)
-if not ok then
+local res_copy = res
+for _, res in ipairs(res_copy) do
+local key = short_key(self, res.key)
+if res.value and type(res.value) ~= "table" then
self:upgrade_version(res.modifiedIndex)
-
-return false, "failed to check item data of ["
- .. self.key .. "] err:" .. err
+return false, "invalid item data of [" .. self.key .. "/" .. key
+.. "], val: " .. tostring(res.value)
Review comment:
bad indentation
##
File path: rockspec/apisix-master-0.rockspec
##
@@ -52,6 +52,7 @@ dependencies = {
"lua-resty-kafka = 0.07",
"lua-resty-logger-socket = 2.0-0",
"skywalking-nginx-lua-plugin = 1.0-0",
+"luaposix = 35.0-1",
Review comment:
why we need this Lua module?
##
File path: bin/apisix
##
@@ -870,35 +870,30 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
-for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
-local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
-return
-end
-end
-
local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
"/proto"}) do
-local cmd = "curl " .. uri .. dir_name
-.. "?prev_exist=false -X PUT -d dir=true "
-.. "--connect-timeout " .. timeout
-.. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+local res = require("posix.stdlib").setenv("ETCDCTL_API", 3)
+local key = (etcd_conf.prefix or "") .. dir_name .. "/"
+-- use suggested v3 ctl `etcdctl` and avoid base64 conversion
+--local base64 = require("base64")
+--uri = host .. "/v3/kv/put"
+--local post_json = '{"value":"' .. base64.encode("null") .. '",
"key":"' .. base64.encode(key) .. '"}'
+--cmd = "curl " .. uri
Review comment:
only one way is easier for user. `etcdctl` is enough.
##
File path: apisix/core/config_etcd.lua
##
@@ -268,86 +289,90 @@ local function sync_data(self)
return false, err
end
-local key = short_key(self, res.key)
-if res.value and type(res.value) ~= "table" then
-self:upgrade_version(res.modifiedIndex)
-return false, "invalid item data of [" .. self.key .. "/" .. key
- .. "], val: " .. tostring(res.value)
- .. ", it shoud be a object"
-end
-
-if res.value and self.item_schema then
-local ok, err = check_schema(self.item_schema, res.value)
-if not ok then
+local res_copy = res
+for _, res in ipairs(res_copy) do
+local key = short_key(self, res.key)
+if res.value and type(res.value) ~= "table" then
self:upgrade_version(res.modifiedIndex)
-
-return false, "failed to check item data of ["
- .. self.key .. "] err:" .. err
+return false, "invalid item data of [" .. self.key .. "/" .. key
+.. "], val: " .. tostring(res.value)
Review comment:
if the `res.value` is a talbe, we need to encode it with `json`
if the `res.value` is non-table, we do not need to call `tostring`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about thi
[GitHub] [apisix] membphis commented on a change in pull request #2036: feature: support etcd v3, by mocking v2 API
membphis commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r475095648
##
File path: apisix/core/etcd.lua
##
@@ -15,11 +15,36 @@
-- limitations under the License.
--
local fetch_local_conf = require("apisix.core.config_local").local_conf
-local etcd = require("resty.etcd")
-local clone_tab = require("table.clone")
+local etcd = require("resty.etcd")
+local clone_tab= require("table.clone")
+local io = io
+local type = type
+local ipairs = ipairs
+local string = string
+local tonumber = tonumber
local _M = {version = 0.1}
+local prefix_v3 = {
+["3.5"] = "/v3",
+["3.4"] = "/v3",
+["3.3"] = "/v3beta",
+["3.2"] = "/v3alpha",
+}
+-- TODO: Default lua-resty-etcd version auto-detection is broken, so directly
get version from cmd
Review comment:
style: two blank lines
##
File path: bin/apisix
##
@@ -870,35 +870,30 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
-for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
-local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
-return
-end
-end
-
local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
"/proto"}) do
-local cmd = "curl " .. uri .. dir_name
-.. "?prev_exist=false -X PUT -d dir=true "
-.. "--connect-timeout " .. timeout
-.. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+local res = require("posix.stdlib").setenv("ETCDCTL_API", 3)
+local key = (etcd_conf.prefix or "") .. dir_name .. "/"
+-- use suggested v3 ctl `etcdctl` and avoid base64 conversion
+--local base64 = require("base64")
+--uri = host .. "/v3/kv/put"
+--local post_json = '{"value":"' .. base64.encode("null") .. '",
"key":"' .. base64.encode(key) .. '"}'
+--cmd = "curl " .. uri
Review comment:
we can remove the old comments?
##
File path: apisix/core/etcd.lua
##
@@ -43,25 +70,140 @@ local function new()
end
_M.new = new
+local function kvs2node(kvs)
+local node = {}
+node.key = kvs.key
+node.value = kvs.value
+node.createdIndex = tonumber(kvs.create_revision)
+node.modifiedIndex = tonumber(kvs.mod_revision)
+return node
+end
+
Review comment:
two blank lines between different functions.
and please fix some other similar points
##
File path: bin/apisix
##
@@ -870,35 +870,30 @@ local function init_etcd(show_output)
local host_count = #(yaml_conf.etcd.host)
--- check whether the user has enabled etcd v2 protocol
-for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/v2/keys"
-local cmd = "curl -i -m ".. timeout * 2 .. " -o /dev/null -s -w
%{http_code} " .. uri
-local res = excute_cmd(cmd)
-if res == "404" then
-io.stderr:write(string.format("failed: please make sure that you
have enabled the v2 protocol of etcd on %s.\n", host))
-return
-end
-end
-
local etcd_ok = false
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-uri = host .. "/v2/keys" .. (etcd_conf.prefix or "")
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
"/proto"}) do
-local cmd = "curl " .. uri .. dir_name
-.. "?prev_exist=false -X PUT -d dir=true "
-.. "--connect-timeout " .. timeout
-.. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+local res = require("posix.stdlib").setenv("ETCDCTL_API", 3)
+local key = (etcd_conf.prefix or "") .. dir_name .. "/"
+-- use suggested v3 ctl `etcdctl` and avoid base64 conversion
+--local base64 = require("base64")
+--uri = host .. "/v3/kv/put"
+--local post_json = '{"value"
