search

ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.31k stars 1.63k forks source link

Load balancer sample using Consul and Least Connection [TCP] #1842

Closed ks1990cn closed 8 months ago

ks1990cn commented 9 months ago

Closes #1811

LoadBalancer Sample using Consul and Least Connection type.

I am creating this PR as a handy guide for local setup and running so any contributor can see loadbalancer in action with minimal repeated setup.

I think this can also help me / anyone as always present setup so just quickly go and make required changes to debug any requested changes / bug.

Proposed Changes

ks1990cn commented 9 months ago

@raman-m / @ggnaegi / @RaynaldM I am raising clean PR again. There was lot of changes and some how lot errors I use to get while working on docker on that branch.

Please add required reviewers again.

I am adding more queries on based on this PR @ggnaegi / @raman-m. Please help me when you get chance.

ks1990cn commented 9 months ago

https://github.com/ThreeMammals/Ocelot/pull/1810#discussion_r1414528706

@ggnaegi I am trying to understand how I can run multiple instances of an Image using docker compose. I am facing an issue here, when my instance starts in container - I am able to reach to one port assigned in container but unable to reach to another port which is assigned for same image.

here is my experimental branch where I tried to do this thing. Can you quickly go through my code and check what I am missing in docker-compose or any other thing?

dockerquery2

raman-m commented 9 months ago

Tanmay, why did you close #1810 ?

ks1990cn commented 9 months ago

Tanmay, why did you close #1810 ?

Code and changes got into mess and my solution started giving lot of local errors. so I had to create branch again & solving them was taking more than creating & chery picking again.

raman-m commented 9 months ago

You've got the perfect code review of #1810 by @ggnaegi Seems you have to take into account old code review issues too...

ks1990cn commented 9 months ago

You've got the perfect code review of #1810 by @ggnaegi Seems you have to take into account old code review issues too...

Yes I am trying to clean referring to same. but I am struggling with docker instances. I need help here.

ks1990cn commented 9 months ago

@ggnaegi Please help me with https://github.com/ThreeMammals/Ocelot/pull/1810#discussion_r1414529390 comment also & how I can set my host name to consul while running on docker? It gets hosted with localhost.

raman-m commented 9 months ago

@ks1990cn commented

Route JSON ServiceName

ks1990cn commented 9 months ago

@ks1990cn commented

Route JSON ServiceName

@raman-m example also suggest for host as localhost, but @ggnaegi suggesting for Consul https://github.com/ThreeMammals/Ocelot/pull/1810#discussion_r1414540043

image

Is he want to explain something different?

raman-m commented 9 months ago

Wait for our Consul expert, @ggnaegi 😉 aka King of the England! 👑 but not Scotland! 🤴

ks1990cn commented 9 months ago

#1810 (comment)

@ggnaegi I am trying to understand how I can run multiple instances of an Image using docker compose. I am facing an issue here, when my instance starts in container - I am able to reach to one port assigned in container but unable to reach to another port which is assigned for same image.

here is my experimental branch where I tried to do this thing. Can you quickly go through my code and check what I am missing in docker-compose or any other thing?

dockerquery2 dockerquery2

@ggnaegi Here I can understand is first docker image builds and creates port and I am trying to assign another port on same built, which is causing this issue. I am waiting for your reply what kind of approach you are suggesting to use only 1 project.

https://github.com/ThreeMammals/Ocelot/pull/1810#discussion_r1414528706 cc: @raman-m

ggnaegi commented 9 months ago

Not even king at home, sometimes in my office, if the dog agrees.

raman-m commented 9 months ago

@ks1990cn commented on Dec 9

Have you ran all your Docker containers for each image? Seems you need to learn Docker more... Which Docker software do you use except Visual Studio Docker support?

ks1990cn commented 9 months ago

@ks1990cn commented on Dec 9

Have you ran all your Docker containers for each image? Seems you need to learn Docker more... Which Docker software do you use except Visual Studio Docker support?

Same container is running all images which are defined in compose file. I use visual studio only, I saw only it as an example in Microsoft tutorials. Not much expert here, I am learning.

raman-m commented 9 months ago

Not much expert here, I am learning.

Aha! Good luck in learning!

Install Docker Desktop if you use Windows OS... It installs also Docker engine, CLI, virtual machine aka WLS, and UI app on top of them. This is Docker engine management GUI tool. It will allow you to manage Docker artifacts easily... Also it is possible to use Docker CLI... But it is a bit complicated thing for Docker newbies who learn this software... Just use Docker Desktop UI. Visual Studio can run one Docker container for current default startup project, but for projects with multiple Docker images you have to run multiple Visual Studio sessions... It is not required if you use Docker Desktop for objects managements! Docker Compose project just allows to create Docker Network for all Docker images in the solution. They can communicate to each other via Docker Network of composing project. But Compose project doesn't manage all Docker objects of the solution, it complies all Docker filed as images and nothing more. You have to manage Docker objects manually somehow...

I will return to this PR back after fixing the problem of placing all this code into the root repo folder... So, work on this issue now ❗

ks1990cn commented 9 months ago

@raman-m commented on Dec 11

Ah! I have docker desktop, I thought you are asking about tool to write docker code.

ggnaegi commented 9 months ago

@ks1990cn I will provide you a sample implementation during the week. Be patient, i'm a bit busy at the minute. Thanks!

ks1990cn commented 9 months ago

Tanmay, you 've created the solution and projects in the root folder! That's wrong!!! We have special folder for all samples: samples

You must move solution/projects into a folder inside of the samples folder!

image

Made respective changes

raman-m commented 9 months ago

@ks1990cn commented on Dec 12:

Made respective changes https://github.com/ThreeMammals/Ocelot/pull/1842/commits/68eed660821fb5757827abb695a2aae6301d8703

Is that your change? Are you kidding? Please, work on major code review issue aka root folder problem After that, as a team, we will perform classic code review: C#, design etc...

ks1990cn commented 8 months ago

@ks1990cn commented on Dec 12:

Made respective changes 68eed66

Is that your change? Are you kidding? Please, work on major code review issue aka root folder problem After that, as a team, we will perform classic code review: C#, design etc...

I already moved to Samples/LoadBalancerDemo , take latest and are you still facing issue with folder structure? 😕

raman-m commented 8 months ago

Your feature branch: https://github.com/ks1990cn/Ocelot/tree/lbsample You've moved NOTHING! image I have last drop of my patience...

raman-m commented 8 months ago

I've rebased the branch. Probably you did the fix in local repo.

ks1990cn commented 8 months ago

Your feature branch: https://github.com/ks1990cn/Ocelot/tree/lbsample You've moved NOTHING! image I have last drop of my patience...

image

Your feature branch: https://github.com/ks1990cn/Ocelot/tree/lbsample You've moved NOTHING! image I have last drop of my patience...

You don't need to loose patience, I am not sure whats wrong, let me figure out.

image

@raman-m Steps you mentioned didn't worked for me. Let me try again, maybe I need to pull through this fork and remove my local folder too.

ks1990cn commented 8 months ago

Closing this PR, cause this had folder structure issue. Clean PR is raised again.

raman-m commented 8 months ago

Tanmay, there is no need to close a PR each time when the code reviewer found an issue. You just need update your feature branch! Nothing more! The changes will be reflected to a PR... This is the 2nd closing... I have no words...

raman-m commented 8 months ago

Also, creation of new PR based on old feature branch has no sense, because files in the feature branch remain the same!

ks1990cn commented 8 months ago

I agree on you @raman-m , but I figured out sample folder structure is little different thing. On File explorer it different and on visual studio naming and structuring is different.

I already had project file into it which was created to wrong path. I had no chance for it. In my new code, I've made some more docker changes which @ggnaegi was suggesting (Just facing another issue with cerficates).

raman-m commented 8 months ago

Tanmay, you had/have misunderstood that real file system structure vs Git, and virtual Visual Studio folders structure are different! So Visual Studio virtual folders tree used solution structure in .sln file. You can organize a solution by adding and including absolutely different file system objects. That was your mistake managing this 2nd PR! Hope you understand Visual Studio virtual folders now better.

ks1990cn commented 8 months ago

Correct