akka / akka-management

Akka Management is a suite of tools for operating Akka Clusters.
https://doc.akka.io/docs/akka-management/
Other
254 stars 160 forks source link

Revisit discovery-consul #392

Open jrudolph opened 5 years ago

jrudolph commented 5 years ago

It seems that discovery consul support will work for bootstrapping. I don't know much about consul but the suggested way of registering entries seems weird.

See this test:

https://github.com/akka/akka-management/blob/ba7481139172a78c7d90639edb8b96ec699bea70/discovery-consul/src/test/scala/akka/cluster/bootstrap/discovery/ConsulDiscoverySpec.scala#L44-L49

Also the documentation in the reference.conf uses confusing terms like "cluster system":

https://github.com/akka/akka-management/blob/ba7481139172a78c7d90639edb8b96ec699bea70/discovery-consul/src/main/resources/reference.conf#L19

It's also doubtful if the module is needed at all as Consul prominently offers a DNS server to query services. The hardest thing about using Consul with discovery / bootstrap would then rather be how to add entries to it when starting up an ActorSystem rather then actually doing the query.

Therefore, it might make sense to remove the module and replace it with some instructions / recipies how you need to setup entries in Consul to use the DNS discovery module for bootstrapping.

@mlosiewicz-pl what do you think about that suggestion?

mlosiewicz-pl commented 5 years ago

I agree that in ideal world this would be enough, but since there is a lot of ways people use consul this module is actually handy in some situations. I use it in my service for example because the architecture of the system does not allow using consul dns.

In my opinion adding entries to Consul is not a responsibility of the application but the "scheduler" or "orchestrator" system (Nomad - from the same company that Consul comes - for example defines the service registration in Consul based on the file with service description instead of relying on application to register itself.

The weird part of registering entries is done this way to kind of bypass Consul limitations related to its tagging of services. If you have any ideas how to "fix" it or make less weird I'd happily do that.

To sum up my chaotic response:

  1. Consul dns is a good way of bootstrapping Akka Cluster when your architecture allows for it
  2. When you have no access to Consul dns you have to have a way to define ports of Akka Management endpoints as well as the name of the actor system in the cluster (you can have multiple clusters from one set of services - think blue/green deployment)
  3. If you or anyone has a better idea on how to name the configuration parameters of how to define this in another way I'm more than happy to help with refactoring the module.
jrudolph commented 5 years ago

Thanks for the information, @mlosiewicz-pl.

WayneWang12 commented 5 years ago

Can we have a better test? Like projects in integration-test?

WayneWang12 commented 5 years ago

And can we have a register function for registration as all messages we need is in configuration, we should do registertion with a simple function.