Skip to content

Commit f40d1f3

Browse files
committed
config: simplify sharding.roles validation
Now there is the `validate_scope` function, so we can write less code for validation of this kind. And we can move the validation to the lower level (`sharding` -> `sharding.roles`), because the `validate` annotation is now supported by the `schema.set()` constructor (see the previous commit). NO_DOC=no behavior changes except the config validation error message NO_CHANGELOG=see NO_DOC
1 parent 62c864d commit f40d1f3

File tree

4 files changed

+37
-54
lines changed

4 files changed

+37
-54
lines changed

src/box/lua/config/instance_config.lua

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1739,11 +1739,12 @@ return schema.new('instance_config', schema.record({
17391739
default = 1,
17401740
validate = validators['sharding.weight'],
17411741
}),
1742-
-- TODO: Add validate.
17431742
roles = schema.set({
17441743
'router',
17451744
'storage',
17461745
'rebalancer',
1746+
}, {
1747+
validate = validators['sharding.roles'],
17471748
}),
17481749
-- Global vshard options.
17491750
shard_index = schema.scalar({
@@ -1811,8 +1812,6 @@ return schema.new('instance_config', schema.record({
18111812
default = 1,
18121813
validate = validators['sharding.sched_move_quota'],
18131814
}),
1814-
}, {
1815-
validate = validators['sharding'],
18161815
}),
18171816
audit_log = enterprise_edition(schema.record({
18181817
-- The same as the destination for the logger, audit logger destination

src/box/lua/config/validators.lua

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -338,29 +338,6 @@ end
338338

339339
-- {{{ sharding
340340

341-
M['sharding'] = function(data, w)
342-
-- Forbid sharding.roles in instance scope.
343-
local scope = w.schema.computed.annotations.scope
344-
if data.roles ~= nil and scope == 'instance' then
345-
w.error('sharding.roles cannot be defined in the instance ' ..
346-
'scope')
347-
end
348-
-- Make sure that if the rebalancer role is present, the storage
349-
-- role is also present.
350-
if data.roles ~= nil then
351-
local has_storage = false
352-
local has_rebalancer = false
353-
for _, role in pairs(data.roles) do
354-
has_storage = has_storage or role == 'storage'
355-
has_rebalancer = has_rebalancer or role == 'rebalancer'
356-
end
357-
if has_rebalancer and not has_storage then
358-
w.error('The rebalancer role cannot be present without ' ..
359-
'the storage role')
360-
end
361-
end
362-
end
363-
364341
M['sharding.bucket_count'] = function(_data, w)
365342
validate_scope(w, {'global'})
366343
end
@@ -397,6 +374,26 @@ M['sharding.rebalancer_mode'] = function(_data, w)
397374
validate_scope(w, {'global'})
398375
end
399376

377+
M['sharding.roles'] = function(roles, w)
378+
-- Forbid in the instance scope, because it has no
379+
-- sense to set the option for a part of a
380+
-- replicaset.
381+
validate_scope(w, {'global', 'group', 'replicaset'})
382+
383+
-- Make sure that if the rebalancer role is present, the storage
384+
-- role is also present.
385+
local has_storage = false
386+
local has_rebalancer = false
387+
for _, role in pairs(roles) do
388+
has_storage = has_storage or role == 'storage'
389+
has_rebalancer = has_rebalancer or role == 'rebalancer'
390+
end
391+
if has_rebalancer and not has_storage then
392+
w.error('The rebalancer role cannot be present without ' ..
393+
'the storage role')
394+
end
395+
end
396+
400397
M['sharding.sched_move_quota'] = function(_data, w)
401398
validate_scope(w, {'global'})
402399
end

test/config-luatest/cluster_config_schema_test.lua

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -625,32 +625,6 @@ g.test_name_failure = function()
625625
end
626626
end
627627

628-
g.test_sharding = function()
629-
local config = {
630-
groups = {
631-
['group-001'] = {
632-
replicasets = {
633-
['replicaset-001'] = {
634-
instances = {
635-
['instance-001'] = {
636-
sharding = {
637-
roles = {'storage'},
638-
},
639-
},
640-
},
641-
},
642-
},
643-
},
644-
},
645-
}
646-
local err = '[cluster_config] groups.group-001.replicasets.' ..
647-
'replicaset-001.instances.instance-001.sharding: ' ..
648-
'sharding.roles cannot be defined in the instance scope'
649-
t.assert_error_msg_equals(err, function()
650-
cluster_config:validate(config)
651-
end)
652-
end
653-
654628
-- Verify options consistency on the global level.
655629
--
656630
-- Expected instance config fields plus the following additional
@@ -778,6 +752,7 @@ end
778752
-- * sharding.shard_index
779753
-- * sharding.sync_timeout
780754
-- * sharding.weight
755+
-- * sharding.roles
781756
g.test_scope = function()
782757
local function exp_err(path, scope)
783758
return ('[cluster_config] %s: The option must not be present in the ' ..
@@ -1003,6 +978,18 @@ g.test_scope = function()
1003978
replicaset = true,
1004979
instance = false,
1005980
},
981+
{
982+
name = 'sharding.roles',
983+
data = {
984+
sharding = {
985+
roles = {'storage'},
986+
},
987+
},
988+
global = true,
989+
group = true,
990+
replicaset = true,
991+
instance = false,
992+
},
1006993
}
1007994

1008995
for _, case in ipairs(cases) do

test/config-luatest/instance_config_schema_test.lua

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,8 +1727,8 @@ g.test_sharding = function()
17271727
roles = {'router', 'rebalancer'},
17281728
},
17291729
}
1730-
local err = '[instance_config] sharding: The rebalancer role cannot be ' ..
1731-
'present without the storage role'
1730+
local err = '[instance_config] sharding.roles: The rebalancer role ' ..
1731+
'cannot be present without the storage role'
17321732
t.assert_error_msg_equals(err, function()
17331733
instance_config:validate(iconfig)
17341734
end)

0 commit comments

Comments
 (0)