From c59116e06206a38cccb553747df69b7fb4ec6256 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Tue, 23 Jun 2015 17:12:57 -0400 Subject: [PATCH 1/3] allow `server_port_override` to be an int since the other `port` config options can be ints --- lib/synapse/service_watcher/ec2tag.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/synapse/service_watcher/ec2tag.rb b/lib/synapse/service_watcher/ec2tag.rb index 43f3e192..40996aa4 100644 --- a/lib/synapse/service_watcher/ec2tag.rb +++ b/lib/synapse/service_watcher/ec2tag.rb @@ -41,7 +41,7 @@ def validate_discovery_opts "Missing server_port_override for service #{@name} - which port are backends listening on?" end - unless @haproxy['server_port_override'].match(/^\d+$/) + unless @haproxy['server_port_override'].to_s.match(/^\d+$/) raise ArgumentError, "Invalid server_port_override value" end From d5d580aaf7f31ac0fb288513037ad30617669662 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Tue, 23 Jun 2015 17:32:13 -0400 Subject: [PATCH 2/3] Don't have synapse require the aws variables - credentials should be able to be nil to allow using iam instance profile. Updates tests to reflect --- lib/synapse/service_watcher/ec2tag.rb | 13 ++++++++----- .../synapse/service_watcher_ec2tags_spec.rb | 18 +++++++++--------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/synapse/service_watcher/ec2tag.rb b/lib/synapse/service_watcher/ec2tag.rb index 40996aa4..7c5d0d1e 100644 --- a/lib/synapse/service_watcher/ec2tag.rb +++ b/lib/synapse/service_watcher/ec2tag.rb @@ -45,11 +45,14 @@ def validate_discovery_opts raise ArgumentError, "Invalid server_port_override value" end - # Required, but can use well-known environment variables. - %w[aws_access_key_id aws_secret_access_key aws_region].each do |attr| - unless (@discovery[attr] || ENV[attr.upcase]) - raise ArgumentError, "Missing #{attr} option or #{attr.upcase} environment variable" - end + # aws region is optional in the SDK, aws will use a default value if not provided + unless @discovery['aws_region'] || ENV['AWS_REGION'] + log.info "aws region is missing, will use default" + end + # access key id & secret are optional, might be using IAM instance profile for credentials + unless ((@discovery['aws_access_key_id'] || ENV['aws_access_key_id']) \ + && (@discovery['aws_secret_access_key'] || ENV['aws_secret_access_key'] )) + log.info "aws access key id & secret not set in config or env variables for service #{name}, will attempt to use IAM instance profile" end end diff --git a/spec/lib/synapse/service_watcher_ec2tags_spec.rb b/spec/lib/synapse/service_watcher_ec2tags_spec.rb index 30aeab37..e04c8fb2 100644 --- a/spec/lib/synapse/service_watcher_ec2tags_spec.rb +++ b/spec/lib/synapse/service_watcher_ec2tags_spec.rb @@ -87,20 +87,20 @@ def munge_haproxy_arg(name, new_value) end context 'when missing arguments' do - it 'complains if aws_region is missing' do + it 'does not break if aws_region is missing' do expect { - Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_region'), mock_synapse) - }.to raise_error(ArgumentError, /Missing aws_region/) + Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_region'), mock_synapse) + }.not_to raise_error end - it 'complains if aws_access_key_id is missing' do + it 'does not break if aws_access_key_id is missing' do expect { - Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_access_key_id'), mock_synapse) - }.to raise_error(ArgumentError, /Missing aws_access_key_id/) + Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_access_key_id'), mock_synapse) + }.not_to raise_error end - it 'complains if aws_secret_access_key is missing' do + it 'does not break if aws_secret_access_key is missing' do expect { - Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_secret_access_key'), mock_synapse) - }.to raise_error(ArgumentError, /Missing aws_secret_access_key/) + Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_secret_access_key'), mock_synapse) + }.not_to raise_error end it 'complains if server_port_override is missing' do expect { From 9d9e5ad6339f1bfe604e900c1255a3e61a5e35a4 Mon Sep 17 00:00:00 2001 From: Danny Cosson Date: Wed, 24 Jun 2015 09:53:37 -0400 Subject: [PATCH 3/3] adds specs for ec2tag watch method, and fixes bug where if an exception was raised the normal sleep got skipped, so it could get stuck calling the failing discover instances code in a tight loop every few ms --- lib/synapse/service_watcher/ec2tag.rb | 3 ++- .../synapse/service_watcher_ec2tags_spec.rb | 27 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/synapse/service_watcher/ec2tag.rb b/lib/synapse/service_watcher/ec2tag.rb index 7c5d0d1e..9df77490 100644 --- a/lib/synapse/service_watcher/ec2tag.rb +++ b/lib/synapse/service_watcher/ec2tag.rb @@ -63,10 +63,11 @@ def watch if set_backends(discover_instances) log.info "synapse: ec2tag watcher backends have changed." end - sleep_until_next_check(start) rescue Exception => e log.warn "synapse: error in ec2tag watcher thread: #{e.inspect}" log.warn e.backtrace + ensure + sleep_until_next_check(start) end end diff --git a/spec/lib/synapse/service_watcher_ec2tags_spec.rb b/spec/lib/synapse/service_watcher_ec2tags_spec.rb index e04c8fb2..bbcf15bd 100644 --- a/spec/lib/synapse/service_watcher_ec2tags_spec.rb +++ b/spec/lib/synapse/service_watcher_ec2tags_spec.rb @@ -89,17 +89,17 @@ def munge_haproxy_arg(name, new_value) context 'when missing arguments' do it 'does not break if aws_region is missing' do expect { - Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_region'), mock_synapse) + Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_region'), mock_synapse) }.not_to raise_error end it 'does not break if aws_access_key_id is missing' do expect { - Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_access_key_id'), mock_synapse) + Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_access_key_id'), mock_synapse) }.not_to raise_error end it 'does not break if aws_secret_access_key is missing' do expect { - Synapse::ServiceWatcher::EC2Watcher.new(remove_discovery_arg('aws_secret_access_key'), mock_synapse) + Synapse::ServiceWatcher::Ec2tagWatcher.new(remove_discovery_arg('aws_secret_access_key'), mock_synapse) }.not_to raise_error end it 'complains if server_port_override is missing' do @@ -122,6 +122,27 @@ def munge_haproxy_arg(name, new_value) let(:instance1) { FakeAWSInstance.new } let(:instance2) { FakeAWSInstance.new } + context 'watch' do + + it 'discovers instances, configures backends, then sleeps' do + fake_backends = [1,2,3] + expect(subject).to receive(:discover_instances).and_return(fake_backends) + expect(subject).to receive(:set_backends).with(fake_backends) { subject.stop } + expect(subject).to receive(:sleep_until_next_check) + subject.send(:watch) + end + + it 'sleeps until next check if discover_instances fails' do + expect(subject).to receive(:discover_instances) do + subject.stop + raise "discover failed" + end + expect(subject).to receive(:sleep_until_next_check) + subject.send(:watch) + end + + end + context 'using the AWS API' do let(:ec2_client) { double('AWS::EC2') } let(:instance_collection) { double('AWS::EC2::InstanceCollection') }