Skip to content

Commit

Permalink
config: extract alert board into its own module
Browse files Browse the repository at this point in the history
It encapsulates all the alerts manipulation in one place, splits it from
the main config logic and simplifies reading of the relevant code.

Several tests are updated to use the public API instead of the internal
one.

Part of tarantool#9574
Part of tarantool#9586

NO_DOC=refactoring, no user-visible changes
NO_CHANGELOG=see NO_DOC
NO_TEST=see NO_DOC
  • Loading branch information
Totktonada committed Jan 25, 2024
1 parent f58bfc9 commit 6594d6f
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 65 deletions.
1 change: 1 addition & 0 deletions src/box/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ lua_source(lua_sources lua/config/init.lua config_init_lua)
lua_source(lua_sources lua/config/instance_config.lua config_instance_config_lua)
lua_source(lua_sources lua/config/source/env.lua config_source_env_lua)
lua_source(lua_sources lua/config/source/file.lua config_source_file_lua)
lua_source(lua_sources lua/config/utils/aboard.lua config_utils_aboard_lua)
lua_source(lua_sources lua/config/utils/expression.lua config_utils_expression_lua)
lua_source(lua_sources lua/config/utils/file.lua config_utils_file_lua)
lua_source(lua_sources lua/config/utils/log.lua config_utils_log_lua)
Expand Down
18 changes: 9 additions & 9 deletions src/box/lua/config/applier/box_cfg.lua
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ local function names_alert_missing(config, missing_names)
'snapshot. It will be automatically set when possible.'
local replicaset_name = config._configdata._replicaset_name
if missing_names[replicaset_name] ~= nil and
config._alerts[replicaset_name] == nil then
config._aboard:get(replicaset_name) == nil then
local replicaset_uuid = missing_names[replicaset_name]
local warning = msg:format(replicaset_name, replicaset_uuid)
config:_alert(replicaset_name, {type = 'warn', message = warning})
config._aboard:set(replicaset_name, {type = 'warn', message = warning})
end

local unknown_msg = 'box_cfg.apply: instance %s is unknown. Possibly ' ..
Expand All @@ -120,9 +120,9 @@ local function names_alert_missing(config, missing_names)
warning = msg:format(name, uuid)
end
-- Alert may be done with 'unknown' uuid. If it was so, update alert.
local alert = config._alerts[name]
local alert = config._aboard:get(name)
if alert == nil or alert.message ~= warning then
config:_alert(name, {type = 'warn', message = warning})
config._aboard:set(name, {type = 'warn', message = warning})
end
end
end
Expand All @@ -141,7 +141,7 @@ local function names_box_cfg(cfg)
end

local function names_on_name_set(name)
names_state.config:_alert_drop(name)
names_state.config._aboard:drop(name)

local config_rs_name = names_state.config._configdata._replicaset_name
local config_inst_name = names_state.config._configdata._instance_name
Expand Down Expand Up @@ -245,13 +245,13 @@ end

local function no_missing_names_alerts(config)
local configdata = config._configdata
if config._alerts[configdata._replicaset_name] ~= nil then
if config._aboard:get(configdata._replicaset_name) ~= nil then
return false
end

local peers = configdata:peers()
for _, name in ipairs(peers) do
if config._alerts[name] ~= nil then
if config._aboard:get(name) ~= nil then
return false
end
end
Expand Down Expand Up @@ -549,8 +549,8 @@ local function apply(config)
if v ~= box.cfg[k] then
local warning = 'box_cfg.apply: non-dynamic option '..k..
' will not be set until the instance is restarted'
config:_alert('box_cfg_apply_non_dynamic',
{type = 'warn', message = warning})
config._aboard:set('box_cfg_apply_non_dynamic',
{type = 'warn', message = warning})
box_cfg[k] = nil
end
end
Expand Down
12 changes: 6 additions & 6 deletions src/box/lua/config/applier/credentials.lua
Original file line number Diff line number Diff line change
Expand Up @@ -544,15 +544,15 @@ local privileges_action_f = function(grant_or_revoke, role_or_user, name, privs,
err = ('credentials.apply: box.schema.%s.%s(%q, %q, %q, %q) failed: %s')
:format(role_or_user, grant_or_revoke, name, privs, obj_type,
obj_name, err)
config:_alert('credentials_grant_or_revoke_err',
{type = 'error', message = err})
config._aboard:set('credentials_grant_or_revoke_err',
{type = 'error', message = err})
else
local msg = "credentials.apply: %s %q hasn't been created yet, " ..
"'box.schema.%s.%s(%q, %q, %q, %q)' will be applied later"
msg = msg:format(obj_type, obj_name, role_or_user, grant_or_revoke,
name, privs, obj_type, obj_name)
config:_alert('credentials_grant_or_revoke_warn',
{type = 'warn', message = msg})
config._aboard:set('credentials_grant_or_revoke_warn',
{type = 'warn', message = msg})
end
end

Expand Down Expand Up @@ -700,8 +700,8 @@ local function set_password(user_name, password)
if user_name == 'guest' then
local message = 'credentials.apply: setting a password for ' ..
'the guest user is not allowed'
config:_alert('credentials_apply_password',
{type = 'error', message = message})
config._aboard:set('credentials_apply_password',
{type = 'error', message = message})
end

local auth_def = box.space._user.index.name:get({user_name})[5]
Expand Down
67 changes: 22 additions & 45 deletions src/box/lua/config/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ local fio = require('fio')
local instance_config = require('internal.config.instance_config')
local cluster_config = require('internal.config.cluster_config')
local configdata = require('internal.config.configdata')
local log = require('internal.config.utils.log')
local aboard = require('internal.config.utils.aboard')
local tarantool = require('tarantool')
local datetime = require('datetime')

-- Tarantool Enterprise Edition has its own additions
-- for config module.
Expand Down Expand Up @@ -96,24 +95,6 @@ local function broadcast(self)
box.broadcast('config.info', self:info())
end

function methods._alert(self, key, alert)
assert(alert.type == 'error' or alert.type == 'warn')
if alert.type == 'error' then
log.error(alert.message)
else
log.warn(alert.message)
end
alert.timestamp = datetime.now()
self._alerts[key] = alert
end

function methods._alert_drop(self, key)
self._alerts[key] = nil
if self._status == 'check_warnings' and table.equals(self._alerts, {}) then
self._status = 'ready'
end
end

function methods._meta(self, source_name, key, value)
local data = self._metadata[source_name] or {}
data[key] = value
Expand Down Expand Up @@ -382,16 +363,7 @@ end

-- Set proper status depending on received alerts.
function methods._set_status_based_on_alerts(self)
local status = 'ready'
for _, alert in pairs(self._alerts) do
assert(alert.type == 'error' or alert.type == 'warn')
if alert.type == 'error' then
status = 'check_errors'
break
end
status = 'check_warnings'
end
self._status = status
self._status = self._aboard:status()
broadcast(self)
end

Expand Down Expand Up @@ -472,7 +444,7 @@ function methods._reload_noexc(self, opts)
self._status = 'reload_in_progress'
broadcast(self)

self._alerts = {}
self._aboard:clean()
self._metadata = {}

local ok, err = pcall(function(opts)
Expand All @@ -484,7 +456,7 @@ function methods._reload_noexc(self, opts)

assert(not ok or err == nil)
if not ok then
self:_alert('reload_error', {type = 'error', message = err})
self._aboard:set('reload_error', {type = 'error', message = err})
end

self:_set_status_based_on_alerts()
Expand All @@ -502,17 +474,8 @@ end

function methods.info(self)
selfcheck(self, 'info')
-- Don't return alert keys from config:info().
local alerts = {}
for _, alert in pairs(self._alerts) do
table.insert(alerts, alert)
end
table.sort(alerts, function(a, b)
return a.timestamp < b.timestamp
end)

return {
alerts = alerts,
alerts = self._aboard:alerts(),
meta = self._metadata,
status = self._status,
}
Expand All @@ -521,7 +484,7 @@ end
-- The object is a singleton. The constructor should be called
-- only once.
local function new()
return setmetatable({
local self = setmetatable({
_sources = {},
_appliers = {},
-- There are values the module need to hold, which are not
Expand All @@ -532,13 +495,27 @@ local function new()
_configdata = nil,
-- Track applied config values as well.
_configdata_applied = nil,
-- Track situations when something is going wrong.
_alerts = {},
-- Metadata from sources.
_metadata = {},
-- Current status.
_status = 'uninitialized',
}, mt)

-- Track situations when something is going wrong.
self._aboard = aboard.new({
-- Transit from 'check_warnings' to 'ready' if the last
-- warning is dropped.
on_drop = function(_, _)
if not self._aboard:is_empty() then
return
end
if self._status == 'check_warnings' then
self._status = 'ready'
end
end,
})

return self
end

return new()
149 changes: 149 additions & 0 deletions src/box/lua/config/utils/aboard.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
-- Alert board.

local datetime = require('datetime')
local log = require('internal.config.utils.log')

-- Set an alert.
--
-- The `key` is an unique identifier of the alert. Attempt to set
-- a new alert with the same key replaces the existing alert.
--
-- The key can be used to drop the alert using :drop().
--
-- The `alert` is a table of the following structure.
--
-- {
-- type = 'warn' or 'error',
-- message = <string>,
-- }
--
-- More fields may be placed into the table and all these fields
-- are returned from :alerts().
--
-- The alert board takes an ownership of the `alert` table and
-- modifies it.
--
-- A timestamp is saved on the :set() method call to show it in
-- an :alerts() result.
local function aboard_set(self, key, alert)
assert(alert.type == 'error' or alert.type == 'warn')
if alert.type == 'error' then
log.error(alert.message)
else
log.warn(alert.message)
end
alert.timestamp = datetime.now()
self._alerts[key] = alert
end

-- Return an alert pointed by the given `key` or nil.
local function aboard_get(self, key)
return self._alerts[key]
end

-- Drop an alert pointed by the given `key`.
--
-- The function is no-op if the alert doesn't exist.
--
-- The `on_drop` callback (see the aboard.new() function) is
-- called if the alert had exist.
local function aboard_drop(self, key)
if self._alerts[key] == nil then
return
end
self._alerts[key] = nil
if self._on_drop ~= nil then
self._on_drop(self, key)
end
end

-- Drop all the alerts.
--
-- The `on_drop` callback is NOT called.
local function aboard_clean(self)
self._alerts = {}
end

-- Serialize the alerts to show them to a user.
--
-- The alerts are sorted by a time of the :set() method call and
-- returned as an array-like table.
--
-- The keys passed to the :set() method are NOT shown in the
-- result of this method: they're considered as internal
-- identifiers.
local function aboard_alerts(self)
-- Don't return alert keys.
local alerts = {}
for _, alert in pairs(self._alerts) do
table.insert(alerts, alert)
end
-- Sort by timestamps.
table.sort(alerts, function(a, b)
return a.timestamp < b.timestamp
end)
return alerts
end

-- Return one of three possible statuses:
--
-- * ready
-- * check_warnings
-- * check_errors
local function aboard_status(self)
local status = 'ready'

for _, alert in pairs(self._alerts) do
assert(alert.type == 'error' or alert.type == 'warn')
if alert.type == 'error' then
return 'check_errors'
end

status = 'check_warnings'
end

return status
end

-- Return `true` if there are no alerts, `false` otherwise.
local function aboard_is_empty(self)
return next(self._alerts) == nil
end

local mt = {
__index = {
set = aboard_set,
get = aboard_get,
drop = aboard_drop,
clean = aboard_clean,
alerts = aboard_alerts,
status = aboard_status,
is_empty = aboard_is_empty,
},
}

-- Create a new alert board.
--
-- The `on_drop` callback is called on the :drop() method call.
-- It is NOT called on the :clean() call.
--
-- The callback should have the following prototype.
--
-- | local function on_drop_cb(aboard, key)
-- | <...>
-- | end
local function new(opts)
local on_drop
if opts ~= nil then
on_drop = opts.on_drop
end

return setmetatable({
_alerts = {},
_on_drop = on_drop,
}, mt)
end

return {
new = new,
}
5 changes: 5 additions & 0 deletions src/box/lua/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ extern char session_lua[],
config_instance_config_lua[],
config_source_env_lua[],
config_source_file_lua[],
config_utils_aboard_lua[],
config_utils_expression_lua[],
config_utils_file_lua[],
config_utils_log_lua[],
Expand Down Expand Up @@ -346,6 +347,10 @@ static const char *lua_sources[] = {
"internal.config.utils.log",
config_utils_log_lua,

"config/utils/aboard",
"internal.config.utils.aboard",
config_utils_aboard_lua,

"config/utils/schema",
"internal.config.utils.schema",
config_utils_schema_lua,
Expand Down

0 comments on commit 6594d6f

Please sign in to comment.