mesos-dns icon indicating copy to clipboard operation
mesos-dns copied to clipboard

Proposal: Add support for multiple resolver backends (eg consul)

Open cheuschober opened this issue 8 years ago • 3 comments

This is a continuation of a conversation back at the end of January in which we discussed how the the real magic of mesos-dns is its records generator that parses state to generate the appropriate state records. This generator was used (by means of a copy) in the mesos-consul project to provide most of its state parsing functions.

This proposal is to further segment the resolver and components to allow mesos-dns to support the records generation and population of any number of other backends with first-class support for consul.

Because code is stronger than words:

https://github.com/Virtustream-OSS/mesos-dns/commit/eeb79da5b7e823c76d9549900ac69e9001a88511

The above is not just a proof of concept, we've been running it in production for roughly three months at this point while we've been seeking the appropriate legal clearances to upstream.

There's a bit of cruft in all the consul libs that have to get vendored (oh, GoLang!) but this was just rebased today and cleanly delineates the consul components, mesos-dns's current resolver (called builtin) and the records generator.

Functionally, I could see this as going a couple ways:

  1. If you don't want to maintain support for the consul resolver within this project, reorganizing the records generator and builtin resolver in the fashion demonstrated in the above commit would allow the records generator to be used in other projects as its own library. At that point I would recommend separating the codebase too just to prevent accidental dependency bleed between the resolver and records generator. At that point we could discuss if we want the config magic and main event loop to also follow that library or if they'd have to be reimplemented in every child project. In this scenario, we'd want to coordinate this with an update in mesos-consul to use the new codebase.

  2. You adopt the featureset part-and-parcel and consul backend support with all the nifty new features we added go into mesos-dns and it becomes the defacto service discovery standard.

We hope you like what you see.

FYI: @sargun @joestein @ChrisAubuchon

cheuschober avatar May 28 '16 00:05 cheuschober

Given that I don't see much conversation and as time moves on, resolving the fork grows increasingly more complicated, should I take it as a mandate that this is not a desired feature or do we just go and open the PR?

cheuschober avatar Jun 28 '16 19:06 cheuschober

Can we split this PR into a couple parts? You guys did a lot of good work on tooling that I'd like to merge, if you submit that in a separate PR, I'd be happy to merge it. That's things like:

  • adding a docker build environment and Makefile to speed and standardize testing
  • updating the golang version to latest stable
  • adding gofmt test targets

The Mesos-Consul stuff I think is more complicated and not straightforward to merge. I'm actually having a hard time pulling it apart. Do you have a set of patches that are isolated to it?

sargun avatar Jul 05 '16 23:07 sargun

We stumbled into the right solutions after knocking all the bugs loose so there's no clean set of history unless you'd like a few hundred extra commits. That-said, everything isolated to consul is here:

https://github.com/Virtustream-OSS/mesos-dns/tree/feature/modular-backend/resolvers/consul

Any other changes made to tree pertain to making the backend modular, in general. References to consul anywhere in these parts we would strip if we broke apart the PR into a modular-backend and consul-backend pair of PR's. References to consul would be re-added in the consul-backend PR to support discovery and loading of that backend. Since that's a fair bit of work I'd like to see a :+1: on at least the modular-backend portions prior to ripping those fellas apart. Forks with rewrites and this much time between them are, alas, never easy to merge.

cheuschober avatar Jul 06 '16 00:07 cheuschober