* [Tarantool-patches] [PATCH] box: set cfg via environment variables @ 2021-03-14 15:34 Roman Khabibov via Tarantool-patches 2021-03-19 12:38 ` Leonid Vasiliev via Tarantool-patches 2021-03-19 13:23 ` Leonid Vasiliev via Tarantool-patches 0 siblings, 2 replies; 7+ messages in thread From: Roman Khabibov via Tarantool-patches @ 2021-03-14 15:34 UTC (permalink / raw) To: tarantool-patches Add ability to set box.cfg options via env variables. These variables should have name "TT_<OPTION>". When Tarantool instance is started under tarantoolctl utility env variables options have more priority than instance file's cfg. Closes #5602 @TarantoolBot document Title: Set box.cfg options via env variables Now, it is possible to set box.cfg options via environment variables. The name of variable should correspond the followng pattern: "TT_<NAME>", where <NAME> is an option name. For example: "TT_LISTEN", "TT_READAHEAD". If it is complex value, for example, "replication" can be set by a table, the corresponding environment varibale value has the syntax of lua array (without keys): `export TT_REPLICATION="{'uri1', 'uri2'}"` Note that every value in arary must be quoted. --- @ChangeLog * Add ability to set box.cfg options via environment variables (gh-5602). Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-5602-environment-variables Issue: https://github.com/tarantool/tarantool/issues/5602 src/box/lua/load_cfg.lua | 74 +++++++++++++++++++ test/app-tap/tarantoolctl.test.lua | 33 ++++++++- .../gh-5602-environment-vars-cfg.result | 42 +++++++++++ .../gh-5602-environment-vars-cfg.test.lua | 53 +++++++++++++ test/box-tap/gh-5602_script.lua | 70 ++++++++++++++++++ 5 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 test/box-tap/gh-5602-environment-vars-cfg.result create mode 100755 test/box-tap/gh-5602-environment-vars-cfg.test.lua create mode 100755 test/box-tap/gh-5602_script.lua diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index 574c8bef4..eace25d46 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -616,6 +616,79 @@ setmetatable(box, { end }) +local function process_option_value(option, raw_value) + local param_type = template_cfg[option] + if param_type == nil then + return nil + end + local value = nil + if param_type == 'number' or option == 'log_level' then + value = tonumber(raw_value) + if value == nil then + error("Environment variable "..'TT_'..string.upper(option).." has".. + " incorrect value for option '"..option.."': should be ".. + "converted to number") + end + elseif param_type == 'boolean' or option == 'log_nonblock' then + if raw_value == 'true' then + value = true + elseif raw_value == 'false' then + value = false + else + error("Environment variable "..'TT_'..string.upper(option).." has".. + " incorrect value for option '"..option.."': should be ".. + "'true' or 'false'") + end + elseif param_type == 'string, number' then + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + elseif param_type == 'string, number, table' then + if string.sub(raw_value, 1, 1) == '{' and + string.sub(raw_value, -1) == '}' then + value = string.sub(raw_value, 2, -2) + value = require('csv').load(value)[1] + for i, str in pairs(value) do + if not (string.sub(str, 1, 1) == '\'' and + string.sub(str, -1) == '\'') and + not (string.sub(str, 1, 1) == '"' and + string.sub(str, -1) == '"') then + error("Environment variable "..'TT_'..string.upper(option).. + " has incorrect value for option '"..option.."': ".. + "elements of array should be quoted strings") + end + str = string.sub(str, 2, -2) + value[i] = str + end + else + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + end + else + value = raw_value + end + + return value +end + +local function collect_env_cfg(cfg) + if cfg == nil then + cfg = {} + end + for option, _ in pairs(template_cfg) do + if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then + local env_var_name = 'TT_'..string.upper(option) + local raw_value = os.getenv(env_var_name) + if raw_value ~= nil then + cfg[option] = process_option_value(option, raw_value) + end + end + end +end + -- Whether box is loaded. -- -- `false` when box is not configured or when the initialization @@ -627,6 +700,7 @@ setmetatable(box, { local box_is_configured = false local function load_cfg(cfg) + collect_env_cfg(cfg) -- A user may save box.cfg (this function) before box loading -- and call it afterwards. We should reconfigure box in the -- case. diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua index 3c3023d34..1f15fe103 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -160,7 +160,7 @@ local function merge(...) end local test = tap.test('tarantoolctl') -test:plan(8) +test:plan(9) -- basic start/stop test -- must be stopped afterwards @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) end end) +-- gh-5602: Check that environment cfg variables are more +-- prioritized than tarantoolctl cfg. +do + local dir = fio.tempdir() + local code = [[ box.cfg ]] + create_script(dir, 'show_cfg.lua', code) + local code = [[ box.cfg{listen=3302, readahead=10001} ]] + create_script(dir, 'cfg_script.lua', code) + + local status, err = pcall(function() + test:test("check answers in case of call", function(test_i) + test_i:plan(4) + os.setenv('TT_LISTEN', '3301') + os.setenv('TT_READAHEAD', '10000') + check_ok(test_i, dir, 'start', 'cfg_script', 0) + tctl_wait_start(dir, 'cfg_script') + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, + '---\n- 3301\n- 10000\n...', nil) + check_ok(test_i, dir, 'stop', 'cfg_script', 0) + end) + end) + + cleanup_instance(dir, 'cfg_script') + recursive_rmdir(dir) + + if status == false then + print(("Error: %s"):format(err)) + os.exit() + end +end + os.exit(test:check() == true and 0 or -1) diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result new file mode 100644 index 000000000..495e02a73 --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.result @@ -0,0 +1,42 @@ +TAP version 13 +1..5 +ok - box.cfg is successful +ok - listen +ok - readahead +ok - strip_core +ok - log_format is not set +TAP version 13 +1..7 +ok - box.cfg is successful +ok - listen +ok - replication is table +ok - replication URI 1 +ok - replication URI 2 +ok - replication_connect_timeout +ok - replication_synchro_quorum +TAP version 13 +1..3 +ok - box.cfg is successful +ok - box.cfg{} background value is prioritized +ok - box.cfg{} vinyl_timeout value is prioritized +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad sql_cache_size value +TAP version 13 +1..2 +ok - box.cfg is not successful +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:638: Environment variable TT_STRIP_CORE has incorrect value for option 'strip_core': should be 'true' or 'false' +ok - bad strip_core value +TAP version 13 +1..2 +ok - box.cfg is not successful +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:657: Environment variable TT_REPLICATION has incorrect value for option 'replication': elements of array should be quoted strings +ok - bad replication value +TAP version 13 +ok - set 1 +ok - set 2 +ok - set 3 +ok - set 4 +ok - set 5 +ok - set 6 diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua new file mode 100755 index 000000000..84b49b7eb --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua @@ -0,0 +1,53 @@ +#!/usr/bin/env tarantool + +local os = require('os') +local fio = require('fio') +local tap = require('tap') +local test = tap.test('gh-5602') + +-- gh-5602: Check that environment cfg variables working. +local TARANTOOL_PATH = arg[-1] +local script_name = 'gh-5602_script.lua' +local path_to_script = fio.pathjoin( + os.getenv('PWD'), + 'box-tap', + script_name) +local function shell_command(set, i) + string.gsub(set, 'TT_', '') + return ('%s %s %s %d'):format( + set, + TARANTOOL_PATH, + path_to_script, + i) +end + +local set_1 = 'TT_LISTEN=3301 ' .. + 'TT_READAHEAD=10000 ' .. + 'TT_STRIP_CORE=false ' .. + 'TT_LOG_FORMAT=json' +local set_2 = 'TT_LISTEN=3301 ' .. + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', ' .. + '\'1.1.1.1:12345\'}\" ' .. + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01 ' .. + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'' +local set_3 = 'TT_BACKGROUND=true ' .. + 'TT_VINYL_TIMEOUT=60.1' +local set_4 = 'TT_SQL_CACHE_SIZE=a' +local set_5 = 'TT_STRIP_CORE=a' +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', ' .. + '\'1.1.1.1:12345\'}\"' +local sets = {} +sets[1] = set_1 +sets[2] = set_2 +sets[3] = set_3 +sets[4] = set_4 +sets[5] = set_5 +sets[6] = set_6 + +for i, set in ipairs(sets) do + local tmpdir = fio.tempdir() + local new_path = fio.pathjoin(tmpdir, script_name) + fio.copyfile(path_to_script, new_path) + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) +end +os.exit(0) diff --git a/test/box-tap/gh-5602_script.lua b/test/box-tap/gh-5602_script.lua new file mode 100755 index 000000000..8ff34deb2 --- /dev/null +++ b/test/box-tap/gh-5602_script.lua @@ -0,0 +1,70 @@ +local tap = require('tap') +local test = tap.test('gh-5602') + +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) + +-- Check that environment cfg values are set correctly. +if arg[1] == '1' then + test:plan(5) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], '3301', 'listen') + test:is(box.cfg['readahead'], 10000, 'readahead') + test:is(box.cfg['strip_core'], false, 'strip_core') + test:is(box.cfg['log_format'], 'json', 'log_format is not set') +end +if arg[1] == '2' then + test:plan(7) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], '3301', 'listen') + local replication = box.cfg['replication'] + test:is(type(replication), 'table', 'replication is table') + test:ok(replication[1] == '0.0.0.0:12345' or + replication[1] == '1.1.1.1:12345', 'replication URI 1') + test:ok(replication[2] == '0.0.0.0:12345' or + replication[2] == '1.1.1.1:12345', 'replication URI 2') + test:is(box.cfg['replication_connect_timeout'], 0.01, + 'replication_connect_timeout') + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', + 'replication_synchro_quorum') +end + +-- Check that box.cfg{} values are more prioritized than +-- environment cfg values. +if arg[1] == '3' then + test:plan(3) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['background'], false, + 'box.cfg{} background value is prioritized') + test:is(box.cfg['vinyl_timeout'], 70.1, + 'box.cfg{} vinyl_timeout value is prioritized') +end + +-- Check bad environment cfg values. +if arg[1] == '4' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(err, 'Environment variable TT_SQL_CACHE_SIZE '.. + 'has incorrect value for option \'sql_cache_size\': should be '.. + 'converted to number') + test:ok(index ~= nil, 'bad sql_cache_size value') +end +if arg[1] == '5' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + print(err) + local index = string.find(err, 'Environment variable TT_STRIP_CORE has '.. + 'incorrect value for option \'strip_core\': should be \'true\' '.. + 'or \'false\'') + test:ok(index ~= nil, 'bad strip_core value') +end +if arg[1] == '6' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + print(err) + local index = string.find(err, 'Environment variable TT_REPLICATION has '.. + 'incorrect value for option \'replication\': elements of array should'.. + ' be quoted strings') + test:ok(index ~= nil, 'bad replication value') +end + +os.exit(test:check() and 0 or 1) \ No newline at end of file -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-03-14 15:34 [Tarantool-patches] [PATCH] box: set cfg via environment variables Roman Khabibov via Tarantool-patches @ 2021-03-19 12:38 ` Leonid Vasiliev via Tarantool-patches 2021-04-09 8:56 ` Roman Khabibov via Tarantool-patches 2021-03-19 13:23 ` Leonid Vasiliev via Tarantool-patches 1 sibling, 1 reply; 7+ messages in thread From: Leonid Vasiliev via Tarantool-patches @ 2021-03-19 12:38 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Hi! Thank you for the patch. See comments bellow: On 3/14/21 6:34 PM, Roman Khabibov wrote: > Add ability to set box.cfg options via env variables. These > variables should have name "TT_<OPTION>". When Tarantool instance > is started under tarantoolctl utility env variables options have > more priority than instance file's cfg. > > Closes #5602 > > @TarantoolBot document > Title: Set box.cfg options via env variables > > Now, it is possible to set box.cfg options via environment > variables. The name of variable should correspond the followng 1) followng -> following > pattern: "TT_<NAME>", where <NAME> is an option name. For example: > "TT_LISTEN", "TT_READAHEAD". > If it is complex value, for example, "replication" can be set by a > table, the corresponding environment varibale value has the syntax 2) varibale -> variable > of lua array (without keys): > > `export TT_REPLICATION="{'uri1', 'uri2'}"` > > Note that every value in arary must be quoted. 1) arary -> array > --- > > @ChangeLog > * Add ability to set box.cfg options via environment variables (gh-5602). > > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-5602-environment-variables > Issue: https://github.com/tarantool/tarantool/issues/5602 > > src/box/lua/load_cfg.lua | 74 +++++++++++++++++++ > test/app-tap/tarantoolctl.test.lua | 33 ++++++++- > .../gh-5602-environment-vars-cfg.result | 42 +++++++++++ > .../gh-5602-environment-vars-cfg.test.lua | 53 +++++++++++++ > test/box-tap/gh-5602_script.lua | 70 ++++++++++++++++++ > 5 files changed, 271 insertions(+), 1 deletion(-) > create mode 100644 test/box-tap/gh-5602-environment-vars-cfg.result > create mode 100755 test/box-tap/gh-5602-environment-vars-cfg.test.lua > create mode 100755 test/box-tap/gh-5602_script.lua > > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 574c8bef4..eace25d46 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -616,6 +616,79 @@ setmetatable(box, { > end > }) > 4) Add descriptions to functions (must start with `---`). > +local function process_option_value(option, raw_value) > + local param_type = template_cfg[option] > + if param_type == nil then > + return nil > + end > + local value = nil > + if param_type == 'number' or option == 'log_level' then > + value = tonumber(raw_value) > + if value == nil then > + error("Environment variable "..'TT_'..string.upper(option).." has".. > + " incorrect value for option '"..option.."': should be ".. > + "converted to number") 5) Codestyle (apply to the whole patch): - Indetation. From https://dev.minetest.net/Lua_code_style_guidelines : "When strings don't fit into the line, you should add the string (changes) to the next line(s) indented by one tab." (4 spaces in our case). - String format. From https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/#indentation-and-formatting: "Avoid multiple concatenations in one statement, use string.format instead" > + end > + elseif param_type == 'boolean' or option == 'log_nonblock' then > + if raw_value == 'true' then > + value = true > + elseif raw_value == 'false' then > + value = false > + else > + error("Environment variable "..'TT_'..string.upper(option).." has".. > + " incorrect value for option '"..option.."': should be ".. > + "'true' or 'false'") > + end > + elseif param_type == 'string, number' then > + value = tonumber(raw_value) > + if value == nil then > + value = raw_value > + end > + elseif param_type == 'string, number, table' then 6) OMG! We have to get the table from the env variable. Nothing good to expect. - What about a nested table? - Also, what about the cases "string, table" or "table, string, number" ...? Now we don't have such a `param_type`, but they will appear tomorrow. > + if string.sub(raw_value, 1, 1) == '{' and > + string.sub(raw_value, -1) == '}' then > + value = string.sub(raw_value, 2, -2) > + value = require('csv').load(value)[1] > + for i, str in pairs(value) do > + if not (string.sub(str, 1, 1) == '\'' and > + string.sub(str, -1) == '\'') and > + not (string.sub(str, 1, 1) == '"' and > + string.sub(str, -1) == '"') then > + error("Environment variable "..'TT_'..string.upper(option).. > + " has incorrect value for option '"..option.."': ".. > + "elements of array should be quoted strings") > + end > + str = string.sub(str, 2, -2) 7) Shouldn't we be checking types inside a table? Or can there only be strings? > + value[i] = str > + end > + else > + value = tonumber(raw_value) > + if value == nil then > + value = raw_value > + end > + end > + else > + value = raw_value > + end > + > + return value > +end > + > +local function collect_env_cfg(cfg) > + if cfg == nil then > + cfg = {} > + end 8) Please, separate logical blocks of code from each other with empty lines. Write comments.(apply to the whole patch). > + for option, _ in pairs(template_cfg) do > + if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then > + local env_var_name = 'TT_'..string.upper(option) > + local raw_value = os.getenv(env_var_name) > + if raw_value ~= nil then > + cfg[option] = process_option_value(option, raw_value) > + end > + end > + end > +end > + > -- Whether box is loaded. > -- > -- `false` when box is not configured or when the initialization > @@ -627,6 +700,7 @@ setmetatable(box, { > local box_is_configured = false > > local function load_cfg(cfg) > + collect_env_cfg(cfg) > -- A user may save box.cfg (this function) before box loading > -- and call it afterwards. We should reconfigure box in the > -- case. 9) It looks like in this case we can overload the value that was set in the previous call with the value from the environment. Or not? > diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua > index 3c3023d34..1f15fe103 100755 > --- a/test/app-tap/tarantoolctl.test.lua > +++ b/test/app-tap/tarantoolctl.test.lua > @@ -160,7 +160,7 @@ local function merge(...) > end > > local test = tap.test('tarantoolctl') > -test:plan(8) > +test:plan(9) > > -- basic start/stop test > -- must be stopped afterwards > @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) > end > end) > > +-- gh-5602: Check that environment cfg variables are more > +-- prioritized than tarantoolctl cfg. > +do 10) As for me, it is preferable to use `test:test()` instead of `do...end`. Up to you. > + local dir = fio.tempdir() > + local code = [[ box.cfg ]] 11) Maybe `[[ box.cfg{} ]]` > + create_script(dir, 'show_cfg.lua', code) 12) Two spaces before `code`, but one is needed. > + local code = [[ box.cfg{listen=3302, readahead=10001} ]] > + create_script(dir, 'cfg_script.lua', code) > + > + local status, err = pcall(function() > + test:test("check answers in case of call", function(test_i) > + test_i:plan(4) > + os.setenv('TT_LISTEN', '3301') 13) We will not have problems due to a specific port when many tests are run in parallel (such as this port will be already occupied). > + os.setenv('TT_READAHEAD', '10000') > + check_ok(test_i, dir, 'start', 'cfg_script', 0) > + tctl_wait_start(dir, 'cfg_script') > + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, > + '---\n- 3301\n- 10000\n...', nil) > + check_ok(test_i, dir, 'stop', 'cfg_script', 0) > + end) > + end) > + > + cleanup_instance(dir, 'cfg_script') > + recursive_rmdir(dir) > + > + if status == false then > + print(("Error: %s"):format(err)) > + os.exit() > + end > +end > + > os.exit(test:check() == true and 0 or -1) > diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result > new file mode 100644 > index 000000000..495e02a73 > --- /dev/null > +++ b/test/box-tap/gh-5602-environment-vars-cfg.result > @@ -0,0 +1,42 @@ > +TAP version 13 > +1..5 > +ok - box.cfg is successful > +ok - listen > +ok - readahead > +ok - strip_core > +ok - log_format is not set > +TAP version 13 > +1..7 > +ok - box.cfg is successful > +ok - listen > +ok - replication is table > +ok - replication URI 1 > +ok - replication URI 2 > +ok - replication_connect_timeout > +ok - replication_synchro_quorum > +TAP version 13 > +1..3 > +ok - box.cfg is successful > +ok - box.cfg{} background value is prioritized > +ok - box.cfg{} vinyl_timeout value is prioritized > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +ok - bad sql_cache_size value > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:638: Environment variable TT_STRIP_CORE has incorrect value for option 'strip_core': should be 'true' or 'false' > +ok - bad strip_core value > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:657: Environment variable TT_REPLICATION has incorrect value for option 'replication': elements of array should be quoted strings > +ok - bad replication value > +TAP version 13 > +ok - set 1 > +ok - set 2 > +ok - set 3 > +ok - set 4 > +ok - set 5 > +ok - set 6 > diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua > new file mode 100755 > index 000000000..84b49b7eb > --- /dev/null > +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua > @@ -0,0 +1,53 @@ > +#!/usr/bin/env tarantool > + > +local os = require('os') > +local fio = require('fio') > +local tap = require('tap') 13) It's nice to separate the variable definition and the `requires` with empty line.(apply to the whole patch) > +local test = tap.test('gh-5602') > + > +-- gh-5602: Check that environment cfg variables working. > +local TARANTOOL_PATH = arg[-1] > +local script_name = 'gh-5602_script.lua' > +local path_to_script = fio.pathjoin( > + os.getenv('PWD'), > + 'box-tap', > + script_name) 14) Please, separate the function definition with an empty line. 15) Describe the test mechanism using a comment. > +local function shell_command(set, i) > + string.gsub(set, 'TT_', '') > + return ('%s %s %s %d'):format( > + set, > + TARANTOOL_PATH, > + path_to_script, > + i) > +end > + > +local set_1 = 'TT_LISTEN=3301 ' .. > + 'TT_READAHEAD=10000 ' .. > + 'TT_STRIP_CORE=false ' .. > + 'TT_LOG_FORMAT=json' > +local set_2 = 'TT_LISTEN=3301 ' .. > + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', ' .. > + '\'1.1.1.1:12345\'}\" ' .. > + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01 ' .. > + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'' > +local set_3 = 'TT_BACKGROUND=true ' .. > + 'TT_VINYL_TIMEOUT=60.1' > +local set_4 = 'TT_SQL_CACHE_SIZE=a' > +local set_5 = 'TT_STRIP_CORE=a' > +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', ' .. > + '\'1.1.1.1:12345\'}\"' > +local sets = {} > +sets[1] = set_1 > +sets[2] = set_2 > +sets[3] = set_3 > +sets[4] = set_4 > +sets[5] = set_5 > +sets[6] = set_6 > + > +for i, set in ipairs(sets) do > + local tmpdir = fio.tempdir() > + local new_path = fio.pathjoin(tmpdir, script_name) > + fio.copyfile(path_to_script, new_path) > + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) > +end 16) Add `test:plan()` and `test:check()`. > +os.exit(0) > diff --git a/test/box-tap/gh-5602_script.lua b/test/box-tap/gh-5602_script.lua 17) Use a more self-documenting name. > new file mode 100755 > index 000000000..8ff34deb2 > --- /dev/null > +++ b/test/box-tap/gh-5602_script.lua > @@ -0,0 +1,70 @@ > +local tap = require('tap') > +local test = tap.test('gh-5602') > + > +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) > + > +-- Check that environment cfg values are set correctly. > +if arg[1] == '1' then > + test:plan(5) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['listen'], '3301', 'listen') > + test:is(box.cfg['readahead'], 10000, 'readahead') > + test:is(box.cfg['strip_core'], false, 'strip_core') > + test:is(box.cfg['log_format'], 'json', 'log_format is not set') > +end > +if arg[1] == '2' then > + test:plan(7) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['listen'], '3301', 'listen') > + local replication = box.cfg['replication'] > + test:is(type(replication), 'table', 'replication is table') > + test:ok(replication[1] == '0.0.0.0:12345' or > + replication[1] == '1.1.1.1:12345', 'replication URI 1') > + test:ok(replication[2] == '0.0.0.0:12345' or > + replication[2] == '1.1.1.1:12345', 'replication URI 2') > + test:is(box.cfg['replication_connect_timeout'], 0.01, > + 'replication_connect_timeout') > + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', > + 'replication_synchro_quorum') > +end > + > +-- Check that box.cfg{} values are more prioritized than > +-- environment cfg values. > +if arg[1] == '3' then > + test:plan(3) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['background'], false, > + 'box.cfg{} background value is prioritized') > + test:is(box.cfg['vinyl_timeout'], 70.1, > + 'box.cfg{} vinyl_timeout value is prioritized') > +end > + > +-- Check bad environment cfg values. > +if arg[1] == '4' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + local index = string.find(err, 'Environment variable TT_SQL_CACHE_SIZE '.. > + 'has incorrect value for option \'sql_cache_size\': should be '.. > + 'converted to number') > + test:ok(index ~= nil, 'bad sql_cache_size value') > +end > +if arg[1] == '5' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + print(err) > + local index = string.find(err, 'Environment variable TT_STRIP_CORE has '.. > + 'incorrect value for option \'strip_core\': should be \'true\' '.. > + 'or \'false\'') > + test:ok(index ~= nil, 'bad strip_core value') > +end > +if arg[1] == '6' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + print(err) > + local index = string.find(err, 'Environment variable TT_REPLICATION has '.. > + 'incorrect value for option \'replication\': elements of array should'.. > + ' be quoted strings') > + test:ok(index ~= nil, 'bad replication value') > +end > + > +os.exit(test:check() and 0 or 1) > \ No newline at end of file 18) Add newline to the end of file. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-03-19 12:38 ` Leonid Vasiliev via Tarantool-patches @ 2021-04-09 8:56 ` Roman Khabibov via Tarantool-patches 2021-04-12 8:37 ` Leonid Vasiliev via Tarantool-patches 2021-04-12 9:52 ` Leonid Vasiliev via Tarantool-patches 0 siblings, 2 replies; 7+ messages in thread From: Roman Khabibov via Tarantool-patches @ 2021-04-09 8:56 UTC (permalink / raw) To: Leonid Vasiliev; +Cc: tarantool-patches Hi! Thanks for review. > On Mar 19, 2021, at 15:38, Leonid Vasiliev <lvasiliev@tarantool.org> wrote: > > Hi! Thank you for the patch. > See comments bellow: > > On 3/14/21 6:34 PM, Roman Khabibov wrote: >> Add ability to set box.cfg options via env variables. These >> variables should have name "TT_<OPTION>". When Tarantool instance >> is started under tarantoolctl utility env variables options have >> more priority than instance file's cfg. >> Closes #5602 >> @TarantoolBot document >> Title: Set box.cfg options via env variables >> Now, it is possible to set box.cfg options via environment >> variables. The name of variable should correspond the followng > > 1) followng -> following > >> pattern: "TT_<NAME>", where <NAME> is an option name. For example: >> "TT_LISTEN", "TT_READAHEAD". >> If it is complex value, for example, "replication" can be set by a >> table, the corresponding environment varibale value has the syntax > > 2) varibale -> variable > >> of lua array (without keys): >> `export TT_REPLICATION="{'uri1', 'uri2'}"` >> Note that every value in arary must be quoted. > > 1) arary -> array box: set cfg via environment variables Add ability to set box.cfg options via env variables. These variables should have name "TT_<OPTION>". When Tarantool instance is started under tarantoolctl utility env variables options have more priority than instance file's cfg. Closes #5602 @TarantoolBot document Title: Set box.cfg options via env variables Now, it is possible to set box.cfg options via environment variables. The name of variable should correspond the following pattern: "TT_<NAME>", where <NAME> is an option name. For example: "TT_LISTEN", "TT_READAHEAD". If it is complex value, for example, "replication" can be set by a table, the corresponding environment variable value has the syntax of lua array (without keys): `export TT_REPLICATION="{'uri1', 'uri2'}”` >> --- >> @ChangeLog >> * Add ability to set box.cfg options via environment variables (gh-5602). >> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-5602-environment-variables >> Issue: https://github.com/tarantool/tarantool/issues/5602 >> src/box/lua/load_cfg.lua | 74 +++++++++++++++++++ >> test/app-tap/tarantoolctl.test.lua | 33 ++++++++- >> .../gh-5602-environment-vars-cfg.result | 42 +++++++++++ >> .../gh-5602-environment-vars-cfg.test.lua | 53 +++++++++++++ >> test/box-tap/gh-5602_script.lua | 70 ++++++++++++++++++ >> 5 files changed, 271 insertions(+), 1 deletion(-) >> create mode 100644 test/box-tap/gh-5602-environment-vars-cfg.result >> create mode 100755 test/box-tap/gh-5602-environment-vars-cfg.test.lua >> create mode 100755 test/box-tap/gh-5602_script.lua >> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua >> index 574c8bef4..eace25d46 100644 >> --- a/src/box/lua/load_cfg.lua >> +++ b/src/box/lua/load_cfg.lua >> @@ -616,6 +616,79 @@ setmetatable(box, { >> end >> }) > > 4) Add descriptions to functions (must start with `---`). Done. >> +local function process_option_value(option, raw_value) >> + local param_type = template_cfg[option] >> + if param_type == nil then >> + return nil >> + end >> + local value = nil >> + if param_type == 'number' or option == 'log_level' then >> + value = tonumber(raw_value) >> + if value == nil then >> + error("Environment variable "..'TT_'..string.upper(option).." has".. >> + " incorrect value for option '"..option.."': should be ".. >> + "converted to number") > > 5) Codestyle (apply to the whole patch): > - Indetation. > From https://dev.minetest.net/Lua_code_style_guidelines : > "When strings don't fit into the line, you should add the string > (changes) to the next line(s) indented by one tab." (4 spaces in our case). > - String format. > From https://www.tarantool.io/en/doc/latest/dev_guide/lua_style_guide/#indentation-and-formatting: > "Avoid multiple concatenations in one statement, use string.format instead” Done. >> + end >> + elseif param_type == 'boolean' or option == 'log_nonblock' then >> + if raw_value == 'true' then >> + value = true >> + elseif raw_value == 'false' then >> + value = false >> + else >> + error("Environment variable "..'TT_'..string.upper(option).." has".. >> + " incorrect value for option '"..option.."': should be ".. >> + "'true' or 'false'") >> + end >> + elseif param_type == 'string, number' then >> + value = tonumber(raw_value) >> + if value == nil then >> + value = raw_value >> + end >> + elseif param_type == 'string, number, table' then > > 6) OMG! We have to get the table from the env variable. Nothing good to expect. > - What about a nested table? > - Also, what about the cases "string, table" or "table, string, number" ...? Now we don't have such a `param_type`, but they will appear tomorrow. Now, it is 'string, number, table’ only. >> + if string.sub(raw_value, 1, 1) == '{' and >> + string.sub(raw_value, -1) == '}' then >> + value = string.sub(raw_value, 2, -2) >> + value = require('csv').load(value)[1] >> + for i, str in pairs(value) do >> + if not (string.sub(str, 1, 1) == '\'' and >> + string.sub(str, -1) == '\'') and >> + not (string.sub(str, 1, 1) == '"' and >> + string.sub(str, -1) == '"') then >> + error("Environment variable "..'TT_'..string.upper(option).. >> + " has incorrect value for option '"..option.."': ".. >> + "elements of array should be quoted strings") >> + end >> + str = string.sub(str, 2, -2) > > 7) Shouldn't we be checking types inside a table? Or can there only be strings? > >> + value[i] = str >> + end >> + else >> + value = tonumber(raw_value) >> + if value == nil then >> + value = raw_value >> + end >> + end >> + else >> + value = raw_value >> + end >> + >> + return value >> +end >> + >> +local function collect_env_cfg(cfg) >> + if cfg == nil then >> + cfg = {} >> + end > > 8) Please, separate logical blocks of code from each other with empty lines. Write comments.(apply to the whole patch). Done. >> + for option, _ in pairs(template_cfg) do >> + if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then >> + local env_var_name = 'TT_'..string.upper(option) >> + local raw_value = os.getenv(env_var_name) >> + if raw_value ~= nil then >> + cfg[option] = process_option_value(option, raw_value) >> + end >> + end >> + end >> +end >> + >> -- Whether box is loaded. >> -- >> -- `false` when box is not configured or when the initialization >> @@ -627,6 +700,7 @@ setmetatable(box, { >> local box_is_configured = false >> local function load_cfg(cfg) >> + collect_env_cfg(cfg) >> -- A user may save box.cfg (this function) before box loading >> -- and call it afterwards. We should reconfigure box in the >> -- case. > > 9) It looks like in this case we can overload the value that was set in > the previous call with the value from the environment. Or not? Discussed. >> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua >> index 3c3023d34..1f15fe103 100755 >> --- a/test/app-tap/tarantoolctl.test.lua >> +++ b/test/app-tap/tarantoolctl.test.lua >> @@ -160,7 +160,7 @@ local function merge(...) >> end >> local test = tap.test('tarantoolctl') >> -test:plan(8) >> +test:plan(9) >> -- basic start/stop test >> -- must be stopped afterwards >> @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) >> end >> end) >> +-- gh-5602: Check that environment cfg variables are more >> +-- prioritized than tarantoolctl cfg. >> +do > > 10) As for me, it is preferable to use `test:test()` instead of `do...end`. Up to you. > >> + local dir = fio.tempdir() >> + local code = [[ box.cfg ]] > > 11) Maybe `[[ box.cfg{} ]]` No, box.cfg. It prints cfg. >> + create_script(dir, 'show_cfg.lua', code) > > 12) Two spaces before `code`, but one is needed. Done. >> + local code = [[ box.cfg{listen=3302, readahead=10001} ]] >> + create_script(dir, 'cfg_script.lua', code) >> + >> + local status, err = pcall(function() >> + test:test("check answers in case of call", function(test_i) >> + test_i:plan(4) >> + os.setenv('TT_LISTEN', '3301') > > 13) We will not have problems due to a specific port when many tests are > run in parallel (such as this port will be already occupied). Changed to checkpoint_interval. >> + os.setenv('TT_READAHEAD', '10000') >> + check_ok(test_i, dir, 'start', 'cfg_script', 0) >> + tctl_wait_start(dir, 'cfg_script') >> + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, >> + '---\n- 3301\n- 10000\n...', nil) >> + check_ok(test_i, dir, 'stop', 'cfg_script', 0) >> + end) >> + end) >> + >> + cleanup_instance(dir, 'cfg_script') >> + recursive_rmdir(dir) >> + >> + if status == false then >> + print(("Error: %s"):format(err)) >> + os.exit() >> + end >> +end >> + >> os.exit(test:check() == true and 0 or -1) >> diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result >> new file mode 100644 >> index 000000000..495e02a73 >> --- /dev/null >> +++ b/test/box-tap/gh-5602-environment-vars-cfg.result >> @@ -0,0 +1,42 @@ >> +TAP version 13 >> +1..5 >> +ok - box.cfg is successful >> +ok - listen >> +ok - readahead >> +ok - strip_core >> +ok - log_format is not set >> +TAP version 13 >> +1..7 >> +ok - box.cfg is successful >> +ok - listen >> +ok - replication is table >> +ok - replication URI 1 >> +ok - replication URI 2 >> +ok - replication_connect_timeout >> +ok - replication_synchro_quorum >> +TAP version 13 >> +1..3 >> +ok - box.cfg is successful >> +ok - box.cfg{} background value is prioritized >> +ok - box.cfg{} vinyl_timeout value is prioritized >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +ok - bad sql_cache_size value >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:638: Environment variable TT_STRIP_CORE has incorrect value for option 'strip_core': should be 'true' or 'false' >> +ok - bad strip_core value >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +builtin/box/load_cfg.lua:19: builtin/box/load_cfg.lua:657: Environment variable TT_REPLICATION has incorrect value for option 'replication': elements of array should be quoted strings >> +ok - bad replication value >> +TAP version 13 >> +ok - set 1 >> +ok - set 2 >> +ok - set 3 >> +ok - set 4 >> +ok - set 5 >> +ok - set 6 >> diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua >> new file mode 100755 >> index 000000000..84b49b7eb >> --- /dev/null >> +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua >> @@ -0,0 +1,53 @@ >> +#!/usr/bin/env tarantool >> + >> +local os = require('os') >> +local fio = require('fio') >> +local tap = require('tap') > > 13) It's nice to separate the variable definition and the `requires` with empty line.(apply to the whole patch) Done. >> +local test = tap.test('gh-5602') >> + >> +-- gh-5602: Check that environment cfg variables working. >> +local TARANTOOL_PATH = arg[-1] >> +local script_name = 'gh-5602_script.lua' >> +local path_to_script = fio.pathjoin( >> + os.getenv('PWD'), >> + 'box-tap', >> + script_name) > > 14) Please, separate the function definition with an empty line. Done. > 15) Describe the test mechanism using a comment. Done. >> +local function shell_command(set, i) >> + string.gsub(set, 'TT_', '') >> + return ('%s %s %s %d'):format( >> + set, >> + TARANTOOL_PATH, >> + path_to_script, >> + i) >> +end >> + >> +local set_1 = 'TT_LISTEN=3301 ' .. >> + 'TT_READAHEAD=10000 ' .. >> + 'TT_STRIP_CORE=false ' .. >> + 'TT_LOG_FORMAT=json' >> +local set_2 = 'TT_LISTEN=3301 ' .. >> + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', ' .. >> + '\'1.1.1.1:12345\'}\" ' .. >> + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01 ' .. >> + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'' >> +local set_3 = 'TT_BACKGROUND=true ' .. >> + 'TT_VINYL_TIMEOUT=60.1' >> +local set_4 = 'TT_SQL_CACHE_SIZE=a' >> +local set_5 = 'TT_STRIP_CORE=a' >> +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', ' .. >> + '\'1.1.1.1:12345\'}\"' >> +local sets = {} >> +sets[1] = set_1 >> +sets[2] = set_2 >> +sets[3] = set_3 >> +sets[4] = set_4 >> +sets[5] = set_5 >> +sets[6] = set_6 >> + >> +for i, set in ipairs(sets) do >> + local tmpdir = fio.tempdir() >> + local new_path = fio.pathjoin(tmpdir, script_name) >> + fio.copyfile(path_to_script, new_path) >> + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) >> +end > > 16) Add `test:plan()` and `test:check()`. done. >> +os.exit(0) >> diff --git a/test/box-tap/gh-5602_script.lua b/test/box-tap/gh-5602_script.lua > > 17) Use a more self-documenting name. Done. >> new file mode 100755 >> index 000000000..8ff34deb2 >> --- /dev/null >> +++ b/test/box-tap/gh-5602_script.lua >> @@ -0,0 +1,70 @@ >> +local tap = require('tap') >> +local test = tap.test('gh-5602') >> + >> +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) >> + >> +-- Check that environment cfg values are set correctly. >> +if arg[1] == '1' then >> + test:plan(5) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['listen'], '3301', 'listen') >> + test:is(box.cfg['readahead'], 10000, 'readahead') >> + test:is(box.cfg['strip_core'], false, 'strip_core') >> + test:is(box.cfg['log_format'], 'json', 'log_format is not set') >> +end >> +if arg[1] == '2' then >> + test:plan(7) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['listen'], '3301', 'listen') >> + local replication = box.cfg['replication'] >> + test:is(type(replication), 'table', 'replication is table') >> + test:ok(replication[1] == '0.0.0.0:12345' or >> + replication[1] == '1.1.1.1:12345', 'replication URI 1') >> + test:ok(replication[2] == '0.0.0.0:12345' or >> + replication[2] == '1.1.1.1:12345', 'replication URI 2') >> + test:is(box.cfg['replication_connect_timeout'], 0.01, >> + 'replication_connect_timeout') >> + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', >> + 'replication_synchro_quorum') >> +end >> + >> +-- Check that box.cfg{} values are more prioritized than >> +-- environment cfg values. >> +if arg[1] == '3' then >> + test:plan(3) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['background'], false, >> + 'box.cfg{} background value is prioritized') >> + test:is(box.cfg['vinyl_timeout'], 70.1, >> + 'box.cfg{} vinyl_timeout value is prioritized') >> +end >> + >> +-- Check bad environment cfg values. >> +if arg[1] == '4' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + local index = string.find(err, 'Environment variable TT_SQL_CACHE_SIZE '.. >> + 'has incorrect value for option \'sql_cache_size\': should be '.. >> + 'converted to number') >> + test:ok(index ~= nil, 'bad sql_cache_size value') >> +end >> +if arg[1] == '5' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + print(err) >> + local index = string.find(err, 'Environment variable TT_STRIP_CORE has '.. >> + 'incorrect value for option \'strip_core\': should be \'true\' '.. >> + 'or \'false\'') >> + test:ok(index ~= nil, 'bad strip_core value') >> +end >> +if arg[1] == '6' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + print(err) >> + local index = string.find(err, 'Environment variable TT_REPLICATION has '.. >> + 'incorrect value for option \'replication\': elements of array should'.. >> + ' be quoted strings') >> + test:ok(index ~= nil, 'bad replication value') >> +end >> + >> +os.exit(test:check() and 0 or 1) >> \ No newline at end of file > > 18) Add newline to the end of file. commit ee6525f843a347c1df8a92a015414f4cbcceafb9 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Tue Mar 9 16:47:11 2021 +0300 box: set cfg via environment variables Add ability to set box.cfg options via env variables. These variables should have name "TT_<OPTION>". When Tarantool instance is started under tarantoolctl utility env variables options have more priority than instance file's cfg. Closes #5602 @TarantoolBot document Title: Set box.cfg options via env variables Now, it is possible to set box.cfg options via environment variables. The name of variable should correspond the following pattern: "TT_<NAME>", where <NAME> is an option name. For example: "TT_LISTEN", "TT_READAHEAD". If it is complex value, for example, "replication" can be set by a table, the corresponding environment variable value has the syntax of lua array (without keys): `export TT_REPLICATION="{'uri1', 'uri2'}"` diff --git a/changelogs/unreleased/environment-cfg.md b/changelogs/unreleased/environment-cfg.md new file mode 100755 index 000000000..c5d5005ac --- /dev/null +++ b/changelogs/unreleased/environment-cfg.md @@ -0,0 +1,5 @@ +## feature/core + +* Now, it is possibly to set box.cfg options with environment variables. +The priority of sources of configuration options is the following (from low +to high): default, tarantoolctl, environment, box.cfg{}. (gh-5602). \ No newline at end of file diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index 574c8bef4..a1439ae46 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -416,6 +416,20 @@ local translate_cfg = { end}, } +-- Configuration options from environment is taken into account +-- only at the first box.cfg{} call. When box.cfg is reload, they +-- are ignored. +-- +-- The priority of sources of configuration options is the +-- following (from low to high): default, tarantoolctl, +-- environment, box.cfg{}. +local env_cfg = {} +-- This is auxiliary table and it needed to keep values which +-- passed or typed to box.cfg{} at the first call. Then we +-- distinguish values (it was from environment or not) and decide +-- ignore it or not during reload. +local user_cfg = {} + -- Upgrade old config local function upgrade_cfg(cfg, translate_cfg) if cfg == nil then @@ -515,6 +529,22 @@ local function prepare_cfg(cfg, default_cfg, template_cfg, return new_cfg end +--- +-- Transfer options from env_cfg to cfg. +-- +local function apply_env_cfg(cfg, env_cfg) + if env_cfg == nil then + return + end + for k, v in pairs(env_cfg) do + if cfg[k] == nil or os.getenv('TARANTOOLCTL') == 'true' then + cfg[k] = v + elseif type(v) == 'table' then + apply_env_cfg(cfg[k], v) + end + end +end + local function apply_default_cfg(cfg, default_cfg, module_cfg) for k,v in pairs(default_cfg) do if cfg[k] == nil then @@ -549,7 +579,19 @@ local function compare_cfg(cfg1, cfg2) return true end +--- +-- Nullify option values from environment in old configuration. +-- +local function reset_env_options(oldcfg) + for k, v in pairs(env_cfg) do + if user_cfg[k] == nil then + oldcfg[k] = nil + end + end +end + local function reload_cfg(oldcfg, cfg) + reset_env_options(oldcfg) cfg = upgrade_cfg(cfg, translate_cfg) local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, module_cfg, modify_cfg) @@ -616,6 +658,104 @@ setmetatable(box, { end }) +--- +-- Erase quotes at the begin and the end of string. +-- +local function dequote(str) + if string.sub(str, 1, 1) == '\'' and string.sub(str, -1) == '\'' or + string.sub(str, 1, 1) == '"' and string.sub(str, -1) == '"' then + return string.sub(str, 2, -2) + end + return str +end + +--- +-- Convert raw option value becoming from the corresponding +-- environment variable represented as string to string, number or +-- boolean (depending on option). +-- +local function process_option_value(option, raw_value) + local param_type = template_cfg[option] + + -- May be peculiar to "feedback*" parameters. + if param_type == nil then + return nil + end + + local value = nil + local err_msg_fmt = 'Environment variable TT_%s has '.. + 'incorrect value for option \'%s\': should be %s' + + -- "log_level" option should be a number. + if param_type == 'number' or option == 'log_level' then + value = tonumber(raw_value) + if value == nil then + error(err_msg_fmt:format(string.upper(option), option, + "convertible to number")) + end + -- "log_nonblock" option should be boolean. + elseif param_type == 'boolean' or option == 'log_nonblock' then + if raw_value == 'true' then + value = true + elseif raw_value == 'false' then + value = false + else + error(err_msg_fmt:format(string.upper(option), option, + "'true' or 'false'")) + end + elseif param_type == 'string, number' then + -- Firstly, try to convert to number. + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + elseif param_type == 'string, number, table' then + if string.sub(raw_value, 1, 1) == '{' and + string.sub(raw_value, -1) == '}' then + value = string.sub(raw_value, 2, -2) + value = require('csv').load(value)[1] + for i, item in pairs(value) do + value[i] = dequote(item) + end + else + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + end + else + value = raw_value + end + + return value +end + +-- local function collect_env_cfg() +-- local env_cfg = {} +-- for option, _ in pairs(template_cfg) do +-- if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then +-- local env_var_name = 'TT_'..string.upper(option) +-- local raw_value = os.getenv(env_var_name) +-- if raw_value ~= nil then +-- cfg[option] = process_option_value(option, raw_value) +-- end +-- end +-- end +-- end + +--- +-- Pull and process configuration values from environment. +-- +local function collect_env_cfg() + for option, _ in pairs(template_cfg) do + local env_var_name = 'TT_'..string.upper(option) + local raw_value = os.getenv(env_var_name) + if raw_value ~= nil then + env_cfg[option] = process_option_value(option, raw_value) + end + end +end + -- Whether box is loaded. -- -- `false` when box is not configured or when the initialization @@ -627,6 +767,7 @@ setmetatable(box, { local box_is_configured = false local function load_cfg(cfg) + user_cfg = cfg -- A user may save box.cfg (this function) before box loading -- and call it afterwards. We should reconfigure box in the -- case. @@ -638,6 +779,8 @@ local function load_cfg(cfg) cfg = upgrade_cfg(cfg, translate_cfg) cfg = prepare_cfg(cfg, default_cfg, template_cfg, module_cfg, modify_cfg) + collect_env_cfg() + apply_env_cfg(cfg, env_cfg) apply_default_cfg(cfg, default_cfg, module_cfg); -- Save new box.cfg box.cfg = cfg diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua index 3c3023d34..060316714 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -160,7 +160,7 @@ local function merge(...) end local test = tap.test('tarantoolctl') -test:plan(8) +test:plan(9) -- basic start/stop test -- must be stopped afterwards @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) end end) +-- gh-5602: Check that environment cfg variables are more +-- prioritized than tarantoolctl cfg. +do + local dir = fio.tempdir() + local code = [[ box.cfg ]] + create_script(dir, 'show_cfg.lua', code) + local code = [[ box.cfg{checkpoint_interval=3302, readahead=10001} ]] + create_script(dir, 'cfg_script.lua', code) + + local status, err = pcall(function() + test:test("check answers in case of call", function(test_i) + test_i:plan(4) + os.setenv('TT_CHECKPOINT_INTERVAL', '3301') + os.setenv('TT_READAHEAD', '10000') + check_ok(test_i, dir, 'start', 'cfg_script', 0) + tctl_wait_start(dir, 'cfg_script') + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, + '---\n- 3301\n- 10000\n...', nil) + check_ok(test_i, dir, 'stop', 'cfg_script', 0) + end) + end) + + cleanup_instance(dir, 'cfg_script') + recursive_rmdir(dir) + + if status == false then + print(("Error: %s"):format(err)) + os.exit() + end +end + os.exit(test:check() == true and 0 or -1) diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result new file mode 100644 index 000000000..c90dd301b --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.result @@ -0,0 +1,41 @@ +TAP version 13 +1..5 +ok - box.cfg is successful +ok - listen +ok - readahead +ok - strip_core +ok - log_format is not set +TAP version 13 +1..7 +ok - box.cfg is successful +ok - listen +ok - replication is table +ok - replication URI 1 +ok - replication URI 2 +ok - replication_connect_timeout +ok - replication_synchro_quorum +TAP version 13 +1..3 +ok - box.cfg is successful +ok - box.cfg{} background value is prioritized +ok - box.cfg{} vinyl_timeout value is prioritized +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad sql_cache_size value +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad strip_core value +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad replication value +TAP version 13 +1..6 +ok - set 1 +ok - set 2 +ok - set 3 +ok - set 4 +ok - set 5 +ok - set 6 diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua new file mode 100755 index 000000000..3d19fdc90 --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua @@ -0,0 +1,57 @@ +#!/usr/bin/env tarantool + +local os = require('os') +local fio = require('fio') +local tap = require('tap') + +local test = tap.test('gh-5602') + +-- gh-5602: Check that environment cfg variables working. +local TARANTOOL_PATH = arg[-1] +local script_name = 'gh-5602-test-cases-script.lua' +local path_to_script = fio.pathjoin( + os.getenv('PWD'), + 'box-tap', + script_name) + +--- +-- Generate shell command where set is sequence of environment +-- variables, then path to tarantool, the first arg is path to +-- test script and the second one is number of set. +-- +local function shell_command(set, i) + return ('%s %s %s %d'):format( + set, + TARANTOOL_PATH, + path_to_script, + i) +end + +local set_1 = ('%s %s %s %s'):format('TT_LISTEN=3301', 'TT_READAHEAD=10000', + 'TT_STRIP_CORE=false', 'TT_LOG_FORMAT=json') +local set_2 = ('%s %s %s %s'):format('TT_LISTEN=3301', + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', \'1.1.1.1:12345\'}\"', + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01', + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'') +local set_3 = 'TT_BACKGROUND=true TT_VINYL_TIMEOUT=60.1' +local set_4 = 'TT_SQL_CACHE_SIZE=a' +local set_5 = 'TT_STRIP_CORE=a' +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', \'1.1.1.1:12345\'}\"' + +local sets = {} +sets[1] = set_1 +sets[2] = set_2 +sets[3] = set_3 +sets[4] = set_4 +sets[5] = set_5 +sets[6] = set_6 + +test:plan(#sets) +for i, set in ipairs(sets) do + local tmpdir = fio.tempdir() + local new_path = fio.pathjoin(tmpdir, script_name) + fio.copyfile(path_to_script, new_path) + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) +end + +os.exit(test:check() and 0 or 1) diff --git a/test/box-tap/gh-5602-test-cases-script.lua b/test/box-tap/gh-5602-test-cases-script.lua new file mode 100755 index 000000000..a8b2fc987 --- /dev/null +++ b/test/box-tap/gh-5602-test-cases-script.lua @@ -0,0 +1,69 @@ +local tap = require('tap') + +local test = tap.test('gh-5602') + +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) + +-- Check that environment cfg values are set correctly. +if arg[1] == '1' then + test:plan(5) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], 3301, 'listen') + test:is(box.cfg['readahead'], 10000, 'readahead') + test:is(box.cfg['strip_core'], false, 'strip_core') + test:is(box.cfg['log_format'], 'json', 'log_format is not set') +end +if arg[1] == '2' then + test:plan(7) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], 3301, 'listen') + local replication = box.cfg['replication'] + test:is(type(replication), 'table', 'replication is table') + test:ok(replication[1] == '0.0.0.0:12345' or + replication[1] == '1.1.1.1:12345', 'replication URI 1') + test:ok(replication[2] == '0.0.0.0:12345' or + replication[2] == '1.1.1.1:12345', 'replication URI 2') + test:is(box.cfg['replication_connect_timeout'], 0.01, + 'replication_connect_timeout') + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', + 'replication_synchro_quorum') +end + +-- Check that box.cfg{} values are more prioritized than +-- environment cfg values. +if arg[1] == '3' then + test:plan(3) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['background'], false, + 'box.cfg{} background value is prioritized') + test:is(box.cfg['vinyl_timeout'], 70.1, + 'box.cfg{} vinyl_timeout value is prioritized') +end + +local err_msg_fmt = 'Environment variable TT_%s has '.. + 'incorrect value for option \'%s\': should be %s' + +-- Check bad environment cfg values. +if arg[1] == '4' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(err, err_msg_fmt:format('SQL_CACHE_SIZE', + 'sql_cache_size', 'convertible to number')) + test:ok(index ~= nil, 'bad sql_cache_size value') +end +if arg[1] == '5' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(err, err_msg_fmt:format('STRIP_CORE', + 'strip_core', '\'true\' or \'false\'')) + test:ok(index ~= nil, 'bad strip_core value') +end +if arg[1] == '6' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(tostring(err), 'Incorrect value for option '.. + '\'replication\': expected host:service or /unix.socket') + test:ok(index ~= nil, 'bad replication value') +end + +os.exit(test:check() and 0 or 1) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-04-09 8:56 ` Roman Khabibov via Tarantool-patches @ 2021-04-12 8:37 ` Leonid Vasiliev via Tarantool-patches 2021-04-13 12:47 ` Roman Khabibov via Tarantool-patches 2021-04-12 9:52 ` Leonid Vasiliev via Tarantool-patches 1 sibling, 1 reply; 7+ messages in thread From: Leonid Vasiliev via Tarantool-patches @ 2021-04-12 8:37 UTC (permalink / raw) To: Roman Khabibov; +Cc: tarantool-patches Hi! Thank you for the patch. See 9 comments below: > > commit ee6525f843a347c1df8a92a015414f4cbcceafb9 > Author: Roman Khabibov <roman.habibov@tarantool.org> > Date: Tue Mar 9 16:47:11 2021 +0300 > > box: set cfg via environment variables > > Add ability to set box.cfg options via env variables. These > variables should have name "TT_<OPTION>". When Tarantool instance > is started under tarantoolctl utility env variables options have > more priority than instance file's cfg. > > Closes #5602 > > @TarantoolBot document > Title: Set box.cfg options via env variables > > Now, it is possible to set box.cfg options via environment > variables. The name of variable should correspond the following > pattern: "TT_<NAME>", where <NAME> is an option name. For example: > "TT_LISTEN", "TT_READAHEAD". > If it is complex value, for example, "replication" can be set by a > table, the corresponding environment variable value has the syntax > of lua array (without keys): > > `export TT_REPLICATION="{'uri1', 'uri2'}"` > > diff --git a/changelogs/unreleased/environment-cfg.md b/changelogs/unreleased/environment-cfg.md > new file mode 100755 > index 000000000..c5d5005ac > --- /dev/null > +++ b/changelogs/unreleased/environment-cfg.md > @@ -0,0 +1,5 @@ > +## feature/core > + > +* Now, it is possibly to set box.cfg options with environment variables. > +The priority of sources of configuration options is the following (from low > +to high): default, tarantoolctl, environment, box.cfg{}. (gh-5602). > \ No newline at end of file > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 574c8bef4..a1439ae46 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -416,6 +416,20 @@ local translate_cfg = { > end}, > } > > +-- Configuration options from environment is taken into account > +-- only at the first box.cfg{} call. When box.cfg is reload, they > +-- are ignored. > +-- > +-- The priority of sources of configuration options is the > +-- following (from low to high): default, tarantoolctl, > +-- environment, box.cfg{}. > +local env_cfg = {} > +-- This is auxiliary table and it needed to keep values which > +-- passed or typed to box.cfg{} at the first call. Then we > +-- distinguish values (it was from environment or not) and decide > +-- ignore it or not during reload. > +local user_cfg = {} 1) AFAIU we decided what it's not needed. In my mind, on reload, we have no difference between the options that were set using environment and any other way. > + > -- Upgrade old config > local function upgrade_cfg(cfg, translate_cfg) > if cfg == nil then > @@ -515,6 +529,22 @@ local function prepare_cfg(cfg, default_cfg, template_cfg, > return new_cfg > end > > +--- > +-- Transfer options from env_cfg to cfg. > +-- > +local function apply_env_cfg(cfg, env_cfg) > + if env_cfg == nil then > + return > + end > + for k, v in pairs(env_cfg) do > + if cfg[k] == nil or os.getenv('TARANTOOLCTL') == 'true' then 2) Add a comment describing the condition `os.getenv('TARANTOOLCTL') == 'true'`. > + cfg[k] = v > + elseif type(v) == 'table' then > + apply_env_cfg(cfg[k], v) > + end > + end > +end > + > local function apply_default_cfg(cfg, default_cfg, module_cfg) > for k,v in pairs(default_cfg) do > if cfg[k] == nil then > @@ -549,7 +579,19 @@ local function compare_cfg(cfg1, cfg2) > return true > end > > +--- > +-- Nullify option values from environment in old configuration. > +-- > +local function reset_env_options(oldcfg) 3) Why should we nullify option values from environment in old configuration? > + for k, v in pairs(env_cfg) do > + if user_cfg[k] == nil then > + oldcfg[k] = nil > + end > + end > +end > + > local function reload_cfg(oldcfg, cfg) > + reset_env_options(oldcfg) > cfg = upgrade_cfg(cfg, translate_cfg) > local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, > module_cfg, modify_cfg) > @@ -616,6 +658,104 @@ setmetatable(box, { > end > }) > > +--- > +-- Erase quotes at the begin and the end of string. > +-- > +local function dequote(str) > + if string.sub(str, 1, 1) == '\'' and string.sub(str, -1) == '\'' or > + string.sub(str, 1, 1) == '"' and string.sub(str, -1) == '"' then > + return string.sub(str, 2, -2) > + end > + return str > +end > + > +--- > +-- Convert raw option value becoming from the corresponding > +-- environment variable represented as string to string, number or > +-- boolean (depending on option). > +-- > +local function process_option_value(option, raw_value) > + local param_type = template_cfg[option] > + > + -- May be peculiar to "feedback*" parameters. > + if param_type == nil then > + return nil > + end > + > + local value = nil > + local err_msg_fmt = 'Environment variable TT_%s has '.. 4) Add a space before `..`. > + 'incorrect value for option \'%s\': should be %s' > + > + -- "log_level" option should be a number. > + if param_type == 'number' or option == 'log_level' then 5) As for me it's strange, that in a94a9b3fd629b5abbc99a23ba527cd91e5c6ebf5 Cyrill Gorcunov moves the type check to a module, but you (in fact) return it back (determine the type by the parameter name). I think you need to discuss this question with Cyrill and check types either inside the module or in the `load_cfg.lua`. > + value = tonumber(raw_value) > + if value == nil then > + error(err_msg_fmt:format(string.upper(option), option, > + "convertible to number")) > + end > + -- "log_nonblock" option should be boolean. > + elseif param_type == 'boolean' or option == 'log_nonblock' then > + if raw_value == 'true' then > + value = true > + elseif raw_value == 'false' then > + value = false > + else > + error(err_msg_fmt:format(string.upper(option), option, > + "'true' or 'false'")) > + end > + elseif param_type == 'string, number' then 6) Since some options can have several types, I think it's more comfortable for such options to use a table listing all possible types. But such refactoring doesn't seem to be mandatory for the patch. > + -- Firstly, try to convert to number. > + value = tonumber(raw_value) > + if value == nil then > + value = raw_value > + end > + elseif param_type == 'string, number, table' then > + if string.sub(raw_value, 1, 1) == '{' and > + string.sub(raw_value, -1) == '}' then > + value = string.sub(raw_value, 2, -2) > + value = require('csv').load(value)[1] > + for i, item in pairs(value) do > + value[i] = dequote(item) 7) In this case, the number inside the table will be parsed as a "string". We don't want that. > + end > + else > + value = tonumber(raw_value) > + if value == nil then > + value = raw_value > + end > + end > + else > + value = raw_value > + end > + > + return value > +end > + > +-- local function collect_env_cfg() > +-- local env_cfg = {} > +-- for option, _ in pairs(template_cfg) do > +-- if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then > +-- local env_var_name = 'TT_'..string.upper(option) > +-- local raw_value = os.getenv(env_var_name) > +-- if raw_value ~= nil then > +-- cfg[option] = process_option_value(option, raw_value) > +-- end > +-- end > +-- end > +-- end 8) Don't leave commented code after work. > + > +--- > +-- Pull and process configuration values from environment. > +-- > +local function collect_env_cfg() > + for option, _ in pairs(template_cfg) do > + local env_var_name = 'TT_'..string.upper(option) 9) Add spaces around the `..` > + local raw_value = os.getenv(env_var_name) > + if raw_value ~= nil then > + env_cfg[option] = process_option_value(option, raw_value) > + end > + end > +end > + > -- Whether box is loaded. > -- > -- `false` when box is not configured or when the initialization > @@ -627,6 +767,7 @@ setmetatable(box, { > local box_is_configured = false > > local function load_cfg(cfg) > + user_cfg = cfg > -- A user may save box.cfg (this function) before box loading > -- and call it afterwards. We should reconfigure box in the > -- case. > @@ -638,6 +779,8 @@ local function load_cfg(cfg) > cfg = upgrade_cfg(cfg, translate_cfg) > cfg = prepare_cfg(cfg, default_cfg, template_cfg, > module_cfg, modify_cfg) > + collect_env_cfg() > + apply_env_cfg(cfg, env_cfg) > apply_default_cfg(cfg, default_cfg, module_cfg); > -- Save new box.cfg > box.cfg = cfg > diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua > index 3c3023d34..060316714 100755 > --- a/test/app-tap/tarantoolctl.test.lua > +++ b/test/app-tap/tarantoolctl.test.lua > @@ -160,7 +160,7 @@ local function merge(...) > end > > local test = tap.test('tarantoolctl') > -test:plan(8) > +test:plan(9) > > -- basic start/stop test > -- must be stopped afterwards > @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) > end > end) > > +-- gh-5602: Check that environment cfg variables are more > +-- prioritized than tarantoolctl cfg. > +do > + local dir = fio.tempdir() > + local code = [[ box.cfg ]] > + create_script(dir, 'show_cfg.lua', code) > + local code = [[ box.cfg{checkpoint_interval=3302, readahead=10001} ]] > + create_script(dir, 'cfg_script.lua', code) > + > + local status, err = pcall(function() > + test:test("check answers in case of call", function(test_i) > + test_i:plan(4) > + os.setenv('TT_CHECKPOINT_INTERVAL', '3301') > + os.setenv('TT_READAHEAD', '10000') > + check_ok(test_i, dir, 'start', 'cfg_script', 0) > + tctl_wait_start(dir, 'cfg_script') > + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, > + '---\n- 3301\n- 10000\n...', nil) > + check_ok(test_i, dir, 'stop', 'cfg_script', 0) > + end) > + end) > + > + cleanup_instance(dir, 'cfg_script') > + recursive_rmdir(dir) > + > + if status == false then > + print(("Error: %s"):format(err)) > + os.exit() > + end > +end > + > os.exit(test:check() == true and 0 or -1) > diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result > new file mode 100644 > index 000000000..c90dd301b > --- /dev/null > +++ b/test/box-tap/gh-5602-environment-vars-cfg.result > @@ -0,0 +1,41 @@ > +TAP version 13 > +1..5 > +ok - box.cfg is successful > +ok - listen > +ok - readahead > +ok - strip_core > +ok - log_format is not set > +TAP version 13 > +1..7 > +ok - box.cfg is successful > +ok - listen > +ok - replication is table > +ok - replication URI 1 > +ok - replication URI 2 > +ok - replication_connect_timeout > +ok - replication_synchro_quorum > +TAP version 13 > +1..3 > +ok - box.cfg is successful > +ok - box.cfg{} background value is prioritized > +ok - box.cfg{} vinyl_timeout value is prioritized > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +ok - bad sql_cache_size value > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +ok - bad strip_core value > +TAP version 13 > +1..2 > +ok - box.cfg is not successful > +ok - bad replication value > +TAP version 13 > +1..6 > +ok - set 1 > +ok - set 2 > +ok - set 3 > +ok - set 4 > +ok - set 5 > +ok - set 6 > diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua > new file mode 100755 > index 000000000..3d19fdc90 > --- /dev/null > +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua > @@ -0,0 +1,57 @@ > +#!/usr/bin/env tarantool > + > +local os = require('os') > +local fio = require('fio') > +local tap = require('tap') > + > +local test = tap.test('gh-5602') > + > +-- gh-5602: Check that environment cfg variables working. > +local TARANTOOL_PATH = arg[-1] > +local script_name = 'gh-5602-test-cases-script.lua' > +local path_to_script = fio.pathjoin( > + os.getenv('PWD'), > + 'box-tap', > + script_name) > + > +--- > +-- Generate shell command where set is sequence of environment > +-- variables, then path to tarantool, the first arg is path to > +-- test script and the second one is number of set. > +-- > +local function shell_command(set, i) > + return ('%s %s %s %d'):format( > + set, > + TARANTOOL_PATH, > + path_to_script, > + i) > +end > + > +local set_1 = ('%s %s %s %s'):format('TT_LISTEN=3301', 'TT_READAHEAD=10000', > + 'TT_STRIP_CORE=false', 'TT_LOG_FORMAT=json') > +local set_2 = ('%s %s %s %s'):format('TT_LISTEN=3301', > + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', \'1.1.1.1:12345\'}\"', > + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01', > + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'') > +local set_3 = 'TT_BACKGROUND=true TT_VINYL_TIMEOUT=60.1' > +local set_4 = 'TT_SQL_CACHE_SIZE=a' > +local set_5 = 'TT_STRIP_CORE=a' > +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', \'1.1.1.1:12345\'}\"' > + > +local sets = {} > +sets[1] = set_1 > +sets[2] = set_2 > +sets[3] = set_3 > +sets[4] = set_4 > +sets[5] = set_5 > +sets[6] = set_6 > + > +test:plan(#sets) > +for i, set in ipairs(sets) do > + local tmpdir = fio.tempdir() > + local new_path = fio.pathjoin(tmpdir, script_name) > + fio.copyfile(path_to_script, new_path) > + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) > +end > + > +os.exit(test:check() and 0 or 1) > diff --git a/test/box-tap/gh-5602-test-cases-script.lua b/test/box-tap/gh-5602-test-cases-script.lua > new file mode 100755 > index 000000000..a8b2fc987 > --- /dev/null > +++ b/test/box-tap/gh-5602-test-cases-script.lua > @@ -0,0 +1,69 @@ > +local tap = require('tap') > + > +local test = tap.test('gh-5602') > + > +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) > + > +-- Check that environment cfg values are set correctly. > +if arg[1] == '1' then > + test:plan(5) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['listen'], 3301, 'listen') > + test:is(box.cfg['readahead'], 10000, 'readahead') > + test:is(box.cfg['strip_core'], false, 'strip_core') > + test:is(box.cfg['log_format'], 'json', 'log_format is not set') > +end > +if arg[1] == '2' then > + test:plan(7) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['listen'], 3301, 'listen') > + local replication = box.cfg['replication'] > + test:is(type(replication), 'table', 'replication is table') > + test:ok(replication[1] == '0.0.0.0:12345' or > + replication[1] == '1.1.1.1:12345', 'replication URI 1') > + test:ok(replication[2] == '0.0.0.0:12345' or > + replication[2] == '1.1.1.1:12345', 'replication URI 2') > + test:is(box.cfg['replication_connect_timeout'], 0.01, > + 'replication_connect_timeout') > + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', > + 'replication_synchro_quorum') > +end > + > +-- Check that box.cfg{} values are more prioritized than > +-- environment cfg values. > +if arg[1] == '3' then > + test:plan(3) > + test:ok(status ~= false, 'box.cfg is successful') > + test:is(box.cfg['background'], false, > + 'box.cfg{} background value is prioritized') > + test:is(box.cfg['vinyl_timeout'], 70.1, > + 'box.cfg{} vinyl_timeout value is prioritized') > +end > + > +local err_msg_fmt = 'Environment variable TT_%s has '.. > + 'incorrect value for option \'%s\': should be %s' > + > +-- Check bad environment cfg values. > +if arg[1] == '4' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + local index = string.find(err, err_msg_fmt:format('SQL_CACHE_SIZE', > + 'sql_cache_size', 'convertible to number')) > + test:ok(index ~= nil, 'bad sql_cache_size value') > +end > +if arg[1] == '5' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + local index = string.find(err, err_msg_fmt:format('STRIP_CORE', > + 'strip_core', '\'true\' or \'false\'')) > + test:ok(index ~= nil, 'bad strip_core value') > +end > +if arg[1] == '6' then > + test:plan(2) > + test:ok(status == false, 'box.cfg is not successful') > + local index = string.find(tostring(err), 'Incorrect value for option '.. > + '\'replication\': expected host:service or /unix.socket') > + test:ok(index ~= nil, 'bad replication value') > +end > + > +os.exit(test:check() and 0 or 1) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-04-12 8:37 ` Leonid Vasiliev via Tarantool-patches @ 2021-04-13 12:47 ` Roman Khabibov via Tarantool-patches 0 siblings, 0 replies; 7+ messages in thread From: Roman Khabibov via Tarantool-patches @ 2021-04-13 12:47 UTC (permalink / raw) To: Leonid Vasiliev; +Cc: tarantool-patches Hi! Thanks for the review. > On Apr 12, 2021, at 11:37, Leonid Vasiliev <lvasiliev@tarantool.org> wrote: > > Hi! Thank you for the patch. > See 9 comments below: > >> commit ee6525f843a347c1df8a92a015414f4cbcceafb9 >> Author: Roman Khabibov <roman.habibov@tarantool.org> >> Date: Tue Mar 9 16:47:11 2021 +0300 >> box: set cfg via environment variables >> Add ability to set box.cfg options via env variables. These >> variables should have name "TT_<OPTION>". When Tarantool instance >> is started under tarantoolctl utility env variables options have >> more priority than instance file's cfg. >> Closes #5602 >> @TarantoolBot document >> Title: Set box.cfg options via env variables >> Now, it is possible to set box.cfg options via environment >> variables. The name of variable should correspond the following >> pattern: "TT_<NAME>", where <NAME> is an option name. For example: >> "TT_LISTEN", "TT_READAHEAD". >> If it is complex value, for example, "replication" can be set by a >> table, the corresponding environment variable value has the syntax >> of lua array (without keys): >> `export TT_REPLICATION="{'uri1', 'uri2'}"` >> diff --git a/changelogs/unreleased/environment-cfg.md b/changelogs/unreleased/environment-cfg.md >> new file mode 100755 >> index 000000000..c5d5005ac >> --- /dev/null >> +++ b/changelogs/unreleased/environment-cfg.md >> @@ -0,0 +1,5 @@ >> +## feature/core >> + >> +* Now, it is possibly to set box.cfg options with environment variables. >> +The priority of sources of configuration options is the following (from low >> +to high): default, tarantoolctl, environment, box.cfg{}. (gh-5602). >> \ No newline at end of file >> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua >> index 574c8bef4..a1439ae46 100644 >> --- a/src/box/lua/load_cfg.lua >> +++ b/src/box/lua/load_cfg.lua >> @@ -416,6 +416,20 @@ local translate_cfg = { >> end}, >> } >> +-- Configuration options from environment is taken into account >> +-- only at the first box.cfg{} call. When box.cfg is reload, they >> +-- are ignored. >> +-- >> +-- The priority of sources of configuration options is the >> +-- following (from low to high): default, tarantoolctl, >> +-- environment, box.cfg{}. >> +local env_cfg = {} >> +-- This is auxiliary table and it needed to keep values which >> +-- passed or typed to box.cfg{} at the first call. Then we >> +-- distinguish values (it was from environment or not) and decide >> +-- ignore it or not during reload. >> +local user_cfg = {} > > 1) AFAIU we decided what it's not needed. In my mind, on reload, we have > no difference between the options that were set using environment and any > other way. Removed. >> + >> -- Upgrade old config >> local function upgrade_cfg(cfg, translate_cfg) >> if cfg == nil then >> @@ -515,6 +529,22 @@ local function prepare_cfg(cfg, default_cfg, template_cfg, >> return new_cfg >> end >> +--- >> +-- Transfer options from env_cfg to cfg. >> +-- >> +local function apply_env_cfg(cfg, env_cfg) >> + if env_cfg == nil then >> + return >> + end >> + for k, v in pairs(env_cfg) do >> + if cfg[k] == nil or os.getenv('TARANTOOLCTL') == 'true' then > > 2) Add a comment describing the condition `os.getenv('TARANTOOLCTL') == 'true'`. +--- +-- Transfer options from env_cfg to cfg. +-- +local function apply_env_cfg(cfg, env_cfg) + if env_cfg == nil then + return + end + + for k, v in pairs(env_cfg) do + -- If Tarantool started with tarantoolctl then we have to + -- ignore configuration from tarantoolctl, because they + -- has priority lower than environment variables. + if cfg[k] == nil or os.getenv('TARANTOOLCTL') == 'true' then + cfg[k] = v + elseif type(v) == 'table' then + apply_env_cfg(cfg[k], v) + end + end +end + >> + cfg[k] = v >> + elseif type(v) == 'table' then >> + apply_env_cfg(cfg[k], v) >> + end >> + end >> +end >> + >> local function apply_default_cfg(cfg, default_cfg, module_cfg) >> for k,v in pairs(default_cfg) do >> if cfg[k] == nil then >> @@ -549,7 +579,19 @@ local function compare_cfg(cfg1, cfg2) >> return true >> end >> +--- >> +-- Nullify option values from environment in old configuration. >> +-- >> +local function reset_env_options(oldcfg) > > 3) Why should we nullify option values from environment in old > configuration? > >> + for k, v in pairs(env_cfg) do >> + if user_cfg[k] == nil then >> + oldcfg[k] = nil >> + end >> + end >> +end >> + >> local function reload_cfg(oldcfg, cfg) >> + reset_env_options(oldcfg) >> cfg = upgrade_cfg(cfg, translate_cfg) >> local newcfg = prepare_cfg(cfg, default_cfg, template_cfg, >> module_cfg, modify_cfg) >> @@ -616,6 +658,104 @@ setmetatable(box, { >> end >> }) >> +--- >> +-- Erase quotes at the begin and the end of string. >> +-- >> +local function dequote(str) >> + if string.sub(str, 1, 1) == '\'' and string.sub(str, -1) == '\'' or >> + string.sub(str, 1, 1) == '"' and string.sub(str, -1) == '"' then >> + return string.sub(str, 2, -2) >> + end >> + return str >> +end >> + >> +--- >> +-- Convert raw option value becoming from the corresponding >> +-- environment variable represented as string to string, number or >> +-- boolean (depending on option). >> +-- >> +local function process_option_value(option, raw_value) >> + local param_type = template_cfg[option] >> + >> + -- May be peculiar to "feedback*" parameters. >> + if param_type == nil then >> + return nil >> + end >> + >> + local value = nil >> + local err_msg_fmt = 'Environment variable TT_%s has '.. > > 4) Add a space before `..`. Done. >> + 'incorrect value for option \'%s\': should be %s' >> + >> + -- "log_level" option should be a number. >> + if param_type == 'number' or option == 'log_level' then > > 5) As for me it's strange, that in a94a9b3fd629b5abbc99a23ba527cd91e5c6ebf5 Cyrill Gorcunov > moves the type check to a module, but you (in fact) return it back > (determine the type by the parameter name). I think you need to discuss > this question with Cyrill and check types either inside the module or in the `load_cfg.lua`. Done. See the first patch. >> + value = tonumber(raw_value) >> + if value == nil then >> + error(err_msg_fmt:format(string.upper(option), option, >> + "convertible to number")) >> + end >> + -- "log_nonblock" option should be boolean. >> + elseif param_type == 'boolean' or option == 'log_nonblock' then >> + if raw_value == 'true' then >> + value = true >> + elseif raw_value == 'false' then >> + value = false >> + else >> + error(err_msg_fmt:format(string.upper(option), option, >> + "'true' or 'false'")) >> + end >> + elseif param_type == 'string, number' then > > 6) Since some options can have several types, I think it's more > comfortable for such options to use a table listing all possible types. > But such refactoring doesn't seem to be mandatory for the patch. > >> + -- Firstly, try to convert to number. >> + value = tonumber(raw_value) >> + if value == nil then >> + value = raw_value >> + end >> + elseif param_type == 'string, number, table' then >> + if string.sub(raw_value, 1, 1) == '{' and >> + string.sub(raw_value, -1) == '}' then >> + value = string.sub(raw_value, 2, -2) >> + value = require('csv').load(value)[1] >> + for i, item in pairs(value) do >> + value[i] = dequote(item) > > 7) In this case, the number inside the table will be parsed as a > "string". We don't want that. +--- +-- Convert raw option value becoming from the corresponding +-- environment variable represented as string to string, number or +-- boolean (depending on option). +-- +local function process_option_value(option, raw_value) + local param_type = template_cfg[option] + + -- May be peculiar to "feedback*" parameters. + if param_type == nil then + return nil + end + + local value = nil + local err_msg_fmt = 'Environment variable TT_%s has ' .. + 'incorrect value for option \'%s\': should be %s' + + -- "log_level" option should be a number. + if param_type == 'number' then + value = tonumber(raw_value) + if value == nil then + error(err_msg_fmt:format(string.upper(option), option, + "convertible to number")) + end + -- "log_nonblock" option should be boolean. + elseif param_type == 'boolean' then + if raw_value == 'true' then + value = true + elseif raw_value == 'false' then + value = false + else + error(err_msg_fmt:format(string.upper(option), option, + "'true' or 'false'")) + end + elseif param_type == 'string, number' then + -- Firstly, try to convert to number. + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + elseif param_type == 'string, number, table' then + if string.sub(raw_value, 1, 1) == '{' and + string.sub(raw_value, -1) == '}' then + value = string.sub(raw_value, 2, -2) + value = require('csv').load(value)[1] + for i, item in pairs(value) do + item = dequote(item) + value[i] = tonumber(item) + if value[i] == nil then + value[i] = item + end + end + else + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + end + else + value = raw_value + end + + return value +end + >> + end >> + else >> + value = tonumber(raw_value) >> + if value == nil then >> + value = raw_value >> + end >> + end >> + else >> + value = raw_value >> + end >> + >> + return value >> +end >> + >> +-- local function collect_env_cfg() >> +-- local env_cfg = {} >> +-- for option, _ in pairs(template_cfg) do >> +-- if cfg[option] == nil or os.getenv('TARANTOOLCTL') == 'true' then >> +-- local env_var_name = 'TT_'..string.upper(option) > +-- local raw_value = os.getenv(env_var_name) >> +-- if raw_value ~= nil then >> +-- cfg[option] = process_option_value(option, raw_value) >> +-- end >> +-- end >> +-- end >> +-- end > > 8) Don't leave commented code after work. Removed. >> + >> +--- >> +-- Pull and process configuration values from environment. >> +-- >> +local function collect_env_cfg() >> + for option, _ in pairs(template_cfg) do >> + local env_var_name = 'TT_'..string.upper(option) > > 9) Add spaces around the `..` Done. >> + local raw_value = os.getenv(env_var_name) >> + if raw_value ~= nil then >> + env_cfg[option] = process_option_value(option, raw_value) >> + end >> + end >> +end >> + >> -- Whether box is loaded. >> -- >> -- `false` when box is not configured or when the initialization >> @@ -627,6 +767,7 @@ setmetatable(box, { >> local box_is_configured = false >> local function load_cfg(cfg) >> + user_cfg = cfg >> -- A user may save box.cfg (this function) before box loading >> -- and call it afterwards. We should reconfigure box in the >> -- case. >> @@ -638,6 +779,8 @@ local function load_cfg(cfg) >> cfg = upgrade_cfg(cfg, translate_cfg) >> cfg = prepare_cfg(cfg, default_cfg, template_cfg, >> module_cfg, modify_cfg) >> + collect_env_cfg() >> + apply_env_cfg(cfg, env_cfg) >> apply_default_cfg(cfg, default_cfg, module_cfg); >> -- Save new box.cfg >> box.cfg = cfg >> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua >> index 3c3023d34..060316714 100755 >> --- a/test/app-tap/tarantoolctl.test.lua >> +++ b/test/app-tap/tarantoolctl.test.lua >> @@ -160,7 +160,7 @@ local function merge(...) >> end >> local test = tap.test('tarantoolctl') >> -test:plan(8) >> +test:plan(9) >> -- basic start/stop test >> -- must be stopped afterwards >> @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) >> end >> end) >> +-- gh-5602: Check that environment cfg variables are more >> +-- prioritized than tarantoolctl cfg. >> +do >> + local dir = fio.tempdir() >> + local code = [[ box.cfg ]] >> + create_script(dir, 'show_cfg.lua', code) >> + local code = [[ box.cfg{checkpoint_interval=3302, readahead=10001} ]] >> + create_script(dir, 'cfg_script.lua', code) >> + >> + local status, err = pcall(function() >> + test:test("check answers in case of call", function(test_i) >> + test_i:plan(4) >> + os.setenv('TT_CHECKPOINT_INTERVAL', '3301') >> + os.setenv('TT_READAHEAD', '10000') >> + check_ok(test_i, dir, 'start', 'cfg_script', 0) >> + tctl_wait_start(dir, 'cfg_script') >> + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, >> + '---\n- 3301\n- 10000\n...', nil) >> + check_ok(test_i, dir, 'stop', 'cfg_script', 0) >> + end) >> + end) >> + >> + cleanup_instance(dir, 'cfg_script') >> + recursive_rmdir(dir) >> + >> + if status == false then >> + print(("Error: %s"):format(err)) >> + os.exit() >> + end >> +end >> + >> os.exit(test:check() == true and 0 or -1) >> diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result >> new file mode 100644 >> index 000000000..c90dd301b >> --- /dev/null >> +++ b/test/box-tap/gh-5602-environment-vars-cfg.result >> @@ -0,0 +1,41 @@ >> +TAP version 13 >> +1..5 >> +ok - box.cfg is successful >> +ok - listen >> +ok - readahead >> +ok - strip_core >> +ok - log_format is not set >> +TAP version 13 >> +1..7 >> +ok - box.cfg is successful >> +ok - listen >> +ok - replication is table >> +ok - replication URI 1 >> +ok - replication URI 2 >> +ok - replication_connect_timeout >> +ok - replication_synchro_quorum >> +TAP version 13 >> +1..3 >> +ok - box.cfg is successful >> +ok - box.cfg{} background value is prioritized >> +ok - box.cfg{} vinyl_timeout value is prioritized >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +ok - bad sql_cache_size value >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +ok - bad strip_core value >> +TAP version 13 >> +1..2 >> +ok - box.cfg is not successful >> +ok - bad replication value >> +TAP version 13 >> +1..6 >> +ok - set 1 >> +ok - set 2 >> +ok - set 3 >> +ok - set 4 >> +ok - set 5 >> +ok - set 6 >> diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua >> new file mode 100755 >> index 000000000..3d19fdc90 >> --- /dev/null >> +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua >> @@ -0,0 +1,57 @@ >> +#!/usr/bin/env tarantool >> + >> +local os = require('os') >> +local fio = require('fio') >> +local tap = require('tap') >> + >> +local test = tap.test('gh-5602') >> + >> +-- gh-5602: Check that environment cfg variables working. >> +local TARANTOOL_PATH = arg[-1] >> +local script_name = 'gh-5602-test-cases-script.lua' >> +local path_to_script = fio.pathjoin( >> + os.getenv('PWD'), >> + 'box-tap', >> + script_name) >> + >> +--- >> +-- Generate shell command where set is sequence of environment >> +-- variables, then path to tarantool, the first arg is path to >> +-- test script and the second one is number of set. >> +-- >> +local function shell_command(set, i) >> + return ('%s %s %s %d'):format( >> + set, >> + TARANTOOL_PATH, >> + path_to_script, >> + i) >> +end >> + >> +local set_1 = ('%s %s %s %s'):format('TT_LISTEN=3301', 'TT_READAHEAD=10000', >> + 'TT_STRIP_CORE=false', 'TT_LOG_FORMAT=json') >> +local set_2 = ('%s %s %s %s'):format('TT_LISTEN=3301', >> + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', \'1.1.1.1:12345\'}\"', >> + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01', >> + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'') >> +local set_3 = 'TT_BACKGROUND=true TT_VINYL_TIMEOUT=60.1' >> +local set_4 = 'TT_SQL_CACHE_SIZE=a' >> +local set_5 = 'TT_STRIP_CORE=a' >> +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', \'1.1.1.1:12345\'}\"' >> + >> +local sets = {} >> +sets[1] = set_1 >> +sets[2] = set_2 >> +sets[3] = set_3 >> +sets[4] = set_4 >> +sets[5] = set_5 >> +sets[6] = set_6 >> + >> +test:plan(#sets) >> +for i, set in ipairs(sets) do >> + local tmpdir = fio.tempdir() >> + local new_path = fio.pathjoin(tmpdir, script_name) >> + fio.copyfile(path_to_script, new_path) >> + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) >> +end >> + >> +os.exit(test:check() and 0 or 1) >> diff --git a/test/box-tap/gh-5602-test-cases-script.lua b/test/box-tap/gh-5602-test-cases-script.lua >> new file mode 100755 >> index 000000000..a8b2fc987 >> --- /dev/null >> +++ b/test/box-tap/gh-5602-test-cases-script.lua >> @@ -0,0 +1,69 @@ >> +local tap = require('tap') >> + >> +local test = tap.test('gh-5602') >> + >> +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) >> + >> +-- Check that environment cfg values are set correctly. >> +if arg[1] == '1' then >> + test:plan(5) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['listen'], 3301, 'listen') >> + test:is(box.cfg['readahead'], 10000, 'readahead') >> + test:is(box.cfg['strip_core'], false, 'strip_core') >> + test:is(box.cfg['log_format'], 'json', 'log_format is not set') >> +end >> +if arg[1] == '2' then >> + test:plan(7) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['listen'], 3301, 'listen') >> + local replication = box.cfg['replication'] >> + test:is(type(replication), 'table', 'replication is table') >> + test:ok(replication[1] == '0.0.0.0:12345' or >> + replication[1] == '1.1.1.1:12345', 'replication URI 1') >> + test:ok(replication[2] == '0.0.0.0:12345' or >> + replication[2] == '1.1.1.1:12345', 'replication URI 2') >> + test:is(box.cfg['replication_connect_timeout'], 0.01, >> + 'replication_connect_timeout') >> + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', >> + 'replication_synchro_quorum') >> +end >> + >> +-- Check that box.cfg{} values are more prioritized than >> +-- environment cfg values. >> +if arg[1] == '3' then >> + test:plan(3) >> + test:ok(status ~= false, 'box.cfg is successful') >> + test:is(box.cfg['background'], false, >> + 'box.cfg{} background value is prioritized') >> + test:is(box.cfg['vinyl_timeout'], 70.1, >> + 'box.cfg{} vinyl_timeout value is prioritized') >> +end >> + >> +local err_msg_fmt = 'Environment variable TT_%s has '.. >> + 'incorrect value for option \'%s\': should be %s' >> + >> +-- Check bad environment cfg values. >> +if arg[1] == '4' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + local index = string.find(err, err_msg_fmt:format('SQL_CACHE_SIZE', >> + 'sql_cache_size', 'convertible to number')) >> + test:ok(index ~= nil, 'bad sql_cache_size value') >> +end >> +if arg[1] == '5' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + local index = string.find(err, err_msg_fmt:format('STRIP_CORE', >> + 'strip_core', '\'true\' or \'false\'')) >> + test:ok(index ~= nil, 'bad strip_core value') >> +end >> +if arg[1] == '6' then >> + test:plan(2) >> + test:ok(status == false, 'box.cfg is not successful') >> + local index = string.find(tostring(err), 'Incorrect value for option '.. >> + '\'replication\': expected host:service or /unix.socket') >> + test:ok(index ~= nil, 'bad replication value') >> +end >> + >> +os.exit(test:check() and 0 or 1) box: set cfg via environment variables Add ability to set box.cfg options via env variables. These variables should have name "TT_<OPTION>". When Tarantool instance is started under tarantoolctl utility env variables options have more priority than instance file's cfg. Closes #5602 @TarantoolBot document Title: Set box.cfg options via env variables Now, it is possible to set box.cfg options via environment variables. The name of variable should correspond the following pattern: "TT_<NAME>", where <NAME> is an option name. For example: "TT_LISTEN", "TT_READAHEAD". If it is complex value, for example, "replication" can be set by a table, the corresponding environment variable value has the syntax of lua array (without keys): `export TT_REPLICATION="{'uri1', 'uri2'}"` --- changelogs/unreleased/environment-cfg.md | 5 + src/box/lua/load_cfg.lua | 114 ++++++++++++++++++ test/app-tap/tarantoolctl.test.lua | 33 ++++- .../gh-5602-environment-vars-cfg.result | 41 +++++++ .../gh-5602-environment-vars-cfg.test.lua | 57 +++++++++ test/box-tap/gh-5602-test-cases-script.lua | 69 +++++++++++ 6 files changed, 318 insertions(+), 1 deletion(-) create mode 100755 changelogs/unreleased/environment-cfg.md create mode 100644 test/box-tap/gh-5602-environment-vars-cfg.result create mode 100755 test/box-tap/gh-5602-environment-vars-cfg.test.lua create mode 100755 test/box-tap/gh-5602-test-cases-script.lua diff --git a/changelogs/unreleased/environment-cfg.md b/changelogs/unreleased/environment-cfg.md new file mode 100755 index 000000000..894c40bf6 --- /dev/null +++ b/changelogs/unreleased/environment-cfg.md @@ -0,0 +1,5 @@ +## feature/core + +* Now, it is possibly to set box.cfg options with environment variables. +The priority of sources of configuration options is the following (from low +to high): default, tarantoolctl, environment, box.cfg{}. (gh-5602). diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index f90ba8a9a..df6a52e94 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -525,6 +525,26 @@ local function prepare_cfg(cfg, default_cfg, template_cfg, return new_cfg end +--- +-- Transfer options from env_cfg to cfg. +-- +local function apply_env_cfg(cfg, env_cfg) + if env_cfg == nil then + return + end + + for k, v in pairs(env_cfg) do + -- If Tarantool started with tarantoolctl then we have to + -- ignore configuration from tarantoolctl, because they + -- has priority lower than environment variables. + if cfg[k] == nil or os.getenv('TARANTOOLCTL') == 'true' then + cfg[k] = v + elseif type(v) == 'table' then + apply_env_cfg(cfg[k], v) + end + end +end + local function apply_default_cfg(cfg, default_cfg, module_cfg) for k,v in pairs(default_cfg) do if cfg[k] == nil then @@ -647,6 +667,98 @@ local function on_initial_config() end end +--- +-- Erase quotes at the begin and the end of string. +-- +local function dequote(str) + if string.sub(str, 1, 1) == '\'' and string.sub(str, -1) == '\'' or + string.sub(str, 1, 1) == '"' and string.sub(str, -1) == '"' then + return string.sub(str, 2, -2) + end + return str +end + +--- +-- Convert raw option value becoming from the corresponding +-- environment variable represented as string to string, number or +-- boolean (depending on option). +-- +local function process_option_value(option, raw_value) + local param_type = template_cfg[option] + + -- May be peculiar to "feedback*" parameters. + if param_type == nil then + return nil + end + + local value = nil + local err_msg_fmt = 'Environment variable TT_%s has ' .. + 'incorrect value for option \'%s\': should be %s' + + -- "log_level" option should be a number. + if param_type == 'number' then + value = tonumber(raw_value) + if value == nil then + error(err_msg_fmt:format(string.upper(option), option, + "convertible to number")) + end + -- "log_nonblock" option should be boolean. + elseif param_type == 'boolean' then + if raw_value == 'true' then + value = true + elseif raw_value == 'false' then + value = false + else + error(err_msg_fmt:format(string.upper(option), option, + "'true' or 'false'")) + end + elseif param_type == 'string, number' then + -- Firstly, try to convert to number. + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + elseif param_type == 'string, number, table' then + if string.sub(raw_value, 1, 1) == '{' and + string.sub(raw_value, -1) == '}' then + value = string.sub(raw_value, 2, -2) + value = require('csv').load(value)[1] + for i, item in pairs(value) do + item = dequote(item) + value[i] = tonumber(item) + if value[i] == nil then + value[i] = item + end + end + else + value = tonumber(raw_value) + if value == nil then + value = raw_value + end + end + else + value = raw_value + end + + return value +end + +--- +-- Pull and process configuration values from environment. +-- +local function collect_env_cfg() + local env_cfg = {} + for option, _ in pairs(template_cfg) do + local env_var_name = 'TT_' .. string.upper(option) + local raw_value = os.getenv(env_var_name) + if raw_value ~= nil then + env_cfg[option] = process_option_value(option, raw_value) + end + end + + return env_cfg +end + -- Whether box is loaded. -- -- `false` when box is not configured or when the initialization @@ -669,6 +781,8 @@ local function load_cfg(cfg) cfg = upgrade_cfg(cfg, translate_cfg) cfg = prepare_cfg(cfg, default_cfg, template_cfg, module_cfg, modify_cfg) + local env_cfg = collect_env_cfg() + apply_env_cfg(cfg, env_cfg) apply_default_cfg(cfg, default_cfg, module_cfg); -- Save new box.cfg box.cfg = cfg diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua index 3c3023d34..060316714 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -160,7 +160,7 @@ local function merge(...) end local test = tap.test('tarantoolctl') -test:plan(8) +test:plan(9) -- basic start/stop test -- must be stopped afterwards @@ -632,4 +632,35 @@ test:test('filter_xlog', function(test) end end) +-- gh-5602: Check that environment cfg variables are more +-- prioritized than tarantoolctl cfg. +do + local dir = fio.tempdir() + local code = [[ box.cfg ]] + create_script(dir, 'show_cfg.lua', code) + local code = [[ box.cfg{checkpoint_interval=3302, readahead=10001} ]] + create_script(dir, 'cfg_script.lua', code) + + local status, err = pcall(function() + test:test("check answers in case of call", function(test_i) + test_i:plan(4) + os.setenv('TT_CHECKPOINT_INTERVAL', '3301') + os.setenv('TT_READAHEAD', '10000') + check_ok(test_i, dir, 'start', 'cfg_script', 0) + tctl_wait_start(dir, 'cfg_script') + check_ok(test_i, dir, 'eval', 'cfg_script show_cfg.lua', 0, + '---\n- 3301\n- 10000\n...', nil) + check_ok(test_i, dir, 'stop', 'cfg_script', 0) + end) + end) + + cleanup_instance(dir, 'cfg_script') + recursive_rmdir(dir) + + if status == false then + print(("Error: %s"):format(err)) + os.exit() + end +end + os.exit(test:check() == true and 0 or -1) diff --git a/test/box-tap/gh-5602-environment-vars-cfg.result b/test/box-tap/gh-5602-environment-vars-cfg.result new file mode 100644 index 000000000..c90dd301b --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.result @@ -0,0 +1,41 @@ +TAP version 13 +1..5 +ok - box.cfg is successful +ok - listen +ok - readahead +ok - strip_core +ok - log_format is not set +TAP version 13 +1..7 +ok - box.cfg is successful +ok - listen +ok - replication is table +ok - replication URI 1 +ok - replication URI 2 +ok - replication_connect_timeout +ok - replication_synchro_quorum +TAP version 13 +1..3 +ok - box.cfg is successful +ok - box.cfg{} background value is prioritized +ok - box.cfg{} vinyl_timeout value is prioritized +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad sql_cache_size value +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad strip_core value +TAP version 13 +1..2 +ok - box.cfg is not successful +ok - bad replication value +TAP version 13 +1..6 +ok - set 1 +ok - set 2 +ok - set 3 +ok - set 4 +ok - set 5 +ok - set 6 diff --git a/test/box-tap/gh-5602-environment-vars-cfg.test.lua b/test/box-tap/gh-5602-environment-vars-cfg.test.lua new file mode 100755 index 000000000..3d19fdc90 --- /dev/null +++ b/test/box-tap/gh-5602-environment-vars-cfg.test.lua @@ -0,0 +1,57 @@ +#!/usr/bin/env tarantool + +local os = require('os') +local fio = require('fio') +local tap = require('tap') + +local test = tap.test('gh-5602') + +-- gh-5602: Check that environment cfg variables working. +local TARANTOOL_PATH = arg[-1] +local script_name = 'gh-5602-test-cases-script.lua' +local path_to_script = fio.pathjoin( + os.getenv('PWD'), + 'box-tap', + script_name) + +--- +-- Generate shell command where set is sequence of environment +-- variables, then path to tarantool, the first arg is path to +-- test script and the second one is number of set. +-- +local function shell_command(set, i) + return ('%s %s %s %d'):format( + set, + TARANTOOL_PATH, + path_to_script, + i) +end + +local set_1 = ('%s %s %s %s'):format('TT_LISTEN=3301', 'TT_READAHEAD=10000', + 'TT_STRIP_CORE=false', 'TT_LOG_FORMAT=json') +local set_2 = ('%s %s %s %s'):format('TT_LISTEN=3301', + 'TT_REPLICATION=\"{\'0.0.0.0:12345\', \'1.1.1.1:12345\'}\"', + 'TT_REPLICATION_CONNECT_TIMEOUT=0.01', + 'TT_REPLICATION_SYNCHRO_QUORUM=\'4 + 1\'') +local set_3 = 'TT_BACKGROUND=true TT_VINYL_TIMEOUT=60.1' +local set_4 = 'TT_SQL_CACHE_SIZE=a' +local set_5 = 'TT_STRIP_CORE=a' +local set_6 = 'TT_REPLICATION=\"{0.0.0.0:12345\', \'1.1.1.1:12345\'}\"' + +local sets = {} +sets[1] = set_1 +sets[2] = set_2 +sets[3] = set_3 +sets[4] = set_4 +sets[5] = set_5 +sets[6] = set_6 + +test:plan(#sets) +for i, set in ipairs(sets) do + local tmpdir = fio.tempdir() + local new_path = fio.pathjoin(tmpdir, script_name) + fio.copyfile(path_to_script, new_path) + test:is(os.execute(shell_command(set, i)), 0, 'set '..i) +end + +os.exit(test:check() and 0 or 1) diff --git a/test/box-tap/gh-5602-test-cases-script.lua b/test/box-tap/gh-5602-test-cases-script.lua new file mode 100755 index 000000000..a8b2fc987 --- /dev/null +++ b/test/box-tap/gh-5602-test-cases-script.lua @@ -0,0 +1,69 @@ +local tap = require('tap') + +local test = tap.test('gh-5602') + +local status, err = pcall(box.cfg, {background = false, vinyl_timeout = 70.1}) + +-- Check that environment cfg values are set correctly. +if arg[1] == '1' then + test:plan(5) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], 3301, 'listen') + test:is(box.cfg['readahead'], 10000, 'readahead') + test:is(box.cfg['strip_core'], false, 'strip_core') + test:is(box.cfg['log_format'], 'json', 'log_format is not set') +end +if arg[1] == '2' then + test:plan(7) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['listen'], 3301, 'listen') + local replication = box.cfg['replication'] + test:is(type(replication), 'table', 'replication is table') + test:ok(replication[1] == '0.0.0.0:12345' or + replication[1] == '1.1.1.1:12345', 'replication URI 1') + test:ok(replication[2] == '0.0.0.0:12345' or + replication[2] == '1.1.1.1:12345', 'replication URI 2') + test:is(box.cfg['replication_connect_timeout'], 0.01, + 'replication_connect_timeout') + test:is(box.cfg['replication_synchro_quorum'], '4 + 1', + 'replication_synchro_quorum') +end + +-- Check that box.cfg{} values are more prioritized than +-- environment cfg values. +if arg[1] == '3' then + test:plan(3) + test:ok(status ~= false, 'box.cfg is successful') + test:is(box.cfg['background'], false, + 'box.cfg{} background value is prioritized') + test:is(box.cfg['vinyl_timeout'], 70.1, + 'box.cfg{} vinyl_timeout value is prioritized') +end + +local err_msg_fmt = 'Environment variable TT_%s has '.. + 'incorrect value for option \'%s\': should be %s' + +-- Check bad environment cfg values. +if arg[1] == '4' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(err, err_msg_fmt:format('SQL_CACHE_SIZE', + 'sql_cache_size', 'convertible to number')) + test:ok(index ~= nil, 'bad sql_cache_size value') +end +if arg[1] == '5' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(err, err_msg_fmt:format('STRIP_CORE', + 'strip_core', '\'true\' or \'false\'')) + test:ok(index ~= nil, 'bad strip_core value') +end +if arg[1] == '6' then + test:plan(2) + test:ok(status == false, 'box.cfg is not successful') + local index = string.find(tostring(err), 'Incorrect value for option '.. + '\'replication\': expected host:service or /unix.socket') + test:ok(index ~= nil, 'bad replication value') +end + +os.exit(test:check() and 0 or 1) -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-04-09 8:56 ` Roman Khabibov via Tarantool-patches 2021-04-12 8:37 ` Leonid Vasiliev via Tarantool-patches @ 2021-04-12 9:52 ` Leonid Vasiliev via Tarantool-patches 1 sibling, 0 replies; 7+ messages in thread From: Leonid Vasiliev via Tarantool-patches @ 2021-04-12 9:52 UTC (permalink / raw) To: Roman Khabibov; +Cc: tarantool-patches The "luachek" test failed. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] box: set cfg via environment variables 2021-03-14 15:34 [Tarantool-patches] [PATCH] box: set cfg via environment variables Roman Khabibov via Tarantool-patches 2021-03-19 12:38 ` Leonid Vasiliev via Tarantool-patches @ 2021-03-19 13:23 ` Leonid Vasiliev via Tarantool-patches 1 sibling, 0 replies; 7+ messages in thread From: Leonid Vasiliev via Tarantool-patches @ 2021-03-19 13:23 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches On 3/14/21 6:34 PM, Roman Khabibov wrote: > Add ability to set box.cfg options via env variables. These > variables should have name "TT_<OPTION>". When Tarantool instance > is started under tarantoolctl utility env variables options have > more priority than instance file's cfg. > > Closes #5602 > > @TarantoolBot document > Title: Set box.cfg options via env variables > > Now, it is possible to set box.cfg options via environment > variables. The name of variable should correspond the followng > pattern: "TT_<NAME>", where <NAME> is an option name. For example: > "TT_LISTEN", "TT_READAHEAD". > If it is complex value, for example, "replication" can be set by a > table, the corresponding environment varibale value has the syntax > of lua array (without keys): > > `export TT_REPLICATION="{'uri1', 'uri2'}"` > > Note that every value in arary must be quoted. > --- > > @ChangeLog > * Add ability to set box.cfg options via environment variables (gh-5602). @Changelog - deprecated. See tarantool/doc/changelogs.md ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-13 12:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-14 15:34 [Tarantool-patches] [PATCH] box: set cfg via environment variables Roman Khabibov via Tarantool-patches 2021-03-19 12:38 ` Leonid Vasiliev via Tarantool-patches 2021-04-09 8:56 ` Roman Khabibov via Tarantool-patches 2021-04-12 8:37 ` Leonid Vasiliev via Tarantool-patches 2021-04-13 12:47 ` Roman Khabibov via Tarantool-patches 2021-04-12 9:52 ` Leonid Vasiliev via Tarantool-patches 2021-03-19 13:23 ` Leonid Vasiliev via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox