diff --git a/files/gitlab-cookbooks/praefect/libraries/praefect.rb b/files/gitlab-cookbooks/praefect/libraries/praefect.rb index 74239bbf06489d839228b9b021506c3517e21fab_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC9saWJyYXJpZXMvcHJhZWZlY3QucmI=..a311f420b8c7042b66ad538dddb041784cf0b7e6_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC9saWJyYXJpZXMvcHJhZWZlY3QucmI= 100644 --- a/files/gitlab-cookbooks/praefect/libraries/praefect.rb +++ b/files/gitlab-cookbooks/praefect/libraries/praefect.rb @@ -4,12 +4,8 @@ parse_virtual_storages end - # parse_virtual_storages converts the virtual storage's config object in to a format that better represents - # the structure of Praefect's virtual storage configuration. Historically, virtual storages were configured - # in omnibus as a hash of virtual storage names to nodes by name. parse_virtual_storages retains backwards - # compatibility with this by moving unknown keys in a virtual storage's config under the 'nodes' key. def parse_virtual_storages return if Gitlab['praefect']['virtual_storages'].nil? raise "Praefect virtual_storages must be a hash" unless Gitlab['praefect']['virtual_storages'].is_a?(Hash) @@ -11,25 +7,8 @@ def parse_virtual_storages return if Gitlab['praefect']['virtual_storages'].nil? raise "Praefect virtual_storages must be a hash" unless Gitlab['praefect']['virtual_storages'].is_a?(Hash) - # These are the known keys of virtual storage's configuration. Values under - # these keys are placed in to the root of the virtual storage's configuration. Unknown - # keys are assumed to be nodes of the virtual storage and are moved under the 'nodes' - # key. - known_keys = ['default_replication_factor'] - deprecation_logged = false - - virtual_storages = {} - Gitlab['praefect']['virtual_storages'].map do |virtual_storage, config_keys| - raise "nodes of a Praefect virtual_storage must be a hash" unless config_keys.is_a?(Hash) - - config = { 'nodes' => config_keys['nodes'] || {} } - config_keys.map do |key, value| - next if key == 'nodes' - - if known_keys.include? key - config[key] = value - next - end + Gitlab['praefect']['virtual_storages'].each do |virtual_storage, config_keys| + next unless config_keys.key?('nodes') @@ -35,19 +14,3 @@ - unless deprecation_logged - LoggingHelper.deprecation( - <<~EOS - Configuring the Gitaly nodes directly in the virtual storage's root configuration object has - been deprecated in GitLab 13.12 and will no longer be supported in GitLab 15.0. Move the Gitaly - nodes under the 'nodes' key as described in step 6 of https://docs.gitlab.com/ee/administration/gitaly/praefect.html#praefect. - EOS - ) - deprecation_logged = true - end - - raise "Virtual storage '#{virtual_storage}' contains duplicate configuration for node '#{key}'" if config['nodes'][key] - - config['nodes'][key] = value - end - - virtual_storages[virtual_storage] = config + raise "Nodes of Praefect virtual storage `#{virtual_storage}` must be a hash" unless config_keys['nodes'].is_a?(Hash) end @@ -53,6 +16,4 @@ end - - Gitlab['praefect']['virtual_storages'] = virtual_storages end end end diff --git a/spec/chef/cookbooks/praefect/recipes/praefect_spec.rb b/spec/chef/cookbooks/praefect/recipes/praefect_spec.rb index 74239bbf06489d839228b9b021506c3517e21fab_c3BlYy9jaGVmL2Nvb2tib29rcy9wcmFlZmVjdC9yZWNpcGVzL3ByYWVmZWN0X3NwZWMucmI=..a311f420b8c7042b66ad538dddb041784cf0b7e6_c3BlYy9jaGVmL2Nvb2tib29rcy9wcmFlZmVjdC9yZWNpcGVzL3ByYWVmZWN0X3NwZWMucmI= 100644 --- a/spec/chef/cookbooks/praefect/recipes/praefect_spec.rb +++ b/spec/chef/cookbooks/praefect/recipes/praefect_spec.rb @@ -101,8 +101,7 @@ 'praefect2' => { address: 'tcp://node2.internal', token: "praefect2-token" }, 'praefect3' => { address: 'tcp://node3.internal', token: "praefect3-token" }, 'praefect4' => { address: 'tcp://node4.internal', token: "praefect4-token" } - }, - 'praefect5' => { address: 'tcp://node5.internal', token: "praefect5-token" } + } } } end @@ -236,11 +235,6 @@ 'address' => 'tcp://node4.internal', 'storage' => 'praefect4', 'token' => 'praefect4-token' - }, - { - 'address' => 'tcp://node5.internal', - 'storage' => 'praefect5', - 'token' => 'praefect5-token' } ] } @@ -263,7 +257,27 @@ end end - context 'with duplicate virtual storage node configured via fallback' do - let(:virtual_storages) { { 'default' => { 'node-1' => {}, 'nodes' => { 'node-1' => {} } } } } + context 'with nodes of virtual storage as an array' do + let(:virtual_storages) do + { + 'default' => { + 'nodes' => { + 'node-1' => { + 'address' => 'tcp://node1.internal', + 'token' => 'praefect1-token' + } + } + }, + 'external' => { + 'nodes' => [ + { + 'storage' => 'node-2', + 'address' => 'tcp://node2.external', + 'token' => 'praefect2-token' + } + ] + } + } + end it 'raises an error' do @@ -268,23 +282,6 @@ it 'raises an error' do - allow(LoggingHelper).to receive(:deprecation) - expect(LoggingHelper).to receive(:deprecation).with( - <<~EOS - Configuring the Gitaly nodes directly in the virtual storage's root configuration object has - been deprecated in GitLab 13.12 and will no longer be supported in GitLab 15.0. Move the Gitaly - nodes under the 'nodes' key as described in step 6 of https://docs.gitlab.com/ee/administration/gitaly/praefect.html#praefect. - EOS - ) - - expect { chef_run }.to raise_error("Virtual storage 'default' contains duplicate configuration for node 'node-1'") - end - end - - context 'with nodes within virtual_storages as an array' do - let(:virtual_storages) { { 'default' => [{ storage: 'praefect1', address: 'tcp://node1.internal', token: "praefect1-token" }] } } - - it 'raises an error' do - expect { chef_run }.to raise_error("nodes of a Praefect virtual_storage must be a hash") + expect { chef_run }.to raise_error('Nodes of Praefect virtual storage `external` must be a hash') end end end