[tarantool-patches] [PATCH][vshard] Prohibit reconfigure non-dynamic options

[AKhatskevich <avkhatskevich@tarantool.org>]

By now there are two non-dynamic options:
 * bucket_count
 * shard_index

They cannot be changed by passing a new value to
router/storage.cfg.

Closes #114
---
Branch: https://github.com/tarantool/vshard/tree/kh/gh-114-non-dynamic
Issue: https://github.com/tarantool/vshard/issues/114

 test/router/router.result     | 14 ++++++++++++++
 test/router/router.test.lua   |  6 ++++++
 test/storage/storage.result   | 14 ++++++++++++++
 test/storage/storage.test.lua |  6 ++++++
 test/unit/config.result       | 20 ++++++++++++++++++++
 test/unit/config.test.lua     |  8 ++++++++
 vshard/cfg.lua                | 19 +++++++++++++++++--
 vshard/router/init.lua        |  4 +++-
 vshard/storage/init.lua       |  4 +++-
 9 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/test/router/router.result b/test/router/router.result
index 2ee1bff..1aacc0e 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -1057,6 +1057,20 @@ error_messages
 - - Use replica:is_connected(...) instead of replica.is_connected(...)
   - Use replica:safe_uri(...) instead of replica.safe_uri(...)
 ...
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+---
+...
+non_dynamic_cfg.shard_index = 'non_default_name'
+---
+...
+_, err = pcall(vshard.router.cfg, non_dynamic_cfg)
+---
+...
+err:match('Non%-dynamic.*')
+---
+- Non-dynamic option shard_index is changed in new vshard config
+...
 _ = test_run:cmd("switch default")
 ---
 ...
diff --git a/test/router/router.test.lua b/test/router/router.test.lua
index fae8e24..f3b18b1 100644
--- a/test/router/router.test.lua
+++ b/test/router/router.test.lua
@@ -389,6 +389,12 @@ end;
 test_run:cmd("setopt delimiter ''");
 error_messages
 
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+non_dynamic_cfg.shard_index = 'non_default_name'
+_, err = pcall(vshard.router.cfg, non_dynamic_cfg)
+err:match('Non%-dynamic.*')
+
 _ = test_run:cmd("switch default")
 test_run:drop_cluster(REPLICASET_2)
 
diff --git a/test/storage/storage.result b/test/storage/storage.result
index d0bf792..41e3af8 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -720,6 +720,20 @@ test_run:cmd("setopt delimiter ''");
 ---
 - true
 ...
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+---
+...
+non_dynamic_cfg.shard_index = 'non_default_name'
+---
+...
+_, err = pcall(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a)
+---
+...
+err:match('Non%-dynamic.*')
+---
+- Non-dynamic option shard_index is changed in new vshard config
+...
 _ = test_run:cmd("switch default")
 ---
 ...
diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua
index f4bbf0e..4f3faf6 100644
--- a/test/storage/storage.test.lua
+++ b/test/storage/storage.test.lua
@@ -177,6 +177,12 @@ for _, new_replicaset in pairs(new_replicasets) do
 end;
 test_run:cmd("setopt delimiter ''");
 
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+non_dynamic_cfg.shard_index = 'non_default_name'
+_, err = pcall(vshard.storage.cfg, non_dynamic_cfg, names.storage_1_a)
+err:match('Non%-dynamic.*')
+
 _ = test_run:cmd("switch default")
 
 test_run:drop_cluster(REPLICASET_2)
diff --git a/test/unit/config.result b/test/unit/config.result
index 6b4f87b..0d8c740 100644
--- a/test/unit/config.result
+++ b/test/unit/config.result
@@ -506,3 +506,23 @@ _ = lcfg.check(cfg)
 replica.name = 'storage'
 ---
 ...
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+---
+...
+non_dynamic_cfg_old = table.copy(cfg)
+---
+...
+non_dynamic_cfg.shard_index = nil
+---
+...
+non_dynamic_cfg_old.shard_index = 'non_default_name'
+---
+...
+_, err = pcall(lcfg.check, non_dynamic_cfg, non_dynamic_cfg_old)
+---
+...
+err:match('Non%-dynamic.*')
+---
+- Non-dynamic option shard_index is changed in new vshard config
+...
diff --git a/test/unit/config.test.lua b/test/unit/config.test.lua
index 0f549d3..972df65 100644
--- a/test/unit/config.test.lua
+++ b/test/unit/config.test.lua
@@ -206,3 +206,11 @@ _ = lcfg.check(cfg)
 replica.name = nil
 _ = lcfg.check(cfg)
 replica.name = 'storage'
+
+-- gh-114: Check non-dynamic option change during reconfigure.
+non_dynamic_cfg = table.copy(cfg)
+non_dynamic_cfg_old = table.copy(cfg)
+non_dynamic_cfg.shard_index = nil
+non_dynamic_cfg_old.shard_index = 'non_default_name'
+_, err = pcall(lcfg.check, non_dynamic_cfg, non_dynamic_cfg_old)
+err:match('Non%-dynamic.*')
diff --git a/vshard/cfg.lua b/vshard/cfg.lua
index f5db4c0..8d26104 100644
--- a/vshard/cfg.lua
+++ b/vshard/cfg.lua
@@ -222,14 +222,29 @@ local cfg_template = {
 
 --
 -- Check sharding config on correctness. Check types, name and uri
--- uniqueness, master count (in each replicaset must by <= 1).
+-- uniqueness, master count (in each replicaset must be <= 1).
 --
-local function cfg_check(shard_cfg)
+local function cfg_check(shard_cfg, old_cfg)
     if type(shard_cfg) ~= 'table' then
         error('Сonfig must be map of options')
     end
     shard_cfg = table.deepcopy(shard_cfg)
     validate_config(shard_cfg, cfg_template)
+    if not old_cfg then
+        return shard_cfg
+    end
+    assert(not old_cfg or type(old_cfg) == 'table')
+    -- Check non-dynamic after default values are added.
+    local non_dynamic = {
+        'bucket_count', 'shard_index'
+    }
+    for _, f_name in pairs(non_dynamic) do
+        -- New option may be added in new vshard verion.
+        if old_cfg[f_name] and shard_cfg[f_name] ~= old_cfg[f_name] then
+           error(string.format('Non-dynamic option %s ' ..
+                               'is changed in new vshard config', f_name))
+        end
+    end
     return shard_cfg
 end
 
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 21093e5..f29998a 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -457,7 +457,8 @@ end
 --------------------------------------------------------------------------------
 
 local function router_cfg(cfg)
-    cfg = lcfg.check(cfg)
+    cfg = lcfg.check(cfg, M.current_cfg)
+    local new_cfg = table.deepcopy(cfg)
     if not M.replicasets then
         log.info('Starting router configuration')
     else
@@ -478,6 +479,7 @@ local function router_cfg(cfg)
     -- TODO: update existing route map in-place
     M.route_map = {}
     M.replicasets = new_replicasets
+    M.current_cfg = new_cfg
     -- Move connections from an old configuration to a new one.
     -- It must be done with no yields to prevent usage both of not
     -- fully moved old replicasets, and not fully built new ones.
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 57076e1..4c5d771 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -1448,7 +1448,8 @@ local function storage_cfg(cfg, this_replica_uuid)
     if this_replica_uuid == nil then
         error('Usage: cfg(configuration, this_replica_uuid)')
     end
-    cfg = lcfg.check(cfg)
+    cfg = lcfg.check(cfg, M.current_cfg)
+    local new_cfg = table.deepcopy(cfg)
     if cfg.weights or cfg.zone then
         error('Weights and zone are not allowed for storage configuration')
     end
@@ -1574,6 +1575,7 @@ local function storage_cfg(cfg, this_replica_uuid)
     M.shard_index = shard_index
     M.collect_bucket_garbage_interval = collect_bucket_garbage_interval
     M.collect_lua_garbage = collect_lua_garbage
+    M.current_cfg = new_cfg
 
     if was_master and not is_master then
         local_on_master_disable()
-- 
2.14.1