[Tarantool-patches] [PATCH] box: set cfg via environment variables
Roman Khabibov
roman.habibov at tarantool.org
Tue Apr 13 15:47:26 MSK 2021
Hi! Thanks for the review.
> On Apr 12, 2021, at 11:37, Leonid Vasiliev <lvasiliev at tarantool.org> wrote:
>
> Hi! Thank you for the patch.
> See 9 comments below:
>
>> commit ee6525f843a347c1df8a92a015414f4cbcceafb9
>> Author: Roman Khabibov <roman.habibov at 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)
More information about the Tarantool-patches
mailing list