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/10/20 01:32:51 UTC
[GitHub] [apisix] moonming commented on a change in pull request #2455: feat: new plugin, api breaker
moonming commented on a change in pull request #2455:
URL: https://github.com/apache/apisix/pull/2455#discussion_r508153943
##########
File path: apisix/plugins/api-breaker.lua
##########
@@ -0,0 +1,248 @@
+--
+-- 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.
+--
+
+local core = require("apisix.core")
+local plugin_name = "api-breaker"
+local ngx = ngx
+local math = math
+local error = error
+local ipairs = ipairs
+
+local shared_buffer = ngx.shared['plugin-'.. plugin_name]
+if not shared_buffer then
+ error("failed to get ngx.shared dict when load plugin " .. plugin_name)
+end
+
+
+local schema = {
+ type = "object",
+ properties = {
+ break_response_code = {
+ type = "integer",
+ minimum = 200,
+ maximum = 599,
+ },
+ max_breaker_sec = {
+ type = "integer",
+ minimum = 3,
+ default = 300,
+ },
+ unhealthy = {
+ type = "object",
+ properties = {
+ http_statuses = {
+ type = "array",
+ minItems = 1,
+ items = {
+ type = "integer",
+ minimum = 500,
+ maximum = 599,
+ },
+ uniqueItems = true,
+ default = {500}
+ },
+ failures = {
+ type = "integer",
+ minimum = 1,
+ default = 3,
+ }
+ },
+ default = {http_statuses = {500}, failures = 3}
+ },
+ healthy = {
+ type = "object",
+ properties = {
+ http_statuses = {
+ type = "array",
+ minItems = 1,
+ items = {
+ type = "integer",
+ minimum = 200,
+ maximum = 499,
+ },
+ uniqueItems = true,
+ default = {200}
+ },
+ successes = {
+ type = "integer",
+ minimum = 1,
+ default = 3,
+ }
+ },
+ default = {http_statuses = {200}, successes = 3}
+ }
+ },
+ required = {"break_response_code"},
+}
+
+
+-- todo: we can move this into `core.talbe`
Review comment:
please create a issue for this, only comment is not enough
##########
File path: t/plugin/api-breaker.t
##########
@@ -0,0 +1,593 @@
+#
+# 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_shuffle();
+no_root_location();
+log_level('info');
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local ok, err = plugin.check_schema({
+ break_response_code = 502,
+ unhealthy = {
+ http_statuses = {500},
+ failures = 1,
+ },
+ healthy = {
+ http_statuses = {200},
+ successes = 1,
+ },
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: default configuration
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: default `healthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ healthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: default `unhealthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ unhealthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+--- LAST
Review comment:
remove it
##########
File path: apisix/plugins/api-breaker.lua
##########
@@ -0,0 +1,248 @@
+--
+-- 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.
+--
+
+local core = require("apisix.core")
+local plugin_name = "api-breaker"
+local ngx = ngx
+local math = math
+local error = error
+local ipairs = ipairs
+
+local shared_buffer = ngx.shared['plugin-'.. plugin_name]
+if not shared_buffer then
+ error("failed to get ngx.shared dict when load plugin " .. plugin_name)
+end
+
+
+local schema = {
+ type = "object",
+ properties = {
+ break_response_code = {
+ type = "integer",
+ minimum = 200,
+ maximum = 599,
+ },
+ max_breaker_sec = {
+ type = "integer",
+ minimum = 3,
+ default = 300,
+ },
+ unhealthy = {
+ type = "object",
+ properties = {
+ http_statuses = {
+ type = "array",
+ minItems = 1,
+ items = {
+ type = "integer",
+ minimum = 500,
+ maximum = 599,
+ },
+ uniqueItems = true,
+ default = {500}
+ },
+ failures = {
+ type = "integer",
+ minimum = 1,
+ default = 3,
+ }
+ },
+ default = {http_statuses = {500}, failures = 3}
+ },
+ healthy = {
+ type = "object",
+ properties = {
+ http_statuses = {
+ type = "array",
+ minItems = 1,
+ items = {
+ type = "integer",
+ minimum = 200,
+ maximum = 499,
+ },
+ uniqueItems = true,
+ default = {200}
+ },
+ successes = {
+ type = "integer",
+ minimum = 1,
+ default = 3,
+ }
+ },
+ default = {http_statuses = {200}, successes = 3}
+ }
+ },
+ required = {"break_response_code"},
+}
+
+
+-- todo: we can move this into `core.talbe`
+local function array_find(array, val)
+ for i, v in ipairs(array) do
+ if v == val then
+ return i
+ end
+ end
+
+ return nil
+end
+
+
+local function gen_healthy_key(ctx)
+ return "healthy-" .. core.request.get_host(ctx) .. ctx.var.uri
+end
+
+
+local function gen_unhealthy_key(ctx)
+ return "unhealthy-" .. core.request.get_host(ctx) .. ctx.var.uri
+end
+
+
+local function gen_unhealthy_lastime_key(ctx)
+ return "unhealthy-lastime" .. core.request.get_host(ctx) .. ctx.var.uri
+end
+
+
+local _M = {
+ version = 0.1,
+ name = plugin_name,
+ priority = 1005,
+ schema = schema,
+}
+
+
+function _M.check_schema(conf)
+ return core.schema.check(schema, conf)
+end
+
+
+function _M.access(conf, ctx)
+ local unhealthy_key = gen_unhealthy_key(ctx)
+ -- unhealthy counts
+ local unhealthy_count, err = shared_buffer:get(unhealthy_key)
+ if err then
+ core.log.warn("failed to get unhealthy_key: ",
+ unhealthy_key, " err: ", err)
+ return
+ end
+
+ if not unhealthy_count then
+ return
+ end
+
+ -- timestamp of the last time a unhealthy state was triggered
+ local unhealthy_lastime_key = gen_unhealthy_lastime_key(ctx)
+ local unhealthy_lastime, err = shared_buffer:get(unhealthy_lastime_key)
+ if err then
+ core.log.warn("failed to get unhealthy_lastime_key: ",
+ unhealthy_lastime_key, " err: ", err)
+ return
+ end
+
+ if not unhealthy_lastime then
+ return
+ end
+
+ local failure_times = math.ceil(unhealthy_count / conf.unhealthy.failures)
+ if failure_times < 1 then
+ failure_times = 1
+ end
+
+ -- cannot exceed the maximum value of the user configuration
+ local breaker_time = 2 ^ failure_times
+ if breaker_time > conf.max_breaker_sec then
+ breaker_time = conf.max_breaker_sec
+ end
+ core.log.info("breaker_time: ", breaker_time)
+
+ -- breaker
+ if unhealthy_lastime + breaker_time >= ngx.time() then
Review comment:
what is `lastime`?
##########
File path: doc/zh-cn/plugins/api-breaker.md
##########
@@ -0,0 +1,116 @@
+<!--
+#
+# 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.
+#
+-->
+
+- [English](../../plugins/api-blocker.md)
+
+# 目录
+
+- [**定义**](#定义)
+- [**属性列表**](#属性列表)
+- [**启用方式**](#启用方式)
+- [**测试插件**](#测试插件)
+- [**禁用插件**](#禁用插件)
+
+## 定义
+
+该插件实现 API 熔断功能，帮助我们保护上游业务服务。
+
+> 关于熔断超时逻辑
+
+由代码逻辑自动按**触发不健康状态**的次数递增运算：
+
+每当上游服务返回`unhealthy.http_statuses`配置中的状态码(比如：500)，达到`unhealthy.failures`次时(比如：3 次)，认为上游服务处于不健康状态。
+
+第一次触发不健康状态，**熔断 2 秒**。
+
+然后，2 秒过后重新开始转发请求到上游服务，如果继续返回`unhealthy.http_statuses`状态码，记数再次达到`unhealthy.failures`次时，**熔断 4 秒**（倍数方式）。
+
+依次类推，2, 4, 8, 16, 32, 64, ..., 256, 300，最大到 `max_breaker_sec`。
Review comment:
why `300` after `256`?
##########
File path: t/plugin/api-breaker.t
##########
@@ -0,0 +1,593 @@
+#
+# 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_shuffle();
+no_root_location();
+log_level('info');
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local ok, err = plugin.check_schema({
+ break_response_code = 502,
+ unhealthy = {
+ http_statuses = {500},
+ failures = 1,
+ },
+ healthy = {
+ http_statuses = {200},
+ successes = 1,
+ },
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: default configuration
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: default `healthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ healthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: default `unhealthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ unhealthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+--- LAST
+
+
+
+=== TEST 5: add plugin
Review comment:
move this test case before test 10
##########
File path: t/plugin/api-breaker.t
##########
@@ -0,0 +1,593 @@
+#
+# 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_shuffle();
+no_root_location();
+log_level('info');
+run_tests;
+
+__DATA__
+
+=== TEST 1: sanity
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local ok, err = plugin.check_schema({
+ break_response_code = 502,
+ unhealthy = {
+ http_statuses = {500},
+ failures = 1,
+ },
+ healthy = {
+ http_statuses = {200},
+ successes = 1,
+ },
+ })
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say("done")
+ }
+ }
+--- request
+GET /t
+--- response_body
+done
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: default configuration
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: default `healthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ healthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: default `unhealthy`
+--- config
+ location /t {
+ content_by_lua_block {
+ local plugin = require("apisix.plugins.api-breaker")
+ local conf = {
+ break_response_code = 502,
+ unhealthy = {}
+ }
+
+ local ok, err = plugin.check_schema(conf)
+ if not ok then
+ ngx.say(err)
+ end
+
+ ngx.say(require("lib.json_sort").encode(conf))
+ }
+ }
+--- request
+GET /t
+--- response_body
+{"break_response_code":502,"healthy":{"http_statuses":[200],"successes":3},"max_breaker_sec":300,"unhealthy":{"failures":3,"http_statuses":[500]}}
+--- no_error_log
+[error]
+--- LAST
+
+
+
+=== TEST 5: add plugin
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "api-breaker": {
+ "break_response_code": 502,
+ "unhealthy": {
+ "http_statuses": [500, 503],
+ "failures": 3
+ },
+ "healthy": {
+ "http_statuses": [200, 206],
+ "successes": 3
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/api_breaker"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: bad break_response_code
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "api-breaker": {
+ "break_response_code": 199,
+ "unhealthy": {
+ "http_statuses": [500, 503],
+ "failures": 3
+ },
+ "healthy": {
+ "http_statuses": [200, 206],
+ "successes": 3
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/api_breaker"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: bad max_breaker_sec
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "api-breaker": {
+ "break_response_code": 200,
+ "max_breaker_sec": -1
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/api_breaker"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- no_error_log
+[error]
+
+
+
+=== TEST 8: bad unhealthy.http_statuses
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "api-breaker": {
+ "break_response_code": 200,
+ "max_breaker_sec": 40,
+ "unhealthy": {
+ "http_statuses": [500, 603],
+ "failures": 3
+ },
+ "healthy": {
+ "http_statuses": [200, 206],
+ "successes": 3
+ }
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/api_breaker"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- error_code: 400
+--- no_error_log
+[error]
+
+
+
+=== TEST 9: dup http_statuses
Review comment:
Title and content are not the same
----------------------------------------------------------------
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