diff --git a/apisix/admin/init.lua b/apisix/admin/init.lua index c3f543e62949..3ef8fafc8fd0 100644 --- a/apisix/admin/init.lua +++ b/apisix/admin/init.lua @@ -27,11 +27,20 @@ local ngx = ngx local get_method = ngx.req.get_method local ngx_time = ngx.time local ngx_timer_at = ngx.timer.at +local ngx_timer_every = ngx.timer.every local ngx_worker_id = ngx.worker.id local tonumber = tonumber local tostring = tostring local str_lower = string.lower local reload_event = "/apisix/admin/plugins/reload" +local is_http = ngx.config.subsystem == "http" +-- declared unconditionally for the http subsystem in ngx_tpl.lua, and already a +-- hard dependency of the server-info plugin; absent in the stream subsystem +local plugins_conf_ver_dict = is_http and ngx.shared["internal-status"] +local PLUGINS_CONF_VERSION_KEY = "plugins_conf_version" +-- plugins conf version this process has applied, compared against the shared +-- dict by the reconciliation timer registered in init_worker() +local applied_plugins_conf_version = 0 local ipairs = ipairs local error = error local type = type @@ -288,6 +297,21 @@ end local function post_reload_plugins() set_ctx_and_check_token() + if plugins_conf_ver_dict then + -- bump the version before broadcasting, so that a process which never + -- receives the event (e.g. the privileged agent while it is + -- reconnecting to the events broker) still converges through the + -- periodic reconciliation below + local _, err = plugins_conf_ver_dict:incr(PLUGINS_CONF_VERSION_KEY, 1, 0) + if err then + -- if the version cannot be bumped the reconciliation timer will + -- never notice a change, so a worker that misses the broadcast + -- would stay stale forever; fail loud instead of pretending success + core.log.error("failed to increase plugins conf version: ", err) + core.response.exit(503, {error_msg = "failed to record plugins reload"}) + end + end + local success, err = events:post(reload_event, get_method(), ngx_time()) if not success then core.response.exit(503, err) @@ -375,8 +399,21 @@ end local function reload_plugins(data, event, source, pid) core.log.info("start to hot reload plugins") + + -- sample the version before loading: if another reload is accepted while + -- plugin.load() runs, the versions stay unequal and the reconciliation + -- timer applies one more round + local ver + if plugins_conf_ver_dict then + ver = plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) + end + plugin.load() + if ver then + applied_plugins_conf_version = ver + end + if ngx_worker_id() == 0 then sync_local_conf_to_etcd() end @@ -509,6 +546,30 @@ function _M.init_worker() events = require("apisix.events") events:register(reload_plugins, reload_event, "PUT") + if plugins_conf_ver_dict and not is_yaml_config_provider then + -- The events broadcast has no delivery guarantee: a process that is + -- (re)connecting to the events broker loses the event for good, which + -- leaves it running e.g. the timers of plugins that were removed. + -- Reconcile against the version in the shared dict, the same pattern + -- admin/standalone.lua uses for the same reason. + applied_plugins_conf_version = + plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) or 0 + + local ok, err = ngx_timer_every(1, function (premature) + if premature then + return + end + + local ver = plugins_conf_ver_dict:get(PLUGINS_CONF_VERSION_KEY) or 0 + if ver ~= applied_plugins_conf_version then + reload_plugins() + end + end) + if not ok then + core.log.error("failed to create plugins reconciliation timer: ", err) + end + end + if ngx_worker_id() == 0 then -- check if admin_key is required if local_conf.deployment.admin.admin_key_required == false then diff --git a/t/admin/plugins-reload-reconcile.t b/t/admin/plugins-reload-reconcile.t new file mode 100644 index 000000000000..df7a2a396426 --- /dev/null +++ b/t/admin/plugins-reload-reconcile.t @@ -0,0 +1,96 @@ +# +# 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'; + +log_level('info'); +repeat_each(1); +no_long_string(); +no_root_location(); +no_shuffle(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } +}); + +run_tests(); + +__DATA__ + +=== TEST 1: a reload request bumps the shared plugins conf version +--- config + location /t { + content_by_lua_block { + local dict = ngx.shared["internal-status"] + local before = dict:get("plugins_conf_version") or 0 + + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/plugins/reload', ngx.HTTP_PUT) + + local after = dict:get("plugins_conf_version") or 0 + ngx.say(code, " ", body, " bumped=", tostring(after > before)) + } + } +--- response_body +200 passed bumped=true +--- no_error_log +failed to increase plugins conf version + + + +=== TEST 2: a process that missed the broadcast reloads through reconciliation +--- config + location /t { + content_by_lua_block { + -- A reload whose broadcast never arrives leaves the shared version + -- ahead of what this process applied. Reproduce exactly that state + -- without going through the events layer. + local dict = ngx.shared["internal-status"] + local newver, incr_err = dict:incr("plugins_conf_version", 1, 0) + if not newver then + ngx.say("failed to bump version: ", incr_err) + return + end + + -- the reconciliation timer runs once a second + ngx.sleep(2.5) + ngx.say("done") + } + } +--- response_body +done +--- error_log +start to hot reload plugins + + + +=== TEST 3: an unchanged version never triggers a reload +--- config + location /t { + content_by_lua_block { + ngx.sleep(3) + ngx.say("done") + } + } +--- response_body +done +--- no_error_log +start to hot reload plugins