moonming commented on a change in pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#discussion_r477143439



##########
File path: .travis/linux_openresty_mtls_runner.sh
##########
@@ -94,7 +94,8 @@ script() {
     sudo service etcd stop
     mkdir -p ~/etcd-data
     /usr/bin/etcd --listen-client-urls 'http://0.0.0.0:2379' 
--advertise-client-urls='http://0.0.0.0:2379' --data-dir ~/etcd-data > 
/dev/null 2>&1 &
-    etcd --version
+    etcdctl --version
+    export ETCDCTL_API=3

Review comment:
       ditto

##########
File path: .travis/linux_apisix_current_luarocks_runner.sh
##########
@@ -50,7 +50,8 @@ script() {
     sudo service etcd stop
     mkdir -p ~/etcd-data
     /usr/bin/etcd --listen-client-urls 'http://0.0.0.0:2379' 
--advertise-client-urls='http://0.0.0.0:2379' --data-dir ~/etcd-data > 
/dev/null 2>&1 &
-    etcd --version
+    etcdctl --version
+    export ETCDCTL_API=3

Review comment:
       why need `export ETCDCTL_API=3`? Is etcd support v3 by default in CI?

##########
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"
+    local t, err = io.popen(cmd)
+    if not t then
+        return nil, "failed to execute command: " .. cmd .. ", error info:" .. 
err
+    end
+    local data = t:read("*all")
+    t:close()
+    return prefix_v3[data:sub(-4,-2)]
+end
+_M.etcd_version_from_cmd = etcd_version_from_cmd

Review comment:
       if `etcd_version_from_cmd` failed, `_M.etcd_version_from_cmd` will be 
`nil`, is test case cover this?

##########
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",
                                    "/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 .. "/"
+            local cmd = "etcdctl --endpoints=" .. host .. " put " .. key .. " 
init_dir"

Review comment:
       `etcdctl` maybe not install in apisix node

##########
File path: t/core/etcd-auth.t
##########
@@ -18,23 +18,28 @@ BEGIN {
     $ENV{"ETCD_ENABLE_AUTH"} = "true"
 }
 
-use t::APISIX 'no_plan';
+use t::APISIX;
 
 repeat_each(1);
 no_long_string();
 no_root_location();
 log_level("info");
 
-# Authentication is enabled at etcd and credentials are set
-system('etcdctl --endpoints="http://127.0.0.1:2379"; -u root:5tHkHhYkjr6cQY 
user add root:5tHkHhYkjr6cQY');
-system('etcdctl --endpoints="http://127.0.0.1:2379"; -u root:5tHkHhYkjr6cQY 
auth enable');
-system('etcdctl --endpoints="http://127.0.0.1:2379"; -u root:5tHkHhYkjr6cQY 
role revoke --path "/*" -rw guest');
+my $etcd_version = `etcdctl version`;
+if ($etcd_version =~ /etcdctl version: 3.2/) {
+    plan(skip_all => "skip for etcd version v3.2");
+} else {

Review comment:
       why etcd 3.2 is special?

##########
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",

Review comment:
       maybe we can only support 3.4+?

##########
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:
       Is user must install `etcdctl` in the APISIX node?

##########
File path: apisix/core/etcd.lua
##########
@@ -44,24 +73,143 @@ 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
+
+
+local function notfound(res)
+    res.body.message = "Key not found"
+    res.reason = "Not found"
+    res.status = 404
+    return res
+end
+
+
+function _M.postget(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
+

Review comment:
       To be honest, I don’t understand these codes




----------------------------------------------------------------
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]


Reply via email to