Add 'use_previous_backends' option.

README.md

@@ -198,6 +198,10 @@ If you do not list any default servers, no proxy will be created.  The
 `default_servers` will also be used in addition to discovered servers if the
 `keep_default_servers` option is set.
 
+If you do not list any `default_servers`, and all backends for a service
+disappear then the previous known backends will be used.  Disable this behavior
+by unsetting `use_previous_backends`.
+
 #### The `haproxy` Section ####
 
 This section is its own hash, which should contain the following keys:
@@ -335,5 +339,5 @@ end
 3. Implement the `start` and `validate_discovery_opts` methods
 4. Implement whatever additional methods your discovery requires
 
-When your watcher detects a list of new backends, they should be written to `@backends`.
-You should then call `@synapse.configure` to force synapse to update the HAProxy config.
+When your watcher detects a list of new backends, you should call `set_backends` to
+store the new backends and update the HAProxy config.

lib/synapse/service_watcher/base.rb

@@ -1,3 +1,4 @@
+require 'set'
 require 'synapse/log'
 
 module Synapse
@@ -42,6 +43,10 @@ def initialize(opts={}, synapse)
 
       @keep_default_servers = opts['keep_default_servers'] || false
 
+      # If there are no default servers and a watcher reports no backends, then
+      # use the previous backends that we already know about.
+      @use_previous_backends = opts.fetch('use_previous_backends', true)
+
       # set a flag used to tell the watchers to exit
       # this is not used in every watcher
       @should_exit = false
@@ -95,13 +100,41 @@ def validate_discovery_opts
     end
 
     def set_backends(new_backends)
-      if @keep_default_servers
-        @backends = @default_servers + new_backends
+      new_backends = (new_backends + (@keep_default_servers ? @default_servers : [])).uniq
+
+      if new_backends.to_set == @backends.to_set
+        return false
+      end
+
+      if new_backends.empty?
+        if @default_servers.empty?
+          if @use_previous_backends
+            # Discard this update
+            log.warn "synapse: no default servers for service #{@name};" \
+              "  using previous backends: #{@backends.inspect}"
+            return false
+          else
+            log.warn "synapse: no default servers for service #{@name} and" \
+              " 'use_previous_backends' is disabled; dropping all backends"
+            @backends.clear
+          end
+        else
+          log.warn "synapse: no backends for service #{@name};" \
+            " using default servers: #{@default_servers.inspect}"
+          @backends = @default_servers
+        end
       else
+        log.info "synapse: discovered #{new_backends.length} backends for service #{@name}"
         @backends = new_backends
       end
+
+      reconfigure!
+
+      return true
     end
 
+    # Subclasses should not invoke this directly; it's only exposed so that it
+    # can be overridden in subclasses.
     def reconfigure!
       @synapse.reconfigure!
     end

lib/synapse/service_watcher/dns.rb

@@ -89,21 +89,7 @@ def configure_backends(servers)
         end
       end
 
-      if new_backends.empty?
-        if @default_servers.empty?
-          log.warn "synapse: no backends and no default servers for service #{@name};" \
-            " using previous backends: #{@backends.inspect}"
-        else
-          log.warn "synapse: no backends for service #{@name};" \
-            " using default servers: #{@default_servers.inspect}"
-          @backends = @default_servers
-        end
-      else
-        log.info "synapse: discovered #{new_backends.length} backends for service #{@name}"
-        set_backends(new_backends)
-      end
-
-      reconfigure!
+      set_backends(new_backends)
     end
   end
 end

lib/synapse/service_watcher/docker.rb

@@ -23,16 +23,10 @@ def validate_discovery_opts
     end
 
     def watch
-      last_containers = []
       until @should_exit
         begin
           start = Time.now
-          current_containers = containers
-          unless last_containers == current_containers
-            last_containers = current_containers
-            configure_backends(last_containers)
-          end
-
+          set_backends(containers)
           sleep_until_next_check(start)
         rescue Exception => e
           log.warn "synapse: error in watcher thread: #{e.inspect}"
@@ -98,23 +92,5 @@ def containers
       log.warn "synapse: error while polling for containers: #{e.inspect}"
       []
     end
-
-    def configure_backends(new_backends)
-      if new_backends.empty?
-        if @default_servers.empty?
-          log.warn "synapse: no backends and no default servers for service #{@name};" \
-            " using previous backends: #{@backends.inspect}"
-        else
-          log.warn "synapse: no backends for service #{@name};" \
-            " using default servers: #{@default_servers.inspect}"
-          @backends = @default_servers
-        end
-      else
-        log.info "synapse: discovered #{new_backends.length} backends for service #{@name}"
-        set_backends(new_backends)
-      end
-      reconfigure!
-    end
-
   end
 end

lib/synapse/service_watcher/ec2tag.rb

@@ -54,20 +54,12 @@ def validate_discovery_opts
     end
 
     def watch
-      last_backends = []
       until @should_exit
         begin
           start = Time.now
-          current_backends = discover_instances
-
-          if last_backends != current_backends
+          if set_backends(discover_instances)
             log.info "synapse: ec2tag watcher backends have changed."
-            last_backends = current_backends
-            configure_backends(current_backends)
-          else
-            log.info "synapse: ec2tag watcher backends are unchanged."
           end
-
           sleep_until_next_check(start)
         rescue Exception => e
           log.warn "synapse: error in ec2tag watcher thread: #{e.inspect}"
@@ -111,23 +103,6 @@ def instances_with_tags(tag_name, tag_value)
         .tagged_values(tag_value)
         .select { |i| i.status == :running }
     end
-
-    def configure_backends(new_backends)
-      if new_backends.empty?
-        if @default_servers.empty?
-          log.warn "synapse: no backends and no default servers for service #{@name};" \
-            " using previous backends: #{@backends.inspect}"
-        else
-          log.warn "synapse: no backends for service #{@name};" \
-            " using default servers: #{@default_servers.inspect}"
-          @backends = @default_servers
-        end
-      else
-        log.info "synapse: discovered #{new_backends.length} backends for service #{@name}"
-        @backends = new_backends
-      end
-      @synapse.reconfigure!
-    end
   end
 end
 

lib/synapse/service_watcher/zookeeper.rb

@@ -45,7 +45,7 @@ def create(path)
       @zk.create(path, ignore: :node_exists)
     end
 
-    # find the current backends at the discovery path; sets @backends
+    # find the current backends at the discovery path
     def discover
       log.info "synapse: discovering backends for service #{@name}"
 
@@ -69,17 +69,7 @@ def discover
         end
       end
 
-      if new_backends.empty?
-        if @default_servers.empty?
-          log.warn "synapse: no backends and no default servers for service #{@name}; using previous backends: #{@backends.inspect}"
-        else
-          log.warn "synapse: no backends for service #{@name}; using default servers: #{@default_servers.inspect}"
-          @backends = @default_servers
-        end
-      else
-        log.info "synapse: discovered #{new_backends.length} backends for service #{@name}"
-        set_backends(new_backends)
-      end
+      set_backends(new_backends)
     end
 
     # sets up zookeeper callbacks if the data at the discovery path changes
@@ -103,8 +93,6 @@ def watcher_callback
         watch
         # Rediscover
         discover
-        # send a message to calling class to reconfigure
-        reconfigure!
       end
     end
 

spec/lib/synapse/service_watcher_base_spec.rb

@@ -6,7 +6,7 @@ class Synapse::BaseWatcher
 
 describe Synapse::BaseWatcher do
   let(:mocksynapse) { double() }
-  subject { Synapse::BaseWatcher.new(args, mocksynapse) } 
+  subject { Synapse::BaseWatcher.new(args, mocksynapse) }
   let(:testargs) { { 'name' => 'foo', 'discovery' => { 'method' => 'base' }, 'haproxy' => {} }}
 
   def remove_arg(name)
@@ -37,18 +37,75 @@ def remove_arg(name)
     end
   end
 
-  context "with default_servers" do
-    default_servers = ['server1', 'server2']
+  context 'set_backends test' do
+    default_servers = ['default_server_1', 'default_server_2']
     let(:args) { testargs.merge({'default_servers' => default_servers}) }
-    it('sets default backends to default_servers') { expect(subject.backends).to equal(default_servers) }
 
-    context "with keep_default_servers set" do
-      let(:args) { testargs.merge({'default_servers' => default_servers, 'keep_default_servers' => true}) }
-      let(:new_backends) { ['discovered1', 'discovered2'] }
+    it 'sets backends' do
+      expect(subject).to receive(:'reconfigure!').exactly(:once)
+      backends = ['server1', 'server2']
+      expect(subject.send(:set_backends, backends)).to equal(true)
+      expect(subject.backends).to eq(backends)
+    end
+
+    it 'removes duplicate backends' do
+      expect(subject).to receive(:'reconfigure!').exactly(:once)
+      backends = ['server1', 'server2']
+      duplicate_backends = backends + backends
+      expect(subject.send(:set_backends, duplicate_backends)).to equal(true)
+      expect(subject.backends).to eq(backends)
+    end
+
+    it 'sets backends to default_servers if no backends discovered' do
+      expect(subject).to receive(:'reconfigure!').exactly(:once)
+      expect(subject.send(:set_backends, [])).to equal(true)
+      expect(subject.backends).to eq(default_servers)
+    end
+
+    context 'with no default_servers' do
+      let(:args) { remove_arg 'default_servers' }
+      it 'uses previous backends if no default_servers set' do
+        expect(subject).to receive(:'reconfigure!').exactly(:once)
+        backends = ['server1', 'server2']
+        expect(subject.send(:set_backends, backends)).to equal(true)
+        expect(subject.send(:set_backends, [])).to equal(false)
+        expect(subject.backends).to eq(backends)
+      end
+    end
+
+    context 'with no default_servers set and use_previous_backends disabled' do
+      let(:args) {
+        remove_arg 'default_servers'
+        testargs.merge({'use_previous_backends' => false})
+      }
+      it 'removes all backends if no default_servers set and use_previous_backends disabled' do
+        expect(subject).to receive(:'reconfigure!').exactly(:twice)
+        backends = ['server1', 'server2']
+        expect(subject.send(:set_backends, backends)).to equal(true)
+        expect(subject.backends).to eq(backends)
+        expect(subject.send(:set_backends, [])).to equal(true)
+        expect(subject.backends).to eq([])
+      end
+    end
+
+    it 'calls reconfigure only once for duplicate backends' do
+      expect(subject).to receive(:'reconfigure!').exactly(:once)
+      backends = ['server1', 'server2']
+      expect(subject.send(:set_backends, backends)).to equal(true)
+      expect(subject.backends).to eq(backends)
+      expect(subject.send(:set_backends, backends)).to equal(false)
+      expect(subject.backends).to eq(backends)
+    end
 
+    context 'with keep_default_servers set' do
+      let(:args) {
+        testargs.merge({'default_servers' => default_servers, 'keep_default_servers' => true})
+      }
       it('keeps default_servers when setting backends') do
-        subject.send(:set_backends, new_backends)
-        expect(subject.backends).to eq(default_servers + new_backends)
+        backends = ['server1', 'server2']
+        expect(subject).to receive(:'reconfigure!').exactly(:once)
+        expect(subject.send(:set_backends, backends)).to equal(true)
+        expect(subject.backends).to eq(backends + default_servers)
       end
     end
   end

spec/lib/synapse/service_watcher_docker_spec.rb

@@ -46,12 +46,7 @@ def add_arg(name, value)
     end
     it('has a happy first run path, configuring backends') do
       expect(subject).to receive(:containers).and_return(['container1'])
-      expect(subject).to receive(:configure_backends).with(['container1'])
-      subject.send(:watch)
-    end
-    it('does not call configure_backends if there is no change') do
-      expect(subject).to receive(:containers).and_return([])
-      expect(subject).to_not receive(:configure_backends)
+      expect(subject).to receive(:set_backends).with(['container1'])
       subject.send(:watch)
     end
   end
@@ -65,33 +60,6 @@ def add_arg(name, value)
     end
   end
 
-  context "configure_backends tests" do
-    before(:each) do
-      expect(subject.synapse).to receive(:'reconfigure!').at_least(:once)
-    end
-    it 'runs' do
-      expect { subject.send(:configure_backends, []) }.not_to raise_error
-    end
-    it 'sets backends right' do
-      subject.send(:configure_backends, ['foo'])
-      expect(subject.backends).to eq(['foo'])
-    end
-    it 'resets to default backends if no container found' do
-      subject.default_servers = ['fallback1']
-      subject.send(:configure_backends, ['foo'])
-      expect(subject.backends).to eq(['foo'])
-      subject.send(:configure_backends, [])
-      expect(subject.backends).to eq(['fallback1'])
-    end
-    it 'does not reset to default backends if there are no default backends' do
-      subject.default_servers = []
-      subject.send(:configure_backends, ['foo'])
-      expect(subject.backends).to eq(['foo'])
-      subject.send(:configure_backends, [])
-      expect(subject.backends).to eq(['foo'])
-    end
-  end
-
   context "rewrite_container_ports tests" do
     it 'doesnt break if Ports => nil' do
         subject.send(:rewrite_container_ports, nil)