Support multiple DNS queries

[davydog187]

This PR extends the API to allow for a list of DNS queries. This is useful if you have multiple services that you want to cluster together

[davydog187]

This PR needs a little more thought due to the basename restriction on connected nodes. I'll try to give this more time tomorrow

[joshk]

This is really awesome! I was going to create a similar PR but was happy to see it was already done!

Would it be possible to accept a list of domains separated by commas? eg. "myapp.internal" or ["foo.internal", "bar.internal"] or "foo.internal,bar.internal"

This would make it a lot easier to configure via environment variables. eg.DNS_CLUSTER_QUERY="foo.internal,bar.internal"

[davydog187]

hello @joshk! I still need to make some tweaks to this PR before it can be merged, unfortunately.

Would it be possible to accept a list of domains separated by commas? eg. "myapp.internal" or ["foo.internal", "bar.internal"] or "foo.internal,bar.internal"

It is certainly possible, although I don't recommend introducing this to the API of this library. While not explicitly mentioned in the anti-patterns guides, I believe it is trivial to do the string splitting in your application code. e.g.

# config/runtime.exs
config :my_app, dns_cluster_query: String.split(System.get_env("DNS_CLUSTER_QUERY"), ",")

# lib/application.ex
{DNSCluster, query: Application.get_env(:my_app, dns_cluster_query)}

I think this keeps the library API simple while enabling maximum flexibility for application developers using this library

[davydog187]

Also, I suppose that it is valid to have a , in your DNS query, so that's another reason not to support it

[davydog187]

@josevalim @chrismccord I have updated this PR to add support for connecting to nodes with a basename different than the host. To support this, I've extended the API to allow the user to specify a query, a {basename, query} or a list of either.

I've updated the documentation to reflect this change, please let me know if you want to go a different direction. I'm not an expert in erlang distribution, but I'm not aware of another parameter that would warrant a more complicated datastructure. If that is the case, we should probably use a map or struct instead.

[davydog187]

I'm running this branch in production and it works great! (Redacted IPs)

iex> Node.list()
[
    :"sauron-prod@fdaa:1:aaaaaaaaaaaaaaaaaaaaa:2",
    :"leaderboard-prod@fdaa:1:aaaaaaaaaaaaaaaaaaaaaa:2",
    :"leaderboard-prod@fdaa:1:aaaaaaaaaaaaaaaaaaaaaa:2"
]

[davydog187]

For anyone interested in this feature, I encourage you to test it out! I've been running it in production for two weeks now without any issue

[schrockwell]

@davydog187 Thanks, I'm trying this on Fly right now. To set it up, I had to do a couple things:

1. In rel/env.sh.eex, set export RELEASE_NODE="${FLY_APP_NAME}@${FLY_PRIVATE_IP}" (note that $FLY_IMAGE_REF has been removed from the node basename)
2. Parse the $DNS_CLUSTER_QUERIES which is in the format "[email protected] [email protected]":

dns_cluster_queries =
  System.get_env("DNS_CLUSTER_QUERIES", "")
  |> String.split(" ", trim: true)
  |> Enum.map(fn query ->
    case String.split(query, "@") do
      [basename, domain] -> {basename, domain}
      [domain] -> domain
    end
  end)

3. Set DNS_CLUSTER_QUERIES in both apps' TOML configs and deploy.

[davydog187]

Hey @schrockwell 👋🏼

That all sounds well and good, however, I would personally just hardcode the queries in your TOML file, you shouldn't need to parse them given that they should be static per environment

[davydog187]

Sorry to ping you @chrismccord but is there anything holding up this PR besides being a low priority?

[yordis]

Hey folks, any opportunity to get this one in? 🙏🏻

[chrismccord]

❤️❤️❤️🐥🔥