Added support for discovery by enumerating pods via Kubernetes API #29
Conversation
Added support for Kubernetes API discovery
|
Wow! Thanks for your contribution @JessieWadman . Sorry for late reply. I'll try to review your PR over the weekend. |
| private readonly ILoggingAdapter logger = Context.GetLogger(); | ||
|
|
||
| private readonly string actorSystemName; | ||
| private readonly string labelSelector; |
There was a problem hiding this comment.
This field is private and it's not used anywhere.
| private Address DeterminePodAddress(V1Pod pod) | ||
| { | ||
| if (!int.TryParse(pod.GetAnnotation("akka.remote.dot-netty.tcp.port"), out var port)) | ||
| port = Cluster.SelfAddress.Port ?? 0; |
There was a problem hiding this comment.
I don't think, this is a good approach. This method is used for setup the initial list of nodes to contact with. The problem here is that port 0 is a valid port only for local node, but not for any remote connection. Depending on the case this should either result in a:
- Info saying that pod X was not annotated, so we assume current node port. In case when port is not specified throw an exception. This could however end up in weird behavior when current node was configured to use dynamic port (0), as it will be different for every OS process.
- Warning (saying that pod X was not properly annotated with
akka.remote.dot-netty.tcp.port) and filtering out that pod's address. I think this sounds as the best option. - Exception and shutdown the entire actor system.
Another issue is annotation itself: it will work only with akka.remote.dot-netty.tcp.port, which is the most common but not necessarily the only valid transport option. In general this should take into account the current configured transport - it can we taken from configuration using akka.remote.enabled-transports path (see: here).
You can specify namespace and a label selector to use to find pods, as well as annotate pods with akka tcp port (optional).