cbuilder: fix duplicate instance creation

Fix a bug in cbuilder when `set_instance_option()` is called without
`use_group()`/`use_replicaset()`: previously it always assumed
`group-001`/`replicaset-001` and could silently create a duplicate
instance entry with the same name in another replicaset, leading to
duplicated instances in the generated config. Now cbuilder searches for
the instance across all groups/replicasets when no explicit group or
replicaset is set, and raises an error if the instance is not found
instead of creating it implicitly.

Closes #404

Author: mandesero

CHANGELOG.md

@@ -10,6 +10,10 @@
   allowing tests to keep clusters alive between test runs (gh-414).
 - Fixed a bug where URI search would terminate prematurely when multiple
   replicasets existed (gh-427).
+- Fixed `cbuilder:set_instance_option()` to reuse existing instances found in a
+  provided configuration and error when a target instance is missing. Also,
+  `cbuilder:add_instance()` now rejects duplicate instance names across groups
+  and replicasets (gh-404).
 
 ## 1.2.1
 

luatest/cbuilder.lua

@@ -194,16 +194,47 @@ function Builder:set_replicaset_option(path, value)
     return self
 end
 
+local function find_instance(config, instance_name)
+    local groups = config.groups
+    if type(groups) ~= 'table' then
+        return nil
+    end
+
+    for group_name, group in pairs(groups) do
+        local replicasets = group.replicasets
+        if type(replicasets) == 'table' then
+            for replicaset_name, replicaset in pairs(replicasets) do
+                local instances = replicaset.instances
+                if type(instances) == 'table' and
+                        instances[instance_name] ~= nil then
+                    return group_name, replicaset_name
+                end
+            end
+        end
+    end
+
+    return nil
+end
+
 -- Set an option of a particular instance in the selected replicaset.
 --
 -- @string instance_name Instance where the option will be saved.
 -- @string path Option path.
 -- @param value Option value (int, string, table).
 function Builder:set_instance_option(instance_name, path, value)
     checks('table', 'string', 'string', '?')
+
+    local group_name, replicaset_name = find_instance(self._config,
+                                                      instance_name)
+    if group_name == nil then
+        error(('Instance %q is not found in the configuration. ' ..
+              'Use :add_instance() to create it first.')
+              :format(instance_name))
+    end
+
     path = fun.chain({
-        'groups', self._group,
-        'replicasets', self._replicaset,
+        'groups', group_name,
+        'replicasets', replicaset_name,
         'instances', instance_name,
     }, path:split('.')):totable()
 
@@ -217,6 +248,16 @@ end
 -- @tab iconfig Declarative config for the instance.
 function Builder:add_instance(instance_name, iconfig)
     checks('table', 'string', '?')
+
+    local group_name, replicaset_name = find_instance(self._config,
+                                                      instance_name)
+    if group_name ~= nil then
+        error(
+            ('Found instance with the same name %q in ' ..
+            'the replicaset %q in the group %q')
+            :format(instance_name, replicaset_name, group_name))
+    end
+
     local path = {
         'groups', self._group,
         'replicasets', self._replicaset,

test/cbuilder_test.lua

@@ -147,6 +147,62 @@ g.test_set_instance_option = function()
         builder.set_instance_option, builder, 'foo', 'replication.anon', 'bar')
 end
 
+g.test_set_instance_option_uses_existing_instance = function()
+    t.run_only_if(utils.version_current_ge_than(3, 0, 0),
+                  [[Declarative configuration works on Tarantool 3.0.0+.
+                    See tarantool/tarantool@13149d65bc9d for details]])
+
+    local config_1 = config_builder:new()
+        :use_replicaset('r-001')
+        :add_instance('i-001', {})
+        :set_instance_option('i-001', 'replication.timeout', 0.1)
+        :config()
+
+    local config_2 = config_builder:new(config_1)
+        :set_instance_option('i-001', 'replication.timeout', 1000)
+        :config()
+
+    t.assert_equals(config_2, merge_config(config_1, {
+        groups = {
+            ['group-001'] = {
+                replicasets = {
+                    ['r-001'] = {
+                        instances = {
+                            ['i-001'] = {replication = {timeout = 1000}},
+                        },
+                    },
+                },
+            },
+        },
+    }))
+
+    local builder = config_builder:new()
+    t.assert_error_msg_contains(
+        'Instance "missing" is not found in the configuration',
+        builder.set_instance_option, builder, 'missing',
+        'replication.timeout', 0.1)
+end
+
+g.test_instance_names_are_unique = function()
+    t.run_only_if(utils.version_current_ge_than(3, 0, 0),
+                  [[Declarative configuration works on Tarantool 3.0.0+.
+                    See tarantool/tarantool@13149d65bc9d for details]])
+
+    local builder = config_builder:new()
+        :use_group('g-001')
+        :use_replicaset('r-001')
+        :add_instance('duplicate', {})
+        :use_group('g-002')
+        :use_replicaset('r-002')
+
+
+    t.assert_error_msg_contains(
+        'Found instance with the same name "duplicate" in the ' ..
+        'replicaset "r-001" in the group "g-001"',
+        builder.add_instance, builder, 'duplicate', {}
+    )
+end
+
 g.test_set_replicaset_option = function()
     t.run_only_if(utils.version_current_ge_than(3, 0, 0),
                   [[Declarative configuration works on Tarantool 3.0.0+.