You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by me...@apache.org on 2020/10/05 02:22:35 UTC
[apisix] branch master updated: bugfix: create etcd object in
`xpcall`, this step may fail (#2312)
This is an automated email from the ASF dual-hosted git repository.
membphis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new 50c99a5 bugfix: create etcd object in `xpcall`, this step may fail (#2312)
50c99a5 is described below
commit 50c99a5e3ba9b6bb2cd1422b903de038bbce256c
Author: YuanSheng Wang <me...@gmail.com>
AuthorDate: Mon Oct 5 10:22:28 2020 +0800
bugfix: create etcd object in `xpcall`, this step may fail (#2312)
* bugfix: create the etcd object in `xpcall`, it may fail, the return values of `etcd.new` should be `res, err`.
fix issue: #2310
1. The old process, if creating etcd fails, etcd data will no longer be synchronized. We need to create the etcd object in xpcall.
2. the return value should be res, err of etcd.new.
* test: old test case is unstable, should delete some checkpoint which is wrong.
---
apisix/core/config_etcd.lua | 23 ++++++++++++---------
t/core/config_etcd.t | 49 +++++++++++++++++++++++++++++++++++++++++++++
t/node/invalid-service.t | 13 +++++-------
3 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua
index 28590e3..4f3fd55 100644
--- a/apisix/core/config_etcd.lua
+++ b/apisix/core/config_etcd.lua
@@ -82,13 +82,13 @@ end
local function readdir(etcd_cli, key)
if not etcd_cli then
- return nil, nil, "not inited"
+ return nil, "not inited"
end
local res, err = etcd_cli:readdir(key)
if not res then
-- log.error("failed to get key from etcd: ", err)
- return nil, nil, err
+ return nil, err
end
if type(res.body) ~= "table" then
@@ -407,16 +407,20 @@ local function _automatic_fetch(premature, self)
return
end
- local etcd_cli, _, err = etcd.new(self.etcd_conf)
- if not etcd_cli then
- error("failed to start a etcd instance: " .. err)
- end
- self.etcd_cli = etcd_cli
-
local i = 0
while not exiting() and self.running and i <= 32 do
i = i + 1
+
local ok, err = xpcall(function()
+ if not self.etcd_cli then
+ local etcd_cli, err = etcd.new(self.etcd_conf)
+ if not etcd_cli then
+ error("failed to create etcd instance for key ["
+ .. self.key .. "]: " .. (err or "unknown"))
+ end
+ self.etcd_cli = etcd_cli
+ end
+
local ok, err = sync_data(self)
if err then
if err ~= "timeout" and err ~= "Key not found"
@@ -437,6 +441,7 @@ local function _automatic_fetch(premature, self)
elseif not ok then
ngx_sleep(0.05)
end
+
end, debug.traceback)
if not ok then
@@ -499,7 +504,7 @@ function _M.new(key, opts)
ngx_timer_at(0, _automatic_fetch, obj)
else
- local etcd_cli, _, err = etcd.new(etcd_conf)
+ local etcd_cli, err = etcd.new(etcd_conf)
if not etcd_cli then
return nil, "failed to start a etcd instance: " .. err
end
diff --git a/t/core/config_etcd.t b/t/core/config_etcd.t
new file mode 100644
index 0000000..7944537
--- /dev/null
+++ b/t/core/config_etcd.t
@@ -0,0 +1,49 @@
+#
+# 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: wrong etcd port
+--- yaml_config
+apisix:
+ node_listen: 1984
+etcd:
+ host:
+ - "http://127.0.0.1:7777" -- wrong etcd port
+ timeout: 1
+--- config
+ location /t {
+ content_by_lua_block {
+ ngx.sleep(8)
+ ngx.say(body)
+ }
+ }
+--- timeout: 12
+--- request
+GET /t
+--- grep_error_log eval
+qr{failed to fetch data from etcd: connection refused, etcd key: .*routes}
+--- grep_error_log_out eval
+qr/(failed to fetch data from etcd: connection refused, etcd key: .*routes\n){1,}/
diff --git a/t/node/invalid-service.t b/t/node/invalid-service.t
index 1ba86f0..8764dc4 100644
--- a/t/node/invalid-service.t
+++ b/t/node/invalid-service.t
@@ -44,6 +44,7 @@ __DATA__
}
--- request
GET /t
+--- wait: 1
--- grep_error_log eval
qr/\[error\].*/
--- grep_error_log_out eval
@@ -53,7 +54,7 @@ qr/"value":"mexxxxxxxxxxxxxxx"/
-=== TEST 2: /not_found
+=== TEST 2: try /not_found, got error log
--- request
GET /not_found
--- error_code: 404
@@ -67,7 +68,7 @@ qr{invalid item data of \[/apisix/services/1\], val: mexxxxxxxxxxxxxxx, it shoud
-=== TEST 3: set valid service(id: 1)
+=== TEST 3: set valid service(id: 1), cover the old one
--- config
location /t {
content_by_lua_block {
@@ -82,7 +83,7 @@ qr{invalid item data of \[/apisix/services/1\], val: mexxxxxxxxxxxxxxx, it shoud
}]]))
if res.status >= 300 then
- res.status = code
+ ngx.status = code
end
ngx.print(core.json.encode(res.body))
@@ -90,13 +91,9 @@ qr{invalid item data of \[/apisix/services/1\], val: mexxxxxxxxxxxxxxx, it shoud
}
--- request
GET /t
+--- ret_code: 200
--- response_body_like eval
qr/"nodes":\{"127.0.0.1:1980":1\}/
---- wait: 1
---- grep_error_log eval
-qr/\[error\].*/
---- grep_error_log_out eval
-qr{invalid item data of \[/apisix/services/1\], val: mexxxxxxxxxxxxxxx, it shoud be a object}