From 44b7ae9b43e0cb12e7a8554159289a507da7d6f8 Mon Sep 17 00:00:00 2001 From: Cory Minkovich Date: Fri, 19 Feb 2016 10:31:25 -0800 Subject: [PATCH 1/4] Create an option to stop ignoring weights. By default weights will still be ignored. This commit is based on: https://github.com/airbnb/synapse/pull/131 but does the following things differently: 1. By default weights are ignored. This is to maintain current behavior for safety. 2. HAProxy will reconfigure if weights changes. --- README.md | 1 + lib/synapse/haproxy.rb | 5 ++++ lib/synapse/service_watcher/base.rb | 2 +- spec/lib/synapse/haproxy_spec.rb | 27 +++++++++++++++++-- spec/lib/synapse/service_watcher_base_spec.rb | 18 +++++++++++++ spec/support/minimum.conf.yaml | 1 + 6 files changed, 51 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b63f9735..1813e822 100644 --- a/README.md +++ b/README.md @@ -273,6 +273,7 @@ This section is its own hash, which should contain the following keys: * `backend_order`: optional: how backends should be ordered in the `backend` stanza. (default is shuffling). Setting to `asc` means sorting backends in ascending alphabetical order before generating stanza. `desc` means descending alphabetical order. `no_shuffle` means no shuffling or sorting. * `shared_frontend`: optional: haproxy configuration directives for a shared http frontend (see below) * `cookie_value_method`: optional: default value is `name`, it defines the way your backends receive a cookie value in http mode. If equal to `hash`, synapse hashes backend names on cookie value assignation of your discovered backends, useful when you want to use haproxy cookie feature but you do not want that your end users receive a Set-Cookie with your server name and ip readable in clear. +* `ignore_weights`: optional: stops haproxy backend 'weight' options being generated, even if the Nerve registrations contain this information. This will cause all backend servers to be treated equally by haproxy. This defaults to true so weights will *NOT* be used by default. ### Configuring HAProxy ### diff --git a/lib/synapse/haproxy.rb b/lib/synapse/haproxy.rb index 439686e4..85e87fe3 100644 --- a/lib/synapse/haproxy.rb +++ b/lib/synapse/haproxy.rb @@ -538,6 +538,7 @@ def initialize(opts) @opts['do_writes'] = true unless @opts.key?('do_writes') @opts['do_socket'] = true unless @opts.key?('do_socket') @opts['do_reloads'] = true unless @opts.key?('do_reloads') + @opts['ignore_weights'] = true unless @opts.key?('ignore_weights') # how to restart haproxy @restart_interval = @opts.fetch('restart_interval', 2).to_i @@ -747,6 +748,10 @@ def generate_backend_stanza(watcher, config) b = "#{b} cookie #{backend_name}" end end + if !@opts['ignore_weights'] && backend.has_key?('weight') + weight = backend['weight'].to_i + b = "#{b} weight #{weight}" + end b = "#{b} #{watcher.haproxy['server_options']}" if watcher.haproxy['server_options'] b = "#{b} #{backend['haproxy_server_options']}" if backend['haproxy_server_options'] b = "#{b} disabled" unless backend['enabled'] diff --git a/lib/synapse/service_watcher/base.rb b/lib/synapse/service_watcher/base.rb index bf83f22e..5651af6d 100644 --- a/lib/synapse/service_watcher/base.rb +++ b/lib/synapse/service_watcher/base.rb @@ -117,7 +117,7 @@ def set_backends(new_backends) # Aggregate and deduplicate all potential backend service instances. new_backends = (new_backends + @default_servers) if @keep_default_servers new_backends = new_backends.uniq {|b| - [b['host'], b['port'], b.fetch('name', '')] + [b['host'], b['port'], b.fetch('name', ''), b.fetch('weight', 1)] } if new_backends.to_set == @backends.to_set diff --git a/spec/lib/synapse/haproxy_spec.rb b/spec/lib/synapse/haproxy_spec.rb index 7791dd86..0a9b7fac 100644 --- a/spec/lib/synapse/haproxy_spec.rb +++ b/spec/lib/synapse/haproxy_spec.rb @@ -5,15 +5,18 @@ class MockWatcher; end; describe Synapse::Haproxy do subject { Synapse::Haproxy.new(config['haproxy']) } - let(:mockwatcher) do + def createmockwatcher(backends) mockWatcher = double(Synapse::ServiceWatcher) allow(mockWatcher).to receive(:name).and_return('example_service') - backends = [{ 'host' => 'somehost', 'port' => 5555}] allow(mockWatcher).to receive(:backends).and_return(backends) allow(mockWatcher).to receive(:haproxy).and_return({'server_options' => "check inter 2000 rise 3 fall 2"}) mockWatcher end + let(:mockwatcher) do + createmockwatcher [{ 'host' => 'somehost', 'port' => '5555'}] + end + let(:mockwatcher_with_server_options) do mockWatcher = double(Synapse::ServiceWatcher) allow(mockWatcher).to receive(:name).and_return('example_service2') @@ -275,4 +278,24 @@ class MockWatcher; end; expect(subject.generate_frontend_stanza(mockwatcher_frontend_with_bind_address, mockConfig)).to eql(["\nfrontend example_service5", [], "\tbind 127.0.0.3:2200", "\tdefault_backend example_service5"]) end + it 'generates backend stanza with weight' do + mockConfig = [] + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 1, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 1 check inter 2000 rise 3 fall 2"]]) + end + + it 'generates backend stanza with bad weight = 0' do + mockConfig = [] + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 'hi', 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 0 check inter 2000 rise 3 fall 2"]]) + end + + it 'generates backend stanza with nil weight = 0' do + mockConfig = [] + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => nil, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 0 check inter 2000 rise 3 fall 2"]]) + end + + it 'generates backend stanza without weight' do + mockConfig = [] + expect(subject.generate_backend_stanza(createmockwatcher([{ 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 check inter 2000 rise 3 fall 2"]]) + end + end diff --git a/spec/lib/synapse/service_watcher_base_spec.rb b/spec/lib/synapse/service_watcher_base_spec.rb index da317470..ae48a4aa 100644 --- a/spec/lib/synapse/service_watcher_base_spec.rb +++ b/spec/lib/synapse/service_watcher_base_spec.rb @@ -135,5 +135,23 @@ def remove_arg(name) expect(subject.backends).to eq(matching_labeled_backends) end end + + context 'with ignore_weights set to false' do + let(:backends) { [ + { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 11 }, + { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, + ] } + let(:non_matching_weight_backends) { [ + { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 33 }, + { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, + ] } + it 'updates backends only when weights change' do + expect(subject).to receive(:'reconfigure!').exactly(:twice) + expect(subject.send(:set_backends, backends)).to equal(true) + expect(subject.backends).to eq(backends) + expect(subject.send(:set_backends, non_matching_weight_backends)).to equal(true) + expect(subject.backends).to eq(non_matching_weight_backends) + end + end end end diff --git a/spec/support/minimum.conf.yaml b/spec/support/minimum.conf.yaml index 5df62fb9..87775393 100644 --- a/spec/support/minimum.conf.yaml +++ b/spec/support/minimum.conf.yaml @@ -22,6 +22,7 @@ haproxy: config_file_path: "/etc/haproxy/haproxy.cfg" do_writes: false do_reloads: false + ignore_weights: false global: - global_test_option From 2e0c6f26a204cc81ce65fb86535915e762548691 Mon Sep 17 00:00:00 2001 From: Cory Minkovich Date: Fri, 19 Feb 2016 14:10:21 -0800 Subject: [PATCH 2/4] Changes position of weight in HAProxy backend line, prevents two nodes from taking traffic just because they have different weights, add more tests. --- lib/synapse/haproxy.rb | 6 +++++- lib/synapse/service_watcher/base.rb | 2 +- spec/lib/synapse/haproxy_spec.rb | 6 +++--- spec/lib/synapse/service_watcher_base_spec.rb | 20 ++++++++++++++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/synapse/haproxy.rb b/lib/synapse/haproxy.rb index 85e87fe3..8fb9f970 100644 --- a/lib/synapse/haproxy.rb +++ b/lib/synapse/haproxy.rb @@ -748,11 +748,15 @@ def generate_backend_stanza(watcher, config) b = "#{b} cookie #{backend_name}" end end + b = "#{b} #{watcher.haproxy['server_options']}" if watcher.haproxy['server_options'] if !@opts['ignore_weights'] && backend.has_key?('weight') + # Check if server_options already contains weight, is so log a warning + if watcher.haproxy['server_options'].include? 'weight' + log.info "synapse: weight is defined by server_options and by nerve" + end weight = backend['weight'].to_i b = "#{b} weight #{weight}" end - b = "#{b} #{watcher.haproxy['server_options']}" if watcher.haproxy['server_options'] b = "#{b} #{backend['haproxy_server_options']}" if backend['haproxy_server_options'] b = "#{b} disabled" unless backend['enabled'] b } diff --git a/lib/synapse/service_watcher/base.rb b/lib/synapse/service_watcher/base.rb index 5651af6d..bf83f22e 100644 --- a/lib/synapse/service_watcher/base.rb +++ b/lib/synapse/service_watcher/base.rb @@ -117,7 +117,7 @@ def set_backends(new_backends) # Aggregate and deduplicate all potential backend service instances. new_backends = (new_backends + @default_servers) if @keep_default_servers new_backends = new_backends.uniq {|b| - [b['host'], b['port'], b.fetch('name', ''), b.fetch('weight', 1)] + [b['host'], b['port'], b.fetch('name', '')] } if new_backends.to_set == @backends.to_set diff --git a/spec/lib/synapse/haproxy_spec.rb b/spec/lib/synapse/haproxy_spec.rb index 0a9b7fac..534debfc 100644 --- a/spec/lib/synapse/haproxy_spec.rb +++ b/spec/lib/synapse/haproxy_spec.rb @@ -280,17 +280,17 @@ def createmockwatcher(backends) it 'generates backend stanza with weight' do mockConfig = [] - expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 1, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 1 check inter 2000 rise 3 fall 2"]]) + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 1, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 check inter 2000 rise 3 fall 2 weight 1"]]) end it 'generates backend stanza with bad weight = 0' do mockConfig = [] - expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 'hi', 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 0 check inter 2000 rise 3 fall 2"]]) + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => 'hi', 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 check inter 2000 rise 3 fall 2 weight 0"]]) end it 'generates backend stanza with nil weight = 0' do mockConfig = [] - expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => nil, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 weight 0 check inter 2000 rise 3 fall 2"]]) + expect(subject.generate_backend_stanza(createmockwatcher([{ 'weight' => nil, 'host' => 'somehost', 'port' => '5555'}]), mockConfig)).to eql(["\nbackend example_service", [], ["\tserver somehost:5555 somehost:5555 cookie somehost:5555 check inter 2000 rise 3 fall 2 weight 0"]]) end it 'generates backend stanza without weight' do diff --git a/spec/lib/synapse/service_watcher_base_spec.rb b/spec/lib/synapse/service_watcher_base_spec.rb index ae48a4aa..9b7bcbcf 100644 --- a/spec/lib/synapse/service_watcher_base_spec.rb +++ b/spec/lib/synapse/service_watcher_base_spec.rb @@ -137,7 +137,7 @@ def remove_arg(name) end context 'with ignore_weights set to false' do - let(:backends) { [ + let(:backends_with_weight) { [ { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 11 }, { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, ] } @@ -145,12 +145,26 @@ def remove_arg(name) { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 33 }, { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, ] } + let(:backends_with_non_matching_weight_duplicates) { [ + { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 11 }, + { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 33 }, + { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, + ] } it 'updates backends only when weights change' do expect(subject).to receive(:'reconfigure!').exactly(:twice) - expect(subject.send(:set_backends, backends)).to equal(true) - expect(subject.backends).to eq(backends) + expect(subject.send(:set_backends, backends_with_weight)).to equal(true) + expect(subject.backends).to eq(backends_with_weight) expect(subject.send(:set_backends, non_matching_weight_backends)).to equal(true) expect(subject.backends).to eq(non_matching_weight_backends) + expect(subject.send(:set_backends, non_matching_weight_backends)).to equal(false) + expect(subject.backends).to eq(non_matching_weight_backends) + end + it 'ignores duplicates even with non-matching weights' do + expect(subject).to receive(:'reconfigure!').exactly(:once) + expect(subject.send(:set_backends, backends_with_weight)).to equal(true) + expect(subject.backends).to eq(backends_with_weight) + expect(subject.send(:set_backends, backends_with_non_matching_weight_duplicates)).to equal(false) + expect(subject.backends).to eq(backends_with_weight) end end end From 6d9b95a3f4740e695e6e6cb710f5668e1cd5801f Mon Sep 17 00:00:00 2001 From: Cory Minkovich Date: Tue, 23 Feb 2016 17:31:05 -0800 Subject: [PATCH 3/4] Remove option ignore_weights so weights will be used by default and change log level for duplicate weights. --- lib/synapse/haproxy.rb | 5 ++--- spec/lib/synapse/service_watcher_base_spec.rb | 2 +- spec/support/minimum.conf.yaml | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/synapse/haproxy.rb b/lib/synapse/haproxy.rb index 8fb9f970..496f12ee 100644 --- a/lib/synapse/haproxy.rb +++ b/lib/synapse/haproxy.rb @@ -538,7 +538,6 @@ def initialize(opts) @opts['do_writes'] = true unless @opts.key?('do_writes') @opts['do_socket'] = true unless @opts.key?('do_socket') @opts['do_reloads'] = true unless @opts.key?('do_reloads') - @opts['ignore_weights'] = true unless @opts.key?('ignore_weights') # how to restart haproxy @restart_interval = @opts.fetch('restart_interval', 2).to_i @@ -749,10 +748,10 @@ def generate_backend_stanza(watcher, config) end end b = "#{b} #{watcher.haproxy['server_options']}" if watcher.haproxy['server_options'] - if !@opts['ignore_weights'] && backend.has_key?('weight') + if backend.has_key?('weight') # Check if server_options already contains weight, is so log a warning if watcher.haproxy['server_options'].include? 'weight' - log.info "synapse: weight is defined by server_options and by nerve" + log.warn "synapse: weight is defined by server_options and by nerve" end weight = backend['weight'].to_i b = "#{b} weight #{weight}" diff --git a/spec/lib/synapse/service_watcher_base_spec.rb b/spec/lib/synapse/service_watcher_base_spec.rb index 9b7bcbcf..42621575 100644 --- a/spec/lib/synapse/service_watcher_base_spec.rb +++ b/spec/lib/synapse/service_watcher_base_spec.rb @@ -136,7 +136,7 @@ def remove_arg(name) end end - context 'with ignore_weights set to false' do + context 'with weights defined' do let(:backends_with_weight) { [ { 'name' => 'server1', 'host' => 'server1', 'port' => 1111, 'weight' => 11 }, { 'name' => 'server2', 'host' => 'server2', 'port' => 2222, 'weight' => 22 }, diff --git a/spec/support/minimum.conf.yaml b/spec/support/minimum.conf.yaml index 87775393..5df62fb9 100644 --- a/spec/support/minimum.conf.yaml +++ b/spec/support/minimum.conf.yaml @@ -22,7 +22,6 @@ haproxy: config_file_path: "/etc/haproxy/haproxy.cfg" do_writes: false do_reloads: false - ignore_weights: false global: - global_test_option From 42c4d488b692f46697bd46abd4d184c3a76b25ec Mon Sep 17 00:00:00 2001 From: Tiffany Low Date: Mon, 21 Mar 2016 11:43:56 -0700 Subject: [PATCH 4/4] Restart haproxy if backend 'weights' setting is changed --- README.md | 2 +- lib/synapse/haproxy.rb | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1813e822..1e8c00c5 100644 --- a/README.md +++ b/README.md @@ -273,7 +273,7 @@ This section is its own hash, which should contain the following keys: * `backend_order`: optional: how backends should be ordered in the `backend` stanza. (default is shuffling). Setting to `asc` means sorting backends in ascending alphabetical order before generating stanza. `desc` means descending alphabetical order. `no_shuffle` means no shuffling or sorting. * `shared_frontend`: optional: haproxy configuration directives for a shared http frontend (see below) * `cookie_value_method`: optional: default value is `name`, it defines the way your backends receive a cookie value in http mode. If equal to `hash`, synapse hashes backend names on cookie value assignation of your discovered backends, useful when you want to use haproxy cookie feature but you do not want that your end users receive a Set-Cookie with your server name and ip readable in clear. -* `ignore_weights`: optional: stops haproxy backend 'weight' options being generated, even if the Nerve registrations contain this information. This will cause all backend servers to be treated equally by haproxy. This defaults to true so weights will *NOT* be used by default. +* `ignore_weights`: optional: stops haproxy backend 'weight' options being generated, even if the Nerve registrations contain this information. This will cause all backend servers to be treated equally by haproxy. This defaults to true so weights will *NOT* be used by default. ### Configuring HAProxy ### diff --git a/lib/synapse/haproxy.rb b/lib/synapse/haproxy.rb index 496f12ee..41f83ed0 100644 --- a/lib/synapse/haproxy.rb +++ b/lib/synapse/haproxy.rb @@ -714,6 +714,11 @@ def generate_backend_stanza(watcher, config) log.info "synapse: restart required because haproxy_server_options changed for #{backend_name}" @restart_required = true end + if (old_backend.fetch('weight', "") != + backend.fetch('weight', "")) + log.info "synapse: restart required because weight changed for #{backend_name}" + @restart_required = true + end end backends[backend_name] = backend.merge('enabled' => true) end @@ -749,9 +754,12 @@ def generate_backend_stanza(watcher, config) end b = "#{b} #{watcher.haproxy['server_options']}" if watcher.haproxy['server_options'] if backend.has_key?('weight') - # Check if server_options already contains weight, is so log a warning - if watcher.haproxy['server_options'].include? 'weight' - log.warn "synapse: weight is defined by server_options and by nerve" + # Check if server_options/haproxy_server_options already contains weight, if so log a warning + if watcher.haproxy.fetch('server_options', '').include? 'weight' + log.warn "synapse: weight is defined in both server_options and nerve. nerve weight takes precedence" + end + if backend['haproxy_server_options'] and backend['haproxy_server_options'].include? 'weight' + log.warn "synapse: weight is defined in both haproxy_server_options and nerve. nerve weight takes precedence" end weight = backend['weight'].to_i b = "#{b} weight #{weight}"