From 15c0b86cb198e57bec0d083cdb2f0f92ce15a978 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Sat, 9 Oct 2021 01:07:47 +0800 Subject: [PATCH 1/7] feat(ext-plugin): avoid sending conf request more times MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two cases where the conf request needs to be forced to be resent: 1. conf has been updated 2. the plugin runner returns 'conf token not found'， will retry For case 1, since the key used by the shared dict is lrucache_id, the lrucache_id will need to be updated after the conf is updated, so it is satisfied. For case 2, work in progress Signed-off-by: tzssangglass --- apisix/cli/ngx_tpl.lua | 3 +++ apisix/plugins/ext-plugin/init.lua | 23 +++++++++++++++++++++-- conf/config-default.yaml | 1 + t/APISIX.pm | 1 + 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/apisix/cli/ngx_tpl.lua b/apisix/cli/ngx_tpl.lua index 07ad5ea35454..6b78764f875f 100644 --- a/apisix/cli/ngx_tpl.lua +++ b/apisix/cli/ngx_tpl.lua @@ -191,6 +191,9 @@ http { # for authz-keycloak lua_shared_dict access-tokens {* http.lua_shared_dict["access-tokens"] *}; # cache for service account access tokens + # for ext-plugin + lua_shared_dict ext-plugin {* http.lua_shared_dict["ext-plugin"] *}; # cache for ext-plugin + # for custom shared dict {% if http.custom_lua_shared_dict then %} {% for cache_key, cache_size in pairs(http.custom_lua_shared_dict) do %} diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua index 6cb593c8c1d1..5c92ba5f33e0 100644 --- a/apisix/plugins/ext-plugin/init.lua +++ b/apisix/plugins/ext-plugin/init.lua @@ -60,6 +60,7 @@ local ipairs = ipairs local pairs = pairs local tostring = tostring local type = type +local dict = ngx.shared["ext-plugin"] local events_list @@ -292,6 +293,19 @@ local function handle_extra_info(ctx, input) end +local function fetch_token(key) + return dict:get(key) +end + + +local function store_token(key, token) + local exp = helper.get_conf_token_cache_time() + -- early expiry, lrucache in critical state sends prepare_conf_req as original behaviour + exp = exp - 1 + dict:set(key, token, exp) +end + + local rpc_call local rpc_handlers = { nil, @@ -300,6 +314,11 @@ local rpc_handlers = { local key = builder:CreateString(unique_key) + local token = fetch_token(key) + if token then + return token + end + local conf_vec if conf.conf then local len = #conf.conf @@ -344,9 +363,10 @@ local rpc_handlers = { local buf = flatbuffers.binaryArray.New(resp) local pcr = prepare_conf_resp.GetRootAsResp(buf, 0) - local token = pcr:ConfToken() + token = pcr:ConfToken() core.log.notice("get conf token: ", token, " conf: ", core.json.delay_encode(conf.conf)) + store_token(key, token) return token end, function (conf, ctx, sock, entry) @@ -470,7 +490,6 @@ local rpc_handlers = { local buf = flatbuffers.binaryArray.New(resp) local call_resp = http_req_call_resp.GetRootAsResp(buf, 0) local action_type = call_resp:ActionType() - if action_type == http_req_call_action.Stop then local action = call_resp:Action() local stop = http_req_call_stop.New() diff --git a/conf/config-default.yaml b/conf/config-default.yaml index 5232a28f7a1d..bf209422a7bd 100644 --- a/conf/config-default.yaml +++ b/conf/config-default.yaml @@ -253,6 +253,7 @@ nginx_config: # config for render the template to generate n jwks: 1m introspection: 10m access-tokens: 1m + ext-plugin: 1m etcd: host: # it's possible to define multiple etcd hosts addresses of the same etcd cluster. diff --git a/t/APISIX.pm b/t/APISIX.pm index 5b960cc87f8b..83e28c810a7e 100644 --- a/t/APISIX.pm +++ b/t/APISIX.pm @@ -445,6 +445,7 @@ _EOC_ lua_shared_dict plugin-api-breaker 10m; lua_capture_error_log 1m; # plugin error-log-logger lua_shared_dict etcd-cluster-health-check 10m; # etcd health check + lua_shared_dict ext-plugin 1m; proxy_ssl_name \$upstream_host; proxy_ssl_server_name on; From 6cf3c257ffdbd63238c735643c22135239598511 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 14 Oct 2021 01:17:49 +0800 Subject: [PATCH 2/7] fix CI error Signed-off-by: tzssangglass --- apisix/plugins/ext-plugin/init.lua | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua index 5c92ba5f33e0..d6c00008c12e 100644 --- a/apisix/plugins/ext-plugin/init.lua +++ b/apisix/plugins/ext-plugin/init.lua @@ -306,6 +306,12 @@ local function store_token(key, token) end +local function flush_token() + core.log.warn("flush conf token in shared dict") + dict:flush_all() +end + + local rpc_call local rpc_handlers = { nil, @@ -607,6 +613,8 @@ end local function create_lrucache() + flush_token() + if lrucache then core.log.warn("flush conf token lrucache") end From 65796b65a8f2d79ec76835c46c37c611504748ea Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 14 Oct 2021 15:45:50 +0800 Subject: [PATCH 3/7] add test cases Signed-off-by: tzssangglass --- apisix/plugins/ext-plugin/init.lua | 1 + t/plugin/ext-plugin/conf_token.t | 140 +++++++++++++++++++++++++++++ t/plugin/ext-plugin/sanity.t | 2 + 3 files changed, 143 insertions(+) create mode 100644 t/plugin/ext-plugin/conf_token.t diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua index d6c00008c12e..aa83ea564757 100644 --- a/apisix/plugins/ext-plugin/init.lua +++ b/apisix/plugins/ext-plugin/init.lua @@ -322,6 +322,7 @@ local rpc_handlers = { local token = fetch_token(key) if token then + core.log.info("fetch token from shared dict") return token end diff --git a/t/plugin/ext-plugin/conf_token.t b/t/plugin/ext-plugin/conf_token.t new file mode 100644 index 000000000000..fee05f634e2b --- /dev/null +++ b/t/plugin/ext-plugin/conf_token.t @@ -0,0 +1,140 @@ +# +# 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'; + +workers(4); +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); +log_level("info"); + +$ENV{"PATH"} = $ENV{PATH} . ":" . $ENV{TEST_NGINX_HTML_DIR}; + +add_block_preprocessor(sub { + my ($block) = @_; + + $block->set_value("stream_conf_enable", 1); + + if (!defined $block->extra_stream_config) { + my $stream_config = <<_EOC_; + server { + listen unix:\$TEST_NGINX_HTML_DIR/nginx.sock; + + content_by_lua_block { + local ext = require("lib.ext-plugin") + ext.go({}) + } + } + +_EOC_ + $block->set_value("extra_stream_config", $stream_config); + } + + my $unix_socket_path = $ENV{"TEST_NGINX_HTML_DIR"} . "/nginx.sock"; + my $orig_extra_yaml_config = $block->extra_yaml_config // ""; + my $cmd = $block->ext_plugin_cmd // "['sleep', '5s']"; + my $extra_yaml_config = <<_EOC_; +ext-plugin: + path_for_test: $unix_socket_path + cmd: $cmd +_EOC_ + $extra_yaml_config = $extra_yaml_config . $orig_extra_yaml_config; + + $block->set_value("extra_yaml_config", $extra_yaml_config); + + if (!$block->request) { + $block->set_value("request", "GET /t"); + } + + if (!$block->error_log) { + $block->set_value("no_error_log", "[error]\n[alert]"); + } +}); + +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local json = require("toolkit.json") + local t = require("lib.test_admin") + + local code, message, res = t.test('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "plugins": { + "ext-plugin-pre-req": {"a":"b"} + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + } + }]] + ) + + if code >= 300 then + ngx.status = code + ngx.say(message) + return + end + + ngx.say(message) + } + } +--- response_body +passed + + + +=== TEST 2: share conf token in different workers +--- ext_plugin_cmd +["t/plugin/ext-plugin/runner.sh", "3600"] +--- config + location /t { + content_by_lua_block { + local http = require "resty.http" + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local function r() + local httpc = http.new() + local res, err = httpc:request_uri(uri) + if not res then + ngx.log(ngx.ERR, err) + return + end + end + + for i = 1, 20 do + r() + ngx.sleep(0.001) + end + ngx.say("done") + } + } +--- response_body +done +--- timeout: 5 +--- error_log +fetch token from shared dict +--- no_error_log +[error] diff --git a/t/plugin/ext-plugin/sanity.t b/t/plugin/ext-plugin/sanity.t index 75b01f98c26e..2a5e965ffc3a 100644 --- a/t/plugin/ext-plugin/sanity.t +++ b/t/plugin/ext-plugin/sanity.t @@ -270,6 +270,7 @@ sending rpc type: 1 data length: receiving rpc type: 1 data length: --- error_log flush conf token lrucache +flush conf token in shared dict --- no_error_log [error] @@ -382,6 +383,7 @@ hello world } --- error_log refresh cache and try again +flush conf token in shared dict --- no_error_log [error] From 1036df60776d96cd630843a661a400ac5c9d2d15 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Thu, 14 Oct 2021 22:44:10 +0800 Subject: [PATCH 4/7] fix CI error Signed-off-by: tzssangglass --- t/plugin/ext-plugin/conf_token.t | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/t/plugin/ext-plugin/conf_token.t b/t/plugin/ext-plugin/conf_token.t index fee05f634e2b..eabdea1ff66b 100644 --- a/t/plugin/ext-plugin/conf_token.t +++ b/t/plugin/ext-plugin/conf_token.t @@ -22,6 +22,7 @@ no_long_string(); no_root_location(); no_shuffle(); log_level("info"); +worker_connections(1024); $ENV{"PATH"} = $ENV{PATH} . ":" . $ENV{TEST_NGINX_HTML_DIR}; @@ -115,25 +116,27 @@ passed content_by_lua_block { local http = require "resty.http" local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" - local function r() - local httpc = http.new() - local res, err = httpc:request_uri(uri) - if not res then - ngx.log(ngx.ERR, err) - return - end - end - for i = 1, 20 do - r() - ngx.sleep(0.001) + local t = {} + for i = 1, 180 do + local th = assert(ngx.thread.spawn(function(i) + local httpc = http.new() + local res, err = httpc:request_uri(uri) + if not res then + ngx.log(ngx.ERR, err) + return + end + end, i)) + table.insert(t, th) + end + for i, th in ipairs(t) do + ngx.thread.wait(th) end ngx.say("done") } } --- response_body done ---- timeout: 5 --- error_log fetch token from shared dict --- no_error_log From 75d9c1cc9f08735fd703acffbe670985df94bfee Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Fri, 15 Oct 2021 15:58:15 +0800 Subject: [PATCH 5/7] fix CI error Signed-off-by: tzssangglass --- t/plugin/ext-plugin/conf_token.t | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/t/plugin/ext-plugin/conf_token.t b/t/plugin/ext-plugin/conf_token.t index eabdea1ff66b..5d55e6916d6e 100644 --- a/t/plugin/ext-plugin/conf_token.t +++ b/t/plugin/ext-plugin/conf_token.t @@ -16,13 +16,13 @@ # use t::APISIX 'no_plan'; -workers(4); +workers(8); repeat_each(1); no_long_string(); no_root_location(); no_shuffle(); log_level("info"); -worker_connections(1024); +worker_connections(10240); $ENV{"PATH"} = $ENV{PATH} . ":" . $ENV{TEST_NGINX_HTML_DIR}; @@ -118,7 +118,7 @@ passed local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" local t = {} - for i = 1, 180 do + for i = 1, 200 do local th = assert(ngx.thread.spawn(function(i) local httpc = http.new() local res, err = httpc:request_uri(uri) @@ -130,6 +130,7 @@ passed table.insert(t, th) end for i, th in ipairs(t) do + ngx.sleep(0.001) ngx.thread.wait(th) end ngx.say("done") From 0389c6b70b2b9994f69d68cd5d8d7dc989b41306 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Sun, 17 Oct 2021 23:40:51 +0800 Subject: [PATCH 6/7] resolve code review Signed-off-by: tzssangglass --- apisix/plugins/ext-plugin/init.lua | 12 +++++++++--- t/plugin/ext-plugin/conf_token.t | 9 +++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua index aa83ea564757..0c2e078aa17d 100644 --- a/apisix/plugins/ext-plugin/init.lua +++ b/apisix/plugins/ext-plugin/init.lua @@ -301,8 +301,14 @@ end local function store_token(key, token) local exp = helper.get_conf_token_cache_time() -- early expiry, lrucache in critical state sends prepare_conf_req as original behaviour - exp = exp - 1 - dict:set(key, token, exp) + exp = exp * 0.9 + local success, err, forcible = dict:set(key, token, exp) + if not success then + core.log.error("ext-plugin:failed to set conf token, err: ", err) + end + if forcible then + core.log.warn("ext-plugin:set valid items forcibly overwritten") + end end @@ -322,7 +328,7 @@ local rpc_handlers = { local token = fetch_token(key) if token then - core.log.info("fetch token from shared dict") + core.log.info("fetch token from shared dict, token: ", token) return token end diff --git a/t/plugin/ext-plugin/conf_token.t b/t/plugin/ext-plugin/conf_token.t index 5d55e6916d6e..a33c674921b3 100644 --- a/t/plugin/ext-plugin/conf_token.t +++ b/t/plugin/ext-plugin/conf_token.t @@ -118,7 +118,7 @@ passed local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" local t = {} - for i = 1, 200 do + for i = 1, 180 do local th = assert(ngx.thread.spawn(function(i) local httpc = http.new() local res, err = httpc:request_uri(uri) @@ -130,7 +130,6 @@ passed table.insert(t, th) end for i, th in ipairs(t) do - ngx.sleep(0.001) ngx.thread.wait(th) end ngx.say("done") @@ -138,7 +137,9 @@ passed } --- response_body done ---- error_log -fetch token from shared dict +--- grep_error_log eval +qr/fetch token from shared dict, token: 233/ +--- grep_error_log_out eval +qr/(fetch token from shared dict, token: 233){1,}/ --- no_error_log [error] From 2080f086b14a733455541f255c76b7a171c50363 Mon Sep 17 00:00:00 2001 From: tzssangglass Date: Mon, 18 Oct 2021 00:25:52 +0800 Subject: [PATCH 7/7] resolve code review Signed-off-by: tzssangglass --- apisix/plugins/ext-plugin/init.lua | 37 ++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/apisix/plugins/ext-plugin/init.lua b/apisix/plugins/ext-plugin/init.lua index 0c2e078aa17d..afc25ff65908 100644 --- a/apisix/plugins/ext-plugin/init.lua +++ b/apisix/plugins/ext-plugin/init.lua @@ -294,27 +294,40 @@ end local function fetch_token(key) - return dict:get(key) + if dict then + return dict:get(key) + else + core.log.error('shm "ext-plugin" not found') + return nil + end end local function store_token(key, token) - local exp = helper.get_conf_token_cache_time() - -- early expiry, lrucache in critical state sends prepare_conf_req as original behaviour - exp = exp * 0.9 - local success, err, forcible = dict:set(key, token, exp) - if not success then - core.log.error("ext-plugin:failed to set conf token, err: ", err) - end - if forcible then - core.log.warn("ext-plugin:set valid items forcibly overwritten") + if dict then + local exp = helper.get_conf_token_cache_time() + -- early expiry, lrucache in critical state sends prepare_conf_req as original behaviour + exp = exp * 0.9 + local success, err, forcible = dict:set(key, token, exp) + if not success then + core.log.error("ext-plugin:failed to set conf token, err: ", err) + end + if forcible then + core.log.warn("ext-plugin:set valid items forcibly overwritten") + end + else + core.log.error('shm "ext-plugin" not found') end end local function flush_token() - core.log.warn("flush conf token in shared dict") - dict:flush_all() + if dict then + core.log.warn("flush conf token in shared dict") + dict:flush_all() + else + core.log.error('shm "ext-plugin" not found') + end end