ThreeMammals / Ocelot

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

Is there any support with Eureka Client for Service Discovery in Pivotal Cloud Foundry? #262

Closed SrikanthNalam closed 6 years ago

SrikanthNalam commented 6 years ago

Expected Behavior / New Feature

Does Ocelot has Support with Eureka for service discovery in Pivotal Cloud Foundry in near future

Actual Behavior / Motivation for New Feautre

Right now for Service Discovery in Ocelot ,only Consul is there .When deployed in PCF we need to have the support for Eureka client.Right now that is happening with gateway

Steps to Reproduce the Problem

1. 1. 1.

Specifications

TomPallister commented 6 years ago

@SrikanthNalam thanks for your interest in the project! :)

I have no plans to implement this at the moment I just started a new job and I'm prioritising certain features at the moment. Maybe in the future it is something I might look at.

However if you would like to implement this yourself you could submit a PR that makes it work.

You would need to add an implementation of IServiceDiscoveryProvider for Eureka, edit ServiceDiscoveryProviderFactory to return your new provider in the right circumstances. I would suggest just having FileServiceDiscoveryProvider.Type as "Eureka" or something and this will get passed into the factory. It shouldn't be too much work especially if there is already a library for this in .NET because you could just pull it in and implement your IServiceDiscoveryProvider.

Anyway hope that helps and I would welcome a PR for this.

SrikanthNalam commented 6 years ago

Thanks Tom Pallister.We are working on this.Will let you know the update on this

TomPallister commented 6 years ago

@SrikanthNalam awesome! Good luck :)

SivakumarShan commented 6 years ago

Hi Tom, I have implemented Eureka Service Discovery in Ocelot and expecting your comments before merging the changes. DependencyInjection/OcelotBuilder.cs // Steeltoe Service Discovery Implementation if (configurationRoot.GetSection("GlobalConfiguration:ServiceDiscoveryProvider").Key !=null && configurationRoot.GetValue<string>("GlobalConfiguration:ServiceDiscoveryProvider:Type") != null && configurationRoot.GetValue<string>("GlobalConfiguration:ServiceDiscoveryProvider:Type") == "Steeltoe" && configurationRoot.GetSection("eureka").Key != null) { _services.AddDiscoveryClient(configurationRoot); _services.TryAddSingleton<IHttpRequester, HttpDiscoveryClientHttpRequester>(); } else { _services.TryAddSingleton<IHttpRequester, HttpClientHttpRequester>(); }

Middleware/OcelotMiddlewareExtensions.cs

if (configuration != null && configuration.ServiceProviderConfiguration != null && configuration.ServiceProviderConfiguration.Type == "Steeltoe") { builder.UseDiscoveryClient(); }

And I have added Overriding methods for HttpDiscoveryClientRequester under Requester/ folder. The new methods will take care the HttpClient calls through DiscoveryHttpClientHandlers.

Requester/HttpDiscoveryClientHttpRequester.cs `public class HttpDiscoveryClientHttpRequester : IHttpRequester { private readonly IHttpClientCache _cacheHandlers; private readonly IOcelotLogger _logger; private readonly IDelegatingHandlerHandlerHouse _house; private readonly IDiscoveryClient _discoveryClient; private DiscoveryHttpClientHandler _handler;

    public HttpDiscoveryClientHttpRequester(IOcelotLoggerFactory loggerFactory,
        IHttpClientCache cacheHandlers,
        IDelegatingHandlerHandlerHouse house, IDiscoveryClient discoveryClient)
    {
        _logger = loggerFactory.CreateLogger<HttpClientHttpRequester>();
        _cacheHandlers = cacheHandlers;
        _house = house;
        _discoveryClient = discoveryClient;
    }

    public async Task<Response<HttpResponseMessage>> GetResponseAsync(DownstreamContext request)
    {
        _handler = new DiscoveryHttpClientHandler(_discoveryClient);
        var discoveryClientBuilder = new DiscoveryHttpClientBuilder().Create(_handler, request.DownstreamReRoute);
        var httpDiscoveryClient = discoveryClientBuilder.Client;

        try
        {
            var response = await httpDiscoveryClient.SendAsync(request.DownstreamRequest);
            return new OkResponse<HttpResponseMessage>(response);
        }
        catch (TimeoutRejectedException exception)
        {
            return
                new ErrorResponse<HttpResponseMessage>(new RequestTimedOutError(exception));
        }
        catch (BrokenCircuitException exception)
        {
            return
                new ErrorResponse<HttpResponseMessage>(new RequestTimedOutError(exception));
        }
        catch (Exception exception)
        {
            return new ErrorResponse<HttpResponseMessage>(new UnableToCompleteRequestError(exception));
        }
        finally
        {
            //_cacheHandlers.Set(cacheKey, httpClient, TimeSpan.FromHours(24));
        }
    }
}`

Requester/DiscoveryHttpClientBuilder.cs public class DiscoveryHttpClientBuilder : IDiscoveryHttpClientBuilder { public IHttpClient Create(DiscoveryHttpClientHandler handler, DownstreamReRoute request) { var client = new HttpClient(handler); return new HttpClientWrapper(client); } } Update in configuration.json "GlobalConfiguration": { "ServiceDiscoveryProvider": { "Type": "Steeltoe" } }

Please let me know your feedback about this approach. Thanks, Siva

TomPallister commented 6 years ago

@SivakumarShan good stuff :)

I've had a look and think that you can implement this without having to change Ocelot at all.

If you are using the latest version of Ocelot 5.5.1 then you can do the following....

 public class DiscoveryHttpClientBuilder
    {
        public IHttpClient Create(DiscoveryHttpClientHandler handler)
        {
            var client = new HttpClient(handler);
            return new HttpClientWrapper(client);
        }
    }

    public class EurekaDelegatingHandler : DelegatingHandler
    {
        private readonly IDiscoveryClient _discoveryClient;
        private DiscoveryHttpClientHandler _handler;

        public EurekaDelegatingHandler(IDiscoveryClient discoveryClient)
        {
            _discoveryClient = discoveryClient;
        }

        protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
        {
            _handler = new DiscoveryHttpClientHandler(_discoveryClient);
            var discoveryClientBuilder = new DiscoveryHttpClientBuilder().Create(_handler);
            var httpDiscoveryClient = discoveryClientBuilder.Client;
            var response = await httpDiscoveryClient.SendAsync(request, cancellationToken);
            return response;
        }
    }

This creates a delegating handler that will use the IDiscoveryClient stuff. Then to tell Ocelot to use you can do the following in your configure either in startup or program.cs

 .Configure(app =>
      {
            app.UseDiscoveryClient().UseOcelot().Wait();
      })

You also need to configure your services..

.ConfigureServices(s =>
        {      
             s.AddDiscoveryClient(c)
                 .AddOcelot()
                 .AddSingletonDelegatingHandler<EurekaDelegatingHandler>(true);
        })

If you look at this documentation hopefully it will all make sense.

SivakumarShan commented 6 years ago

Hi Tom, Thanks for your guidance on using the above approach. I thought we can integrate the Eureka Service Discovery in Ocelot, so anyone can use it in future based upon their need.

I followed the above approach and have added UseDiscoveryClient() and AddDiscoveryClient() in my Startup class. But I'm getting an exception that

The request message was already sent. Cannot send the same request message multiple times

as we already injected _services.TryAddSingleton<IHttpRequester, HttpClientHttpRequester>(); in OcelotBuilder.cs which needs to be avoided when we use EurekaDelegatingHandler which will make a Http Call through DiscoveryHttpClientHandler.

Could you please guide me how can I override the HttpClientHttpRequester with EurekaDelegatingHandler when I use Eureka?

Thanks, Siva

TomPallister commented 6 years ago

@SivakumarShan If you submit a pull request between your fork and Ocelot's develop branch I will be able to work this out a bit easier.

My best guess is something to do with the order delegating handlers have been registered.

SivakumarShan commented 6 years ago

@TomPallister I'm unable to push the code changes to the Developer branch. I'm getting the below error.

cannot spawn askpass: No such file or directory could not read Username for 'https://github.com': terminal prompts disabled Pushing to https://github.com/ThreeMammals/Ocelot.git

Should I have the access to push my changes to repo?

TomPallister commented 6 years ago

@SivakumarShan you need to follow this process

  1. https://help.github.com/articles/fork-a-repo/
  2. https://help.github.com/articles/creating-a-pull-request-from-a-fork/

Unfortunately you cannot push code straight to Ocelot!

SivakumarShan commented 6 years ago

Thanks @TomPallister I have created a PR with my approach. Please have a look at it.

TomPallister commented 6 years ago

@SivakumarShan cool, I'm taking a look now.

I have started with the following code https://github.com/ThreeMammals/Ocelot/commit/8532a5b5201363d58c169536291ac16bf2e70ae6 which is just going to try and use Steeltoe.

However I get an error when Ocelot starts because I don't have whatever service running that Eureka / Steeltoe / Pivotal needs to work. Do you know where I can get this for local development?

Also is there anywhere good I can quickly learn about how all this works? Because there might be a much better way to do this and hook into Ocelot's normal service discovery pipeline.

SivakumarShan commented 6 years ago

Hi @TomPallister you can create a local Eureka Server by pulling the docker image from https://hub.docker.com/r/steeltoeoss/servers/

Steeltoe Service Discovery - https://steeltoe.io/docs/steeltoe-discovery/

TomPallister commented 6 years ago

@SivakumarShan cool thanks, ill give this a go asap!

TomPallister commented 6 years ago

@SivakumarShan I’ve had a good look at this and we won’t be able to use the delegatinghandler approach because of how ocelot and eureka work. I will work on this issue now as it’s quite complicated with what needs to happen.

TomPallister commented 6 years ago

@SivakumarShan I have made the changes for this to work, you can see them here. I will probably merge & release them tomorrow if I get a chance. Let me know if you have any suggestions.

TomPallister commented 6 years ago

@SivakumarShan @SrikanthNalam I have just released this feature in version 5.5.4 please check it out and let me know if it works. Docs are here http://ocelot.readthedocs.io/en/latest/features/servicediscovery.html

SivakumarShan commented 6 years ago

@TomPallister Thanks a lot for integrating the changes for Steeltoe Service Discovery. Ocelot is able to discover the Services from Service Discovery. But still I suspect, ocelot is making a normal HttpClient calls instead of the HttpClient(new DiscoveryHttpClientHandler(client)). Please refer the given link. https://steeltoe.io/docs/steeltoe-discovery/#1-2-6-discovering-services

And also we need to update

"shouldRegisterWithEureka": false

in http://ocelot.readthedocs.io/en/latest/features/servicediscovery.html

Could you please integrate those changes in Ocelot?

TomPallister commented 6 years ago

@SivakumarShan Ocelot doesn't need to use the DiscoveryHttpClientHandler so I won't be using it. We use the IDiscoveryClient directly.

Ocelot doesn't need to register with Eureka in order to get services from it so it doesn't need to be set to true.

SivakumarShan commented 6 years ago

@TomPallister I meant, there are two properties in eureka service Registry/Discovery. By default both the properties are true. So in-order to ensure Ocelot is discovering the service, we need to set the "shouldRegisterWithEureka": false and automatically shouldFetchRegistry will be set to true.

And also, I'm trying to understand the approach which you implemented through EurekaServiceDiscoveryProvider and I'm getting some exceptions. I'll let you know my updates, after I complete the testing.

TomPallister commented 6 years ago

@SivakumarShan shouldRegisterWithEureka is already set to false in the documentation. I have linked to the Eureka documentation for that though. It's not really Ocelot concern. Is that OK?

OK let me know how your testing goes....hopefully it will work.

All I have done is use the IDiscoveryClient directly instead of the HttpHandler they provide.

TomPallister commented 6 years ago

@SivakumarShan sorry I have checked the documentation now you are correct! I'm not sure why it is wrong :( will fix!

SivakumarShan commented 6 years ago

@TomPallister I have set the below attributes in configuration.json and EurekaServiceDiscoveryProvider is able to pick the service which is registered in the Eureka. But getting Timeout exception, when I try calling my Downstream service. I'm further looking into the cause of this issue.

"UseServiceDiscovery": true, "ServiceName": "ncore-rat"

Error Message: Error Code: RequestTimedOutError Message: Timeout making http request, exception: System.Threading.Tasks.TaskCanceledException: A task was canceled.

SivakumarShan commented 6 years ago

@TomPallister I found out the root cause of the issue. DownStreamRequest.Host is replaced with the Eureka Service Host URL and Ocelot is trying to make a HttpClient call with Eureka host which is wrong. So we are getting the timeout exception.

As per my knowledge, Eureka Server Discovery will be resolved only by creating a HttpClient of DiscoveryHttpClientHandler like it's documented here

TomPallister commented 6 years ago

@SivakumarShan I'm not sure that this is the root cause of your problem. I have tested this and everything works OK for me :(

If you look at the docs it says

The simplest way to use the registry to lookup services is to use the Steeltoe DiscoveryHttpClientHandler together with a HttpClient.

You don't have to use DiscoveryHttpClientHandler, I guess if you get a host and port, the host and port listens to HTTP you can call it with anything?

If you look at the source code here (and this is the best I can do because I cant find the code on GitHub) all the DiscoveryHttpClientHandler does is call GetInstances which is the same as Ocelot, then randomly choose a URI. So there is nothing special about DiscoveryHttpClientHandler as far as I can tell.

Given that the request times out it suggest either SSL problem or port problem to me. Maybe Ocelot is using the wrong port? Can you send me your Eureka and Ocelot configurations?

SivakumarShan commented 6 years ago

@TomPallister Please find the below configurations. ncore-rat is the service name which has been registered in Eureka.

Ocelot { "ReRoutes": [ { "DownstreamPathTemplate": "/api/Category", "DownstreamScheme": "https", "DownstreamHostAndPorts": [ { "Host": "PCF Hosted Service", "Port": 443 } ], "UpstreamPathTemplate": "/Category", "UseServiceDiscovery": true, "ServiceName": "ncore-rat", "UpstreamHttpMethod": [ "Get" ], "QoSOptions": { "ExceptionsAllowedBeforeBreaking": 3, "DurationOfBreak": 10000, "TimeoutValue": 5000 }, "FileCacheOptions": { "TtlSeconds": 15 } } ], "GlobalConfiguration": { "RequestIdKey": "OcRequestId", "AdministrationPath": "/administration", "ServiceDiscoveryProvider": { "Type": "Eureka" } } }

Eureka Configuration { "Logging": { "IncludeScopes": true, "LogLevel": { "Default": "Trace", "System": "Information", "Microsoft": "Information" } }, "spring": { "application": { "name": "Ocelot-Gateway" }, "cloud": { "config": { "uri": "http://localhost:8888", "validateCertificates": false } } }, "eureka": { "client": { "serviceUrl": "http://localhost:8761/eureka/", "shouldRegisterWithEureka": false, "validateCertificates": false } } }

TomPallister commented 6 years ago

@SivakumarShan try DownstreamScheme as http and you don't need the DownstreamHostAndPorts when using service discovery. Can I see the configuration that your service uses to register with Eureka?

That might be causing a problem. I will have a proper look later!

SivakumarShan commented 6 years ago

@TomPallister please find the below Eureka Configuration of the Service.

{ "Logging": { "IncludeScopes": false, "LogLevel": { "Default": "Warning" } }, "spring": { "application": { "name": "ncore-rat" }, "cloud": { "config": { "uri": "http://localhost:8888", "validate_certificates": false } } }, "eureka": { "client": { "serviceUrl": "http://localhost:8761/eureka/", "shouldFetchRegistry": false, "validateCertificates": false }, "instance": { "port": 5000 } } }

Sure. Let me try it out.

SivakumarShan commented 6 years ago

@TomPallister I tried it with the above approaches. But no luck.

TomPallister commented 6 years ago

@SivakumarShan ahh no :( this is so confusing. Please bear with me I'm doing my best!

I don't understand why it doesn't work for you! I noticed you want to call your downstream service over https, is your certificate valid? Do you accept connections over HTTP? Or just HTTPS? I'm thinking you are registering with port 5000 which isnt normally https but in the Ocelot config you are stating HTTPS!

If you call the service without Ocelot e.g. curl over http and https does it work with both?

SivakumarShan commented 6 years ago

@TomPallister The port address of 5000 is for local service registry. Even I tried by running Service and Ocelot in local. But getting the same issue. Actually my application has been hosted in Private PCF. So it accepts Http request too. Once it's deployed to PCF, I'll be binding the Eureka Service with my application which will override the 5000 port.

TomPallister commented 6 years ago

@SivakumarShan I have looked at this some more and your settings didn't work for me either.

I noticed in your service appsettings.json that you have the cloud:config:uri as http://localhost:8888 so I assume that is the address you expect the service to run on but in your instance:port setting you have 5000. If your service is running on 8888 and you set 5000 I don't think it will work. Also the other problem I had was if I have a service running on http://localhost:8888 or http://localhost:5000 when the steeltoe client registers the service it does it using the machine name not localhost. This meant I have registered http://XXXXMACHINE:5000 not http://localhost:5000 in Eureka. When Ocelot made the call to my service on http://XXXXMACHINE:5000 it didn't work because my service was listening for requests on http://localhost:5000. I was getting the task cancelled timeout exception in this scenario.

Anyway I hope this is the same problem you are having.

I have created a sample project for you to run and check that it works on your local environment. Then perhaps use that as the basis going forward. The project is here let me know if you get it working!

SivakumarShan commented 6 years ago

@TomPallister Thanks a lot for sharing the sample project which works fantastically without any issues. I further figured out by changing the ocelot configuration with "DownstreamScheme": "http" instead of https, it worked without any issue. Even though if I mention the downstream URI, it's not affecting the flow. So we are good now.

One quick question, if my downstream service accepts, only HTTPS, how can we handle that situation. Will ocelot take care of that scenario too?

TomPallister commented 6 years ago

@SivakumarShan if your downstream service only accepts HTTPS then you need to set DownstreamScheme as https. The important thing to remember is that you need a valid certificate on the downstream service that is trusted by the host Ocelot is running on. Otherwise the connection between Ocelot and the downstream service will fail.

It is also worth noting by the way to use a load balancer with Ocelot service discovery. Check out the docs here. I would suggest using the RoundRobin load balancer at first and then LeastConnection if that works OK.

I'm happy that it is working :) I will close this one now! Let me know if you need anymore assistance!

SivakumarShan commented 6 years ago

@TomPallister Thanks a lot for the help😄 Yeah understood now. I'll use the Loadbalancer with Ocelto Service Discovery. Thanks again.