From c92bea64155cc50292525ce90157fdb1507fd58b Mon Sep 17 00:00:00 2001 From: xumin Date: Wed, 9 Aug 2023 16:35:48 +0800 Subject: [PATCH] Fix(language): Critical messages during startup We used to check if the plugin server socket is ready by trying to connect, which will generate a lot of critical messages which cannot be suppressed due to OpenResty's limitation. As the logic is not really helpful, we decided to revert it. Revert "fix(pluginserver): error if req come before ready (#9507)" This reverts commit e7b6963f8c01c11232828d2709de08743708bf56. Fix KAG-2136 Fix #11084 Fix kong/kong-js-pdk#308 --- CHANGELOG.md | 2 ++ kong/runloop/plugin_servers/init.lua | 27 +++----------- kong/runloop/plugin_servers/mp_rpc.lua | 22 +++++------- kong/runloop/plugin_servers/pb_rpc.lua | 29 ++++++--------- kong/runloop/plugin_servers/process.lua | 47 ------------------------- 5 files changed, 26 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b72b9c1847a..ddf5f129b21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ [#11244](https://github.com/Kong/kong/pull/11244) - Add beta support for WebAssembly/proxy-wasm [#11218](https://github.com/Kong/kong/pull/11218) +- Fixed critical level logs when starting external plugin servers. Those logs cannot be suppressed due to the limitation of OpenResty. We choose to remove the socket availibilty detection feature. + [#11372](https://github.com/Kong/kong/pull/11372) #### Admin API diff --git a/kong/runloop/plugin_servers/init.lua b/kong/runloop/plugin_servers/init.lua index 43381bf168f..429657384bc 100644 --- a/kong/runloop/plugin_servers/init.lua +++ b/kong/runloop/plugin_servers/init.lua @@ -232,16 +232,6 @@ function get_instance_id(plugin_name, conf) end local plugin_info = get_plugin_info(plugin_name) - local server_def = plugin_info.server_def - - if server_def.socket_err then - return nil, server_def.socket_err - end - - if not server_def.ready then - return nil, "not ready" - end - local server_rpc = get_server_rpc(plugin_info.server_def) local new_instance_info, err = server_rpc:call_start_instance(plugin_name, conf) @@ -386,24 +376,15 @@ end function plugin_servers.start() - -- only worker 0 managers the plugin server - if worker_id() == 0 then - local pluginserver_timer = proc_mgmt.pluginserver_timer - - for _, server_def in ipairs(proc_mgmt.get_server_defs()) do - if server_def.start_command then - native_timer_at(0, pluginserver_timer, server_def) - end - end + if worker_id() ~= 0 then + return end - -- workers != 0 still need to get plugin servers definitions - -- - local connection_check_timer = proc_mgmt.connection_check_timer + local pluginserver_timer = proc_mgmt.pluginserver_timer for _, server_def in ipairs(proc_mgmt.get_server_defs()) do if server_def.start_command then - native_timer_at(0, connection_check_timer, server_def) + native_timer_at(0, pluginserver_timer, server_def) end end diff --git a/kong/runloop/plugin_servers/mp_rpc.lua b/kong/runloop/plugin_servers/mp_rpc.lua index 3763c817320..ebd0943b265 100644 --- a/kong/runloop/plugin_servers/mp_rpc.lua +++ b/kong/runloop/plugin_servers/mp_rpc.lua @@ -1,8 +1,5 @@ local kong_global = require "kong.global" local cjson = require "cjson.safe" -local handle_not_ready = require("kong.runloop.plugin_servers.process").handle_not_ready -local str_find = string.find - local msgpack do msgpack = require "MessagePack" local nil_pack = msgpack.packers["nil"] @@ -22,6 +19,7 @@ local kong = kong local cjson_encode = cjson.encode local mp_pack = msgpack.pack local mp_unpacker = msgpack.unpacker +local str_find = string.find local Rpc = {} @@ -328,20 +326,18 @@ end function Rpc:handle_event(plugin_name, conf, phase) - local instance_id, _, err - instance_id, err = self.get_instance_id(plugin_name, conf) - if not err then - _, err = bridge_loop(self, instance_id, phase) - end + local instance_id = self.get_instance_id(plugin_name, conf) + local _, err = bridge_loop(self, instance_id, phase) if err then - if err == "not ready" then - self.reset_instance(plugin_name, conf) - return handle_not_ready(plugin_name) + local ok, err2 = kong.worker_events.post("plugin_server", "reset_instances", + { plugin_name = plugin_name, conf = conf }) + if not ok then + kong.log.err("failed to post plugin_server reset_instances event: ", err2) end - if err and str_find(err:lower(), "no plugin instance", 1, true) then + + if str_find(err:lower(), "no plugin instance") then kong.log.warn(err) - self.reset_instance(plugin_name, conf) return self:handle_event(plugin_name, conf, phase) end kong.log.err(err) diff --git a/kong/runloop/plugin_servers/pb_rpc.lua b/kong/runloop/plugin_servers/pb_rpc.lua index 10872c33eff..aa170ccbd1b 100644 --- a/kong/runloop/plugin_servers/pb_rpc.lua +++ b/kong/runloop/plugin_servers/pb_rpc.lua @@ -3,8 +3,6 @@ local cjson = require "cjson.safe" local grpc_tools = require "kong.tools.grpc" local pb = require "pb" local lpack = require "lua_pack" -local handle_not_ready = require("kong.runloop.plugin_servers.process").handle_not_ready -local str_find = string.find local ngx = ngx local kong = kong @@ -12,6 +10,7 @@ local cjson_encode = cjson.encode local t_unpack = table.unpack -- luacheck: ignore table local st_pack = lpack.pack local st_unpack = lpack.unpack +local str_find = string.find local proto_fname = "kong/pluginsocket.proto" @@ -400,23 +399,17 @@ function Rpc:handle_event(plugin_name, conf, phase) end if not res or res == "" then - if err then - local ok, err = kong.worker_events.post("plugin_server", "reset_instances", - { plugin_name = plugin_name, conf = conf }) - if not ok then - kong.log.err("failed to post plugin_server reset_instances event: ", err) - end + local ok, err2 = kong.worker_events.post("plugin_server", "reset_instances", + { plugin_name = plugin_name, conf = conf }) + if not ok then + kong.log.err("failed to post plugin_server reset_instances event: ", err2) + end - if err == "not ready" then - self.reset_instance(plugin_name, conf) - return handle_not_ready(plugin_name) - end - if err and (str_find(err:lower(), "no plugin instance", 1, true) - or str_find(err:lower(), "closed", 1, true)) then - kong.log.warn(err) - self.reset_instance(plugin_name, conf) - return self:handle_event(plugin_name, conf, phase) - end + local err_lowered = err and err:lower() or "" + if str_find(err_lowered, "no plugin instance") + or str_find(err_lowered, "closed") then + kong.log.warn(err) + return self:handle_event(plugin_name, conf, phase) end kong.log.err(err) end diff --git a/kong/runloop/plugin_servers/process.lua b/kong/runloop/plugin_servers/process.lua index 5f88809c905..79c5ee44c7b 100644 --- a/kong/runloop/plugin_servers/process.lua +++ b/kong/runloop/plugin_servers/process.lua @@ -1,18 +1,13 @@ local cjson = require "cjson.safe" local pl_path = require "pl.path" local raw_log = require "ngx.errlog".raw_log -local ngx = ngx -local sleep = ngx.sleep -local connect = ngx.socket.connect local is_not_http_subsystem = ngx.config.subsystem ~= "http" local _, ngx_pipe = pcall(require, "ngx.pipe") local kong = kong -local log = ngx.log local ngx_INFO = ngx.INFO -local ngx_WARN = ngx.WARN local cjson_decode = cjson.decode local proc_mgmt = {} @@ -203,36 +198,6 @@ local function grab_logs(proc, name) end end --- if a plugin server is not ready after 20s of waiting we consider it failed -local pluginserver_start_timeout = 20 - -function proc_mgmt.connection_check_timer(premature, server_def) - if premature then - return - end - - if is_not_http_subsystem then - return - end - - local step = 0.1 - - local time = 0 - local uri = "unix:" .. server_def.socket - local c, err - while time < pluginserver_start_timeout do - c, err = connect(uri) - if c then - server_def.ready = true - c:close() - return - end - sleep(step) - time = time + step - end - server_def.socket_err = err -end - function proc_mgmt.pluginserver_timer(premature, server_def) if premature then return @@ -271,20 +236,8 @@ function proc_mgmt.pluginserver_timer(premature, server_def) end end kong.log.notice("Exiting: pluginserver '", server_def.name, "' not respawned.") - end --- limit the number of warning messages -local plugin_already_warned = {} -function proc_mgmt.handle_not_ready(plugin_name) - if plugin_already_warned[plugin_name] then - return - end - - plugin_already_warned[plugin_name] = true - log(ngx_WARN, "plugin server is not ready, ", - plugin_name, " will not be executed in this request") -end return proc_mgmt