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 <membp...@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}