From 687b01a149afc35daadd151068b98d58eae29c22 Mon Sep 17 00:00:00 2001 From: Vladislav Grubov Date: Mon, 26 Feb 2024 23:39:35 +0300 Subject: [PATCH] Fixes load config under tarantoolctl * Bug was introduced in version 0.7.0 and affects only development installations where tarantool is running under tarantoolctl in background (without systemd) * value of `box.cfg` always retreived before use from globals, because it can be changed (as tarantoolctl does) * do_cfg now drops non-boxcfg options checking template_cfg map and drops static values checking only dynamic_cfg map * Each cfg.box is copied before passing to "some" external boxcfg function, because some of them change insides and breaking Reconfiguration after load (as tarantoolctl does) --- .github/workflows/test.yml | 8 ++++-- Dockerfile.test | 2 +- Makefile | 2 +- README.md | 4 +-- config.lua | 59 +++++++++++++++++++++++++++++++------- 5 files changed, 58 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 094017c..e15c86e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,7 @@ jobs: run: mv luacov.stats.out luacov.stats.out-${{matrix.version}} - uses: actions/upload-artifact@master with: - name: luacov.stats.out + name: luacov.stats.out-${{matrix.version}} path: luacov.stats.out-${{matrix.version}} run-coverage-report: runs-on: ubuntu-latest @@ -46,9 +46,11 @@ jobs: run: tarantoolctl rocks install --server=https://luarocks.org luacov-coveralls 0.2.3 - name: install luacov-console 1.2.0 run: tarantoolctl rocks --server http://moonlibs.github.io/rocks install luacov-console 1.2.0 - - uses: actions/download-artifact@master + - name: Download run artifacts + uses: actions/download-artifact@v4 with: - name: luacov.stats.out + pattern: luacov.stats.out-* + merge-multiple: true - name: debug run: ls -la . - name: merge luacov.stats.out diff --git a/Dockerfile.test b/Dockerfile.test index a19e475..017d8ea 100644 --- a/Dockerfile.test +++ b/Dockerfile.test @@ -1,5 +1,5 @@ ARG IMAGE=1.10.14 -FROM config-test-builder as builder +FROM moonlibs-config-test-builder:latest as builder FROM tarantool/tarantool:${IMAGE} diff --git a/Makefile b/Makefile index f4de20f..1542d3c 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ run-etcd: make -C test run-compose-etcd config-test-builder: - docker build -t config-test-builder -f Dockerfile.build . + docker build -t moonlibs-config-test-builder:latest -f Dockerfile.build . config-test-%: config-test-builder run-etcd docker build -t $(@) --build-arg IMAGE=$(subst config-test-,,$@) -f Dockerfile.test . diff --git a/README.md b/README.md index 855515e..164e38f 100644 --- a/README.md +++ b/README.md @@ -36,12 +36,12 @@ Only ETCD APIv2 now supported. Ready for production use. -Latest stable release: `config 0.7.0`. +Latest stable release: `config 0.7.1`. ## Installation ```bash -tarantoolctl rocks --server=https://moonlibs.org install config 0.7.0 +tarantoolctl rocks --server=https://moonlibs.org install config 0.7.1 ``` Starting with Tarantool 2.10.0 you may add configuration of moonlibs.org into `config-5.1.lua` diff --git a/config.lua b/config.lua index 5ffb25c..3541f8f 100644 --- a/config.lua +++ b/config.lua @@ -714,15 +714,31 @@ end local function do_cfg(boxcfg, cfg) for key, val in pairs(cfg) do - if load_cfg.default_cfg[key] == nil and load_cfg.dynamic_cfg[key] == nil then + if load_cfg.template_cfg[key] == nil then local warn = string.format("Dropping non-boxcfg option '%s' given '%s'",key,val) log.warn("%s",warn) print(warn) cfg[key] = nil end end - log.info("Just before box.cfg %s", yaml.encode(cfg)) - boxcfg(cfg) + + if type(box.cfg) ~= 'function' then + for key, val in pairs(cfg) do + if load_cfg.dynamic_cfg[key] == nil and box.cfg[key] ~= val then + local warn = string.format( + "Dropping dynamic option '%s' previous value '%s' new value '%s'", + key,box.cfg[key],val + ) + log.warn("%s",warn) + print(warn) + cfg[key] = nil + end + end + end + + log.info("Just before first box.cfg %s", yaml.encode(cfg)) + -- Some boxcfg loves to rewrite passing table. We pass a copy of configuration + boxcfg(deep_copy(cfg)) end @@ -757,6 +773,7 @@ end ---@type moonlibs.config local M M = setmetatable({ + _VERSION = '0.7.1', console = {}; ---Retrieves value from config ---@overload fun(k: string, def: any?): any? @@ -792,6 +809,7 @@ local M enforce_ro = M._enforced_ro, } end, + _load_cfg = load_cfg, },{ ---Reinitiates moonlibs.config ---@param args moonlibs.config.opts @@ -821,7 +839,6 @@ local M M.master_selection_policy = args.master_selection_policy M.default = args.default M.strict_mode = args.strict_mode or args.strict or false - M._load_cfg = load_cfg -- print("config", "loading ",file, json.encode(args)) if not file then file = get_opt() @@ -958,11 +975,33 @@ local M end end - local boxcfg = box.cfg + -- The code below is very hard to understand and quite hard to fix when any bugs occurs. + -- First, you must remember that this part of code is executed several times in very different environments: + -- 1) Tarantool may be started with tarantool .lua and this part is required from the script + -- 2) Tarantool may be started under tarantoolctl (such as tarantoolctl start .lua) then box.cfg will be wrapped + -- by tarantoolctl itself, and it be returned back to table box.cfg after first successfull execution + -- 3) Tarantool may be started inside docker container and default docker-entrypoint.lua also rewraps box.cfg + -- 4) User might want to overwrite box.cfg with his function via args.boxcfg. Though, this method is not recommended + -- it is possible in some environments + -- 5) User might want to "wrap" box.cfg with his own middleware via (args.wrap_box_cfg). It is more recommended, because + -- full algorithm of tidy_load and ro-enforcing is preserved for the user. + + -- Moreover, first run of box.cfg in the life of the process allows to specify static box.cfg options, such as pid_file, log + -- and many others. + -- But, second reconfiguration of box.cfg (due to reload, or reconfiguration in fencing must never touch static options) + -- Part of this is fixed in `do_cfg` method of this codebase. But! Tarantool is buggy, so some options such as `log` appears to + -- be dynamic (it is listed in load_cfg.dynamic_cfg) but in practices log is static option. + + -- do_cfg calls under the hood first_cfg or redo_cfg based on type of `box.cfg` (when function -- then it is first_cfg, + -- otherwise -- redo_cfg). + + -- Because many wrappers in docker-entrypoint.lua and tarantoolctl LOVES to perform non-redoable actions inside box.cfg and + -- switch box.cfg back to builtin tarantool box.cfg, following code MUST NEVER cache value of box.cfg if args.boxcfg then do_cfg(args.boxcfg, cfg.box) else + local boxcfg if args.wrap_box_cfg then boxcfg = args.wrap_box_cfg end @@ -1026,7 +1065,7 @@ local M end end - do_cfg(boxcfg, cfg.box) + do_cfg(boxcfg or box.cfg, cfg.box) log.info("Reloading config after start") @@ -1054,14 +1093,14 @@ local M if diff_box then log.info("Reconfigure after load with %s",require'json'.encode(diff_box)) - do_cfg(boxcfg, diff_box) + do_cfg(boxcfg or box.cfg, diff_box) else log.info("Config is actual after load") end M._flat = flatten(new_cfg) else - do_cfg(boxcfg, cfg.box) + do_cfg(boxcfg or box.cfg, cfg.box) end else local replication = cfg.box.replication_source or cfg.box.replication @@ -1073,12 +1112,12 @@ local M cfg.box.replication = nil cfg.box.replication_source = nil - do_cfg(boxcfg, cfg.box) + do_cfg(boxcfg or box.cfg, cfg.box) cfg.box.replication = r cfg.box.replication_source = rs else - do_cfg(boxcfg, cfg.box) + do_cfg(boxcfg or box.cfg, cfg.box) end end end