Network.ToEndPoint method does not seem to support a dns based endpoint

[natilivni]

Hello,

I am trying to change my cluster to run in containers and am having a problem with configuring the members. Since docker assigns them their own IPs and makes the nodes available through a dns system (eg https://mydockerraftnodeservice), I have configured the members collection to use the service name. However, when I attempt to bring up the cluster, the constructor of the RaftClusterMember class tries to convert the member to an endpoint and fails since it is not a loopback endpoint:

internal static IPEndPoint? ToEndPoint(this Uri memberUri)
{
    switch (memberUri.HostNameType)
    {
        case UriHostNameType.IPv4:
        case UriHostNameType.IPv6:
            return new IPEndPoint(IPAddress.Parse(memberUri.Host), memberUri.Port);
        case UriHostNameType.Dns:
            return memberUri.IsLoopback ? new IPEndPoint(IPAddress.Loopback, memberUri.Port) : null;
        default:
            return null;
    }
}

This is a problem since I do not know the IPs of the containers ahead of time and since the standard way to refer to the containers is through dns.

Do you have any ideas?

[sakno]

If all the nodes have the same name then they indistinguishable during election process. This is happening because members section will have records of the same value. In this case you need to use service discovery mechanism such as Consul. In this case, a new cluster node should advertise itself using this mechanism.

[sakno]

I'm not Kubernetes guru, but I found that this platform has built-in service discovery mechanism through Service abstraction so you can write your own configuration provider which exposes members section based on information queried from Kubernetes.

Additionally, you can contribute such an implementation as separated library. DotNext.AspNetCore.Cluster is not a good place for that because I don't want to rely on some specific hosting environment.

[natilivni]

That is a good workaround and I will try it but I still don't understand the downside of allowing the endpoints to be configured using dns. Especially for an http implementation, it seems like a desirable layer to support.

[natilivni]

BTW, all the nodes do not have the same name. They have different names but they are dns names.

[sakno]

From my point of view, it's not possible to provide a universal implementation applicable for all variations of underlying hosting infrastructure and network environment. In case of Kubernetes, all cluster nodes running in the private virtual network. This network may or may not have its own DNS. And there is not guarantee that each node has its own unique name. If so, how these names are managed and allocated dynamically? If they are visible to external clients then it cause node stickiness. It's bad. Moreover, if elastic scaling is enabled then nodes can be provisioned and decommissioned automatically.

[sakno]

I think that Service abstraction is an official way for service discovery in Kubernetes, not a workaround.

[sakno]

According with Kubernetes docs, each POD is nonpermanent resource. Thus, DNS name can be assigned to Service abstraction only.

[natilivni]

I still don't understand the need to enforce using ip4 or ip6 directly. Dns is a valuable way to identity resources and has deep support in the underlying HttpClient used for transport. What is the downside in allowing members to identify each other using dns?

Further, when using docker-compose.yml, the service name is a valid canonical dns entry for the container instance. It is best practice for containers to contract each other using http://myservicename.

How difficult would it be to support dns values for the members collection?

[sakno]

Docker is not the only way to host Raft nodes. Therefore, the proposed behavior is not universal. Moreover, in the current 2.x IClusterMember.EndPoint is IPEndPoint. I cannot change the type without breaking compatibility.

[natilivni]

I created a PR that solved the problem for me and I believe that it does not break compatibility:

https://github.com/sakno/dotNext/pull/49

Please late me know what you think.

[sakno]

I created a PR that solved the problem for me and I believe that it does not break compatibility:

49

Please late me know what you think.

Right :) Because develop branch already contains 3.x version, which is not yet published. In 3.x, IClusterMember.EndPoint is relaxed to EndPoint class instead of IPEndPoint. But 3.x is not ready for publishing.

Also, take a look at my comments in PR.

[natilivni]

I abandoned that PR and created a new one into the 2.x branch: https://github.com/sakno/dotNext/pull/50

Please let me know what you think.

[sakno]

You can try DNS support in 3.x branch which is located in dns-support-3.x branch. According with support policy described in README, 2.x doesn't accept new features, only bug fixes. DNS support is a new feature which onbviously cannot be implemented in 2.x (see my comments for your PR) without breaking changes.

[sakno]

Additionally, 3.x branch supports JSON serialization of log entries out-of-the-box and Interpreter Framework based on Command pattern aimed to help writing custom state machine on top of persistent WAL.

[natilivni]

It looks like you already took care of it. :)

What is the ETA for 3.x release?

[sakno]

It's mostly completed. However, DNS support is in separated branch because I still not sure that this is a good idea. According with your PR, you just tried to resolve DNS name within ToEndPoint method. This is not enough because DNS can be resolved to multiple IPs. Moreover, host IP usually defined as 0.0.0. which is not DNS name and you need to take care about reverse DNS request and make this work in situation when some of nodes defined with IPs and others with names. In other words, algorithm for detection of local node must work correctly in any case.

[sakno]

@natilivni could you please test dns-support-3.x branch in your environment? If everything will be OK then I'll merge this branch with develop.

[natilivni]

I will test it tomorrow. You are correct that in theory the dns could resolve to several values, but then you can just do a union with kestrel's IPs to detect the local node.

Also, regarding support for multiple hosting environments, you could allow users to register a service which receives member name and returns a boolean as to whether or not it is local. This way, users can effectively handle any scenario themselves the current functionality can be the default in case they do not register such a service.

[sakno]

Kestrel's IP may be ephemeral like 0.0.0.0 which is not so uncommon. So union with resolved IPs gives no result.

[natilivni]

Good point. What about doing: Dns.GetHostEntry(Dns.GetHostName()).AddressList

Wouldnt that successfully detect the ips?

[sakno]

Unfortunately no. The machine may or may not be visible externally through these IPs. For instance, my Linux machine has host name which is resolved as a set of externally unreachable IPs:

• Execute hostname command in Bash
• Execute nslookup and the result is

  
  Server:     127.0.0.53
  Address:    127.0.0.53#53

Non-authoritative answer: Name: XXXXXX Address: 127.0.1.1

[sakno]

Any assumptions here are too dangerous. I see no way to implement it in reliable way. However, what's done in dns-support-3.x branch already:

1. If Kestrel hosting address is represented by 0.0.0.0 then use Dns.GetHostName and all visible network interfaces as candidates for searching
2. Otherwise, use specified IP address as IP endpoint and apply reversed DNS query in attempt to obtain DNS aliases for the given IP address.

The algorithm looks complex and implemented here. That's why test result from you is valuable.

[natilivni]

Will definitely test. What do you think of my idea to allow users tk provide node identity through an injected service? That way each node could theoretically be configured with its own address and the injected service could resolve the identity.

[natilivni]

BTW, any significant breaking changes in 3.x I should know about before I attempt integration tomorrow?

[sakno]

Will definitely test. What do you think of my idea to allow users tk provide node identity through an injected service? That way each node could theoretically be configured with its own address and the injected service could resolve the identity.

Already done in the branch. IClusterMemberLifetime now has GetLocalMemberSelectorAsync method for that.

BTW, any significant breaking changes in 3.x I should know about before I attempt integration tomorrow?

High-level API mostly remains the same. Configuration model for Raft/HTTP was not changed in incompatible way. A few options were introduced. For instance, cluster node can be launched as standby node which means that the node cannot be elected as a leader and suitable for building active-standby cluster configuration.

Under the hood, Raft/HTTP has many performance improvements aimed to traffic optimization and memory allocation.

[natilivni]

@sakno, LogEntry's change to implement IDataTransferObject instead of IAsyncBinaryReader is creating significant breaking changes. Is there any guide for moving from one to the other?

[sakno]

@natilivni , IDataTransferObject gives you the same ability. Use TransformAsync method of the log entry for deserialization.

[sakno]

There are two ways:

• Move all your deserialization logic to type implementing IDataTransferObject.ITransformation<T> interface
• Move all your deserialization logic to lambda and pass it to DataTransferObject.TransformAsync extension method accepting Func<IAsyncBinaryReader, CancellationToken, ValueTask<TResult>> delegate

[natilivni]

It does not work in the container for the following reason:

1. The default GetHostingAddresesAsync method adds new DnsEndPoint(Dns.GetHostName(), ip.Port)
2. The member EndPoint property which is build from the external uri has a DnsEndPoint based on the Uri.
3. The comparison between these two fails since the container hostname is not the same as the external uri.

Suggested fix: Has the DnsEndpoint detect when comparing another DnsEndpoint and resolve the IPs of both, if any of the IPs match, then return true.

[natilivni]

I handled the LogEntry change without using ITransformation. I did the following:

var memory = await entry.ToMemoryAsync();
var reader = IAsyncBinaryReader.Create(memory.Memory);

Is there a performance hit for doing it this way?

[sakno]

This will not work as described above. Dns.GetHostName may be resolved as a set of externally unreachable IPs. It's interesting why container's host name returned by Dns.GetHostName differs from externally visibile DNS name.

[sakno]

I handled the LogEntry change without using ITransformation. I did the following:

var memory = await entry.ToMemoryAsync();
var reader = IAsyncBinaryReader.Create(memory.Memory);

Is there a performance hit for doing it this way?

ToMemoryAsync returning disposable object. It's a memory block rented from the pool so the correct code is

using var memory = await entry.ToMemoryAsync();
var reader = IAsyncBinaryReader.Create(memory.Memory);

And yes, you adding another indirection level. IAsyncBinaryReader accessible via TransformAsync method without redundant copying to the memory.

[sakno]

@natilivni , as an option I can give you ability to obtain IAsyncBinaryReader directly from the log entry via GetReader method. Is it OK for you?

[sakno]

I found the discussion about --hostname flag when launching container. I think the problem can be resolved using container configuration.

[natilivni]

@sakno, I think a GetReader functionality is a good idea. Especially good for people who will need to migrate and won't want to invest in refactoring to transformations.

Will try the hostname idea.

[natilivni]

the hostname idea is not a great solution because with containers, you can have any number of orchestration engines (Dockerfile, docker-compose.yml, k8s, etc) and it is not always easy to map commands to docker cli in an obvious way. What do you think about my idea of resolving the dns to IPs?

[sakno]

What do you think about my idea of resolving the dns to IPs?

This will not work. Dns.GetHostName is equal to the value of --hostname flag. This name is not resolvable from outside and resolution gives unreachable IPs. I have been demonstrated this fact above with my Linux machine.

[sakno]

GetReader has been added to dns-supports-3.x branch. However, I recommend to use custom transformation in future because GetReader causes boxing.

[natilivni]

Its true the name is not resolvable from the outside. however, at least one of its IPs will resolve to the same IPs as the external dns entry. I see it on my end: my external service name and the internal container name both share an IP.

[natilivni]

Another point, ClusterMemberSelector uses Predicate<IRaftClusterMember>. You should consider changing it to Func<IRaftClusterMember, Task<bool>> so that it can be used in async scenarios.

[sakno]

However, at least one of its IPs will resolve to the same IPs as the external dns entry.

It's not true for my machine. As a result, I cannot use weak assumption when potential user always utilize Docker for hosting.

[sakno]

Another point, ClusterMemberSelector uses Predicate<IRaftClusterMember>. You should consider changing it to Func<IRaftClusterMember, Task<bool>> so that it can be used in async scenarios.

Done.

[sakno]

Its true the name is not resolvable from the outside. however, at least one of its IPs will resolve to the same IPs as the external dns entry. I see it on my end: my external service name and the internal container name both share an IP.

Currently, hostAddressHint configuration property exists to help Raft/HTTP in understanding of the correct IP for the local member. I can add hostNameHint property to advise the correct DNS name. The value can be populated from environment variable passed to the container on startup.

[natilivni]

Can you check if the following code (in IClusterMemberLifetime) works on your end? This works for me:

private bool IsLocal(IRaftClusterMember member)
{
    var addresses = Dns.GetHostAddresses(Dns.GetHostName());
    if (member.EndPoint is DnsEndPoint dnsEndPoint)
    {
        var memberAddresses = Dns.GetHostAddresses(dnsEndPoint.Host);
        return addresses.Intersect(memberAddresses).Any();
    }
    else
        return addresses.Any(a => a.Equals(member.EndPoint));
}

public ValueTask<ClusterMemberSelector?> GetLocalMemberSelectorAsync(CancellationToken token)
            => token.IsCancellationRequested ? new ValueTask<ClusterMemberSelector?>(Task.FromCanceled<ClusterMemberSelector?>(token)) 
                                             : new ValueTask<ClusterMemberSelector?>(IsLocal);

[natilivni]

The hostNameHint is also a good idea

[sakno]

It doesn't work in my case: publicly visible DNS name of my machine doesn't match to Dns.GetHostName. If all nodes identified by IP address it doesn't work as well. The list of IP addresses returned by Dns.GetHostAddresses(Dns.GetHostName()) are not reachable from outside.

[natilivni]

Ok, In any case, the 3.x branch works great for me since I can override the GetLocalMemberSelectorAsync and customized it for my hosting environment. I don't think you need to offer more than that (although the hostNameHint is a great idea as well).

[natilivni]

Do you think you can push prerelease nuget of 3.x with the GetReader and the async LocalMemberSelector?

[natilivni]

Also, can you please add IAsyncBinaryReader GetReader() to IRaftLogEntry interface?

[sakno]

Ok, let's summarize the upcoming work from my side:

• I'll add hostNameHint configuration property
• Async local member selector is already implemented in branch dns-support-3.x. I need to merge it to develop branch.
• I cannot add GetReader to IRaftLogEntry interface because it's not recommended way to decode log entries due to performance reasons (boxing or instantiation of the object for each log entry on high workload produces a lot of garbage).

Not sure that publishing of 3.x as pre-release is a good idea. It will take the time. I'd rather spend the time preparing the final release. Anyway, I'll think about this.