Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Feature - Is there any support with Eureka Client for Service Discovery in Pivotal Cloud Foundry with Ocelot in near by feature ? #262

Closed
SrikanthNalam opened this issue Mar 6, 2018 · 34 comments · Fixed by #324

Comments

@SrikanthNalam
Copy link

@SrikanthNalam SrikanthNalam commented Mar 6, 2018

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

Specifications

  • Version:
  • Platform:
  • Subsystem:
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Mar 7, 2018

@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

This comment has been minimized.

Copy link
Author

@SrikanthNalam SrikanthNalam commented Mar 15, 2018

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

@TomPallister TomPallister changed the title Is there any support with Eureka Client for Service Discovery in Pivotal Cloud Foundry with Ocelot in near by feature ? New Feature - Is there any support with Eureka Client for Service Discovery in Pivotal Cloud Foundry with Ocelot in near by feature ? Mar 16, 2018
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Mar 17, 2018

@SrikanthNalam awesome! Good luck :)

@SivakumarShan

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 12, 2018

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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 12, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 17, 2018

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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 17, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 18, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 18, 2018

@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 pushed a commit to SivakumarShan/Ocelot-1 that referenced this issue Apr 18, 2018
@SivakumarShan

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 18, 2018

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

@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 18, 2018

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

I have started with the following code 8532a5b 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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 18, 2018

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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 18, 2018

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

@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 20, 2018

@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 pushed a commit that referenced this issue Apr 20, 2018
TomPallister pushed a commit that referenced this issue Apr 20, 2018
Tom Pallister
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 20, 2018

@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 added a commit that referenced this issue Apr 20, 2018
* #262 - Integrated Steeltoe Service Discovery with Ocelot for review.

* messing around

* seems to be working with eureka

* acceptance test passing with external lib references

* #262 support for netflix eureka service discovery thanks to pivotal

* #262 fixed warnings
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 21, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 23, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 23, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 23, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 23, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 23, 2018

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

@SivakumarShan

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 24, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 24, 2018

@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 TomPallister reopened this Apr 24, 2018
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 24, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 25, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 25, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 25, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 25, 2018

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

@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 25, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 26, 2018

@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 pushed a commit that referenced this issue Apr 26, 2018
Tom Pallister
TomPallister added a commit that referenced this issue Apr 26, 2018
@TomPallister

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 26, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 26, 2018

@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

This comment has been minimized.

Copy link
Member

@TomPallister TomPallister commented Apr 26, 2018

@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

This comment has been minimized.

Copy link

@SivakumarShan SivakumarShan commented Apr 26, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.