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":"' .. base64.encode("null") .. '",
"key":"' .. base64.encode(key) .. '"}'
+ --cmd = "curl " .. uri
+ -- .. " -X POST -d '" .. post_json
+ -- .. "' --connect-timeout " .. timeout
+ -- .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+ local cmd = "etcdctl --endpoints=" .. host .. " put " .. key .. "
init_dir"
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
+ -- (etcd_version == "v3" and not res:find("revision", 1,
true)) then
Review comment:
if it is useless, we can remove it
##########
File path: doc/install-dependencies.md
##########
@@ -28,12 +28,12 @@
Note
====
-- Apache APISIX currently only supports the v2 protocol storage to etcd, but
the latest version of etcd (starting with 3.4) has turned off the v2 protocol
by default.
+- Since v2.0, Apache APISIX would not support the v2 protocol storage to etcd
anymore. If etcd version is below 3.4, the default protocol is still v2 and you
need to turn on v3 protocol mannually.
Review comment:
we still not release `v2.0` now.
----------------------------------------------------------------
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:
[email protected]
