Add marathon watcher #101
Add marathon watcher #101
Conversation
In case the URL is wrong, this will not hang startup.
We don't want to load-balance across tasks that aren't started.
Sometimes it does JSON by default, sometimes not.
Hopefully these specs will catch some of the most important behavioral changes. Rather than individually stubbing a bunch of Net::HTTP stuff, I pulled in webmock as a development dependency. I also found it easiest to add a silly method Synapse::MarathonWatcher#only_run_once? which is hard-coded to false but stubbed to return true in the test suite.
They can't be sorted by a Hash, since that is not something Ruby can do.
This increases the resiliency in the case that marathon cannot be immediately found. A connection attempt will be made at the start of each watch interval.
| @@ -4,6 +4,7 @@ | |||
| require "synapse/service_watcher/dns" | |||
| require "synapse/service_watcher/docker" | |||
| require "synapse/service_watcher/zookeeper_dns" | |||
| require "synapse/service_watcher/marathon" | |||
There was a problem hiding this comment.
Can you rebase to pick up the auto-loading watcher code? See https://github.com/airbnb/synapse/blob/master/lib/synapse/service_watcher/README.md for the new specification
|
@tdooner Awesome, I think with a little bit of cleanup this is good for merge. Since I'm not too worried about changes in the marathon API (like I am with etcd, docker, etc ...) that might make this direct request approach problematic long term, I think I prefer this to one that adds a dependency on marathon_client. What are your thoughts having run marathon in production for a while? Does it seem like they are good about keeping backwards compatibility? |
| { | ||
| 'name' => task['host'], | ||
| 'host' => task['host'], | ||
| 'port' => task['ports'].first, |
There was a problem hiding this comment.
Can you do a port_index like the other PR? I like that flexibility.
There was a problem hiding this comment.
Good idea, we actually needed this too.
* upstream/master: Fixup mistake in watcher README Update the README concerning ELB's weaknesses Fix minor Ruby 1.9 vs Ruby 2.X compat in install instructions Clarify that the first code block is not installing Ruby Update README.md with better installation directions Allow arbitrary haproxy_server_options to be supplied to HAProxy. 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 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 allow `server_port_override` to be an int since the other `port` config options can be ints Linking from main README to service watcher README Allow pluggable watchers Handle path going away properly Do not unregister the callback in the zk watcher Some minor documentation fixups Explicit watcher haproxy backend names [travis] switch to new infrastructure bump version to v0.12.1 bump version to v0.12.0 Fixed wording of some documentation, added missing options Address feedback from igor and schleyfox Fixups for merge to airbnb/master Turns out it's important to handle session disconnects correctly Try out :per_callback threads and get more debug information Add support for the weight key added in nerve Fix bug in caching logic. Add state file. Rate limit restarts but not stats socket updates ZooKeeper connection pooling. Increase HAProxy restart interval. Revert "Add rate limiter." Add rate limiter. Allow the option allredisp option to haproxy. Explicitly deduplicate registrations Allow registrations to be manifested on the file system Add 'use_previous_backends' option. test synapse on 2.0.0, 2.1.6, 2.2.2 also git ignore .ruby-version update zookeeper dependency to support newer rubies typo synapse expects arrays under shared_frontend, hosts and backend verify doubles run transpec to convert specs to new format fix some broken specs and deprecation warnings bump rspec version Conflicts: Gemfile.lock lib/synapse/service_watcher.rb
A bit has changed - mostly adding the ServiceWatcher class in the hierarchy. safe_yaml needs upgraded because of: dtao/safe_yaml#67
In case a new version of marathon comes out, and the path changes from
/v2/apps/%{app}/tasks, this will allow administrators to not need to
fork synapse.
Rescuing `Exception` instead of `StandardError` is a bad idea. Also setting connection to nil will ensure we reconnect to marathon.
And thus fending off the thundering herd.
7d51288 moved this into ServiceWatcher::Base, so this logic can be removed from here.
This will allow users to expose multiple backends in the same Marathon task.
|
All done! My thoughts on having run this in production: it is a lot of requests to the Marathon API, since it scales up by the product of the number of machines and the number of services you have. For small deployments and relatively-few backend tasks, Marathon will be able to take the heat. At Yelp or Airbnb scale? This won't work. At the time of writing this initially, only the Marathon eventSubscriptions existed -- this was a pretty clunky way to request Marathon POST event updates as they happen. This isn't a good fit for Synapse. But since then, Marathon has introduced the |
I wrote this before I noticed #87, so in case you'd like a version that doesn't add a runtime dependency on
marathon_clientthen this is your PR. Otherwise these are pretty much identical PRs and I'll close this out if you merge the other one.