[WIP] OrleansDockerUtils -> Docker and Docker Swarm Clusters for Orleans #2569
Conversation
galvesribeiro
changed the title
|
|
||
| if (parameters.TryGetValue("Certificate".ToUpper(), out certificate)) | ||
| { | ||
| if (!File.Exists(certificate)) throw new FileNotFoundException("Unable to find certificate file"); |
There was a problem hiding this comment.
Windows users might be confused about this, since they're used to having a cert store and referring to certs using thumbprints rather than file names.
There was a problem hiding this comment.
Yeah I agree. But this is the way Linux users deal with certs. The cert stores are not implemented for linux yet so I rather stick with this filename for now.
There was a problem hiding this comment.
I'm fine with that, I was just pointing out that the name might be a cause for confusion with some users. A name like CertificatePath or CertFile might be clearer.
|
|
||
| Credentials credentials = null; | ||
|
|
||
| if (parameters.TryGetValue("DaemonEndpoint".ToUpper(), out username)) |
There was a problem hiding this comment.
DaemonEndpoint is the username?
The logic in this method isn't straightforward because if the user specifies DaemonEndpoint as well as Certificate, the credentials will first be set to BasicAuthCredentials and then to CertificateCredentials.
There was a problem hiding this comment.
No, it is the Docker Daemon or Docker Swarm endpoint uri like http://10.0.0.1:2375 or tcp://localhost:2375
There was a problem hiding this comment.
The variable is called username
There was a problem hiding this comment.
Oh! now I saw the problem. It was a change before the push which led to this bug... the string on the key is wrong... It should be "Username". Fixed.
| Docker_GatewayProvider_ExceptionNotifyingSubscribers = DockerBase + 1, | ||
| Docker_GatewayProvider_ExceptionRefreshingGateways = DockerBase + 2, | ||
| Docker_MembershipOracle_ExceptionNotifyingSubscribers = DockerBase + 3, | ||
| Docker_MembershipOracle_ExceptionRefreshingPartitions = DockerBase + 4 |
There was a problem hiding this comment.
Ops... Bad copy and paste... Removing
|
|
||
| public async Task Refresh() | ||
| { | ||
| var result = await Task.WhenAll((await dockerClient.Containers.ListContainersAsync( |
There was a problem hiding this comment.
await Task.WhenAll(await ...)?
I'd recommend moving the inner await outside so that it's more obvious what's happening here
| return new DockerSiloInfo(_.Config.Hostname, | ||
| new IPEndPoint(IPAddress.Parse(_.NetworkSettings.Networks.First().Value.IPAddress), | ||
| int.Parse(_.Config.Labels[DockerLabels.SILO_PORT])), | ||
| new IPEndPoint(IPAddress.Parse(_.NetworkSettings.Networks.First().Value.IPAddress), |
There was a problem hiding this comment.
Some variables could be extracted for clarity, eg the IPAddress, maybe _.Config. We tend not to use _ as a parameter name anymore (I used to do it a lot, but it doesn't fit with the codebase well).
Additionally, the body of this select could be extracted into its own method, like GetSiloFromContainer
| private readonly ILocalSiloDetails localSiloDetails; | ||
| private readonly Logger log; | ||
| private readonly GlobalConfiguration globalConfig; | ||
| private readonly IDockerSiloResolver resolver; |
There was a problem hiding this comment.
Just to be sure, which parts of this file differ from the Service Fabric oracle other than this type?
There was a problem hiding this comment.
Nothing is different actually. Just this resolver interface... Like I said yesterday, too much boilerplate code... We should later refactory this as a common abstract class or something in another PR.
|
Can you pleasevprovide a bit more details, describing how exactly you integrate with docker, where is the MBR table or who and how membership Oracle is provided, what is the hosting model, ... So people can understand without reading the code. |
|
@gabikliot sorry, I was actually hoping just @ReubenBond jump in since he just implemented the Service Fabric Membership Oracle and this implementation is almost the same. But yes, I'll edit the PR and include more details. Regarding MBR, there is no MBR table at all, just as in SF. I'll detail more on the PR text. Thanks |
|
Documentation on this would be great - I need to document "Hosting Orleans on Service Fabric", too. |
|
Yes, the docs is the PR right after this one @ReubenBond Unfortunately we sill need to write docs in a separated PR to use the |
|
This is a great effort! Let's open an issue to discuss the design instead of mixing it here with code review feedback. I'm trying to comprehend it, and have some basic initial questions.
Does this mean that there is no way to mark a silo as dead?
|
|
@galvesribeiro, I know how SF works and also what does it mean from Orleans perspective. I was asking about Docker Swarm. I am looking into more high level description of how it is used, both hosting and MBR, like here: http://dotnet.github.io/orleans/Documentation/Runtime-Implementation-Details/Consul-Deployment.html For example, you can see here the long thread we went through when designing Consul based solution: #1267 So can we please open an issue and discuss the design, like @sergeybykov suggested. Ideally, I think we prefer doing that BEFORE code is submitted for review, so that people can actually comprehend the code, after the design is ready. Without seeing the design, its hard to give any valuable feedback. |
|
Also would be interesting to ask why Swarm and not other Docker clustering technologies. Is there a specific reason, or customer who asked for that, or just a first one we picked without any particular reason? |
Ok will do in a minute.
Docker containers are immutable. In no way a same container or its main process can restart. It is discarded and start a fresh one so the generation is pointless in this case. I'll describe in the issue how I do that.
If the silo process is frozen or crash, the container is killed and a new one must be spawned. 1 container has only 1 main process which in this case is the silo process. So if it dies, the whole container die. More on that in the issue. I agree, let me open an issue and link here in a minute.
You can use any docker clustering platform as long as they respect Docker APIs and they do (almost). Swarm is by far Today's most used technology for Docker clustering and the one that comes from Docker Inc. If you look at the core, there is no explicit mention tie to Swarm. The same code that talk with a single Docker Daemon talk to Swarm. Its orthogonal. Just a matter of change endpoints. I'll dive into details in the issue. Anyway, linking the issue here soon. Thanks for the comments. |
|
Btw, sidenote... I didn't added the .Net core projects yet, will do in the end since the code dont change. The tests I'm building some easy way to run from xUnit with a |
|
Thank you for the detailed explanation @galvesribeiro . |
|
|
||
| public async Task Refresh() | ||
| { | ||
| var containerList = await dockerClient.Containers.ListContainersAsync( |
There was a problem hiding this comment.
can you please replace var containerList with the actual type, so its more readable.
| }, | ||
| } | ||
| }); | ||
| var inspectionResult = await Task.WhenAll(containerList.Select(c => dockerClient.Containers.InspectContainerAsync(c.ID))); |
There was a problem hiding this comment.
same comment about var here.
| /// Updates the status of this silo. | ||
| /// </summary> | ||
| /// <param name="status">The updated status.</param> | ||
| private void UpdateStatus(SiloStatus status) |
There was a problem hiding this comment.
UpdateStatus does not really do anything, it does not propagate the new status of this silos remotely to other silo. We need that in Orleans. There is a difference between Starting/Active, or Terminating/Stopping/Dead.
There was a problem hiding this comment.
I don't understand @gabikliot. What you mean by "it does not do anything"?
foreach (var subscriber in subscribers.Values)
{
subscriber.SiloStatusChangeNotification(SiloAddress, status);
}
It is notifying all the registered subscribers about the new status on this method. I don't see your point here, sorry. Can you elaborate?
There was a problem hiding this comment.
It does not propagate the new status of this silo to other silos (yes, it does update local subscribers).
When a silo goes lets say to terminating state (just an example), we need to notify all other silos about it. Its not enough to notify all local components. If we can't do distributed silo states, that's a major gap that we are opening compared to the main MBR Oracle and we need to think through the implications.
There was a problem hiding this comment.
Ok, then we have the same problem in SF implementation because it does the same thing there. o.O
There was a problem hiding this comment.
The SF solution is not merged yet, so not really a good source of example to follow, at least until merged.
We need to solve that, right?
There was a problem hiding this comment.
The propagation of silo status changes happens in FabricMembershipOracle.cs:L348. The only states which are supported are Active and Dead, but they satisfy the distinction for SiloStatus.IsTerminating().
Do you think this is an issue, @gabikliot?
There was a problem hiding this comment.
This is a somewhat critical question I think so it may deserve its own issue.
If we only need 2 states for SF livenes, then maybe we need only 2 for the regular case as well? Maybe we can simplify the silo state machine and get rid of all other states and stay with just 2?
One need to look at what we are getting out if the other states. I wonder what @sergetbykov thinks. Would like to hear his opinion.
One thing is sure in my eyes: we should try to have similar silo state machines in all livenes cases, and not just ditch states in some cases cause we can't implementat them. If u go that route, it may work, or more likely you will be supporting diverging implementaion and at the end paying higher cost.
Alternatively: when I wrote the first integration with SF(WF at a time) 3-4 years ago, it was similar to your code now, and in addition my plan was to use their build in key value store to store silo states. I didn't implemenent it, but maybe it can work.
If u go that route, them yet another alternative is to ditch SF livenes and just use the key value store to implement MBR table. That is exactly what we did for Consul. It works, so here is an advantage. And Consul is comparable to SF, so why not? More choices.:-)
| { | ||
| try | ||
| { | ||
| await resolver.Refresh(); |
There was a problem hiding this comment.
This is a bit weird. Oracle refreshes the docker resolver which notifies its subscribers (this Oracle) back about the MBR. Cycle of dependences? Or did I misunderstand it.
There was a problem hiding this comment.
Yeah you are right. I copy that over from the SF implementation. I just refactored the timer out of both gateway and oracle. Now it is inside the resolver and this dependency don't happen anymore. Thanks for notice that.
@gabikliot Sure, but sorry, I din't understand. You mean the Docker Swarm membership implementation from Docker Inc. or the the Docker Oracle Membership I implemented using Swarm for Orleans? |
|
I meant the former: "Docker Swarm membership implementation from Docker Inc." Mostly not how it is implemented, but what it provides to the users of that protocol. What are the semantics. Like does it notify or do you have to pool it, how fast it detects, does it have an option to specify health check .... That knowledge will help review the latter (" Docker Oracle Membership I implemented using Swarm for Orleans"). |
|
@gabikliot I'm updating the issue related to this PR and will ping you there in a minute. |
Fixed a small bug detected on a test
|
@gabikliot I've updated #2571 with more info related of health and membership. About the Swarm membership implementation (or any other orchestration engine for containers like Mesos, DC/OS and Kubernetes), there nothing that can help us in this case. Swarm has its own Raft implementation for its manager, but it is restricted to Swarm usage. The other orchestration engines have their own and suffer of the same problem. They only care about their cluster, and not the containers. What they care is only if the container is up or down. Nothing more. |
|
Does swarm have key value store? To store silo states or implement MBR table? |
|
@gabikliot no, it doesn't. In fact, none of the current orchestrators have it and that is why we have having this whole discussion here. If we have it, we would be implementing the |
|
Guys I'm closing this PR since it is not required anymore. See here: #2571 (comment) Thanks for all the feedback. |
This PR brings support to Docker and Docker Swarp cluster for Orleans.
It is based on @ReubenBond's work on SF Membership Oracle but using the Docker APIs.
The design of this Membership Oracle is to not have a MBR just as SF doesn't. It works by communicating with Docker Daemon or Swarm endpoints using https://github.com/Microsoft/Docker.DotNet which is a wrapper over Docker APIs, and leveraging the
Labelsfeature of docker.The idea is to group a set of containers by
deploymentIdon the same host (Docker Daemon) or across multiple hosts (Swarm cluster). Whenever a client start, theDockerGatewayProviderconnects to Docker and query for the current list of containers which are a silo, and are in the samedeploymentIdso it can build the gateway list. After that first list, it set a timer to refresh that list from docker. The same happens at the silo side which basically find the other silos by thedeploymentId.Basically there is no persistence at all neither MBR tables since all the containers can be found by the labels and when a container dies, it is automatically removed from docker list so it will not return anymore.
The hosting model is basically any container engine which implement Docker APIs (i.e. Docker Daemon, Docker Swarm, etc.). There is no change on how things are deployed or a new OrleansHost for that. All people need to do is to set the appropriate labels on the container which has the Orleans application code and everything will just works as long as the container is able to reach over network the Docker API host (i.e. the container host or Swarm manager).
There are two things that I'll implement after this PR:
This is a WIP. I was able to run manual tests here and made 2 containers and a client talk perfectly. The current code is totally usable. I've simulated adding and removing silos/containers to the cluster and they join and leave just fine.
I'm now working on get a unit test project with some
DockerFiles so all people will need to run the tests is have Docker installed.I appreciate any feedback while I'm working in the tests so I can address them quickly.