config: don't choose learners as bootstrap leaders

This patch makes Tarantool configured in `replication.failover =
'supervised'` mode not choose instances marked as learners for the
failover coordinator as a bootstrap leader.

Marking instance as a learner means it shouldn't be appointed as a
master by the failover. Thus, the user would expect for the learners not
to become RW during the bootstrap too.

Example.
```
replication:
  failover: supervised

failover:
  replicasets:
    r-001:
      learners:
        - i-001

groups:
  g-001:
    replicasets:
      r-001:
        instances:
          i-001: {}
          i-002: {}
```
`i-002` will be chosen as a replicaset bootstrap leader.

If all non-anonymous instances are marked as learners the initial
cluster bootstrap fails since every instance starts in RO and none
of them might become the leader.

If user applies a config where all non-anonymous instances marked as
learners an alert is issued since this config is likely to be wrong.

```
log.lua:74 W> box_cfg.apply: cannot determine a bootstrap leader based
on the configuration. Make sure learners are properly configured
```

Closes tarantool#10842

NO_DOC=see tarantool/doc#4993

Author: georgiy-belyanin

changelogs/unreleased/gh-10842-learners-not-bootstrap-leaders.md

@@ -0,0 +1,4 @@
+## feature/config
+
+* Now Tarantool instances configured in the supervised failover mode don't
+  choose learner instances as a bootstrap leader (gh-10842).

src/box/lua/config/applier/box_cfg.lua

@@ -158,7 +158,8 @@ end
 
 -- {{{ Set RO/RW
 
-local function set_ro_rw(configdata, box_cfg)
+local function set_ro_rw(config, box_cfg)
+    local configdata = config._configdata
     -- The startup process may need a special handling and differs
     -- from the configuration reloading process.
     local is_startup = type(box.cfg) == 'function'
@@ -258,6 +259,13 @@ local function set_ro_rw(configdata, box_cfg)
             assert(false)
         end
     elseif failover == 'supervised' then
+        if configdata:bootstrap_leader_name() == nil then
+            local warning = 'box_cfg.apply: cannot determine a bootstrap ' ..
+                'leader based on the configuration. Make sure learners are ' ..
+                'properly configured'
+            config._aboard:set({type = 'warn', message = warning})
+        end
+
         -- The startup flow in the 'supervised' failover mode is
         -- the following.
         --
@@ -1058,7 +1066,7 @@ local function apply(config)
     set_replication_peers(configdata, box_cfg)
     set_log(configdata, box_cfg)
     set_audit_log(configdata, box_cfg)
-    set_ro_rw(configdata, box_cfg)
+    set_ro_rw(config, box_cfg)
     revert_non_dynamic_options(config, box_cfg)
     set_names_in_background(config, box_cfg)
     set_bootstrap_leader(configdata, box_cfg)

src/box/lua/config/configdata.lua

@@ -10,6 +10,19 @@ local instance_config = require('internal.config.instance_config')
 local cluster_config = require('internal.config.cluster_config')
 local snapshot = require('internal.config.utils.snapshot')
 
+-- {{{ General-purpose utils
+
+-- {'a', 'b', 'c'} => {a = true, b = true, c = true}
+local function array2set(t)
+    local res = {}
+    for _, v in ipairs(t) do
+        res[v] = true
+    end
+    return res
+end
+
+-- }}} General-purpose utils
+
 local function choose_iconfig(self, opts)
     if opts ~= nil and opts.instance ~= nil then
         local instances = self._instances
@@ -1044,28 +1057,28 @@ local function new(iconfig, cconfig, instance_name)
         end
         assert(bootstrap_leader == nil)
 
-        -- Choose the first non-anonymous instance with the highest
-        -- priority specified in the failover configuration
-        -- section.
+        local failover_replicaset = instance_config:get(iconfig_def,
+            {'failover', 'replicasets', found.replicaset_name}) or {}
+
+        local failover_learners = array2set(failover_replicaset.learners or {})
+        local failover_priorities = failover_replicaset.priority or {}
+
+        -- Choose the first non-anonymous non-learner instance
+        -- with the highest priority specified in the failover
+        -- configuration section.
         local max_priority = -math.huge
         for _, peer_name in ipairs(peer_names) do
             assert(peers[peer_name] ~= nil)
             local iconfig_def = peers[peer_name].iconfig_def
             local is_anon = instance_config:get(iconfig_def, 'replication.anon')
+            local is_learner = failover_learners[peer_name]
+            local priority = failover_priorities[peer_name] or 0
 
-            local priority = instance_config:get(iconfig_def, {
-                'failover',
-                'replicasets',
-                found.replicaset_name,
-                'priority',
-                peer_name}) or 0
-
-            if not is_anon and priority > max_priority then
+            if not is_anon and not is_learner and priority > max_priority then
                 bootstrap_leader_name = peer_name
                 max_priority = priority
             end
         end
-        assert(bootstrap_leader_name ~= nil)
     end
 
     -- Names and UUIDs are always validated: during instance start

test/config-luatest/failover_learners_test.lua

@@ -0,0 +1,113 @@
+local t = require('luatest')
+local cbuilder = require('luatest.cbuilder')
+local cluster = require('luatest.cluster')
+
+local g = t.group()
+
+-- {{{ Helpers
+
+local function check_instance_mode(instance, mode)
+    t.assert_equals(instance:eval('return box.info.ro'), mode == 'ro')
+end
+
+local function find_alert(server, prefix)
+    return server:exec(function(prefix)
+        for _, alert in ipairs(box.info.config.alerts) do
+            if alert.message:startswith(prefix) then
+                return alert
+            end
+        end
+        return nil
+    end, {prefix})
+end
+
+-- }}} Helpers
+
+-- Verify that using `replication.failover = "supervised"` with
+-- `replication.bootstrap_strategy = "auto"` not chooses failover
+-- learners as a bootstrap leader.
+g.test_not_chooses_learners_as_bootstrap_leader = function()
+    local config = cbuilder:new()
+        :set_global_option('failover.replicasets.r-001.learners',
+                           {'i-001', 'i-002'})
+        :set_global_option('replication.failover', 'supervised')
+        :use_replicaset('r-001')
+        :add_instance('i-001', {})
+        :add_instance('i-002', {})
+        :add_instance('i-003', {})
+        :config()
+
+    local cluster = cluster:new(config)
+    cluster:start()
+
+    check_instance_mode(cluster['i-001'], 'ro')
+    check_instance_mode(cluster['i-002'], 'ro')
+    check_instance_mode(cluster['i-003'], 'rw')
+end
+
+-- Check a singleton replicaset bootstrap fails if the instance
+-- is marked as a learner.
+g.test_all_learners = function()
+    local config = cbuilder:new()
+        :set_global_option('failover.replicasets.r-001.learners',
+                           {'i-001'})
+        :set_global_option('replication.failover', 'supervised')
+        :use_replicaset('r-001')
+        :add_instance('i-001', {})
+        :config()
+
+    local cluster = cluster:new(config)
+    cluster:start({wait_until_ready = false})
+
+    -- The instance (with the smallest UUID in the
+    -- lexicographical order) chooses itself as a join bootstrap
+    -- leader and fails since it's unable to bootstrap itself in
+    -- RO mode.
+    --
+    -- If there are other instances within the replicaset the
+    -- behavior is unpredictable. They might connect to the
+    -- leader before it fails. If so they will wait for it.
+    -- Otherwise they will fail.
+    t.helpers.retrying({timeout = 10}, function()
+        t.assert_not(cluster['i-001'].process:is_alive())
+    end)
+
+    local config = cbuilder:new(config)
+        :set_global_option('failover.replicasets.r-001.learners', {})
+        :config()
+    cluster:sync(config)
+
+    -- Now, the cluster should be able to bee successfully
+    -- bootstrapped.
+    cluster:start()
+
+    check_instance_mode(cluster['i-001'], 'rw')
+end
+
+-- Check an alert is issued if all instances are marked as
+-- learners.
+g.test_alert_on_no_leader = function()
+    local config1 = cbuilder:new()
+        :use_replicaset('r-001')
+        :add_instance('i-001', {})
+        :config()
+
+    local cluster = cluster:new(config1)
+    cluster:start()
+
+    local config2 = cbuilder:new(config1)
+        :set_global_option('failover.replicasets.r-001.learners',
+                           {'i-001'})
+        :set_global_option('replication.failover', 'supervised')
+        :config()
+    cluster:reload(config2)
+
+    local msg = 'box_cfg.apply: cannot determine a bootstrap leader based ' ..
+                'on the configuration'
+    find_alert(cluster['i-001'], msg)
+
+    cluster:reload(config1)
+    cluster['i-001']:exec(function()
+        t.assert_equals(box.info.config.alerts, {})
+    end)
+end