[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548829822
##
File path: apisix/cli/etcd.lua
##
@@ -179,14 +179,14 @@ function _M.init(env, show_output)
local post_json_auth = dkjson.encode(json_auth)
local response_body = {}
-local _, err = http.request{url = auth_url, method = "POST",
+local res, err = http.request{url = auth_url, method = "POST",
source =
ltn12.source.string(post_json_auth),
sink = ltn12.sink.table(response_body),
headers = {["Content-Length"] =
#post_json_auth}}
-- In case of failure, request returns nil followed by an error
message.
-- Else the first return value is just the number 1
Review comment:
Got it
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548827770
##
File path: apisix/cli/etcd.lua
##
@@ -179,14 +179,14 @@ function _M.init(env, show_output)
local post_json_auth = dkjson.encode(json_auth)
local response_body = {}
-local _, err = http.request{url = auth_url, method = "POST",
+local res, err = http.request{url = auth_url, method = "POST",
source =
ltn12.source.string(post_json_auth),
sink = ltn12.sink.table(response_body),
headers = {["Content-Length"] =
#post_json_auth}}
-- In case of failure, request returns nil followed by an error
message.
-- Else the first return value is just the number 1
Review comment:
The first return value isn't the response body?
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548771546
##
File path: apisix/cli/etcd.lua
##
@@ -160,31 +163,38 @@ function _M.init(env, show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-local token_head = ""
+local errmsg
+local auth_token
local user = yaml_conf.etcd.user
local password = yaml_conf.etcd.password
if user and password then
-local uri_auth = host .. "/v3/auth/authenticate"
+local auth_url = host .. "/v3/auth/authenticate"
local json_auth = {
name = etcd_conf.user,
password = etcd_conf.password
}
-local post_json_auth = dkjson.encode(json_auth)
-local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" ..
- post_json_auth .. "' --connect-timeout " ..
timeout
- .. " --max-time " .. timeout * 2 .. " --retry 1
2>&1"
-local res_auth = util.execute_cmd(cmd_auth)
-local body_auth, _, err_auth = dkjson.decode(res_auth)
-if err_auth then
-util.die(cmd_auth, "\n", res_auth)
+local post_json_auth = dkjson.encode(json_auth)
+local response_body = {}
+local _, err = http.request{url = auth_url, method = "POST",
+source =
ltn12.source.string(post_json_auth),
+sink = ltn12.sink.table(response_body),
+headers = {["Content-Length"] =
#post_json_auth}}
+-- err is string type
+if err and type(err) == "string" then
Review comment:
I think it would be better to use `if not res then handle err`? Since
the first argument is nil when error happened. This way is more normal.
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548771546
##
File path: apisix/cli/etcd.lua
##
@@ -160,31 +163,38 @@ function _M.init(env, show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-local token_head = ""
+local errmsg
+local auth_token
local user = yaml_conf.etcd.user
local password = yaml_conf.etcd.password
if user and password then
-local uri_auth = host .. "/v3/auth/authenticate"
+local auth_url = host .. "/v3/auth/authenticate"
local json_auth = {
name = etcd_conf.user,
password = etcd_conf.password
}
-local post_json_auth = dkjson.encode(json_auth)
-local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" ..
- post_json_auth .. "' --connect-timeout " ..
timeout
- .. " --max-time " .. timeout * 2 .. " --retry 1
2>&1"
-local res_auth = util.execute_cmd(cmd_auth)
-local body_auth, _, err_auth = dkjson.decode(res_auth)
-if err_auth then
-util.die(cmd_auth, "\n", res_auth)
+local post_json_auth = dkjson.encode(json_auth)
+local response_body = {}
+local _, err = http.request{url = auth_url, method = "POST",
+source =
ltn12.source.string(post_json_auth),
+sink = ltn12.sink.table(response_body),
+headers = {["Content-Length"] =
#post_json_auth}}
+-- err is string type
+if err and type(err) == "string" then
Review comment:
I think it would be better to use `if not res then handle err`? Since
the first argument is nil when error happened.
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548537555
##
File path: apisix/cli/etcd.lua
##
@@ -160,31 +163,38 @@ function _M.init(env, show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-local token_head = ""
+local errmsg
+local auth_token
local user = yaml_conf.etcd.user
local password = yaml_conf.etcd.password
if user and password then
-local uri_auth = host .. "/v3/auth/authenticate"
+local auth_url = host .. "/v3/auth/authenticate"
local json_auth = {
name = etcd_conf.user,
password = etcd_conf.password
}
-local post_json_auth = dkjson.encode(json_auth)
-local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" ..
- post_json_auth .. "' --connect-timeout " ..
timeout
- .. " --max-time " .. timeout * 2 .. " --retry 1
2>&1"
-local res_auth = util.execute_cmd(cmd_auth)
-local body_auth, _, err_auth = dkjson.decode(res_auth)
-if err_auth then
-util.die(cmd_auth, "\n", res_auth)
+local post_json_auth = dkjson.encode(json_auth)
+local response_body = {}
+local _, err = http.request{url = auth_url, method = "POST",
+source =
ltn12.source.string(post_json_auth),
+sink = ltn12.sink.table(response_body),
+headers = {["Content-Length"] =
#post_json_auth}}
+-- err is string type
+if err and type(err) == "string" then
Review comment:
I mean, why we need to check the type of `err`? We need to clarify it in
the comment, so that people can know it without searching luasocket's doc.
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548434807
##
File path: apisix/cli/etcd.lua
##
@@ -160,31 +163,38 @@ function _M.init(env, show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-local token_head = ""
+local errmsg
+local auth_token
local user = yaml_conf.etcd.user
local password = yaml_conf.etcd.password
if user and password then
-local uri_auth = host .. "/v3/auth/authenticate"
+local auth_url = host .. "/v3/auth/authenticate"
local json_auth = {
name = etcd_conf.user,
password = etcd_conf.password
}
-local post_json_auth = dkjson.encode(json_auth)
-local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" ..
- post_json_auth .. "' --connect-timeout " ..
timeout
- .. " --max-time " .. timeout * 2 .. " --retry 1
2>&1"
-local res_auth = util.execute_cmd(cmd_auth)
-local body_auth, _, err_auth = dkjson.decode(res_auth)
-if err_auth then
-util.die(cmd_auth, "\n", res_auth)
+local post_json_auth = dkjson.encode(json_auth)
+local response_body = {}
+local _, err = http.request{url = auth_url, method = "POST",
+source =
ltn12.source.string(post_json_auth),
+sink = ltn12.sink.table(response_body),
+headers = {["Content-Length"] =
#post_json_auth}}
+-- err is string type
+if err and type(err) == "string" then
Review comment:
Better to add more info about the `err` handle, so that people can know
what you are doing without looking into luasocket's doc.
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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965: URL: https://github.com/apache/apisix/pull/2965#discussion_r537210013 ## File path: .travis/apisix_cli_test.sh ## @@ -817,3 +817,53 @@ fi done echo "passed: etcd auth enabled and init kv has been set up correctly" + +# check etcd connect refused +git checkout conf/config.yaml + +echo ' +etcd: + host: +- "http://127.0.0.1:2389; + prefix: "/apisix" +' > conf/config.yaml + +make init &>/tmp/apisix_temp & Review comment: @starsz You can try this way: https://github.com/apache/apisix/blob/c41aa8534a18f43ef0c3cd6780f106595f9f9b88/.travis/apisix_cli_test_in_ci.sh#L43 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] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
spacewander commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r535974232
##
File path: .travis/apisix_cli_test.sh
##
@@ -817,3 +817,53 @@ fi
done
echo "passed: etcd auth enabled and init kv has been set up correctly"
+
+# check etcd connect refused
+git checkout conf/config.yaml
+
+echo '
+etcd:
+ host:
+- "http://127.0.0.1:2389;
+ prefix: "/apisix"
+' > conf/config.yaml
+
+make init &>/tmp/apisix_temp &
Review comment:
Why need to run in the background?
##
File path: apisix/cli/etcd.lua
##
@@ -106,9 +111,6 @@ function _M.init(env, show_output)
local etcd_conf = yaml_conf.etcd
-local timeout = etcd_conf.timeout or 3
Review comment:
Should allow configuring timeout in the new way
##
File path: apisix/cli/etcd.lua
##
@@ -132,22 +134,23 @@ function _M.init(env, show_output)
-- check the etcd cluster version
for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/version"
-local cmd = str_format("curl -s -m %d %s", timeout * 2, uri)
-local res = util.execute_cmd(cmd)
-local errmsg = str_format("got malformed version message: \"%s\" from
etcd\n",
- res)
+local version_url = host .. "/version"
+local errmsg
-local body, _, err = dkjson.decode(res)
-if err then
+local res, err = http.request(version_url)
Review comment:
Look like this library doesn't support max-timeout? Only operation level
timeout is supported.
##
File path: .travis/apisix_cli_test.sh
##
@@ -817,3 +817,53 @@ fi
done
echo "passed: etcd auth enabled and init kv has been set up correctly"
+
+# check etcd connect refused
+git checkout conf/config.yaml
+
+echo '
+etcd:
+ host:
+- "http://127.0.0.1:2389;
+ prefix: "/apisix"
+' > conf/config.yaml
+
+make init &>/tmp/apisix_temp &
+sleep 1
+if [`grep -c "connection refused" /tmp/apisix_temp` -ne '1']; then
+echo "failed: not output connection refused"
+exit 1
+fi
+
+echo "passed: show connection refused info successfully"
+
+# check etcd auth error
+git checkout conf/config.yaml
+
+export ETCDCTL_API=3
+etcdctl version
+etcdctl --endpoints=127.0.0.1:2379 user add "root:apache-api6"
Review comment:
Need to clean the user/role after running the test
##
File path: apisix/cli/etcd.lua
##
@@ -191,26 +206,36 @@ function _M.init(env, show_output)
local key = (etcd_conf.prefix or "") .. dir_name .. "/"
-local uri = host .. "/v3/kv/put"
+local put_url = host .. "/v3/kv/put"
local post_json = '{"value":"' .. base64_encode("init_dir")
.. '", "key":"' .. base64_encode(key) .. '"}'
-local cmd = "curl " .. uri .. token_head .. " -X POST -d '" ..
post_json
-.. "' --connect-timeout " .. timeout
-.. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+local response_body = {}
+local headers = {["Content-Length"] = #post_json}
+if auth_token then
+headers["Authorization"] = auth_token
+end
-local res = util.execute_cmd(cmd)
-if res:find("error", 1, true) then
+local _, err = http.request{url = put_url, method = "POST",
+source =
ltn12.source.string(post_json),
+sink = ltn12.sink.table(response_body),
+headers = headers}
+if err and type(err) == "string" then
+errmsg = str_format("request etcd endpoint \"%s\" error,
%s\n", host, err)
Review comment:
Better to log the full uri
##
File path: apisix/cli/etcd.lua
##
@@ -132,22 +134,23 @@ function _M.init(env, show_output)
-- check the etcd cluster version
for index, host in ipairs(yaml_conf.etcd.host) do
-uri = host .. "/version"
-local cmd = str_format("curl -s -m %d %s", timeout * 2, uri)
-local res = util.execute_cmd(cmd)
-local errmsg = str_format("got malformed version message: \"%s\" from
etcd\n",
- res)
+local version_url = host .. "/version"
+local errmsg
-local body, _, err = dkjson.decode(res)
-if err then
+local res, err = http.request(version_url)
+if err and type(err) == "string" then
+errmsg = str_format("request etcd endpoint \'%s\' error, %s\n",
host, err)
Review comment:
Better to log the full uri
##
File path: apisix/cli/etcd.lua
##
@@ -160,27 +163,39 @@ function _M.init(env, show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
-local token_head = ""
+local errmsg
+
