This is an automated email from the ASF dual-hosted git repository.
nic-6443 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 f3623982e fix(etcd): watch from the revision the configuration was
read at (#13917)
f3623982e is described below
commit f3623982ef0e2d6de41306f637caf08ff0ef59ac
Author: Nic <[email protected]>
AuthorDate: Mon Sep 7 12:31:02 2026 +0800
fix(etcd): watch from the revision the configuration was read at (#13917)
---
apisix/core/config_etcd.lua | 35 +++++++----
t/core/config_etcd.t | 31 +++-------
t/core/etcd-watch-start-revision.t | 116 +++++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+), 35 deletions(-)
diff --git a/apisix/core/config_etcd.lua b/apisix/core/config_etcd.lua
index e59dc6b32..c6b4697c8 100644
--- a/apisix/core/config_etcd.lua
+++ b/apisix/core/config_etcd.lua
@@ -70,6 +70,9 @@ if not is_http then
end
local created_obj = {}
local loaded_configuration = {}
+-- the etcd revision loaded_configuration was read at, kept separately because
+-- its entries are removed as they are consumed
+local loaded_configuration_rev
local configuration_loaded_time
local watch_ctx
@@ -144,18 +147,14 @@ local function do_run_watch(premature)
error("failed to create etcd instance: " .. string(err))
end
- local rev = 0
- if loaded_configuration then
- local _, res = next(loaded_configuration)
- if res then
- rev = tonumber(res.headers["X-Etcd-Index"])
- if not rev or rev <= 0 then
- log.warn("invalid or missing X-Etcd-Index header, ",
- "will fetch revision from etcd directly")
- rev = 0
- end
- end
- end
+ -- Watch from the revision the preloaded configuration was read at, so
+ -- that everything written after that snapshot is still delivered. The
+ -- revision cannot be read back from loaded_configuration here:
+ -- core.config.new() removes each entry as it consumes it, so the table
+ -- is empty once every preloaded type has been registered -- and then
+ -- the fallback below would start the watch at the current revision and
+ -- drop every write made since the snapshot.
+ local rev = loaded_configuration_rev or 0
if rev == 0 then
while true do
@@ -1206,6 +1205,7 @@ end
local function init_loaded_configuration()
loaded_configuration = {}
+ loaded_configuration_rev = nil
local etcd_cli, prefix, err = etcd_apisix.new_without_proxy()
if not etcd_cli then
return "failed to start a etcd instance: " .. err
@@ -1216,6 +1216,17 @@ local function init_loaded_configuration()
return err
end
+ -- One readdir backs every entry create_formatter() stored, so there is a
+ -- single revision to record. Nothing is consumed yet at this point, so an
+ -- empty table here means the read stored nothing -- unlike the same test
+ -- made at watch time, which is the bug being fixed. Leave that case alone:
+ -- with no preloaded data every resource type reads for itself at first
+ -- sync, which already picks up whatever was written since, and the
+ -- rev == 0 path below keeps handling it exactly as before.
+ if next(loaded_configuration) then
+ loaded_configuration_rev = tonumber(res.headers["X-Etcd-Index"])
+ end
+
configuration_loaded_time = ngx_time()
end
diff --git a/t/core/config_etcd.t b/t/core/config_etcd.t
index 02fbee1a7..b25cff005 100644
--- a/t/core/config_etcd.t
+++ b/t/core/config_etcd.t
@@ -605,8 +605,13 @@ etcd watch timeout, upgrade revision to
-=== TEST 15: missing X-Etcd-Index header should not crash init worker
+=== TEST 15: a startup without a preloaded revision must not crash the init
worker
+The watch takes its start revision from the read that preloads the
+configuration. When there is no such read there is no revision either, and the
+watch has to fall back to asking etcd for the current one rather than failing.
--- yaml_config
+apisix:
+ disable_sync_configuration_during_start: true
deployment:
role: traditional
role_traditional:
@@ -615,26 +620,6 @@ deployment:
host:
- "http://127.0.0.1:2379";
prefix: /apisix
---- extra_init_by_lua
- -- Clear loaded_configuration and inject a fake entry with missing
- -- X-Etcd-Index header so do_run_watch exercises the fallback path.
- local config_etcd = require("apisix.core.config_etcd")
- for i = 1, 256 do
- local name, val = debug.getupvalue(config_etcd.new, i)
- if not name then
- break
- end
- if name == "loaded_configuration" and type(val) == "table" then
- for k in pairs(val) do
- val[k] = nil
- end
- val["__test_fake"] = {
- body = { nodes = {} },
- headers = {},
- }
- break
- end
- end
--- config
location /t {
content_by_lua_block {
@@ -647,9 +632,9 @@ GET /t
--- response_body
passed
--- grep_error_log eval
-qr/invalid or missing X-Etcd-Index header/
+qr/main etcd watcher initialised, revision=\d+/
--- grep_error_log_out eval
-qr/(invalid or missing X-Etcd-Index header\n){1,}/
+qr/(main etcd watcher initialised, revision=\d+\n){1,}/
diff --git a/t/core/etcd-watch-start-revision.t
b/t/core/etcd-watch-start-revision.t
new file mode 100644
index 000000000..28eaeecd0
--- /dev/null
+++ b/t/core/etcd-watch-start-revision.t
@@ -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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: a write made after the configuration snapshot is still delivered
+The configuration is preloaded once in init_by_lua and handed to each resource
+type as it registers, which removes it from the preload table. This test drives
+that table empty by giving etcd a prefix holding a single directory, so /routes
+is the only type with anything to consume, and then writes a route between the
+snapshot and the first watch. The route can only reach the worker through the
+watch, so it arrives if and only if the watch starts from the revision the
+snapshot was read at.
+--- yaml_config
+apisix:
+ node_listen: 1984
+deployment:
+ role: traditional
+ role_traditional:
+ config_provider: etcd
+ admin:
+ admin_key: null
+ etcd:
+ prefix: "/apisix-watch-start-revision"
+ host:
+ - "http://127.0.0.1:2379";
+--- extra_init_by_lua_start
+ -- io.popen reports nothing about how curl fared, so the etcd response is
+ -- checked instead: a silently failing write here would look exactly like
+ -- the lost event this test is about
+ local function etcd_call(path, body)
+ local f = assert(io.popen("curl -sS --fail-with-body -X POST "
+ .. "http://127.0.0.1:2379/v3/kv/"; .. path
+ .. " -d '" .. body .. "'"))
+ local out = f:read("*a")
+ f:close()
+ if not out or not out:find('"header"', 1, true) then
+ error("etcd " .. path .. " failed: " .. tostring(out))
+ end
+ end
+
+ local function etcd_put(key, value)
+ etcd_call("put", '{"key":"' .. ngx.encode_base64(key)
+ .. '","value":"' .. ngx.encode_base64(value) .. '"}')
+ end
+ _G.etcd_put_for_test = etcd_put
+
+ local function etcd_clear()
+ etcd_call("deleterange",
+ '{"key":"' .. ngx.encode_base64("/apisix-watch-start-revision/")
+ .. '","range_end":"' ..
ngx.encode_base64("/apisix-watch-start-revision0") .. '"}')
+ end
+ _G.etcd_clear_for_test = etcd_clear
+
+ -- start from an empty prefix, so the route written below can only ever
+ -- reach the worker through the watch and never through the snapshot. The
+ -- run clears up after itself too; this is here for a run that did not.
+ etcd_clear()
+
+ -- Do not remove: this key is what puts /routes on the preload path.
Without
+ -- it the prefix is empty, /routes has nothing to consume and reads for
+ -- itself at first sync, and that read picks the route up whatever revision
+ -- the watch started from -- the test then passes even unpatched. One key
+ -- also means one preload entry, so registering /routes empties the table,
+ -- which is the state being tested.
+ etcd_put("/apisix-watch-start-revision/routes/", "init_dir")
+--- extra_init_by_lua
+ -- after the snapshot was taken, before any worker starts watching
+ _G.etcd_put_for_test("/apisix-watch-start-revision/routes/1",
+
'{"uri":"/hello","upstream":{"type":"roundrobin","nodes":{"127.0.0.1:1980":1}}}')
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+
+ ngx.sleep(1)
+
+ local routes = core.config.fetch_created_obj("/routes")
+ local route = routes and routes:get("1")
+
+ -- the local etcd is shared, so do not leave this prefix populated
+ _G.etcd_clear_for_test()
+
+ if not route then
+ ngx.say("route written during startup was lost")
+ return
+ end
+
+ ngx.say("uri: ", route.value.uri)
+ }
+ }
+--- request
+GET /t
+--- response_body
+uri: /hello
