diff --git a/changelogs/unreleased/smh-default-replication-factor.yml b/changelogs/unreleased/smh-default-replication-factor.yml new file mode 100644 index 0000000000000000000000000000000000000000..3d77bcb5b15a32ad8bbb7a1574101e1fa6d34198_Y2hhbmdlbG9ncy91bnJlbGVhc2VkL3NtaC1kZWZhdWx0LXJlcGxpY2F0aW9uLWZhY3Rvci55bWw= --- /dev/null +++ b/changelogs/unreleased/smh-default-replication-factor.yml @@ -0,0 +1,5 @@ +--- +title: Add default replication factor configuration option to Praefect +merge_request: 4988 +author: +type: added diff --git a/files/gitlab-config-template/gitlab.rb.template b/files/gitlab-config-template/gitlab.rb.template index fba8cfeb45798bf16fd956f47183967e00566d65_ZmlsZXMvZ2l0bGFiLWNvbmZpZy10ZW1wbGF0ZS9naXRsYWIucmIudGVtcGxhdGU=..3d77bcb5b15a32ad8bbb7a1574101e1fa6d34198_ZmlsZXMvZ2l0bGFiLWNvbmZpZy10ZW1wbGF0ZS9naXRsYWIucmIudGVtcGxhdGU= 100644 --- a/files/gitlab-config-template/gitlab.rb.template +++ b/files/gitlab-config-template/gitlab.rb.template @@ -2100,13 +2100,16 @@ # praefect['logging_format'] = "json" # praefect['virtual_storages'] = { # 'default' => { -# 'praefect-internal-0' => { -# 'address' => 'tcp://10.23.56.78:8075', -# 'token' => 'abc123' -# }, -# 'praefect-internal-1' => { -# 'address' => 'tcp://10.76.23.31:8075', -# 'token' => 'xyz456' +# 'default_replication_factor' => 3, +# 'nodes' => { +# 'praefect-internal-0' => { +# 'address' => 'tcp://10.23.56.78:8075', +# 'token' => 'abc123' +# }, +# 'praefect-internal-1' => { +# 'address' => 'tcp://10.76.23.31:8075', +# 'token' => 'xyz456' +# } # } # }, # 'alternative' => { @@ -2110,13 +2113,15 @@ # } # }, # 'alternative' => { -# 'praefect-internal-2' => { -# 'address' => 'tcp://10.34.1.16:8075', -# 'token' => 'abc321' -# }, -# 'praefect-internal-3' => { -# 'address' => 'tcp://10.23.18.6:8075', -# 'token' => 'xyz890' +# 'nodes' => { +# 'praefect-internal-2' => { +# 'address' => 'tcp://10.34.1.16:8075', +# 'token' => 'abc321' +# }, +# 'praefect-internal-3' => { +# 'address' => 'tcp://10.23.18.6:8075', +# 'token' => 'xyz890' +# } # } # } # } diff --git a/files/gitlab-cookbooks/praefect/libraries/praefect.rb b/files/gitlab-cookbooks/praefect/libraries/praefect.rb index fba8cfeb45798bf16fd956f47183967e00566d65_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC9saWJyYXJpZXMvcHJhZWZlY3QucmI=..3d77bcb5b15a32ad8bbb7a1574101e1fa6d34198_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC9saWJyYXJpZXMvcHJhZWZlY3QucmI= 100644 --- a/files/gitlab-cookbooks/praefect/libraries/praefect.rb +++ b/files/gitlab-cookbooks/praefect/libraries/praefect.rb @@ -4,8 +4,12 @@ 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) @@ -7,8 +11,31 @@ 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) - Gitlab['praefect']['virtual_storages'].each do |name, nodes| - raise "nodes of a Praefect virtual_storage must be a hash" unless nodes.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'] + + 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 + + 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 end @@ -14,4 +41,6 @@ end + + Gitlab['praefect']['virtual_storages'] = virtual_storages end end end diff --git a/files/gitlab-cookbooks/praefect/templates/default/praefect-config.toml.erb b/files/gitlab-cookbooks/praefect/templates/default/praefect-config.toml.erb index fba8cfeb45798bf16fd956f47183967e00566d65_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC90ZW1wbGF0ZXMvZGVmYXVsdC9wcmFlZmVjdC1jb25maWcudG9tbC5lcmI=..3d77bcb5b15a32ad8bbb7a1574101e1fa6d34198_ZmlsZXMvZ2l0bGFiLWNvb2tib29rcy9wcmFlZmVjdC90ZW1wbGF0ZXMvZGVmYXVsdC9wcmFlZmVjdC1jb25maWcudG9tbC5lcmI= 100644 --- a/files/gitlab-cookbooks/praefect/templates/default/praefect-config.toml.erb +++ b/files/gitlab-cookbooks/praefect/templates/default/praefect-config.toml.erb @@ -60,6 +60,6 @@ format = '<%= @logging_format %>' <% end %> -<% @virtual_storages.each do |name, nodes| %> +<% @virtual_storages.each do |name, config| %> [[virtual_storage]] name = '<%= name %>' @@ -64,6 +64,9 @@ [[virtual_storage]] name = '<%= name %>' -<% nodes.each do |storage, node| %> +<% if config['default_replication_factor'] %> +default_replication_factor = <%= config['default_replication_factor'] %> +<% end %> +<% config['nodes'].each do |storage, node| %> [[virtual_storage.node]] storage = '<%= storage %>' address = '<%= node['address'] %>' diff --git a/spec/chef/recipes/praefect_spec.rb b/spec/chef/recipes/praefect_spec.rb index fba8cfeb45798bf16fd956f47183967e00566d65_c3BlYy9jaGVmL3JlY2lwZXMvcHJhZWZlY3Rfc3BlYy5yYg==..3d77bcb5b15a32ad8bbb7a1574101e1fa6d34198_c3BlYy9jaGVmL3JlY2lwZXMvcHJhZWZlY3Rfc3BlYy5yYg== 100644 --- a/spec/chef/recipes/praefect_spec.rb +++ b/spec/chef/recipes/praefect_spec.rb @@ -92,10 +92,13 @@ let(:virtual_storages) do { 'default' => { - 'praefect1' => { address: 'tcp://node1.internal', token: "praefect1-token" }, - 'praefect2' => { address: 'tcp://node2.internal', token: "praefect2-token" }, - 'praefect3' => { address: 'tcp://node3.internal', token: "praefect3-token" }, - 'praefect4' => { address: 'tcp://node4.internal', token: "praefect4-token" }, + 'default_replication_factor' => 2, + 'nodes' => { + 'praefect1' => { address: 'tcp://node1.internal', token: "praefect1-token" }, + '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" } } } @@ -154,5 +157,5 @@ it 'renders the config.toml' do expect(chef_run).to render_file(config_path).with_content { |content| - expect(Tomlrb.parse(content)).to include( + expect(Tomlrb.parse(content)).to eq( { @@ -158,2 +161,31 @@ { + 'auth' => { + 'token' => 'secrettoken123', + 'transitioning' => false + }, + 'database' => { + 'dbname' => 'praefect_production', + 'host' => 'pg.external', + 'host_no_proxy' => 'pg.internal', + 'password' => 'praefect-pg-pass', + 'port' => 2234, + 'port_no_proxy' => 1234, + 'sslcert' => '/path/to/client-cert', + 'sslkey' => '/path/to/client-key', + 'sslmode' => 'require', + 'sslrootcert' => '/path/to/rootcert', + 'user' => 'praefect-pg' + }, + 'failover' => { + 'election_strategy' => 'local', + 'enabled' => true + }, + 'logging' => { + 'format' => 'text', + 'level' => 'debug' + }, + 'listen_addr' => 'localhost:4444', + 'prometheus' => { + 'grpc_latency_buckets' => [0.001, 0.005, 0.025, 0.1, 0.5, 1.0, 10.0, 30.0, 60.0, 300.0, 1500.0] + }, 'reconciliation' => { @@ -159,7 +191,51 @@ 'reconciliation' => { - 'scheduling_interval' => '1m', - 'histogram_buckets' => [1.0, 2.0] - } + 'histogram_buckets' => [1.0, 2.0], + 'scheduling_interval' => '1m' + }, + 'sentry' => { + 'sentry_dsn' => 'https://my_key:my_secret@sentry.io/test_project', + 'sentry_environment' => 'production' + }, + 'prometheus_listen_addr' => 'localhost:1234', + 'socket_path' => '/var/opt/gitlab/praefect/praefect.socket', + 'tls' => { + 'certificate_path' => '/path/to/cert.pem', + 'key_path' => '/path/to/key.pem' + }, + 'tls_listen_addr' => 'localhost:5555', + 'virtual_storage' => [ + { + 'name' => 'default', + 'default_replication_factor' => 2, + 'node' => [ + { + 'address' => 'tcp://node1.internal', + 'storage' => 'praefect1', + 'token' => 'praefect1-token' + }, + { + 'address' => 'tcp://node2.internal', + 'storage' => 'praefect2', + 'token' => 'praefect2-token' + }, + { + 'address' => 'tcp://node3.internal', + 'storage' => 'praefect3', + 'token' => 'praefect3-token' + }, + { + 'address' => 'tcp://node4.internal', + 'storage' => 'praefect4', + 'token' => 'praefect4-token' + }, + { + 'address' => 'tcp://node5.internal', + 'storage' => 'praefect5', + 'token' => 'praefect5-token' + } + ] + } + ] } ) } @@ -163,54 +239,6 @@ } ) } - - expect(chef_run).to render_file(config_path) - .with_content("listen_addr = '#{listen_addr}'") - expect(chef_run).to render_file(config_path) - .with_content(%r{^tls_listen_addr = '#{tls_listen_addr}'\n\[tls\]\ncertificate_path = '#{certificate_path}'\nkey_path = '#{key_path}'\n}) - expect(chef_run).to render_file(config_path) - .with_content("socket_path = '#{socket_path}'") - expect(chef_run).to render_file(config_path) - .with_content("prometheus_listen_addr = '#{prom_addr}'") - expect(chef_run).to render_file(config_path) - .with_content("level = '#{log_level}'") - expect(chef_run).to render_file(config_path) - .with_content("format = '#{log_format}'") - expect(chef_run).to render_file(config_path) - .with_content("sentry_dsn = '#{sentry_dsn}'") - expect(chef_run).to render_file(config_path) - .with_content("sentry_environment = '#{sentry_environment}'") - expect(chef_run).to render_file(config_path) - .with_content(%r{\[failover\]\s+enabled = true\s+election_strategy = 'local'}) - expect(chef_run).to render_file(config_path) - .with_content(%r{\[prometheus\]\s+grpc_latency_buckets = #{Regexp.escape(prometheus_grpc_latency_buckets)}}) - expect(chef_run).to render_file(config_path) - .with_content(%r{^\[auth\]\ntoken = '#{auth_token}'\ntransitioning = #{auth_transitioning}\n}) - - virtual_storages.each do |name, nodes| - expect(chef_run).to render_file(config_path).with_content(%r{^\[\[virtual_storage\]\]\nname = '#{name}'\n}) - nodes.each do |storage, node| - expect(chef_run).to render_file(config_path) - .with_content(%r{^\[\[virtual_storage.node\]\]\nstorage = '#{storage}'\naddress = '#{node[:address]}'\ntoken = '#{node[:token]}'\n}) - end - end - - database_section = Regexp.new([ - %r{\[database\]}, - %r{host = '#{database_host}'}, - %r{port = #{database_port}}, - %r{user = '#{database_user}'}, - %r{password = '#{database_password}'}, - %r{dbname = '#{database_dbname}'}, - %r{sslmode = '#{database_sslmode}'}, - %r{sslcert = '#{database_sslcert}'}, - %r{sslkey = '#{database_sslkey}'}, - %r{sslrootcert = '#{database_sslrootcert}'}, - %r{host_no_proxy = '#{database_host_no_proxy}'}, - %r{port_no_proxy = #{database_port_no_proxy}}, - ].map(&:to_s).join('\n')) - - expect(chef_run).to render_file(config_path).with_content(database_section) end it 'renders the env dir files correctly' do @@ -226,6 +254,14 @@ end end + context 'with duplicate virtual storage node configured via fallback' do + let(:virtual_storages) { { 'default' => { 'node-1' => {}, 'nodes' => { 'node-1' => {} } } } } + + it 'raises an error' do + 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" }] } }