fix: removed restrictions on role inclusion order

(cherry picked from commit dcbef05550f478761bdf6eb1227fdece8e814a4e)

Author: a1div0

CHANGELOG.md

@@ -13,7 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Fixed
 
-- Fix `is_master_only` option. Now it runs on master correctly.
+- Fix `is_master_only` option. Now it runs on master correctly. 
+- Fixed restrictions on the order of role enable.
 
 ## 1.6.0 - 2024-03-25
 

README.md

@@ -131,6 +131,7 @@ expirationd.start(job_name, space.id, is_expired, {
 ## Testing
 
 ```
+$ tt install tarantool-ee 1.10.15-0-r648
 $ make deps-full
 $ make test
 ```

cartridge/roles/expirationd.lua

@@ -2,49 +2,35 @@ local expirationd = require("expirationd")
 local role_name = "expirationd"
 local started = require("cartridge.vars").new(role_name)
 
-local function dumb_fn() end
+local issue_map = {}
 
-local function load_function(func_name)
-    if func_name == nil or type(func_name) ~= 'string' then
-        return nil
-    end
-
-    local func = rawget(_G, func_name)
-    if func == nil then
-        if type(box.cfg) == 'function' then
-            -- After restart `validate_config` sometimes is run before box.cfg
-            -- call and it leads to error like `Please call box.cfg first` and
-            -- at this moment we are not able to check real availability of
-            -- functions in box.func. If we return nil, we fail configration
-            -- but it is not an error actually, because we cannot do any
-            -- checks.  Thus we decided to return dumb_fn to avoid
-            -- misconfiguration at this stage and in hope that `apply_config`
-            -- will do real check.
-            -- P.S. Of course it is a bad solution, but...
-            return dumb_fn
-        end
-        if not box.schema.func.exists(func_name) then
-            return nil
-        end
+function _G.expirationd_enable_issue(task_name, message, ...)
+    issue_map[task_name] = {
+        level = 'warning',
+        topic = 'expirationd',
+        message = ('EXPIRATIOND, task name "%s": ' .. message):format(task_name, ...),
+    }
+end
 
-        return function(...)
-            return box.func[func_name]:call({...})
-        end
-    end
+function _G.expirationd_disable_issue(task_name)
+    issue_map[task_name] = nil
+end
 
-    if type(func) ~= 'function' then
-        return nil
+local function get_issues()
+    local res = {}
+    for _, issue in pairs(issue_map) do
+        table.insert(res, issue)
     end
 
-    return func
+    return res
 end
 
 local function get_param(param_name, value, types)
     local types_map = {
         b = {type = "boolean", err = "a boolean"},
         n = {type = "number", err = "a number"},
         s = {type = "string", err = "a string"},
-        f = {type = "string", transform = load_function, err = "a function name in _G or in box.func"},
+        f = {type = "string", err = "a string"},
         t = {type = "table", err = "a table"},
         any = {err = "any type"},
     }
@@ -273,4 +259,5 @@ return setmetatable({
     validate_config = validate_config,
     apply_config = apply_config,
     stop = stop,
+    get_issues = get_issues,
 }, { __index = expirationd })

expirationd/init.lua

@@ -22,6 +22,27 @@ local function get_fiber_id(fiber)
     return fid
 end
 
+-- This method adds an issue to the cluster instance that is displayed in the admin panel
+-- and metrics. The method calls a global function that must be declared in the role module.
+-- If the function is not declared, then instead of an issue, a warning is written to the log.
+local function add_issue(task_name, message, ...)
+    if rawget(_G, 'expirationd_enable_issue') ~= nil then
+        _G.expirationd_enable_issue(task_name, message, ...)
+    else
+        local message_tpl = 'Expirationd warning: task "%s", ' .. message
+        log.warn(message_tpl:format(task_name, ...))
+    end
+end
+
+-- This method removes a previously added issue from the cluster instance, which is displayed in
+-- the admin panel and metrics. The method calls a global function that must be declared in the
+-- role module. If the function is not declared, then nothing is called.
+local function remove_issue(task_name)
+    if rawget(_G, 'expirationd_disable_issue') ~= nil then
+        _G.expirationd_disable_issue(task_name)
+    end
+end
+
 local stash_names = {
     cfg = '__expirationd_cfg',
     metrics_stats = '__expirationd_metrics_stats',
@@ -186,6 +207,67 @@ local function check_space_and_index_exist(task)
     return true
 end
 
+local function load_function(func_name)
+    if func_name == nil or type(func_name) ~= 'string' then
+        return nil
+    end
+
+    local func = rawget(_G, func_name)
+    if func ~= nil then
+        if type(func) ~= 'function' then
+            return nil
+        end
+
+        return func
+    elseif box.schema.func.exists(func_name) then
+        return function(...)
+            return box.func[func_name]:call({...})
+        end
+    else
+        return nil
+    end
+end
+
+local function load_functions(task)
+    local func_list = {
+        'is_tuple_expired',
+        'iterate_with',
+        'on_full_scan_complete',
+        'on_full_scan_error',
+        'on_full_scan_start',
+        'on_full_scan_success',
+        'process_expired_tuple',
+        'process_while',
+    }
+
+    local result = {}
+    for _, func_name in ipairs(func_list) do
+        if task[func_name] and type(task[func_name]) == 'string' then
+            result[func_name] = load_function(task[func_name])
+            if result[func_name] == nil then
+                add_issue(
+                    task.name,
+                    'Function "%s" (for option "%s") -- not loaded',
+                    task[func_name],
+                    func_name
+                )
+                return false
+            end
+        end
+    end
+
+    for func_name, func_ptr in pairs(result) do
+        task[func_name] = func_ptr
+    end
+
+    if task.is_tuple_expired == nil then
+        add_issue(task.name, 'Function "is_tuple_expired" must be declare')
+        return false
+    end
+
+    return true
+end
+
 local function check_space_and_index(task)
     local ok, err = check_space_and_index_exist(task)
     if not ok then
@@ -374,11 +456,14 @@ local function worker_loop(task)
             fiber.self():cancel()
         end
         -- Full scan iteration is complete, yield
+        remove_issue(task.name)
         fiber.sleep(task.full_scan_delay)
     end
 end
 
 local function guardian_loop(task)
+    add_issue(task.name, 'Task is not running')
+
     -- detach the guardian from the creator and attach it to sched
     fiber.name(string.format("guardian of %q", task.name), { truncate = true })
 
@@ -389,6 +474,13 @@ local function guardian_loop(task)
         fiber.sleep(constants.check_interval)
     end
 
+    while true do
+        if load_functions(task) then
+            break
+        end
+        fiber.sleep(constants.check_interval)
+    end
+
     while true do
         -- if fiber doesn't exist
         if get_fiber_id(task.worker_fiber) == 0 then
@@ -447,6 +539,8 @@ local Task_methods = {
     --
     -- @function task.stop
     stop = function (self)
+        remove_issue(self.name)
+
         if (get_fiber_id(self.guardian_fiber) ~= 0) then
             self.guardian_fiber:cancel()
             while self.guardian_fiber:status() ~= "dead" do
@@ -991,23 +1085,23 @@ end
 --
 -- @function expirationd.start
 local function expirationd_run_task(name, space, is_tuple_expired, options)
-    checks('string', 'number|string', 'function', {
+    checks('string', 'number|string', 'string|function', {
         args = '?',
         atomic_iteration = '?boolean',
         force = '?boolean',
         force_allow_functional_index = '?boolean',
         full_scan_delay = '?number|cdata',
         full_scan_time = '?number|cdata',
         index = '?number|string',
-        iterate_with = '?function',
+        iterate_with = '?string|function',
         iteration_delay = '?number|cdata',
         iterator_type = '?number|string',
-        on_full_scan_complete = '?function',
-        on_full_scan_error = '?function',
-        on_full_scan_start = '?function',
-        on_full_scan_success = '?function',
-        process_expired_tuple = '?function',
-        process_while = '?function',
+        on_full_scan_complete = '?string|function',
+        on_full_scan_error = '?string|function',
+        on_full_scan_start = '?string|function',
+        on_full_scan_success = '?string|function',
+        process_expired_tuple = '?string|function',
+        process_while = '?string|function',
         start_key = '?',
         tuples_per_iteration = '?number|cdata',
         vinyl_assumed_space_len_factor = '?number|cdata',

roles/expirationd.lua

@@ -1,37 +1,32 @@
 local expirationd = require("expirationd")
+local config = require('config')
 local fiber = require("fiber")
 local log = require("log")
 
 local role_name = "roles.expirationd"
 local started = {}
 
+function _G.expirationd_enable_issue(task_name, message, ...)
+    config._aboard:set(
+        {
+            type = 'warn',
+            message = ('EXPIRATIOND, task name "%s": ' .. message):format(task_name, ...),
+        },
+        {
+            key = task_name,
+        }
+    )
+end
 
-local function load_function(func_name)
-    if func_name == nil or type(func_name) ~= 'string' then
-        return nil
-    end
-
-    local func = rawget(_G, func_name)
-    if func ~= nil then
-        if type(func) ~= 'function' then
-            return nil
-        end
-
-        return func
-    elseif box.schema.func.exists(func_name) then
-        return function(...)
-            return box.func[func_name]:call({...})
-        end
-    else
-        return nil
-    end
+function _G.expirationd_disable_issue(task_name)
+    config._aboard:drop(task_name)
 end
 
 local types_map = {
     b = {type = "boolean", err = "a boolean"},
     n = {type = "number", err = "a number"},
     s = {type = "string", err = "a string"},
-    f = {type = "string", transform = load_function, err = "a function name in _G or in box.func"},
+    f = {type = "string", err = "a string"},
     t = {type = "table", err = "a table"},
     any = {err = "any type"},
 }

test/entrypoint/empty.lua

@@ -0,0 +1,27 @@
+#!/usr/bin/env tarantool
+
+require('strict').on()
+_G.is_initialized = function() return false end
+
+local log = require('log')
+local errors = require('errors')
+local cartridge = require('cartridge')
+
+local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
+    advertise_uri = 'localhost:3301',
+    http_port = 8081,
+    bucket_count = 3000,
+    roles = {
+        'cartridge.roles.vshard-router',
+        'cartridge.roles.vshard-storage',
+        'cartridge.roles.expirationd'
+    },
+    roles_reload_allowed = true,
+})
+
+if not ok then
+    log.error('%s', err)
+    os.exit(1)
+end
+
+_G.is_initialized = cartridge.is_healthy

test/helper.lua

@@ -322,6 +322,21 @@ function helpers.create_persistent_function(name, body)
     })
 end
 
+function helpers.create_persistent_function_on_server(server, name, body)
+    if _TARANTOOL >= "2" then
+        server:exec(function(name_on_server, body_on_server)
+            box.schema.func.create(name_on_server, {
+                language = 'LUA',
+                if_not_exists = true,
+                body = body_on_server
+            })
+        end, {name, body})
+    else
+        local expr = ([[rawset(_G, '%s', %s)]]):format(name, body)
+        server:eval(expr)
+    end
+end
+
 local root = fio.dirname(fio.dirname(fio.abspath(package.search('test.helper'))))
 
 helpers.lua_path = root .. '/?.lua;' ..