You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/12/04 06:30:50 UTC
[GitHub] [apisix] starsz opened a new pull request #2965: feat: use luasocket instead of curl in etcd.lua
starsz opened a new pull request #2965:
URL: https://github.com/apache/apisix/pull/2965
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
fix: #2818
fix: #2718
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r537210247
##########
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:
Right. The library doesn't support max-timeout.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548792861
##########
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:
Well. Agree with you.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r552671455
##########
File path: apisix/cli/etcd.lua
##########
@@ -195,31 +210,44 @@ 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 res = util.execute_cmd(cmd)
- if res:find("404 page not found", 1, true) then
- util.die("gRPC gateway is not enabled in your etcd cluster, ",
- "which is required by Apache APISIX.", "\n")
+ local response_body = {}
+ local headers = {["Content-Length"] = #post_json}
+ if auth_token then
+ headers["Authorization"] = auth_token
+ end
+
+ local res, err = http.request{url = put_url, method = "POST",
Review comment:
style: I prefer `http.request({...})`, it is easy to read
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548542977
##########
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:
OK. I got it 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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548546322
##########
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:
Added.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] tokers merged pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #2965:
URL: https://github.com/apache/apisix/pull/2965
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548829689
##########
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:
Yep. It has two forms.
You can see more details about it.
>In case of failure, the function returns nil followed by an error message. If successful, the simple form returns the response body as a string, followed by the response status code, the response headers, and the response status line. The generic function returns the same information, except the first return value is just the number 1 (the body goes to the sink).
http://w3.impa.br/~diego/software/luasocket/http.html
##########
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:
Yep. It has two forms.
You can see more details about it.
>In case of failure, the function returns nil followed by an error message. If successful, the simple form returns the response body as a string, followed by the response status code, the response headers and the response status line. The generic function returns the same information, except the first return value is just the number 1 (the body goes to the sink).
http://w3.impa.br/~diego/software/luasocket/http.html
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548536838
##########
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:
@spacewander.Sorry, I can't get your point.
That we need to distinguish the `connection refused` and `timeout`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#issuecomment-750021190
Hi, @spacewander, please have a look when you are free.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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
+ 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 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
+ errmsg = str_format("request etcd endpoint \"%s\" error, %s\n", host, err)
Review comment:
Better to log the full uri
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r548536838
##########
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:
@spacewander Sorry, I can't get your point.
That we need to distinguish the `connection refused` and `timeout`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a change in pull request #2965: feat: use luasocket instead of curl in etcd.lua
Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #2965:
URL: https://github.com/apache/apisix/pull/2965#discussion_r537208917
##########
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:
Because the exit code of `make init` is 1. It will terminate the shell.
----------------------------------------------------------------
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:
users@infra.apache.org