diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 78f65140a..91cbe6cad 100644 --- a/ReleaseNotes.md +++ b/ReleaseNotes.md @@ -1 +1,8 @@ Technical release, version {0} + +### Breaking changes + +- The `ILoadBalancer` interface: The `Lease` method was renamed to `LeaseAsync`. + Interface FQN: `Ocelot.LoadBalancer.LoadBalancers.ILoadBalancer` + Method FQN: `Ocelot.LoadBalancer.LoadBalancers.ILoadBalancer.LeaseAsync` +- TO BE Written \ No newline at end of file diff --git a/docs/features/servicediscovery.rst b/docs/features/servicediscovery.rst index a4e4f96c5..ed2915a63 100644 --- a/docs/features/servicediscovery.rst +++ b/docs/features/servicediscovery.rst @@ -246,10 +246,12 @@ However, the quickest and most streamlined approach is to inherit directly from public class MyConsulServiceBuilder : DefaultConsulServiceBuilder { - public MyConsulServiceBuilder(Func configurationFactory, IConsulClientFactory clientFactory, IOcelotLoggerFactory loggerFactory) - : base(configurationFactory, clientFactory, loggerFactory) { } + public MyConsulServiceBuilder(IHttpContextAccessor contextAccessor, IConsulClientFactory clientFactory, IOcelotLoggerFactory loggerFactory) + : base(contextAccessor, clientFactory, loggerFactory) { } + // I want to use the agent service IP address as the downstream hostname - protected override string GetDownstreamHost(ServiceEntry entry, Node node) => entry.Service.Address; + protected override string GetDownstreamHost(ServiceEntry entry, Node node) + => entry.Service.Address; } **Second**, we must inject the new behavior into DI, as demonstrated in the Ocelot versus Consul setup: @@ -543,7 +545,7 @@ But you can leave this ``Type`` option for compatibility between both designs. .. _KV Store: https://developer.hashicorp.com/consul/docs/dynamic-app-config/kv .. _3 seconds TTL: https://github.com/search?q=repo%3AThreeMammals%2FOcelot+TimeSpan.FromSeconds%283%29&type=code .. _catalog nodes: https://developer.hashicorp.com/consul/api-docs/catalog#list-nodes -.. _the acceptance test: https://github.com/search?q=repo%3AThreeMammals%2FOcelot+Should_return_service_address_by_overridden_service_builder_when_there_is_a_node&type=code +.. _the acceptance test: https://github.com/search?q=repo%3AThreeMammals%2FOcelot+ShouldReturnServiceAddressByOverriddenServiceBuilderWhenThereIsANode&type=code .. _346: https://github.com/ThreeMammals/Ocelot/issues/346 .. _909: https://github.com/ThreeMammals/Ocelot/pull/909 .. _954: https://github.com/ThreeMammals/Ocelot/issues/954 diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index 27b5b4422..9be0128e0 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -33,21 +33,16 @@ public virtual async Task> GetAsync() var entries = entriesTask.Result.Response ?? Array.Empty(); var nodes = nodesTask.Result.Response ?? Array.Empty(); - var services = new List(); - - if (entries.Length != 0) - { - _logger.LogDebug(() => $"{nameof(Consul)} Provider: Found total {entries.Length} service entries for '{_configuration.KeyOfServiceInConsul}' service."); - _logger.LogDebug(() => $"{nameof(Consul)} Provider: Found total {nodes.Length} catalog nodes."); - var collection = BuildServices(entries, nodes); - services.AddRange(collection); - } - else + if (entries.Length == 0) { _logger.LogWarning(() => $"{nameof(Consul)} Provider: No service entries found for '{_configuration.KeyOfServiceInConsul}' service!"); + return new(); } - return services; + _logger.LogDebug(() => $"{nameof(Consul)} Provider: Found total {entries.Length} service entries for '{_configuration.KeyOfServiceInConsul}' service."); + _logger.LogDebug(() => $"{nameof(Consul)} Provider: Found total {nodes.Length} catalog nodes."); + return BuildServices(entries, nodes) + .ToList(); } protected virtual IEnumerable BuildServices(ServiceEntry[] entries, Node[] nodes) diff --git a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs index f7c5c0c0c..fdece1d99 100644 --- a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs @@ -4,10 +4,15 @@ namespace Ocelot.Provider.Consul; public class ConsulClientFactory : IConsulClientFactory { + // TODO We need this overloaded method -> + //public IConsulClient Get(ServiceProviderConfiguration config) public IConsulClient Get(ConsulRegistryConfiguration config) => new ConsulClient(c => OverrideConfig(c, config)); - private static void OverrideConfig(ConsulClientConfiguration to, ConsulRegistryConfiguration from) + // TODO -> + //private static void OverrideConfig(ConsulClientConfiguration to, ServiceProviderConfiguration from) + // Factory which consumes concrete types is a bad factory! A more abstract types are required + private static void OverrideConfig(ConsulClientConfiguration to, ConsulRegistryConfiguration from) // TODO Why ConsulRegistryConfiguration? We use ServiceProviderConfiguration props only! :) { to.Address = new Uri($"{from.Scheme}://{from.Host}:{from.Port}"); diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 00c2715ee..dbc4b9155 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -1,4 +1,5 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; using Ocelot.Logging; using Ocelot.Provider.Consul.Interfaces; @@ -6,36 +7,37 @@ namespace Ocelot.Provider.Consul; -public static class ConsulProviderFactory +/// +/// TODO It must be refactored converting to real factory-class and add to DI. +/// +/// +/// Must inherit from interface. +/// Also the must be removed from the design. +/// +public static class ConsulProviderFactory // TODO : IServiceDiscoveryProviderFactory { - /// - /// String constant used for provider type definition. - /// + /// String constant used for provider type definition. public const string PollConsul = nameof(Provider.Consul.PollConsul); - private static readonly List ServiceDiscoveryProviders = new(); - private static readonly object LockObject = new(); + private static readonly List ServiceDiscoveryProviders = new(); // TODO It must be scoped service in DI-container + private static readonly object SyncRoot = new(); public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; - - private static ConsulRegistryConfiguration configuration; - private static ConsulRegistryConfiguration ConfigurationGetter() => configuration; - public static Func GetConfiguration { get; } = ConfigurationGetter; - - private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, - ServiceProviderConfiguration config, DownstreamRoute route) + private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var factory = provider.GetService(); var consulFactory = provider.GetService(); + var contextAccessor = provider.GetService(); - configuration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); - var serviceBuilder = provider.GetService(); + var configuration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); // TODO Why not to pass 2 args only: config, route? LoL + contextAccessor.HttpContext.Items[nameof(ConsulRegistryConfiguration)] = configuration; // initialize data + var serviceBuilder = provider.GetService(); // consume data in default/custom builder - var consulProvider = new Consul(configuration, factory, consulFactory, serviceBuilder); + var consulProvider = new Consul(configuration, factory, consulFactory, serviceBuilder); // TODO It must be added to DI-container! if (PollConsul.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) { - lock (LockObject) + lock (SyncRoot) { var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); if (discoveryProvider != null) diff --git a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs index 255c7b686..2109f9eaf 100644 --- a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs +++ b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs @@ -1,6 +1,6 @@ namespace Ocelot.Provider.Consul; -public class ConsulRegistryConfiguration +public class ConsulRegistryConfiguration // TODO Inherit from ServiceProviderConfiguration ? { /// /// Consul HTTP client default port. @@ -12,6 +12,7 @@ public class ConsulRegistryConfiguration public ConsulRegistryConfiguration(string scheme, string host, int port, string keyOfServiceInConsul, string token) { + // TODO Why not to encapsulate this biz logic right in ConsulProviderFactory? LoL Host = string.IsNullOrEmpty(host) ? "localhost" : host; Port = port > 0 ? port : DefaultHttpPort; Scheme = string.IsNullOrEmpty(scheme) ? Uri.UriSchemeHttp : scheme; diff --git a/src/Ocelot.Provider.Consul/DefaultConsulServiceBuilder.cs b/src/Ocelot.Provider.Consul/DefaultConsulServiceBuilder.cs index 7526bea65..4d9abe7a7 100644 --- a/src/Ocelot.Provider.Consul/DefaultConsulServiceBuilder.cs +++ b/src/Ocelot.Provider.Consul/DefaultConsulServiceBuilder.cs @@ -1,4 +1,5 @@ -using Ocelot.Infrastructure.Extensions; +using Microsoft.AspNetCore.Http; +using Ocelot.Infrastructure.Extensions; using Ocelot.Logging; using Ocelot.Provider.Consul.Interfaces; using Ocelot.Values; @@ -7,23 +8,31 @@ namespace Ocelot.Provider.Consul; public class DefaultConsulServiceBuilder : IConsulServiceBuilder { - private readonly ConsulRegistryConfiguration _configuration; - private readonly IConsulClient _client; - private readonly IOcelotLogger _logger; + private readonly HttpContext _context; + private readonly IConsulClientFactory _clientFactory; + private readonly IOcelotLoggerFactory _loggerFactory; + + private ConsulRegistryConfiguration _configuration; + private IConsulClient _client; + private IOcelotLogger _logger; public DefaultConsulServiceBuilder( - Func configurationFactory, + IHttpContextAccessor contextAccessor, IConsulClientFactory clientFactory, IOcelotLoggerFactory loggerFactory) { - _configuration = configurationFactory.Invoke(); - _client = clientFactory.Get(_configuration); - _logger = loggerFactory.CreateLogger(); + _context = contextAccessor.HttpContext; + _clientFactory = clientFactory; + _loggerFactory = loggerFactory; } - public ConsulRegistryConfiguration Configuration => _configuration; - protected IConsulClient Client => _client; - protected IOcelotLogger Logger => _logger; + // TODO See comment in the interface about the privacy. The goal is to eliminate IBC! + // So, we need more abstract type, and ServiceProviderConfiguration is a good choice. The rest of props can be obtained from HttpContext + protected /*public*/ ConsulRegistryConfiguration Configuration => _configuration + ??= _context.Items.TryGetValue(nameof(ConsulRegistryConfiguration), out var value) + ? value as ConsulRegistryConfiguration : default; + protected IConsulClient Client => _client ??= _clientFactory.Get(Configuration); + protected IOcelotLogger Logger => _logger ??= _loggerFactory.CreateLogger(); public virtual bool IsValid(ServiceEntry entry) { @@ -36,7 +45,7 @@ public virtual bool IsValid(ServiceEntry entry) if (!valid) { - _logger.LogWarning( + Logger.LogWarning( () => $"Unable to use service address: '{service.Address}' and port: {service.Port} as it is invalid for the service: '{service.Service}'. Address must contain host only e.g. 'localhost', and port must be greater than 0."); } diff --git a/src/Ocelot.Provider.Consul/Interfaces/IConsulServiceBuilder.cs b/src/Ocelot.Provider.Consul/Interfaces/IConsulServiceBuilder.cs index 0555b0144..fab45dfe4 100644 --- a/src/Ocelot.Provider.Consul/Interfaces/IConsulServiceBuilder.cs +++ b/src/Ocelot.Provider.Consul/Interfaces/IConsulServiceBuilder.cs @@ -4,7 +4,8 @@ namespace Ocelot.Provider.Consul.Interfaces; public interface IConsulServiceBuilder { - ConsulRegistryConfiguration Configuration { get; } + // Keep config private (deep encapsulation) until an architectural decision is made. + // ConsulRegistryConfiguration Configuration { get; } bool IsValid(ServiceEntry entry); IEnumerable BuildServices(ServiceEntry[] entries, Node[] nodes); Service CreateService(ServiceEntry serviceEntry, Node serviceNode); diff --git a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs index 0c064f780..aed4a528d 100644 --- a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs @@ -24,9 +24,8 @@ public static IOcelotBuilder AddConsul(this IOcelotBuilder builder) { builder.Services .AddSingleton(ConsulProviderFactory.Get) - .AddSingleton(ConsulProviderFactory.GetConfiguration) .AddSingleton() - .AddSingleton() + .AddScoped() .RemoveAll(typeof(IFileConfigurationPollerOptions)) .AddSingleton(); return builder; @@ -49,7 +48,7 @@ public static IOcelotBuilder AddConsul(this IOcelotBuilder buil { AddConsul(builder).Services .RemoveAll() - .AddSingleton(typeof(IConsulServiceBuilder), typeof(TServiceBuilder)); + .AddScoped(typeof(IConsulServiceBuilder), typeof(TServiceBuilder)); return builder; } diff --git a/src/Ocelot/DependencyInjection/OcelotBuilder.cs b/src/Ocelot/DependencyInjection/OcelotBuilder.cs index 217280e37..146ba3c65 100644 --- a/src/Ocelot/DependencyInjection/OcelotBuilder.cs +++ b/src/Ocelot/DependencyInjection/OcelotBuilder.cs @@ -4,7 +4,6 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Options; using Ocelot.Authorization; -using Ocelot.Cache; using Ocelot.Claims; using Ocelot.Configuration; using Ocelot.Configuration.ChangeTracking; @@ -27,7 +26,6 @@ using Ocelot.Multiplexer; using Ocelot.PathManipulation; using Ocelot.QueryStrings; -using Ocelot.RateLimiting; using Ocelot.Request.Creator; using Ocelot.Request.Mapper; using Ocelot.Requester; @@ -119,10 +117,8 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo Services.AddOcelotMetadata(); Services.AddOcelotMessageInvokerPool(); - // See this for why we register this as singleton: - // http://stackoverflow.com/questions/37371264/invalidoperationexception-unable-to-resolve-service-for-type-microsoft-aspnetc - // Could maybe use a scoped data repository - Services.TryAddSingleton(); + // Chinese developers should read StackOverflow ignoring Microsoft Learn docs -> http://stackoverflow.com/questions/37371264/invalidoperationexception-unable-to-resolve-service-for-type-microsoft-aspnetc + Services.AddHttpContextAccessor(); Services.TryAddSingleton(); Services.AddMemoryCache(); Services.TryAddSingleton(); @@ -206,44 +202,50 @@ public IOcelotBuilder AddTransientDefinedAggregator() return this; } - public IOcelotBuilder AddCustomLoadBalancer() - where T : ILoadBalancer, new() + public IOcelotBuilder AddCustomLoadBalancer() + where TLoadBalancer : ILoadBalancer, new() { - AddCustomLoadBalancer((provider, route, serviceDiscoveryProvider) => new T()); - return this; + TLoadBalancer Create(IServiceProvider provider, DownstreamRoute route, IServiceDiscoveryProvider discoveryProvider) + => new(); + return AddCustomLoadBalancer(Create); } - public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) - where T : ILoadBalancer + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where TLoadBalancer : ILoadBalancer { - AddCustomLoadBalancer((provider, route, serviceDiscoveryProvider) => - loadBalancerFactoryFunc()); - return this; + TLoadBalancer Create(IServiceProvider provider, DownstreamRoute route, IServiceDiscoveryProvider discoveryProvider) + => loadBalancerFactoryFunc(); + return AddCustomLoadBalancer(Create); } - public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) - where T : ILoadBalancer + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where TLoadBalancer : ILoadBalancer { - AddCustomLoadBalancer((provider, route, serviceDiscoveryProvider) => - loadBalancerFactoryFunc(provider)); - return this; + TLoadBalancer Create(IServiceProvider provider, DownstreamRoute route, IServiceDiscoveryProvider discoveryProvider) + => loadBalancerFactoryFunc(provider); + return AddCustomLoadBalancer(Create); } - public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) - where T : ILoadBalancer + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where TLoadBalancer : ILoadBalancer { - AddCustomLoadBalancer((provider, route, serviceDiscoveryProvider) => - loadBalancerFactoryFunc(route, serviceDiscoveryProvider)); - return this; + TLoadBalancer Create(IServiceProvider provider, DownstreamRoute route, IServiceDiscoveryProvider discoveryProvider) + => loadBalancerFactoryFunc(route, discoveryProvider); + return AddCustomLoadBalancer(Create); } - public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) - where T : ILoadBalancer + public IOcelotBuilder AddCustomLoadBalancer(Func loadBalancerFactoryFunc) + where TLoadBalancer : ILoadBalancer { - Services.AddSingleton(provider => - new DelegateInvokingLoadBalancerCreator( - (route, serviceDiscoveryProvider) => - loadBalancerFactoryFunc(provider, route, serviceDiscoveryProvider))); + ILoadBalancer Create(DownstreamRoute route, IServiceDiscoveryProvider discoveryProvider) + => loadBalancerFactoryFunc(_serviceProvider, route, discoveryProvider); + ILoadBalancerCreator implementationFactory(IServiceProvider provider) + { + _serviceProvider = provider; + return new DelegateInvokingLoadBalancerCreator(Create); + } + + Services.AddSingleton(implementationFactory); return this; } @@ -257,9 +259,9 @@ public IOcelotBuilder AddDelegatingHandler(Type delegateType, bool global = fals if (global) { Services.AddTransient(delegateType); - Services.AddTransient(s => + Services.AddTransient(provider => { - var service = s.GetService(delegateType) as DelegatingHandler; + var service = provider.GetService(delegateType) as DelegatingHandler; return new GlobalDelegatingHandler(service); }); } @@ -277,9 +279,9 @@ public IOcelotBuilder AddDelegatingHandler(bool global = false) if (global) { Services.AddTransient(); - Services.AddTransient(s => + Services.AddTransient(provider => { - var service = s.GetService(); + var service = provider.GetService(); return new GlobalDelegatingHandler(service); }); } @@ -302,15 +304,20 @@ public IOcelotBuilder AddConfigPlaceholders() Services.Replace(ServiceDescriptor.Describe( typeof(IPlaceholders), - s => (IPlaceholders)objectFactory(s, - new[] { CreateInstance(s, wrappedDescriptor) }), + provider => (IPlaceholders)objectFactory( + provider, + new[] { CreateInstance(provider, wrappedDescriptor) }), wrappedDescriptor.Lifetime )); return this; } - private static object CreateInstance(IServiceProvider services, ServiceDescriptor descriptor) + + /// For local implementation purposes, so it MUST NOT be public!.. + private IServiceProvider _serviceProvider; // TODO Reuse ActivatorUtilities factories? + + private static object CreateInstance(IServiceProvider provider, ServiceDescriptor descriptor) { if (descriptor.ImplementationInstance != null) { @@ -319,10 +326,10 @@ private static object CreateInstance(IServiceProvider services, ServiceDescripto if (descriptor.ImplementationFactory != null) { - return descriptor.ImplementationFactory(services); + return descriptor.ImplementationFactory(provider); } - return ActivatorUtilities.GetServiceOrCreateInstance(services, descriptor.ImplementationType); + return ActivatorUtilities.GetServiceOrCreateInstance(provider, descriptor.ImplementationType); } } } diff --git a/src/Ocelot/Infrastructure/Placeholders.cs b/src/Ocelot/Infrastructure/Placeholders.cs index c0ca6a151..e69fbe578 100644 --- a/src/Ocelot/Infrastructure/Placeholders.cs +++ b/src/Ocelot/Infrastructure/Placeholders.cs @@ -12,24 +12,24 @@ public class Placeholders : IPlaceholders private readonly Dictionary> _requestPlaceholders; private readonly IBaseUrlFinder _finder; private readonly IRequestScopedDataRepository _repo; - private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IHttpContextAccessor _contextAccessor; - public Placeholders(IBaseUrlFinder finder, IRequestScopedDataRepository repo, IHttpContextAccessor httpContextAccessor) + public Placeholders(IBaseUrlFinder finder, IRequestScopedDataRepository repo, IHttpContextAccessor contextAccessor) { _repo = repo; - _httpContextAccessor = httpContextAccessor; + _contextAccessor = contextAccessor; _finder = finder; _placeholders = new Dictionary>> { - { "{BaseUrl}", GetBaseUrl() }, - { "{TraceId}", GetTraceId() }, - { "{RemoteIpAddress}", GetRemoteIpAddress() }, - { "{UpstreamHost}", GetUpstreamHost() }, + { "{BaseUrl}", GetBaseUrl }, + { "{TraceId}", GetTraceId }, + { "{RemoteIpAddress}", GetRemoteIpAddress }, + { "{UpstreamHost}", GetUpstreamHost }, }; _requestPlaceholders = new Dictionary> { - { "{DownstreamBaseUrl}", GetDownstreamBaseUrl() }, + { "{DownstreamBaseUrl}", GetDownstreamBaseUrl }, }; } @@ -49,23 +49,16 @@ public Response Get(string key) public Response Get(string key, DownstreamRequest request) { - if (_requestPlaceholders.ContainsKey(key)) - { - return new OkResponse(_requestPlaceholders[key].Invoke(request)); - } - - return new ErrorResponse(new CouldNotFindPlaceholderError(key)); + return _requestPlaceholders.TryGetValue(key, out var func) + ? new OkResponse(func.Invoke(request)) + : new ErrorResponse(new CouldNotFindPlaceholderError(key)); } public Response Add(string key, Func> func) { - if (_placeholders.ContainsKey(key)) - { - return new ErrorResponse(new CannotAddPlaceholderError($"Unable to add placeholder: {key}, placeholder already exists")); - } - - _placeholders.Add(key, func); - return new OkResponse(); + return _placeholders.TryAdd(key, func) + ? new OkResponse() + : new ErrorResponse(new CannotAddPlaceholderError($"Unable to add placeholder: {key}, placeholder already exists")); } public Response Remove(string key) @@ -79,75 +72,53 @@ public Response Remove(string key) return new OkResponse(); } - private Func> GetRemoteIpAddress() + private Response GetRemoteIpAddress() { - return () => + // this can blow up so adding try catch and return error + try { - // this can blow up so adding try catch and return error - try - { - var remoteIdAddress = _httpContextAccessor.HttpContext.Connection.RemoteIpAddress.ToString(); - return new OkResponse(remoteIdAddress); - } - catch - { - return new ErrorResponse(new CouldNotFindPlaceholderError("{RemoteIpAddress}")); - } - }; - } - - private static Func GetDownstreamBaseUrl() - { - return x => + var remoteIdAddress = _contextAccessor.HttpContext.Connection.RemoteIpAddress.ToString(); + return new OkResponse(remoteIdAddress); + } + catch { - var downstreamUrl = $"{x.Scheme}://{x.Host}"; - - if (x.Port != 80 && x.Port != 443) - { - downstreamUrl = $"{downstreamUrl}:{x.Port}"; - } - - return $"{downstreamUrl}/"; - }; + return new ErrorResponse(new CouldNotFindPlaceholderError("{RemoteIpAddress}")); + } } - private Func> GetTraceId() + private static string GetDownstreamBaseUrl(DownstreamRequest x) { - return () => + var downstreamUrl = $"{x.Scheme}://{x.Host}"; + if (x.Port != 80 && x.Port != 443) { - var traceId = _repo.Get("TraceId"); - if (traceId.IsError) - { - return new ErrorResponse(traceId.Errors); - } + downstreamUrl = $"{downstreamUrl}:{x.Port}"; + } - return new OkResponse(traceId.Data); - }; + return $"{downstreamUrl}/"; } - private Func> GetBaseUrl() + private Response GetTraceId() { - return () => new OkResponse(_finder.Find()); + var traceId = _repo.Get("TraceId"); + return traceId.IsError + ? new ErrorResponse(traceId.Errors) + : new OkResponse(traceId.Data); } - private Func> GetUpstreamHost() + private Response GetBaseUrl() => new OkResponse(_finder.Find()); + + private Response GetUpstreamHost() { - return () => + try { - try - { - if (_httpContextAccessor.HttpContext.Request.Headers.TryGetValue("Host", out var upstreamHost)) - { - return new OkResponse(upstreamHost.First()); - } - - return new ErrorResponse(new CouldNotFindPlaceholderError("{UpstreamHost}")); - } - catch - { - return new ErrorResponse(new CouldNotFindPlaceholderError("{UpstreamHost}")); - } - }; + return _contextAccessor.HttpContext.Request.Headers.TryGetValue("Host", out var upstreamHost) + ? new OkResponse(upstreamHost.First()) + : new ErrorResponse(new CouldNotFindPlaceholderError("{UpstreamHost}")); + } + catch + { + return new ErrorResponse(new CouldNotFindPlaceholderError("{UpstreamHost}")); + } } } } diff --git a/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs b/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs index 24e0607f1..f1170d895 100644 --- a/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs +++ b/src/Ocelot/Infrastructure/RequestData/HttpDataRepository.cs @@ -5,18 +5,18 @@ namespace Ocelot.Infrastructure.RequestData { public class HttpDataRepository : IRequestScopedDataRepository { - private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IHttpContextAccessor _contextAccessor; - public HttpDataRepository(IHttpContextAccessor httpContextAccessor) + public HttpDataRepository(IHttpContextAccessor contextAccessor) { - _httpContextAccessor = httpContextAccessor; + _contextAccessor = contextAccessor; } public Response Add(string key, T value) { try { - _httpContextAccessor.HttpContext.Items.Add(key, value); + _contextAccessor.HttpContext.Items.Add(key, value); return new OkResponse(); } catch (Exception exception) @@ -29,7 +29,7 @@ public Response Update(string key, T value) { try { - _httpContextAccessor.HttpContext.Items[key] = value; + _contextAccessor.HttpContext.Items[key] = value; return new OkResponse(); } catch (Exception exception) @@ -40,18 +40,14 @@ public Response Update(string key, T value) public Response Get(string key) { - if (_httpContextAccessor.HttpContext == null || _httpContextAccessor.HttpContext.Items == null) + if (_contextAccessor?.HttpContext?.Items == null) { return new ErrorResponse(new CannotFindDataError($"Unable to find data for key: {key} because HttpContext or HttpContext.Items is null")); } - if (_httpContextAccessor.HttpContext.Items.TryGetValue(key, out var obj)) - { - var data = (T)obj; - return new OkResponse(data); - } - - return new ErrorResponse(new CannotFindDataError($"Unable to find data for key: {key}")); + return _contextAccessor.HttpContext.Items.TryGetValue(key, out var item) + ? new OkResponse((T)item) + : new ErrorResponse(new CannotFindDataError($"Unable to find data for key: {key}")); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/Lease.cs b/src/Ocelot/LoadBalancer/Lease.cs similarity index 92% rename from src/Ocelot/LoadBalancer/LoadBalancers/Lease.cs rename to src/Ocelot/LoadBalancer/Lease.cs index 46d120e4c..f0ba048c3 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/Lease.cs +++ b/src/Ocelot/LoadBalancer/Lease.cs @@ -1,63 +1,63 @@ -using Ocelot.Values; - -namespace Ocelot.LoadBalancer.LoadBalancers; - -public struct Lease : IEquatable -{ - public Lease() - { - HostAndPort = null; - Connections = 0; - } - - public Lease(Lease from) - { - HostAndPort = from.HostAndPort; - Connections = from.Connections; - } - - public Lease(ServiceHostAndPort hostAndPort) - { - HostAndPort = hostAndPort; - Connections = 0; - } - - public Lease(ServiceHostAndPort hostAndPort, int connections) - { - HostAndPort = hostAndPort; - Connections = connections; - } - - public ServiceHostAndPort HostAndPort { get; } - public int Connections { get; set; } - - public static Lease Null => new(); - - public override readonly string ToString() => $"({HostAndPort}+{Connections})"; - public override readonly int GetHashCode() => HostAndPort.GetHashCode(); - public override readonly bool Equals(object obj) => obj is Lease l && this == l; - public readonly bool Equals(Lease other) => this == other; - - /// Checks equality of two leases. - /// - /// Override default implementation of because we want to ignore the property. - /// Microsoft Learn | .NET | C# Docs: - /// - /// Equality operators - /// System.Object.Equals method - /// IEquatable<T>.Equals(T) Method - /// ValueType.Equals(Object) Method - /// - /// - /// First operand. - /// Second operand. - /// if both operands are equal; otherwise, . - public static bool operator ==(Lease x, Lease y) => x.HostAndPort == y.HostAndPort; // ignore -> x.Connections == y.Connections; - public static bool operator !=(Lease x, Lease y) => !(x == y); - - public static bool operator ==(ServiceHostAndPort h, Lease l) => h == l.HostAndPort; - public static bool operator !=(ServiceHostAndPort h, Lease l) => !(h == l); - - public static bool operator ==(Lease l, ServiceHostAndPort h) => l.HostAndPort == h; - public static bool operator !=(Lease l, ServiceHostAndPort h) => !(l == h); -} +using Ocelot.Values; + +namespace Ocelot.LoadBalancer; + +public struct Lease : IEquatable +{ + public Lease() + { + HostAndPort = null; + Connections = 0; + } + + public Lease(Lease from) + { + HostAndPort = from.HostAndPort; + Connections = from.Connections; + } + + public Lease(ServiceHostAndPort hostAndPort) + { + HostAndPort = hostAndPort; + Connections = 0; + } + + public Lease(ServiceHostAndPort hostAndPort, int connections) + { + HostAndPort = hostAndPort; + Connections = connections; + } + + public ServiceHostAndPort HostAndPort { get; } + public int Connections { get; set; } + + public static Lease Null => new(); + + public override readonly string ToString() => $"({HostAndPort}+{Connections})"; + public override readonly int GetHashCode() => HostAndPort.GetHashCode(); + public override readonly bool Equals(object obj) => obj is Lease l && this == l; + public readonly bool Equals(Lease other) => this == other; + + /// Checks equality of two leases. + /// + /// Override default implementation of because we want to ignore the property. + /// Microsoft Learn | .NET | C# Docs: + /// + /// Equality operators + /// System.Object.Equals method + /// IEquatable<T>.Equals(T) Method + /// ValueType.Equals(Object) Method + /// + /// + /// First operand. + /// Second operand. + /// if both operands are equal; otherwise, . + public static bool operator ==(Lease x, Lease y) => x.HostAndPort == y.HostAndPort; // ignore -> x.Connections == y.Connections; + public static bool operator !=(Lease x, Lease y) => !(x == y); + + public static bool operator ==(ServiceHostAndPort h, Lease l) => h == l.HostAndPort; + public static bool operator !=(ServiceHostAndPort h, Lease l) => !(h == l); + + public static bool operator ==(Lease l, ServiceHostAndPort h) => l.HostAndPort == h; + public static bool operator !=(Lease l, ServiceHostAndPort h) => !(l == h); +} diff --git a/src/Ocelot/LoadBalancer/LeaseEventArgs.cs b/src/Ocelot/LoadBalancer/LeaseEventArgs.cs new file mode 100644 index 000000000..f15e37e85 --- /dev/null +++ b/src/Ocelot/LoadBalancer/LeaseEventArgs.cs @@ -0,0 +1,17 @@ +using Ocelot.Values; + +namespace Ocelot.LoadBalancer; + +public class LeaseEventArgs : EventArgs +{ + public LeaseEventArgs(Lease lease, Service service, int serviceIndex) + { + Lease = lease; + Service = service; + ServiceIndex = serviceIndex; + } + + public Lease Lease { get; } + public Service Service { get; } + public int ServiceIndex { get; } +} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs index a135855a4..5499c9b19 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/CookieStickySessions.cs @@ -16,6 +16,8 @@ public class CookieStickySessions : ILoadBalancer private static readonly object Locker = new(); private static readonly Dictionary Stored = new(); // TODO Inject instead of static sharing + public string Type => nameof(CookieStickySessions); + public CookieStickySessions(ILoadBalancer loadBalancer, string cookieName, int keyExpiryInMs, IBus bus) { _bus = bus; @@ -40,7 +42,7 @@ private void CheckExpiry(StickySession sticky) } } - public Task> Lease(HttpContext httpContext) + public Task> LeaseAsync(HttpContext httpContext) { var route = httpContext.Items.DownstreamRoute(); var serviceName = route.LoadBalancerKey; @@ -56,7 +58,7 @@ public Task> Lease(HttpContext httpContext) } // There is no value in the store, so lease it now! - var next = _loadBalancer.Lease(httpContext).GetAwaiter().GetResult(); // unfortunately the operation must be synchronous + var next = _loadBalancer.LeaseAsync(httpContext).GetAwaiter().GetResult(); // unfortunately the operation must be synchronous if (next.IsError) { return Task.FromResult>(new ErrorResponse(next.Errors)); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs index ccf997c55..4070bc01f 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/ILoadBalancer.cs @@ -1,13 +1,20 @@ using Microsoft.AspNetCore.Http; using Ocelot.Responses; using Ocelot.Values; +using System.Reflection; namespace Ocelot.LoadBalancer.LoadBalancers { + // TODO Add sync & async pairs public interface ILoadBalancer { - Task> Lease(HttpContext httpContext); + Task> LeaseAsync(HttpContext httpContext); void Release(ServiceHostAndPort hostAndPort); + + /// Static name of the load balancer instance. + /// To avoid reflection calls of the property of the objects. + /// A object with type name value. + string Type { get; } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs index bede3ba8e..21d19b11b 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnection.cs @@ -9,7 +9,9 @@ public class LeastConnection : ILoadBalancer private readonly Func>> _services; private readonly List _leases; private readonly string _serviceName; - private static readonly object SyncLock = new(); + private static readonly object SyncRoot = new(); + + public string Type => nameof(LeastConnection); public LeastConnection(Func>> services, string serviceName) { @@ -18,31 +20,38 @@ public LeastConnection(Func>> services, string serviceName) _leases = new List(); } - public async Task> Lease(HttpContext httpContext) + public event EventHandler Leased; + protected virtual void OnLeased(LeaseEventArgs e) => Leased?.Invoke(this, e); + + public async Task> LeaseAsync(HttpContext httpContext) { var services = await _services.Invoke(); if ((services?.Count ?? 0) == 0) { - return new ErrorResponse(new ServicesAreNullError($"Services were null/empty in {nameof(LeastConnection)} for '{_serviceName}' during {nameof(Lease)} operation!")); + return new ErrorResponse(new ServicesAreNullError($"Services were null/empty in {Type} for '{_serviceName}' during {nameof(LeaseAsync)} operation!")); } - lock (SyncLock) + lock (SyncRoot) { //todo - maybe this should be moved somewhere else...? Maybe on a repeater on seperate thread? loop every second and update or something? UpdateLeasing(services); Lease wanted = GetLeaseWithLeastConnections(); _ = Update(ref wanted, true); + + var index = services.FindIndex(s => s.HostAndPort == wanted); + OnLeased(new(wanted, services[index], index)); + return new OkResponse(new(wanted.HostAndPort)); } } public void Release(ServiceHostAndPort hostAndPort) { - lock (SyncLock) + lock (SyncRoot) { var matchingLease = _leases.Find(l => l == hostAndPort); - if (matchingLease != LoadBalancers.Lease.Null) + if (matchingLease != Lease.Null) { _ = Update(ref matchingLease, false); } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs index e5d15fa2d..faa071e9d 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LeastConnectionCreator.cs @@ -8,7 +8,12 @@ public class LeastConnectionCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - return new OkResponse(new LeastConnection(async () => await serviceProvider.GetAsync(), route.ServiceName)); + var loadBalancer = new LeastConnection( + serviceProvider.GetAsync, + !string.IsNullOrEmpty(route.ServiceName) + ? route.ServiceName + : route.LoadBalancerKey); // if service discovery mode then use service name; otherwise use balancer key + return new OkResponse(loadBalancer); } public string Type => nameof(LeastConnection); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs index dfa6279e6..79cb72ad1 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/LoadBalancerHouse.cs @@ -1,5 +1,4 @@ using Ocelot.Configuration; -using Ocelot.Errors; using Ocelot.Responses; namespace Ocelot.LoadBalancer.LoadBalancers @@ -7,57 +6,45 @@ namespace Ocelot.LoadBalancer.LoadBalancers public class LoadBalancerHouse : ILoadBalancerHouse { private readonly ILoadBalancerFactory _factory; - private readonly ConcurrentDictionary _loadBalancers; + private readonly Dictionary _loadBalancers; + private static readonly object SyncRoot = new(); public LoadBalancerHouse(ILoadBalancerFactory factory) { _factory = factory; - _loadBalancers = new ConcurrentDictionary(); + _loadBalancers = new(); } public Response Get(DownstreamRoute route, ServiceProviderConfiguration config) { try { - if (_loadBalancers.TryGetValue(route.LoadBalancerKey, out var loadBalancer)) - { - // TODO Fix ugly reflection issue of dymanic detection in favor of static type property - if (route.LoadBalancerOptions.Type != loadBalancer.GetType().Name) - { - return GetResponse(route, config); - } - - return new OkResponse(loadBalancer); + lock (SyncRoot) + { + return (_loadBalancers.TryGetValue(route.LoadBalancerKey, out var loadBalancer) && + route.LoadBalancerOptions.Type == loadBalancer.Type) // TODO Case insensitive? + ? new OkResponse(loadBalancer) + : GetResponse(route, config); } - - return GetResponse(route, config); } catch (Exception ex) { - return new ErrorResponse(new List() - { - new UnableToFindLoadBalancerError($"Unable to find load balancer for '{route.LoadBalancerKey}'. Exception: {ex};"), - }); + return new ErrorResponse( + new UnableToFindLoadBalancerError($"Unable to find load balancer for '{route.LoadBalancerKey}'. Exception: {ex};")); } } private Response GetResponse(DownstreamRoute route, ServiceProviderConfiguration config) { var result = _factory.Get(route, config); - if (result.IsError) { return new ErrorResponse(result.Errors); } - var loadBalancer = result.Data; - AddLoadBalancer(route.LoadBalancerKey, loadBalancer); - return new OkResponse(loadBalancer); - } - - private void AddLoadBalancer(string key, ILoadBalancer loadBalancer) - { - _loadBalancers.AddOrUpdate(key, loadBalancer, (x, y) => loadBalancer); + var balancer = result.Data; + _loadBalancers[route.LoadBalancerKey] = balancer; // TODO TryAdd ? + return new OkResponse(balancer); } } } diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs index 725b0d33d..6d3c0a94c 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/NoLoadBalancer.cs @@ -13,13 +13,15 @@ public NoLoadBalancer(Func>> services) _services = services; } - public async Task> Lease(HttpContext httpContext) + public string Type => nameof(NoLoadBalancer); + + public async Task> LeaseAsync(HttpContext httpContext) { var services = await _services(); if (services == null || services.Count == 0) { - return new ErrorResponse(new ServicesAreEmptyError("There were no services in NoLoadBalancer")); + return new ErrorResponse(new ServicesAreEmptyError($"There were no services in {Type}!")); } var service = await Task.FromResult(services.FirstOrDefault()); diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs index 9c087f77a..8febb5b29 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobin.cs @@ -10,6 +10,8 @@ public class RoundRobin : ILoadBalancer private readonly string _serviceName; private readonly List _leasing; + public string Type => nameof(RoundRobin); + public RoundRobin(Func>> services, string serviceName) { _servicesDelegate = services; @@ -23,12 +25,12 @@ public RoundRobin(Func>> services, string serviceName) public event EventHandler Leased; protected virtual void OnLeased(LeaseEventArgs e) => Leased?.Invoke(this, e); - public virtual async Task> Lease(HttpContext httpContext) + public virtual async Task> LeaseAsync(HttpContext httpContext) { var services = await _servicesDelegate?.Invoke() ?? new List(); if (services.Count == 0) { - return new ErrorResponse(new ServicesAreEmptyError($"There were no services in {nameof(RoundRobin)} for '{_serviceName}' during {nameof(Lease)} operation!")); + return new ErrorResponse(new ServicesAreEmptyError($"There were no services in {Type} for '{_serviceName}' during {nameof(LeaseAsync)} operation!")); } lock (SyncRoot) @@ -36,7 +38,7 @@ public virtual async Task> Lease(HttpContext httpCo var readMe = CaptureState(services, out int count); if (!TryScanNext(readMe, out Service next, out int index)) { - return new ErrorResponse(new ServicesAreNullError($"The service at index {index} was null in {nameof(RoundRobin)} for {_serviceName} during the {nameof(Lease)} operation. Total services count: {count}.")); + return new ErrorResponse(new ServicesAreNullError($"The service at index {index} was null in {Type} for {_serviceName} during the {nameof(LeaseAsync)} operation. Total services count: {count}.")); } ProcessLeasing(readMe, next, index); // Happy path: Lease now @@ -117,17 +119,3 @@ private void UpdateLeasing(IList services) _leasing.AddRange(newLeases); } } - -public class LeaseEventArgs : EventArgs -{ - public LeaseEventArgs(Lease lease, Service service, int serviceIndex) - { - Lease = lease; - Service = service; - ServiceIndex = serviceIndex; - } - - public Lease Lease { get; } - public Service Service { get; } - public int ServiceIndex { get; } -} diff --git a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs index 057fa95e6..c45720e28 100644 --- a/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs +++ b/src/Ocelot/LoadBalancer/LoadBalancers/RoundRobinCreator.cs @@ -8,7 +8,11 @@ public class RoundRobinCreator : ILoadBalancerCreator { public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) { - var loadBalancer = new RoundRobin(serviceProvider.GetAsync, route.ServiceName); + var loadBalancer = new RoundRobin( + serviceProvider.GetAsync, + !string.IsNullOrEmpty(route.ServiceName) + ? route.ServiceName + : route.LoadBalancerKey); // if service discovery mode then use service name; otherwise use balancer key return new OkResponse(loadBalancer); } diff --git a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs index bc894fc55..fa455198b 100644 --- a/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs +++ b/src/Ocelot/LoadBalancer/Middleware/LoadBalancingMiddleware.cs @@ -34,7 +34,7 @@ public async Task Invoke(HttpContext httpContext) return; } - var hostAndPort = await loadBalancer.Data.Lease(httpContext); + var hostAndPort = await loadBalancer.Data.LeaseAsync(httpContext); if (hostAndPort.IsError) { Logger.LogDebug("there was an error leasing the loadbalancer, setting pipeline error"); diff --git a/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs b/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs index b417c4c7c..dc09c94b9 100644 --- a/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs +++ b/src/Ocelot/ServiceDiscovery/Providers/ConfigurationServiceProvider.cs @@ -1,19 +1,12 @@ using Ocelot.Values; -namespace Ocelot.ServiceDiscovery.Providers +namespace Ocelot.ServiceDiscovery.Providers; + +public class ConfigurationServiceProvider : IServiceDiscoveryProvider { - public class ConfigurationServiceProvider : IServiceDiscoveryProvider - { - private readonly List _services; + private readonly List _services; - public ConfigurationServiceProvider(List services) - { - _services = services; - } + public ConfigurationServiceProvider(List services) => _services = services; - public async Task> GetAsync() - { - return await Task.FromResult(_services); - } - } + public Task> GetAsync() => ValueTask.FromResult(_services).AsTask(); } diff --git a/test/Ocelot.AcceptanceTests/ConcurrentSteps.cs b/test/Ocelot.AcceptanceTests/ConcurrentSteps.cs new file mode 100644 index 000000000..6d323452f --- /dev/null +++ b/test/Ocelot.AcceptanceTests/ConcurrentSteps.cs @@ -0,0 +1,271 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +using Ocelot.AcceptanceTests.LoadBalancer; +using Ocelot.LoadBalancer; +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Text; + +namespace Ocelot.AcceptanceTests; + +public class ConcurrentSteps : Steps, IDisposable +{ + protected Task[] _tasks; + protected ServiceHandler[] _handlers; + protected ConcurrentDictionary _responses; + protected volatile int[] _counters; + + public ConcurrentSteps() + { + _tasks = Array.Empty(); + _handlers = Array.Empty(); + _responses = new(); + _counters = Array.Empty(); + } + + public override void Dispose() + { + foreach (var handler in _handlers) + { + handler?.Dispose(); + } + + foreach (var response in _responses.Values) + { + response?.Dispose(); + } + + foreach (var task in _tasks) + { + task?.Dispose(); + } + + base.Dispose(); + GC.SuppressFinalize(this); + } + + protected void GivenServiceInstanceIsRunning(string url, string response) + => GivenServiceInstanceIsRunning(url, response, HttpStatusCode.OK); + + protected void GivenServiceInstanceIsRunning(string url, string response, HttpStatusCode statusCode) + { + _handlers = new ServiceHandler[1]; // allocate single instance + _counters = new int[1]; // single counter + GivenServiceIsRunning(url, response, 0, statusCode); + _counters[0] = 0; + } + + protected void GivenThereIsAServiceRunningOn(string url, string basePath, string responseBody) + { + var handler = new ServiceHandler(); + _handlers = new ServiceHandler[] { handler }; + handler.GivenThereIsAServiceRunningOn(url, basePath, MapGet(basePath, responseBody)); + } + + protected void GivenMultipleServiceInstancesAreRunning(string[] urls, [CallerMemberName] string serviceName = null) + { + serviceName ??= new Uri(urls[0]).Host; + string[] responses = urls.Select(u => $"{serviceName}|url({u})").ToArray(); + GivenMultipleServiceInstancesAreRunning(urls, responses, HttpStatusCode.OK); + } + + protected void GivenMultipleServiceInstancesAreRunning(string[] urls, string[] responses) + => GivenMultipleServiceInstancesAreRunning(urls, responses, HttpStatusCode.OK); + + protected void GivenMultipleServiceInstancesAreRunning(string[] urls, string[] responses, HttpStatusCode statusCode) + { + Debug.Assert(urls.Length == responses.Length, "Length mismatch!"); + _handlers = new ServiceHandler[urls.Length]; // allocate multiple instances + _counters = new int[urls.Length]; // multiple counters + for (int i = 0; i < urls.Length; i++) + { + GivenServiceIsRunning(urls[i], responses[i], i, statusCode); + _counters[i] = 0; + } + } + + private void GivenServiceIsRunning(string url, string response) + => GivenServiceIsRunning(url, response, 0, HttpStatusCode.OK); + private void GivenServiceIsRunning(string url, string response, int index) + => GivenServiceIsRunning(url, response, index, HttpStatusCode.OK); + + private void GivenServiceIsRunning(string url, string response, int index, HttpStatusCode successCode) + { + response ??= successCode.ToString(); + _handlers[index] ??= new(); + var serviceHandler = _handlers[index]; + serviceHandler.GivenThereIsAServiceRunningOn(url, MapGet(index, response, successCode)); + } + + protected static RequestDelegate MapGet(string path, string responseBody) => MapGet(path, responseBody, HttpStatusCode.OK); + protected static RequestDelegate MapGet(string path, string responseBody, HttpStatusCode statusCode) => async context => + { + var downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) + ? context.Request.PathBase.Value + : context.Request.Path.Value; + bool isMatch = downstreamPath == path; + context.Response.StatusCode = (int)(isMatch ? statusCode : HttpStatusCode.NotFound); + await context.Response.WriteAsync(isMatch ? responseBody : "Not Found"); + }; + + public static class HeaderNames + { + public const string ServiceIndex = nameof(LeaseEventArgs.ServiceIndex); + public const string Host = nameof(Uri.Host); + public const string Port = nameof(Uri.Port); + public const string Counter = nameof(Counter); + } + + protected RequestDelegate MapGet(int index, string body) => MapGet(index, body, HttpStatusCode.OK); + protected RequestDelegate MapGet(int index, string body, HttpStatusCode successCode) => async context => + { + // Don't delay during the first service call + if (Volatile.Read(ref _counters[index]) > 0) + { + await Task.Delay(Random.Shared.Next(5, 15)); // emulate integration delay up to 15 milliseconds + } + + string responseBody; + var request = context.Request; + var response = context.Response; + try + { + int count = Interlocked.Increment(ref _counters[index]); + responseBody = string.Concat(count, ':', body); + + response.StatusCode = (int)successCode; + response.Headers.Append(HeaderNames.ServiceIndex, new StringValues(index.ToString())); + response.Headers.Append(HeaderNames.Host, new StringValues(request.Host.Host)); + response.Headers.Append(HeaderNames.Port, new StringValues(request.Host.Port.ToString())); + response.Headers.Append(HeaderNames.Counter, new StringValues(count.ToString())); + await response.WriteAsync(responseBody); + } + catch (Exception exception) + { + responseBody = string.Concat(1, ':', exception.StackTrace); + response.StatusCode = (int)HttpStatusCode.InternalServerError; + await response.WriteAsync(responseBody); + } + }; + + public Task[] WhenIGetUrlOnTheApiGatewayConcurrently(string url, int times) + => RunParallelRequests(times, (i) => url); + + public Task[] WhenIGetUrlOnTheApiGatewayConcurrently(int times, params string[] urls) + => RunParallelRequests(times, (i) => urls[i % urls.Length]); + + protected Task[] RunParallelRequests(int times, Func urlFunc) + { + _tasks = new Task[times]; + _responses = new(times, times); + for (var i = 0; i < times; i++) + { + var url = urlFunc(i); + _tasks[i] = GetParallelResponse(url, i); + _responses[i] = null; + } + + Task.WaitAll(_tasks); + return _tasks; + } + + private async Task GetParallelResponse(string url, int threadIndex) + { + var response = await _ocelotClient.GetAsync(url); + var content = await response.Content.ReadAsStringAsync(); + var counterString = content.Contains(':') + ? content.Split(':')[0] // let the first fragment is counter value + : "0"; + int count = int.Parse(counterString); + count.ShouldBeGreaterThan(0); + _responses[threadIndex] = response; + } + + public void ThenAllStatusCodesShouldBe(HttpStatusCode expected) + => _responses.ShouldAllBe(response => response.Value.StatusCode == expected); + public void ThenAllResponseBodiesShouldBe(string expectedBody) + => _responses.ShouldAllBe(response => response.Value.Content.ReadAsStringAsync().Result == expectedBody); + + protected string CalledTimesMessage() + => $"All values are [{string.Join(',', _counters)}]"; + + public void ThenAllServicesShouldHaveBeenCalledTimes(int expected) + => _counters.Sum().ShouldBe(expected, CalledTimesMessage()); + + public void ThenServiceShouldHaveBeenCalledTimes(int index, int expected) + => _counters[index].ShouldBe(expected, CalledTimesMessage()); + + public void ThenServicesShouldHaveBeenCalledTimes(params int[] expected) + { + for (int i = 0; i < expected.Length; i++) + { + _counters[i].ShouldBe(expected[i], CalledTimesMessage()); + } + } + + public static int Bottom(int totalRequests, int totalServices) + => totalRequests / totalServices; + public static int Top(int totalRequests, int totalServices) + { + int bottom = Bottom(totalRequests, totalServices); + return totalRequests - (bottom * totalServices) + bottom; + } + + public void ThenAllServicesCalledRealisticAmountOfTimes(int bottom, int top) + { + var customMessage = new StringBuilder() + .AppendLine($"{nameof(bottom)}: {bottom}") + .AppendLine($" {nameof(top)}: {top}") + .AppendLine($" All values are [{string.Join(',', _counters)}]") + .ToString(); + int sum = 0, totalSum = _counters.Sum(); + + // Last offline services cannot be called at all, thus don't assert zero counters + for (int i = 0; i < _counters.Length && sum < totalSum; i++) + { + int actual = _counters[i]; + actual.ShouldBeInRange(bottom, top, customMessage); + sum += actual; + } + } + + public void ThenAllServicesCalledOptimisticAmountOfTimes(ILoadBalancerAnalyzer analyzer) + { + if (analyzer == null) return; + int bottom = analyzer.BottomOfConnections(), + top = analyzer.TopOfConnections(); + ThenAllServicesCalledRealisticAmountOfTimes(bottom, top); // with unstable checkings + } + + public void ThenServiceCountersShouldMatchLeasingCounters(ILoadBalancerAnalyzer analyzer, int[] ports, int totalRequests) + { + if (analyzer == null || ports == null) + return; + + analyzer.ShouldNotBeNull().Analyze(); + analyzer.Events.Count.ShouldBe(totalRequests, $"{nameof(ILoadBalancerAnalyzer.ServiceName)}: {analyzer.ServiceName}"); + + var leasingCounters = analyzer?.GetHostCounters() ?? new(); + var sortedLeasingCountersByPort = ports.Select(port => leasingCounters.FirstOrDefault(kv => kv.Key.DownstreamPort == port).Value).ToArray(); + for (int i = 0; i < ports.Length; i++) + { + var host = leasingCounters.Keys.FirstOrDefault(k => k.DownstreamPort == ports[i]); + + // Leasing info/counters can be absent because of offline service instance with exact port in unstable scenario + if (host != null) + { + var customMessage = new StringBuilder() + .AppendLine($"{nameof(ILoadBalancerAnalyzer.ServiceName)}: {analyzer.ServiceName}") + .AppendLine($" Port: {ports[i]}") + .AppendLine($" Host: {host}") + .AppendLine($" Service counters: [{string.Join(',', _counters)}]") + .AppendLine($" Leasing counters: [{string.Join(',', sortedLeasingCountersByPort)}]") // should have order of _counters + .ToString(); + int counter1 = _counters[i]; + int counter2 = leasingCounters[host]; + counter1.ShouldBe(counter2, customMessage); + } + } + } +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/ILoadBalancerAnalyzer.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/ILoadBalancerAnalyzer.cs new file mode 100644 index 000000000..5b2acc6ca --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/ILoadBalancerAnalyzer.cs @@ -0,0 +1,18 @@ +using Ocelot.LoadBalancer; +using Ocelot.Values; +using System.Collections.Concurrent; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +public interface ILoadBalancerAnalyzer +{ + string ServiceName { get; } + string GenerationPrefix { get; } + ConcurrentBag Events { get; } + object Analyze(); + Dictionary GetHostCounters(); + Dictionary ToHostCountersDictionary(IEnumerable> grouping); + bool HasManyServiceGenerations(int maxGeneration); + int BottomOfConnections(); + int TopOfConnections(); +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzer.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzer.cs new file mode 100644 index 000000000..79ee9cf72 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzer.cs @@ -0,0 +1,28 @@ +using Microsoft.AspNetCore.Http; +using Ocelot.LoadBalancer; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.Values; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +internal sealed class LeastConnectionAnalyzer : LoadBalancerAnalyzer, ILoadBalancer +{ + private readonly LeastConnection loadBalancer; + + public LeastConnectionAnalyzer(Func>> services, string serviceName) + : base(serviceName) + { + loadBalancer = new(services, serviceName); + loadBalancer.Leased += Me_Leased; + } + + private void Me_Leased(object sender, LeaseEventArgs args) => Events.Add(args); + + public override string Type => nameof(LeastConnectionAnalyzer); + public override Task> LeaseAsync(HttpContext httpContext) => loadBalancer.LeaseAsync(httpContext); + public override void Release(ServiceHostAndPort hostAndPort) => loadBalancer.Release(hostAndPort); + + public override Dictionary ToHostCountersDictionary(IEnumerable> grouping) + => grouping.ToDictionary(g => g.Key, g => g.Count(e => e.Lease == g.Key)); +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzerCreator.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzerCreator.cs new file mode 100644 index 000000000..785189ce2 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/LeastConnectionAnalyzerCreator.cs @@ -0,0 +1,22 @@ +using Ocelot.Configuration; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.ServiceDiscovery.Providers; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +internal sealed class LeastConnectionAnalyzerCreator : ILoadBalancerCreator +{ + // We need to adhere to the same implementations of RoundRobinCreator, which results in a significant design overhead, (until redesigned) + public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) + { + var loadBalancer = new LeastConnectionAnalyzer( + serviceProvider.GetAsync, + !string.IsNullOrEmpty(route.ServiceName) // if service discovery mode then use service name; otherwise use balancer key + ? route.ServiceName + : route.LoadBalancerKey); + return new OkResponse(loadBalancer); + } + + public string Type => nameof(LeastConnectionAnalyzer); +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerAnalyzer.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerAnalyzer.cs new file mode 100644 index 000000000..dc19a51e4 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerAnalyzer.cs @@ -0,0 +1,118 @@ +using Microsoft.AspNetCore.Http; +using Ocelot.LoadBalancer; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.Values; +using System.Collections.Concurrent; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +internal class LoadBalancerAnalyzer : ILoadBalancerAnalyzer, ILoadBalancer +{ + protected readonly string _serviceName; + protected LoadBalancerAnalyzer(string serviceName) => _serviceName = serviceName; + + public string ServiceName => _serviceName; + public virtual string GenerationPrefix => "Gen:"; + public ConcurrentBag Events { get; } = new(); + + public virtual object Analyze() + { + var allGenerations = Events + .Select(e => e.Service.Tags.FirstOrDefault(t => t.StartsWith(GenerationPrefix))) + .Where(generation => !string.IsNullOrEmpty(generation)) + .Distinct().ToArray(); + var allIndices = Events.Select(e => e.ServiceIndex) + .Distinct().OrderBy(index => index).ToArray(); + + Dictionary> eventsPerGeneration = new(); + foreach (var generation in allGenerations) + { + var l = Events.Where(e => e.Service.Tags.Contains(generation)).ToList(); + eventsPerGeneration.Add(generation, l); + } + + Dictionary> generationIndices = new(); + foreach (var generation in allGenerations) + { + var l = eventsPerGeneration[generation].Select(e => e.ServiceIndex).Distinct().ToList(); + generationIndices.Add(generation, l); + } + + Dictionary> generationLeases = new(); + foreach (var generation in allGenerations) + { + var l = eventsPerGeneration[generation].Select(e => e.Lease).ToList(); + generationLeases.Add(generation, l); + } + + Dictionary> generationHosts = new(); + foreach (var generation in allGenerations) + { + var l = eventsPerGeneration[generation].Select(e => e.Lease.HostAndPort).Distinct().ToList(); + generationHosts.Add(generation, l); + } + + Dictionary> generationLeasesWithMaxConnections = new(); + foreach (var generation in allGenerations) + { + List leases = new(); + var uniqueHosts = generationHosts[generation]; + foreach (var host in uniqueHosts) + { + int max = generationLeases[generation].Where(l => l == host).Max(l => l.Connections); + Lease wanted = generationLeases[generation].Find(l => l == host && l.Connections == max); + leases.Add(wanted); + } + + leases = leases.OrderBy(l => l.HostAndPort.DownstreamPort).ToList(); + generationLeasesWithMaxConnections.Add(generation, leases); + } + + return generationLeasesWithMaxConnections; + } + + public virtual bool HasManyServiceGenerations(int maxGeneration) + { + int[] generations = new int[maxGeneration + 1]; + string[] tags = new string[maxGeneration + 1]; + for (int i = 0; i < generations.Length; i++) + { + generations[i] = i; + tags[i] = GenerationPrefix + i; + } + + var all = Events + .Select(e => e.Service.Tags.FirstOrDefault(t => t.StartsWith(GenerationPrefix))) + .Distinct().ToArray(); + return all.All(tags.Contains); + } + + public virtual Dictionary GetHostCounters() + { + var hosts = Events.Select(e => e.Lease.HostAndPort).Distinct().ToList(); + var grouping = Events + .GroupBy(e => e.Lease.HostAndPort) + .OrderBy(g => g.Key.DownstreamPort); + return ToHostCountersDictionary(grouping); + } + + public virtual Dictionary ToHostCountersDictionary(IEnumerable> grouping) + => grouping.ToDictionary(g => g.Key, g => g.Count(e => e.Lease == g.Key)); + + public virtual int BottomOfConnections() + { + var hostCounters = GetHostCounters(); + return hostCounters.Min(_ => _.Value); + } + + public virtual int TopOfConnections() + { + var hostCounters = GetHostCounters(); + return hostCounters.Max(_ => _.Value); + } + + public virtual string Type => nameof(LoadBalancerAnalyzer); + public virtual Task> LeaseAsync(HttpContext httpContext) => Task.FromResult>(new ErrorResponse(new UnableToFindLoadBalancerError(GetType().Name))); + public virtual void Release(ServiceHostAndPort hostAndPort) { } +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerTests.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerTests.cs new file mode 100644 index 000000000..b92a3d392 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/LoadBalancerTests.cs @@ -0,0 +1,129 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Ocelot.Configuration; +using Ocelot.Configuration.File; +using Ocelot.DependencyInjection; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.ServiceDiscovery.Providers; +using Ocelot.Values; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +public sealed class LoadBalancerTests : ConcurrentSteps, IDisposable +{ + [Theory] + [Trait("Feat", "211")] + [InlineData(false)] // original scenario, clean config + [InlineData(true)] // extended scenario using analyzer + public void ShouldLoadBalanceRequestWithLeastConnection(bool withAnalyzer) + { + var ports = PortFinder.GetPorts(2); + var route = GivenRoute(withAnalyzer ? nameof(LeastConnectionAnalyzer) : nameof(LeastConnection), ports); + var configuration = GivenConfiguration(route); + var downstreamServiceUrls = ports.Select(DownstreamUrl).ToArray(); + LeastConnectionAnalyzer lbAnalyzer = null; + LeastConnectionAnalyzer getAnalyzer(DownstreamRoute route, IServiceDiscoveryProvider provider) + { + //lock (LoadBalancerHouse.SyncRoot) // Note, synch locking is implemented in LoadBalancerHouse + return lbAnalyzer ??= new LeastConnectionAnalyzerCreator().Create(route, provider)?.Data as LeastConnectionAnalyzer; + } + Action withLeastConnectionAnalyzer = (s) + => s.AddOcelot().AddCustomLoadBalancer(getAnalyzer); + GivenMultipleServiceInstancesAreRunning(downstreamServiceUrls); + this.Given(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(withAnalyzer ? withLeastConnectionAnalyzer : WithAddOcelot)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 99)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(99)) + .And(x => ThenAllServicesCalledOptimisticAmountOfTimes(lbAnalyzer)) + .And(x => ThenServiceCountersShouldMatchLeasingCounters(lbAnalyzer, ports, 99)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(Bottom(99, ports.Length), Top(99, ports.Length))) + .And(x => ThenServicesShouldHaveBeenCalledTimes(50, 49)) // strict assertion + .BDDfy(); + } + + [Theory] + [Trait("Bug", "365")] + [InlineData(false)] // original scenario, clean config + [InlineData(true)] // extended scenario using analyzer + public void ShouldLoadBalanceRequestWithRoundRobin(bool withAnalyzer) + { + var ports = PortFinder.GetPorts(2); + var route = GivenRoute(withAnalyzer ? nameof(RoundRobinAnalyzer) : nameof(RoundRobin), ports); + var configuration = GivenConfiguration(route); + var downstreamServiceUrls = ports.Select(DownstreamUrl).ToArray(); + RoundRobinAnalyzer lbAnalyzer = null; + RoundRobinAnalyzer getAnalyzer(DownstreamRoute route, IServiceDiscoveryProvider provider) + { + //lock (LoadBalancerHouse.SyncRoot) // Note, synch locking is implemented in LoadBalancerHouse + return lbAnalyzer ??= new RoundRobinAnalyzerCreator().Create(route, provider)?.Data as RoundRobinAnalyzer; + } + Action withRoundRobinAnalyzer = (s) + => s.AddOcelot().AddCustomLoadBalancer(getAnalyzer); + GivenMultipleServiceInstancesAreRunning(downstreamServiceUrls); + this.Given(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(withAnalyzer ? withRoundRobinAnalyzer : WithAddOcelot)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 99)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(99)) + .And(x => ThenAllServicesCalledOptimisticAmountOfTimes(lbAnalyzer)) + .And(x => ThenServiceCountersShouldMatchLeasingCounters(lbAnalyzer, ports, 99)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(Bottom(99, ports.Length), Top(99, ports.Length))) + .And(x => ThenServicesShouldHaveBeenCalledTimes(50, 49)) // strict assertion + .BDDfy(); + } + + [Fact] + [Trait("Feat", "961")] + public void ShouldLoadBalanceRequestWithCustomLoadBalancer() + { + Func loadBalancerFactoryFunc = + (serviceProvider, route, discoveryProvider) => new CustomLoadBalancer(discoveryProvider.GetAsync); + var ports = PortFinder.GetPorts(2); + var route = GivenRoute(nameof(CustomLoadBalancer), ports); + var configuration = GivenConfiguration(route); + var downstreamServiceUrls = ports.Select(DownstreamUrl).ToArray(); + Action withCustomLoadBalancer = (s) + => s.AddOcelot().AddCustomLoadBalancer(loadBalancerFactoryFunc); + GivenMultipleServiceInstancesAreRunning(downstreamServiceUrls); + this.Given(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(withCustomLoadBalancer)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 50)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(50)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(Bottom(50, ports.Length), Top(50, ports.Length))) + .And(x => ThenServicesShouldHaveBeenCalledTimes(25, 25)) // strict assertion + .BDDfy(); + } + + private sealed class CustomLoadBalancer : ILoadBalancer + { + private readonly Func>> _services; + private static readonly object _lock = new(); + private int _last; + + public string Type => nameof(CustomLoadBalancer); + public CustomLoadBalancer(Func>> services) => _services = services; + + public async Task> LeaseAsync(HttpContext httpContext) + { + var services = await _services(); + lock (_lock) + { + if (_last >= services.Count) _last = 0; + var next = services[_last++]; + return new OkResponse(next.HostAndPort); + } + } + + public void Release(ServiceHostAndPort hostAndPort) { } + } + + private FileRoute GivenRoute(string loadBalancer, params int[] ports) => new() + { + DownstreamPathTemplate = "/", + DownstreamScheme = Uri.UriSchemeHttp, + UpstreamPathTemplate = "/", + UpstreamHttpMethod = new() { HttpMethods.Get }, + LoadBalancerOptions = new() { Type = loadBalancer ?? nameof(LeastConnection) }, + DownstreamHostAndPorts = ports.Select(Localhost).ToList(), + }; +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzer.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzer.cs new file mode 100644 index 000000000..8f5f479ee --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzer.cs @@ -0,0 +1,31 @@ +using KubeClient.Models; +using Microsoft.AspNetCore.Http; +using Ocelot.LoadBalancer; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.Values; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +internal sealed class RoundRobinAnalyzer : LoadBalancerAnalyzer, ILoadBalancer +{ + private readonly RoundRobin loadBalancer; + + public RoundRobinAnalyzer(Func>> services, string serviceName) + : base(serviceName) + { + loadBalancer = new(services, serviceName); + loadBalancer.Leased += Me_Leased; + } + + private void Me_Leased(object sender, LeaseEventArgs args) => Events.Add(args); + + public override string Type => nameof(RoundRobinAnalyzer); + public override Task> LeaseAsync(HttpContext httpContext) => loadBalancer.LeaseAsync(httpContext); + public override void Release(ServiceHostAndPort hostAndPort) => loadBalancer.Release(hostAndPort); + + public override string GenerationPrefix => nameof(EndpointsV1.Metadata.Generation) + ":"; + + public override Dictionary ToHostCountersDictionary(IEnumerable> grouping) + => grouping.ToDictionary(g => g.Key, g => g.Max(e => e.Lease.Connections)); +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzerCreator.cs b/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzerCreator.cs new file mode 100644 index 000000000..a8f7a2c44 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/LoadBalancer/RoundRobinAnalyzerCreator.cs @@ -0,0 +1,22 @@ +using Ocelot.Configuration; +using Ocelot.LoadBalancer.LoadBalancers; +using Ocelot.Responses; +using Ocelot.ServiceDiscovery.Providers; + +namespace Ocelot.AcceptanceTests.LoadBalancer; + +internal sealed class RoundRobinAnalyzerCreator : ILoadBalancerCreator +{ + // We need to adhere to the same implementations of RoundRobinCreator, which results in a significant design overhead, (until redesigned) + public Response Create(DownstreamRoute route, IServiceDiscoveryProvider serviceProvider) + { + var loadBalancer = new RoundRobinAnalyzer( + serviceProvider.GetAsync, + !string.IsNullOrEmpty(route.ServiceName) // if service discovery mode then use service name; otherwise use balancer key + ? route.ServiceName + : route.LoadBalancerKey); + return new OkResponse(loadBalancer); + } + + public string Type => nameof(RoundRobinAnalyzer); +} diff --git a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs b/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs deleted file mode 100644 index 104cdaaea..000000000 --- a/test/Ocelot.AcceptanceTests/LoadBalancerTests.cs +++ /dev/null @@ -1,274 +0,0 @@ -using Microsoft.AspNetCore.Http; -using Ocelot.Configuration; -using Ocelot.Configuration.File; -using Ocelot.LoadBalancer.LoadBalancers; -using Ocelot.Responses; -using Ocelot.ServiceDiscovery.Providers; -using Ocelot.Values; - -namespace Ocelot.AcceptanceTests; - -public sealed class LoadBalancerTests : IDisposable -{ - private readonly Steps _steps; - private int _counterOne; - private int _counterTwo; - private static readonly object SyncLock = new(); - private readonly ServiceHandler _serviceHandler; - - public LoadBalancerTests() - { - _serviceHandler = new ServiceHandler(); - _steps = new Steps(); - } - - [Fact] - public void Should_load_balance_request_with_least_connection() - { - var portOne = PortFinder.GetRandomPort(); - var portTwo = PortFinder.GetRandomPort(); - - var downstreamServiceOneUrl = $"http://localhost:{portOne}"; - var downstreamServiceTwoUrl = $"http://localhost:{portTwo}"; - - var configuration = new FileConfiguration - { - Routes = new List - { - new() - { - DownstreamPathTemplate = "/", - DownstreamScheme = "http", - UpstreamPathTemplate = "/", - UpstreamHttpMethod = new List { "Get" }, - LoadBalancerOptions = new FileLoadBalancerOptions { Type = nameof(LeastConnection) }, - DownstreamHostAndPorts = new List - { - new() - { - Host = "localhost", - Port = portOne, - }, - new() - { - Host = "localhost", - Port = portTwo, - }, - }, - }, - }, - GlobalConfiguration = new FileGlobalConfiguration(), - }; - - this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) - .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) - .And(x => _steps.GivenThereIsAConfiguration(configuration)) - .And(x => _steps.GivenOcelotIsRunning()) - .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - - // Quite risky assertion because the actual values based on health checks and threading - //.And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(24, 26)) - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 49)) - .BDDfy(); - } - - [Fact] - public void Should_load_balance_request_with_round_robin() - { - var downstreamPortOne = PortFinder.GetRandomPort(); - var downstreamPortTwo = PortFinder.GetRandomPort(); - var downstreamServiceOneUrl = $"http://localhost:{downstreamPortOne}"; - var downstreamServiceTwoUrl = $"http://localhost:{downstreamPortTwo}"; - - var configuration = new FileConfiguration - { - Routes = new List - { - new() - { - DownstreamPathTemplate = "/", - DownstreamScheme = "http", - UpstreamPathTemplate = "/", - UpstreamHttpMethod = new List { "Get" }, - LoadBalancerOptions = new FileLoadBalancerOptions { Type = nameof(RoundRobin) }, - DownstreamHostAndPorts = new List - { - new() - { - Host = "localhost", - Port = downstreamPortOne, - }, - new() - { - Host = "localhost", - Port = downstreamPortTwo, - }, - }, - }, - }, - GlobalConfiguration = new FileGlobalConfiguration(), - }; - - this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) - .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) - .And(x => _steps.GivenThereIsAConfiguration(configuration)) - .And(x => _steps.GivenOcelotIsRunning()) - .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - - // Quite risky assertion because the actual values based on health checks and threading - //.And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(24, 26)) - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 49)) - .BDDfy(); - } - - [Fact] - public void Should_load_balance_request_with_custom_load_balancer() - { - var downstreamPortOne = PortFinder.GetRandomPort(); - var downstreamPortTwo = PortFinder.GetRandomPort(); - var downstreamServiceOneUrl = $"http://localhost:{downstreamPortOne}"; - var downstreamServiceTwoUrl = $"http://localhost:{downstreamPortTwo}"; - - var configuration = new FileConfiguration - { - Routes = new List - { - new() - { - DownstreamPathTemplate = "/", - DownstreamScheme = "http", - UpstreamPathTemplate = "/", - UpstreamHttpMethod = new List { "Get" }, - LoadBalancerOptions = new FileLoadBalancerOptions { Type = nameof(CustomLoadBalancer) }, - DownstreamHostAndPorts = new List - { - new() - { - Host = "localhost", - Port = downstreamPortOne, - }, - new() - { - Host = "localhost", - Port = downstreamPortTwo, - }, - }, - }, - }, - GlobalConfiguration = new FileGlobalConfiguration(), - }; - - Func loadBalancerFactoryFunc = (serviceProvider, route, serviceDiscoveryProvider) => new CustomLoadBalancer(serviceDiscoveryProvider.GetAsync); - - this.Given(x => x.GivenProductServiceOneIsRunning(downstreamServiceOneUrl, 200)) - .And(x => x.GivenProductServiceTwoIsRunning(downstreamServiceTwoUrl, 200)) - .And(x => _steps.GivenThereIsAConfiguration(configuration)) - .And(x => _steps.GivenOcelotIsRunningWithCustomLoadBalancer(loadBalancerFactoryFunc)) - .When(x => _steps.WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - - // Quite risky assertion because the actual values based on health checks and threading - //.And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(24, 26)) - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 49)) - .BDDfy(); - } - - private class CustomLoadBalancer : ILoadBalancer - { - private readonly Func>> _services; - private readonly object _lock = new(); - - private int _last; - - public CustomLoadBalancer(Func>> services) - { - _services = services; - } - - public async Task> Lease(HttpContext httpContext) - { - var services = await _services(); - lock (_lock) - { - if (_last >= services.Count) - { - _last = 0; - } - - var next = services[_last]; - _last++; - return new OkResponse(next.HostAndPort); - } - } - - public void Release(ServiceHostAndPort hostAndPort) - { - } - } - - private void ThenBothServicesCalledRealisticAmountOfTimes(int bottom, int top) - { - _counterOne.ShouldBeInRange(bottom, top); - _counterTwo.ShouldBeInRange(bottom, top); - } - - private void ThenTheTwoServicesShouldHaveBeenCalledTimes(int expected) - { - var total = _counterOne + _counterTwo; - total.ShouldBe(expected); - } - - private void GivenProductServiceOneIsRunning(string url, int statusCode) - { - _serviceHandler.GivenThereIsAServiceRunningOn(url, async context => - { - try - { - string response; - lock (SyncLock) - { - _counterOne++; - response = _counterOne.ToString(); - } - - context.Response.StatusCode = statusCode; - await context.Response.WriteAsync(response); - } - catch (Exception exception) - { - await context.Response.WriteAsync(exception.StackTrace); - } - }); - } - - private void GivenProductServiceTwoIsRunning(string url, int statusCode) - { - _serviceHandler.GivenThereIsAServiceRunningOn(url, async context => - { - try - { - string response; - lock (SyncLock) - { - _counterTwo++; - response = _counterTwo.ToString(); - } - - context.Response.StatusCode = statusCode; - await context.Response.WriteAsync(response); - } - catch (Exception exception) - { - await context.Response.WriteAsync(exception.StackTrace); - } - }); - } - - public void Dispose() - { - _serviceHandler?.Dispose(); - _steps.Dispose(); - } -} diff --git a/test/Ocelot.AcceptanceTests/Properties/GlobalSuppressions.cs b/test/Ocelot.AcceptanceTests/Properties/GlobalSuppressions.cs index 903242484..c7da8e7ec 100644 --- a/test/Ocelot.AcceptanceTests/Properties/GlobalSuppressions.cs +++ b/test/Ocelot.AcceptanceTests/Properties/GlobalSuppressions.cs @@ -7,3 +7,4 @@ [assembly: SuppressMessage("StyleCop.CSharp.ReadabilityRules", "SA1132:Do not combine fields", Justification = "Has no much sense in test projects", Scope = "namespaceanddescendants", Target = "~N:Ocelot.AcceptanceTests")] [assembly: SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1513:Closing brace should be followed by blank line", Justification = "Has no much sense in test projects", Scope = "namespaceanddescendants", Target = "~N:Ocelot.AcceptanceTests")] +[assembly: SuppressMessage("StyleCop.CSharp.LayoutRules", "SA1503:Braces should not be omitted", Justification = "For if-shortcuts")] diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulServiceDiscoveryTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulServiceDiscoveryTests.cs index dce57ba0f..5e9fae3c2 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulServiceDiscoveryTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/ConsulServiceDiscoveryTests.cs @@ -3,12 +3,15 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Net.Http.Headers; using Newtonsoft.Json; +using Ocelot.AcceptanceTests.LoadBalancer; +using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.DependencyInjection; using Ocelot.LoadBalancer.LoadBalancers; using Ocelot.Logging; using Ocelot.Provider.Consul; using Ocelot.Provider.Consul.Interfaces; +using Ocelot.ServiceDiscovery.Providers; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; @@ -17,59 +20,49 @@ namespace Ocelot.AcceptanceTests.ServiceDiscovery; /// /// Tests for the provider. /// -public sealed partial class ConsulServiceDiscoveryTests : Steps, IDisposable +public sealed partial class ConsulServiceDiscoveryTests : ConcurrentSteps, IDisposable { + private readonly ServiceHandler _consulHandler; private readonly List _consulServices; private readonly List _consulNodes; - private int _counterOne; - private int _counterTwo; - private int _counterConsul; - private int _counterNodes; - private static readonly object SyncLock = new(); - private string _downstreamPath; + private string _receivedToken; - private readonly ServiceHandler _serviceHandler; - private readonly ServiceHandler _serviceHandler2; - private readonly ServiceHandler _consulHandler; + + private volatile int _counterConsul; + private volatile int _counterNodes; public ConsulServiceDiscoveryTests() { - _serviceHandler = new ServiceHandler(); - _serviceHandler2 = new ServiceHandler(); _consulHandler = new ServiceHandler(); - _consulServices = new(); - _consulNodes = new(); + _consulServices = new List(); + _consulNodes = new List(); } public override void Dispose() { - _serviceHandler?.Dispose(); - _serviceHandler2?.Dispose(); _consulHandler?.Dispose(); + base.Dispose(); } [Fact] - public void Should_use_consul_service_discovery_and_load_balance_request() + [Trait("Feat", "28")] + public void ShouldDiscoverServicesInConsulAndLoadBalanceByLeastConnectionWhenConfigInRoute() { const string serviceName = "product"; var consulPort = PortFinder.GetRandomPort(); - var port1 = PortFinder.GetRandomPort(); - var port2 = PortFinder.GetRandomPort(); - var serviceEntryOne = GivenServiceEntry(port1, serviceName: serviceName); - var serviceEntryTwo = GivenServiceEntry(port2, serviceName: serviceName); - var route = GivenRoute(serviceName: serviceName); + var ports = PortFinder.GetPorts(2); + var serviceEntries = ports.Select(port => GivenServiceEntry(port, serviceName: serviceName)).ToArray(); + var route = GivenRoute(serviceName: serviceName, loadBalancerType: nameof(LeastConnection)); var configuration = GivenServiceDiscovery(consulPort, route); - this.Given(x => x.GivenProductServiceOneIsRunning(DownstreamUrl(port1), 200)) - .And(x => x.GivenProductServiceTwoIsRunning(DownstreamUrl(port2), 200)) + var urls = ports.Select(DownstreamUrl).ToArray(); + this.Given(x => GivenMultipleServiceInstancesAreRunning(urls, serviceName)) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) - .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntryOne, serviceEntryTwo)) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntries)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) - .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - - // Quite risky assertion because the actual values based on health checks and threading - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 49)) //(24, 26)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 50)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(50)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(/*25*/24, /*25*/26)) // TODO Check strict assertion .BDDfy(); } @@ -77,7 +70,9 @@ public void Should_use_consul_service_discovery_and_load_balance_request() private static readonly string[] GetVsOptionsMethods = new[] { "Get", "Options" }; [Fact] - public void Should_handle_request_to_consul_for_downstream_service_and_make_request() + [Trait("Feat", "201")] + [Trait("Bug", "213")] + public void ShouldHandleRequestToConsulForDownstreamServiceAndMakeRequest() { const string serviceName = "web"; var consulPort = PortFinder.GetRandomPort(); @@ -85,11 +80,11 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ var serviceEntryOne = GivenServiceEntry(servicePort, "localhost", "web_90_0_2_224_8080", VersionV1Tags, serviceName); var route = GivenRoute("/api/home", "/home", serviceName, httpMethods: GetVsOptionsMethods); var configuration = GivenServiceDiscovery(consulPort, route); - this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", HttpStatusCode.OK, "Hello from Laura")) + this.Given(x => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", "Hello from Laura")) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntryOne)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) .When(x => WhenIGetUrlOnTheApiGateway("/home")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) @@ -97,7 +92,9 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ } [Fact] - public void Should_handle_request_to_consul_for_downstream_service_and_make_request_no_re_routes() + [Trait("Bug", "213")] + [Trait("Feat", "201 340")] + public void ShouldHandleRequestToConsulForDownstreamServiceAndMakeRequestWhenDynamicRoutingWithNoRoutes() { const string serviceName = "web"; var consulPort = PortFinder.GetRandomPort(); @@ -113,11 +110,11 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ UseTracing = false, }; - this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/something", HttpStatusCode.OK, "Hello from Laura")) + this.Given(x => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/something", "Hello from Laura")) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) .When(x => WhenIGetUrlOnTheApiGateway("/web/something")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) @@ -125,35 +122,33 @@ public void Should_handle_request_to_consul_for_downstream_service_and_make_requ } [Fact] - public void Should_use_consul_service_discovery_and_load_balance_request_no_re_routes() + [Trait("Feat", "340")] + public void ShouldUseConsulServiceDiscoveryAndLoadBalanceRequestWhenDynamicRoutingWithNoRoutes() { const string serviceName = "product"; var consulPort = PortFinder.GetRandomPort(); - var port1 = PortFinder.GetRandomPort(); - var port2 = PortFinder.GetRandomPort(); - var serviceEntry1 = GivenServiceEntry(port1, serviceName: serviceName); - var serviceEntry2 = GivenServiceEntry(port2, serviceName: serviceName); + var ports = PortFinder.GetPorts(2); + var serviceEntries = ports.Select(port => GivenServiceEntry(port, serviceName: serviceName)).ToArray(); var configuration = GivenServiceDiscovery(consulPort); configuration.GlobalConfiguration.LoadBalancerOptions = new() { Type = nameof(LeastConnection) }; configuration.GlobalConfiguration.DownstreamScheme = "http"; - this.Given(x => x.GivenProductServiceOneIsRunning(DownstreamUrl(port1), 200)) - .And(x => x.GivenProductServiceTwoIsRunning(DownstreamUrl(port2), 200)) + var urls = ports.Select(DownstreamUrl).ToArray(); + this.Given(x => GivenMultipleServiceInstancesAreRunning(urls, serviceName)) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) - .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry1, serviceEntry2)) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntries)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) - .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimes($"/{serviceName}/", 50)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(50)) - - // Quite risky assertion because the actual values based on health checks and threading - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 49)) //(24, 26)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently($"/{serviceName}/", 50)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(50)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(/*25*/24, /*25*/26)) // TODO Check strict assertion .BDDfy(); } [Fact] - public void Should_use_token_to_make_request_to_consul() + [Trait("Feat", "295")] + public void ShouldUseAclTokenToMakeRequestToConsul() { const string serviceName = "web"; const string token = "abctoken"; @@ -165,54 +160,52 @@ public void Should_use_token_to_make_request_to_consul() var configuration = GivenServiceDiscovery(consulPort, route); configuration.GlobalConfiguration.ServiceDiscoveryProvider.Token = token; - this.Given(_ => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", HttpStatusCode.OK, "Hello from Laura")) - .And(_ => GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) - .And(_ => GivenTheServicesAreRegisteredWithConsul(serviceEntry)) - .And(_ => GivenThereIsAConfiguration(configuration)) - .And(_ => GivenOcelotIsRunningWithConsul()) - .When(_ => WhenIGetUrlOnTheApiGateway("/home")) - .Then(_ => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) - .And(_ => ThenTheResponseBodyShouldBe("Hello from Laura")) - .And(_ => ThenTheTokenIs(token)) + this.Given(x => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", "Hello from Laura")) + .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry)) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) + .When(x => WhenIGetUrlOnTheApiGateway("/home")) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) + .And(x => x.ThenTheTokenIs(token)) .BDDfy(); } [Fact] - public void Should_send_request_to_service_after_it_becomes_available_in_consul() + [Trait("Bug", "181")] + public void ShouldSendRequestToServiceAfterItBecomesAvailableInConsul() { const string serviceName = "product"; var consulPort = PortFinder.GetRandomPort(); - var port1 = PortFinder.GetRandomPort(); - var port2 = PortFinder.GetRandomPort(); - var serviceEntry1 = GivenServiceEntry(port1, serviceName: serviceName); - var serviceEntry2 = GivenServiceEntry(port2, serviceName: serviceName); + var ports = PortFinder.GetPorts(2); + var serviceEntries = ports.Select(port => GivenServiceEntry(port, serviceName: serviceName)).ToArray(); var route = GivenRoute(serviceName: serviceName); var configuration = GivenServiceDiscovery(consulPort, route); - this.Given(x => x.GivenProductServiceOneIsRunning(DownstreamUrl(port1), 200)) - .And(x => x.GivenProductServiceTwoIsRunning(DownstreamUrl(port2), 200)) + var urls = ports.Select(DownstreamUrl).ToArray(); + this.Given(_ => GivenMultipleServiceInstancesAreRunning(urls, serviceName)) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) - .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry1, serviceEntry2)) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntries)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) - .And(x => WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) - .And(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(10)) - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 9)) //(4, 6)) - .And(x => WhenIRemoveAService(serviceEntry2)) - .And(x => GivenIResetCounters()) - .And(x => WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) - .And(x => ThenOnlyOneServiceHasBeenCalled()) - .And(x => WhenIAddAServiceBackIn(serviceEntry2)) - .And(x => GivenIResetCounters()) - .When(x => WhenIGetUrlOnTheApiGatewayMultipleTimes("/", 10)) - .Then(x => x.ThenTheTwoServicesShouldHaveBeenCalledTimes(10)) - - // Quite risky assertion because the actual values based on health checks and threading - .And(x => x.ThenBothServicesCalledRealisticAmountOfTimes(1, 9)) //(4, 6)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) + .And(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 10)) + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(10)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(/*5*/4, /*5*/6)) // TODO Check strict assertion + .And(x => x.WhenIRemoveAService(serviceEntries[1])) // 2nd entry + .And(x => x.GivenIResetCounters()) + .And(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 10)) + .And(x => ThenServicesShouldHaveBeenCalledTimes(10, 0)) // 2nd is offline + .And(x => x.WhenIAddAServiceBackIn(serviceEntries[1])) // 2nd entry + .And(x => x.GivenIResetCounters()) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently("/", 10)) + .Then(x => ThenAllServicesShouldHaveBeenCalledTimes(10)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(/*5*/4, /*5*/6)) // TODO Check strict assertion .BDDfy(); } [Fact] - public void Should_handle_request_to_poll_consul_for_downstream_service_and_make_request() + [Trait("Feat", "374")] + public void ShouldPollConsulForDownstreamServiceAndMakeRequest() { const string serviceName = "web"; var consulPort = PortFinder.GetRandomPort(); @@ -222,15 +215,15 @@ public void Should_handle_request_to_poll_consul_for_downstream_service_and_make var configuration = GivenServiceDiscovery(consulPort, route); var sd = configuration.GlobalConfiguration.ServiceDiscoveryProvider; - sd.Type = nameof(PollConsul); + sd.Type = nameof(PollConsul); // !!! sd.PollingInterval = 0; sd.Namespace = string.Empty; - this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", HttpStatusCode.OK, "Hello from Laura")) + this.Given(x => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", "Hello from Laura")) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) .When(x => WhenIGetUrlOnTheApiGatewayWaitingForTheResponseToBeOk("/home")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .And(x => ThenTheResponseBodyShouldBe("Hello from Laura")) @@ -240,11 +233,11 @@ public void Should_handle_request_to_poll_consul_for_downstream_service_and_make [Theory] [Trait("PR", "1944")] [Trait("Bugs", "849 1496")] - [InlineData(nameof(LeastConnection))] - [InlineData(nameof(RoundRobin))] [InlineData(nameof(NoLoadBalancer))] + [InlineData(nameof(RoundRobin))] + [InlineData(nameof(LeastConnection))] [InlineData(nameof(CookieStickySessions))] - public void Should_use_consul_service_discovery_based_on_upstream_host(string loadBalancerType) + public void ShouldUseConsulServiceDiscoveryWhenThereAreTwoUpstreamHosts(string loadBalancerType) { // Simulate two DIFFERENT downstream services (e.g. product services for US and EU markets) // with different ServiceNames (e.g. product-us and product-eu), @@ -272,12 +265,13 @@ public void Should_use_consul_service_discovery_based_on_upstream_host(string lo // Ocelot request for http://us-shop/ should find 'product-us' in Consul, call /products and return "Phone chargers with US plug" // Ocelot request for http://eu-shop/ should find 'product-eu' in Consul, call /products and return "Phone chargers with EU plug" - this.Given(x => x._serviceHandler.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePortUS), "/products", MapGet("/products", responseBodyUS))) - .Given(x => x._serviceHandler2.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePortEU), "/products", MapGet("/products", responseBodyEU))) + _handlers = new ServiceHandler[2] { new(), new() }; + this.Given(x => _handlers[0].GivenThereIsAServiceRunningOn(DownstreamUrl(servicePortUS), "/products", MapGet("/products", responseBodyUS))) + .Given(x => _handlers[1].GivenThereIsAServiceRunningOn(DownstreamUrl(servicePortEU), "/products", MapGet("/products", responseBodyEU))) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntryUS, serviceEntryEU)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul(publicUrlUS, publicUrlEU)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) .When(x => x.WhenIGetUrl(publicUrlUS, sessionCookieUS), "When I get US shop for the first time") .Then(x => x.ThenConsulShouldHaveBeenCalledTimes(1)) .And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) @@ -299,7 +293,7 @@ public void Should_use_consul_service_discovery_based_on_upstream_host(string lo [Fact] [Trait("Bug", "954")] - public void Should_return_service_address_by_overridden_service_builder_when_there_is_a_node() + public void ShouldReturnServiceAddressByOverriddenServiceBuilderWhenThereIsANode() { const string serviceName = "OpenTestService"; string[] methods = new[] { HttpMethods.Post, HttpMethods.Get }; @@ -314,12 +308,12 @@ public void Should_return_service_address_by_overridden_service_builder_when_the var route = GivenRoute("/api/{url}", "/open/{url}", serviceName, httpMethods: methods); var configuration = GivenServiceDiscovery(consulPort, route); - this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", HttpStatusCode.OK, "Hello from Raman")) + this.Given(x => GivenThereIsAServiceRunningOn(DownstreamUrl(servicePort), "/api/home", "Hello from Raman")) .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) .And(x => x.GivenTheServicesAreRegisteredWithConsul(serviceEntry)) .And(x => x.GivenTheServiceNodesAreRegisteredWithConsul(serviceNode)) .And(x => GivenThereIsAConfiguration(configuration)) - .And(x => GivenOcelotIsRunningWithConsul()) // default services registration results with the bug: "n1" host issue + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) // default services registration results with the bug: "n1" host issue .When(x => WhenIGetUrlOnTheApiGateway("/open/home")) .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.BadGateway)) .And(x => ThenTheResponseBodyShouldBe("")) @@ -336,13 +330,192 @@ public void Should_return_service_address_by_overridden_service_builder_when_the .BDDfy(); } - private static void WithOverriddenConsulServiceBuilder(IServiceCollection services) - => services.AddOcelot().AddConsul(); + private static readonly string[] Bug2119ServiceNames = new string[] { "ProjectsService", "CustomersService" }; + private readonly ILoadBalancer[] _lbAnalyzers = new ILoadBalancer[Bug2119ServiceNames.Length]; // emulate LoadBalancerHouse's collection + + private TLoadBalancer GetAnalyzer(DownstreamRoute route, IServiceDiscoveryProvider provider) + where TLoadBalancer : class, ILoadBalancer + where TLoadBalancerCreator : class, ILoadBalancerCreator, new() + { + //lock (LoadBalancerHouse.SyncRoot) // Note, synch locking is implemented in LoadBalancerHouse + int index = Array.IndexOf(Bug2119ServiceNames, route.ServiceName); // LoadBalancerHouse should return different balancers for different service names + _lbAnalyzers[index] ??= new TLoadBalancerCreator().Create(route, provider)?.Data; + return (TLoadBalancer)_lbAnalyzers[index]; + } + + private void WithLbAnalyzer(IServiceCollection services) + where TLoadBalancer : class, ILoadBalancer + where TLoadBalancerCreator : class, ILoadBalancerCreator, new() + => services.AddOcelot().AddConsul().AddCustomLoadBalancer(GetAnalyzer); + + [Theory] + [Trait("Bug", "2119")] + [InlineData(nameof(NoLoadBalancer))] + [InlineData(nameof(RoundRobin))] + [InlineData(nameof(LeastConnection))] // original scenario + public void ShouldReturnDifferentServicesWhenThereAre2SequentialRequestsToDifferentServices(string loadBalancer) + { + var consulPort = PortFinder.GetRandomPort(); + var ports = PortFinder.GetPorts(Bug2119ServiceNames.Length); + var service1 = GivenServiceEntry(ports[0], serviceName: Bug2119ServiceNames[0]); + var service2 = GivenServiceEntry(ports[1], serviceName: Bug2119ServiceNames[1]); + var route1 = GivenRoute("/{all}", "/projects/{all}", serviceName: Bug2119ServiceNames[0], loadBalancerType: loadBalancer); + var route2 = GivenRoute("/{all}", "/customers/{all}", serviceName: Bug2119ServiceNames[1], loadBalancerType: loadBalancer); + route1.UpstreamHttpMethod = route2.UpstreamHttpMethod = new() { HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Delete }; + var configuration = GivenServiceDiscovery(consulPort, route1, route2); + var urls = ports.Select(DownstreamUrl).ToArray(); + this.Given(x => GivenMultipleServiceInstancesAreRunning(urls, Bug2119ServiceNames)) + .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(service1, service2)) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(WithConsul)) + + // Step 1 + .When(x => WhenIGetUrlOnTheApiGateway("/projects/api/projects")) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => ThenServiceShouldHaveBeenCalledTimes(0, 1)) + .And(x => x.ThenTheResponseBodyShouldBe($"1:{Bug2119ServiceNames[0]}")) // ! + + // Step 2 + .When(x => WhenIGetUrlOnTheApiGateway("/customers/api/customers")) + .Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => ThenServiceShouldHaveBeenCalledTimes(1, 1)) + .And(x => x.ThenTheResponseBodyShouldBe($"1:{Bug2119ServiceNames[1]}")) // !! + + // Finally + .Then(x => ThenAllStatusCodesShouldBe(HttpStatusCode.OK)) + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(2)) + .And(x => ThenServicesShouldHaveBeenCalledTimes(1, 1)) + .BDDfy(); + } + + [Theory] + [Trait("Bug", "2119")] + [InlineData(false, nameof(NoLoadBalancer))] + [InlineData(false, nameof(LeastConnection))] // original scenario, clean config + [InlineData(true, nameof(LeastConnectionAnalyzer))] // extended scenario using analyzer + [InlineData(false, nameof(RoundRobin))] + [InlineData(true, nameof(RoundRobinAnalyzer))] + public void ShouldReturnDifferentServicesWhenSequentiallylyRequestingToDifferentServices(bool withAnalyzer, string loadBalancer) + { + var consulPort = PortFinder.GetRandomPort(); + var ports = PortFinder.GetPorts(Bug2119ServiceNames.Length); + var service1 = GivenServiceEntry(ports[0], serviceName: Bug2119ServiceNames[0]); + var service2 = GivenServiceEntry(ports[1], serviceName: Bug2119ServiceNames[1]); + var route1 = GivenRoute("/{all}", "/projects/{all}", serviceName: Bug2119ServiceNames[0], loadBalancerType: loadBalancer); + var route2 = GivenRoute("/{all}", "/customers/{all}", serviceName: Bug2119ServiceNames[1], loadBalancerType: loadBalancer); + route1.UpstreamHttpMethod = route2.UpstreamHttpMethod = new() { HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Delete }; + var configuration = GivenServiceDiscovery(consulPort, route1, route2); + var urls = ports.Select(DownstreamUrl).ToArray(); + Action requestToProjectsAndThenRequestToCustomersAndAssert = (i) => + { + // Step 1 + int count = i + 1; + WhenIGetUrlOnTheApiGateway("/projects/api/projects"); + ThenTheStatusCodeShouldBe(HttpStatusCode.OK); + ThenServiceShouldHaveBeenCalledTimes(0, count); + ThenTheResponseBodyShouldBe($"{count}:{Bug2119ServiceNames[0]}", $"i is {i}"); + _responses[2 * i] = _response; + + // Step 2 + WhenIGetUrlOnTheApiGateway("/customers/api/customers"); + ThenTheStatusCodeShouldBe(HttpStatusCode.OK); + ThenServiceShouldHaveBeenCalledTimes(1, count); + ThenTheResponseBodyShouldBe($"{count}:{Bug2119ServiceNames[1]}", $"i is {i}"); + _responses[(2 * i) + 1] = _response; + }; + this.Given(x => GivenMultipleServiceInstancesAreRunning(urls, Bug2119ServiceNames)) // service names as responses + .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(service1, service2)) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(withAnalyzer ? WithLbAnalyzer(loadBalancer) : WithConsul)) + .When(x => WhenIDoActionMultipleTimes(50, requestToProjectsAndThenRequestToCustomersAndAssert)) + .Then(x => ThenAllStatusCodesShouldBe(HttpStatusCode.OK)) + .And(x => x.ThenResponsesShouldHaveBodyFromDifferentServices(ports, Bug2119ServiceNames)) // !!! + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(100)) + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(50, 50)) + .And(x => ThenServicesShouldHaveBeenCalledTimes(50, 50)) // strict assertion + .BDDfy(); + } + + [Theory] + [Trait("Bug", "2119")] + [InlineData(false, nameof(NoLoadBalancer))] + [InlineData(false, nameof(LeastConnection))] // original scenario, clean config + [InlineData(true, nameof(LeastConnectionAnalyzer))] // extended scenario using analyzer + [InlineData(false, nameof(RoundRobin))] + [InlineData(true, nameof(RoundRobinAnalyzer))] + public void ShouldReturnDifferentServicesWhenConcurrentlyRequestingToDifferentServices(bool withAnalyzer, string loadBalancer) + { + const int total = 100; // concurrent requests + var consulPort = PortFinder.GetRandomPort(); + var ports = PortFinder.GetPorts(Bug2119ServiceNames.Length); + var service1 = GivenServiceEntry(ports[0], serviceName: Bug2119ServiceNames[0]); + var service2 = GivenServiceEntry(ports[1], serviceName: Bug2119ServiceNames[1]); + var route1 = GivenRoute("/{all}", "/projects/{all}", serviceName: Bug2119ServiceNames[0], loadBalancerType: loadBalancer); + var route2 = GivenRoute("/{all}", "/customers/{all}", serviceName: Bug2119ServiceNames[1], loadBalancerType: loadBalancer); + route1.UpstreamHttpMethod = route2.UpstreamHttpMethod = new() { HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Delete }; + var configuration = GivenServiceDiscovery(consulPort, route1, route2); + var urls = ports.Select(DownstreamUrl).ToArray(); + this.Given(x => GivenMultipleServiceInstancesAreRunning(urls, Bug2119ServiceNames)) // service names as responses + .And(x => x.GivenThereIsAFakeConsulServiceDiscoveryProvider(DownstreamUrl(consulPort))) + .And(x => x.GivenTheServicesAreRegisteredWithConsul(service1, service2)) + .And(x => GivenThereIsAConfiguration(configuration)) + .And(x => GivenOcelotIsRunningWithServices(withAnalyzer ? WithLbAnalyzer(loadBalancer) : WithConsul)) + .When(x => WhenIGetUrlOnTheApiGatewayConcurrently(total, "/projects/api/projects", "/customers/api/customers")) + .Then(x => ThenAllStatusCodesShouldBe(HttpStatusCode.OK)) + .And(x => x.ThenResponsesShouldHaveBodyFromDifferentServices(ports, Bug2119ServiceNames)) // !!! + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(total)) + .And(x => ThenServiceCountersShouldMatchLeasingCounters((ILoadBalancerAnalyzer)_lbAnalyzers[0], ports, 50)) // ProjectsService + .And(x => ThenServiceCountersShouldMatchLeasingCounters((ILoadBalancerAnalyzer)_lbAnalyzers[1], ports, 50)) // CustomersService + .And(x => ThenAllServicesCalledRealisticAmountOfTimes(Bottom(total, ports.Length), Top(total, ports.Length))) + .And(x => ThenServicesShouldHaveBeenCalledTimes(50, 50)) // strict assertion + .BDDfy(); + } + + private Action WithLbAnalyzer(string loadBalancer) => loadBalancer switch + { + nameof(LeastConnection) => WithLbAnalyzer, + nameof(LeastConnectionAnalyzer) => WithLbAnalyzer, + nameof(RoundRobin) => WithLbAnalyzer, + nameof(RoundRobinAnalyzer) => WithLbAnalyzer, + _ => WithLbAnalyzer, + }; + + private void ThenResponsesShouldHaveBodyFromDifferentServices(int[] ports, string[] serviceNames) + { + foreach (var response in _responses) + { + var headers = response.Value.Headers; + headers.TryGetValues(HeaderNames.ServiceIndex, out var indexValues).ShouldBeTrue(); + int serviceIndex = int.Parse(indexValues.FirstOrDefault() ?? "-1"); + serviceIndex.ShouldBeGreaterThanOrEqualTo(0); + + headers.TryGetValues(HeaderNames.Host, out var hostValues).ShouldBeTrue(); + hostValues.FirstOrDefault().ShouldBe("localhost"); + headers.TryGetValues(HeaderNames.Port, out var portValues).ShouldBeTrue(); + portValues.FirstOrDefault().ShouldBe(ports[serviceIndex].ToString()); + + var body = response.Value.Content.ReadAsStringAsync().Result; + var serviceName = serviceNames[serviceIndex]; + body.ShouldNotBeNull().ShouldEndWith(serviceName); + + headers.TryGetValues(HeaderNames.Counter, out var counterValues).ShouldBeTrue(); + var counter = counterValues.ShouldNotBeNull().FirstOrDefault().ShouldNotBeNull(); + body.ShouldBe($"{counter}:{serviceName}"); + } + } + + private static void WithConsul(IServiceCollection services) => services + .AddOcelot().AddConsul(); + + private static void WithOverriddenConsulServiceBuilder(IServiceCollection services) => services + .AddOcelot().AddConsul(); public class MyConsulServiceBuilder : DefaultConsulServiceBuilder { - public MyConsulServiceBuilder(Func configurationFactory, IConsulClientFactory clientFactory, IOcelotLoggerFactory loggerFactory) - : base(configurationFactory, clientFactory, loggerFactory) { } + public MyConsulServiceBuilder(IHttpContextAccessor contextAccessor, IConsulClientFactory clientFactory, IOcelotLoggerFactory loggerFactory) + : base(contextAccessor, clientFactory, loggerFactory) { } protected override string GetDownstreamHost(ServiceEntry entry, Node node) => entry.Service.Address; } @@ -406,12 +579,6 @@ private void WhenIAddAServiceBackIn(ServiceEntry serviceEntry) _consulServices.Add(serviceEntry); } - private void ThenOnlyOneServiceHasBeenCalled() - { - _counterOne.ShouldBe(10); - _counterTwo.ShouldBe(0); - } - private void WhenIRemoveAService(ServiceEntry serviceEntry) { _consulServices.Remove(serviceEntry); @@ -419,23 +586,10 @@ private void WhenIRemoveAService(ServiceEntry serviceEntry) private void GivenIResetCounters() { - _counterOne = 0; - _counterTwo = 0; + _counters[0] = _counters[1] = 0; _counterConsul = 0; } - private void ThenBothServicesCalledRealisticAmountOfTimes(int bottom, int top) - { - _counterOne.ShouldBeInRange(bottom, top); - _counterTwo.ShouldBeInRange(bottom, top); - } - - private void ThenTheTwoServicesShouldHaveBeenCalledTimes(int expected) - { - var total = _counterOne + _counterTwo; - total.ShouldBe(expected); - } - private void GivenTheServicesAreRegisteredWithConsul(params ServiceEntry[] serviceEntries) => _consulServices.AddRange(serviceEntries); private void GivenTheServiceNodesAreRegisteredWithConsul(params Node[] nodes) => _consulNodes.AddRange(nodes); @@ -459,12 +613,18 @@ private void GivenThereIsAFakeConsulServiceDiscoveryProvider(string url) var pathMatch = ServiceNameRegex().Match(context.Request.Path.Value); if (pathMatch.Success) { - _counterConsul++; + //string json; + //lock (ConsulCounterLocker) + //{ + //_counterConsul++; + int count = Interlocked.Increment(ref _counterConsul); // Use the parsed service name to filter the registered Consul services var serviceName = pathMatch.Groups["serviceName"].Value; var services = _consulServices.Where(x => x.Service.Service == serviceName).ToList(); var json = JsonConvert.SerializeObject(services); + + //} context.Response.Headers.Append("Content-Type", "application/json"); await context.Response.WriteAsync(json); return; @@ -472,7 +632,8 @@ private void GivenThereIsAFakeConsulServiceDiscoveryProvider(string url) if (context.Request.Path.Value == "/v1/catalog/nodes") { - _counterNodes++; + //_counterNodes++; + int count = Interlocked.Increment(ref _counterNodes); var json = JsonConvert.SerializeObject(_consulNodes); context.Response.Headers.Append("Content-Type", "application/json"); await context.Response.WriteAsync(json); @@ -482,84 +643,4 @@ private void GivenThereIsAFakeConsulServiceDiscoveryProvider(string url) private void ThenConsulShouldHaveBeenCalledTimes(int expected) => _counterConsul.ShouldBe(expected); private void ThenConsulNodesShouldHaveBeenCalledTimes(int expected) => _counterNodes.ShouldBe(expected); - - private void GivenProductServiceOneIsRunning(string url, int statusCode) - { - _serviceHandler.GivenThereIsAServiceRunningOn(url, async context => - { - try - { - string response; - lock (SyncLock) - { - _counterOne++; - response = _counterOne.ToString(); - } - - context.Response.StatusCode = statusCode; - await context.Response.WriteAsync(response); - } - catch (Exception exception) - { - await context.Response.WriteAsync(exception.StackTrace); - } - }); - } - - private void GivenProductServiceTwoIsRunning(string url, int statusCode) - { - _serviceHandler2.GivenThereIsAServiceRunningOn(url, async context => - { - try - { - string response; - lock (SyncLock) - { - _counterTwo++; - response = _counterTwo.ToString(); - } - - context.Response.StatusCode = statusCode; - await context.Response.WriteAsync(response); - } - catch (Exception exception) - { - await context.Response.WriteAsync(exception.StackTrace); - } - }); - } - - private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, HttpStatusCode statusCode, string responseBody) - { - _serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, basePath, async context => - { - _downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) ? context.Request.PathBase.Value : context.Request.Path.Value; - - if (_downstreamPath != basePath) - { - context.Response.StatusCode = (int)HttpStatusCode.NotFound; - await context.Response.WriteAsync("Downstream path doesn't match base path"); - } - else - { - context.Response.StatusCode = (int)statusCode; - await context.Response.WriteAsync(responseBody); - } - }); - } - - private static RequestDelegate MapGet(string path, string responseBody) => async context => - { - var downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) ? context.Request.PathBase.Value : context.Request.Path.Value; - if (downstreamPath == path) - { - context.Response.StatusCode = (int)HttpStatusCode.OK; - await context.Response.WriteAsync(responseBody); - } - else - { - context.Response.StatusCode = (int)HttpStatusCode.NotFound; - await context.Response.WriteAsync("Not Found"); - } - }; } diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/EurekaServiceDiscoveryTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/EurekaServiceDiscoveryTests.cs index 5ec8f194b..0a5967e11 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/EurekaServiceDiscoveryTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/EurekaServiceDiscoveryTests.cs @@ -20,9 +20,10 @@ public EurekaServiceDiscoveryTests() } [Theory] + [Trait("Feat", "262")] [InlineData(true)] [InlineData(false)] - public void should_use_eureka_service_discovery_and_make_request(bool dotnetRunningInContainer) + public void Should_use_eureka_service_discovery_and_make_request(bool dotnetRunningInContainer) { Environment.SetEnvironmentVariable("DOTNET_RUNNING_IN_CONTAINER", dotnetRunningInContainer.ToString()); var eurekaPort = 8761; diff --git a/test/Ocelot.AcceptanceTests/ServiceDiscovery/KubernetesServiceDiscoveryTests.cs b/test/Ocelot.AcceptanceTests/ServiceDiscovery/KubernetesServiceDiscoveryTests.cs index 99fe18f14..daaa40911 100644 --- a/test/Ocelot.AcceptanceTests/ServiceDiscovery/KubernetesServiceDiscoveryTests.cs +++ b/test/Ocelot.AcceptanceTests/ServiceDiscovery/KubernetesServiceDiscoveryTests.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Newtonsoft.Json; +using Ocelot.AcceptanceTests.LoadBalancer; using Ocelot.Configuration; using Ocelot.Configuration.File; using Ocelot.DependencyInjection; @@ -13,17 +14,15 @@ using Ocelot.Provider.Kubernetes.Interfaces; using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; -using System.Collections.Concurrent; using System.Runtime.CompilerServices; using System.Text; namespace Ocelot.AcceptanceTests.ServiceDiscovery; -public sealed class KubernetesServiceDiscoveryTests : Steps, IDisposable +public sealed class KubernetesServiceDiscoveryTests : ConcurrentSteps, IDisposable { private readonly string _kubernetesUrl; private readonly IKubeApiClient _clientFactory; - private readonly List _serviceHandlers; private readonly ServiceHandler _kubernetesHandler; private string _receivedToken; @@ -38,13 +37,11 @@ public KubernetesServiceDiscoveryTests() AllowInsecure = true, }; _clientFactory = KubeApiClient.Create(option); - _serviceHandlers = new(); _kubernetesHandler = new(); } public override void Dispose() { - _serviceHandlers.ForEach(handler => handler?.Dispose()); _kubernetesHandler.Dispose(); base.Dispose(); } @@ -62,13 +59,14 @@ public void ShouldReturnServicesFromK8s() var route = GivenRouteWithServiceName(namespaces); var configuration = GivenKubeConfiguration(namespaces, route); var downstreamResponse = serviceName; - this.Given(x => x.GivenK8sProductServiceIsRunning(downstreamUrl, downstreamResponse)) + this.Given(x => GivenServiceInstanceIsRunning(downstreamUrl, downstreamResponse)) .And(x => x.GivenThereIsAFakeKubernetesProvider(endpoints, serviceName, namespaces)) .And(_ => GivenThereIsAConfiguration(configuration)) .And(_ => GivenOcelotIsRunningWithServices(WithKubernetes)) .When(_ => WhenIGetUrlOnTheApiGateway("/")) .Then(_ => ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .And(_ => ThenTheResponseBodyShouldBe($"1:{downstreamResponse}")) + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(1)) .And(x => x.ThenTheTokenIs("Bearer txpc696iUhbVoudg164r93CxDTrKRVWG")) .BDDfy(); } @@ -101,7 +99,7 @@ public void ShouldReturnServicesByPortNameAsDownstreamScheme(string downstreamSc route.ServiceName = serviceName; // "example-web" var configuration = GivenKubeConfiguration(namespaces, route); - this.Given(x => x.GivenK8sProductServiceIsRunning(downstreamUrl, nameof(ShouldReturnServicesByPortNameAsDownstreamScheme))) + this.Given(x => GivenServiceInstanceIsRunning(downstreamUrl, nameof(ShouldReturnServicesByPortNameAsDownstreamScheme))) .And(x => x.GivenThereIsAFakeKubernetesProvider(endpoints, serviceName, namespaces)) .And(_ => GivenThereIsAConfiguration(configuration)) .And(_ => GivenOcelotIsRunningWithServices(WithKubernetes)) @@ -110,6 +108,7 @@ public void ShouldReturnServicesByPortNameAsDownstreamScheme(string downstreamSc .And(_ => ThenTheResponseBodyShouldBe(downstreamScheme == "http" ? "1:" + nameof(ShouldReturnServicesByPortNameAsDownstreamScheme) : string.Empty)) + .And(x => ThenAllServicesShouldHaveBeenCalledTimes(downstreamScheme == "http" ? 1 : 0)) .And(x => x.ThenTheTokenIs("Bearer txpc696iUhbVoudg164r93CxDTrKRVWG")) .BDDfy(); } @@ -137,7 +136,7 @@ public void ShouldHighlyLoadOnStableKubeProvider_WithRoundRobinLoadBalancing(int int bottom = totalRequests / totalServices, top = totalRequests - (bottom * totalServices) + bottom; ThenAllServicesCalledRealisticAmountOfTimes(bottom, top); - ThenServiceCountersShouldMatchLeasingCounters(servicePorts); + ThenServiceCountersShouldMatchLeasingCounters(_roundRobinAnalyzer, servicePorts, totalRequests); } [Theory] @@ -154,10 +153,8 @@ public void ShouldHighlyLoadOnUnstableKubeProvider_WithRoundRobinLoadBalancing(i HighlyLoadOnKubeProviderAndRoundRobinBalancer(totalRequests, k8sGeneration); - int bottom = _roundRobinAnalyzer.BottomOfConnections(), - top = _roundRobinAnalyzer.TopOfConnections(); - ThenAllServicesCalledRealisticAmountOfTimes(bottom, top); // with unstable checkings - ThenServiceCountersShouldMatchLeasingCounters(servicePorts); + ThenAllServicesCalledOptimisticAmountOfTimes(_roundRobinAnalyzer); // with unstable checkings + ThenServiceCountersShouldMatchLeasingCounters(_roundRobinAnalyzer, servicePorts, totalRequests); } private (EndpointsV1 Endpoints, int[] ServicePorts) ArrangeHighLoadOnKubeProviderAndRoundRobinBalancer( @@ -165,23 +162,21 @@ public void ShouldHighlyLoadOnUnstableKubeProvider_WithRoundRobinLoadBalancing(i [CallerMemberName] string serviceName = nameof(ArrangeHighLoadOnKubeProviderAndRoundRobinBalancer)) { const string namespaces = nameof(KubernetesServiceDiscoveryTests); - var servicePorts = Enumerable.Repeat(0, totalServices) - .Select(_ => PortFinder.GetRandomPort()) - .ToArray(); + var servicePorts = PortFinder.GetPorts(totalServices); var downstreamUrls = servicePorts .Select(port => LoopbackLocalhostUrl(port, Array.IndexOf(servicePorts, port))) - .ToList(); // based on localhost aka loopback network interface + .ToArray(); // based on localhost aka loopback network interface var downstreams = downstreamUrls.Select(url => new Uri(url)) .ToList(); var downstreamResponses = downstreams .Select(ds => $"{serviceName}:{ds.Host}:{ds.Port}") - .ToList(); + .ToArray(); var subset = new EndpointSubsetV1(); downstreams.ForEach(ds => GivenSubsetAddress(ds, subset)); var endpoints = GivenEndpoints(subset, serviceName); // totalServices service instances with different ports var route = GivenRouteWithServiceName(namespaces, serviceName, nameof(RoundRobinAnalyzer)); // !!! var configuration = GivenKubeConfiguration(namespaces, route); - GivenMultipleK8sProductServicesAreRunning(downstreamUrls, downstreamResponses); + GivenMultipleServiceInstancesAreRunning(downstreamUrls, downstreamResponses); GivenThereIsAConfiguration(configuration); GivenOcelotIsRunningWithServices(WithKubernetesAndRoundRobin); return (endpoints, servicePorts); @@ -190,7 +185,7 @@ public void ShouldHighlyLoadOnUnstableKubeProvider_WithRoundRobinLoadBalancing(i private void HighlyLoadOnKubeProviderAndRoundRobinBalancer(int totalRequests, int k8sGenerationNo) { // Act - WhenIGetUrlOnTheApiGatewayMultipleTimes("/", totalRequests); // load by X parallel requests + WhenIGetUrlOnTheApiGatewayConcurrently("/", totalRequests); // load by X parallel requests // Assert _k8sCounter.ShouldBeGreaterThanOrEqualTo(totalRequests); // integration endpoint called times @@ -198,6 +193,7 @@ private void HighlyLoadOnKubeProviderAndRoundRobinBalancer(int totalRequests, in ThenAllStatusCodesShouldBe(HttpStatusCode.OK); ThenAllServicesShouldHaveBeenCalledTimes(totalRequests); _roundRobinAnalyzer.ShouldNotBeNull().Analyze(); + _roundRobinAnalyzer.Events.Count.ShouldBe(totalRequests); _roundRobinAnalyzer.HasManyServiceGenerations(k8sGenerationNo).ShouldBeTrue(); } @@ -240,8 +236,7 @@ private static EndpointSubsetV1 GivenSubsetAddress(Uri downstream, EndpointSubse private FileRoute GivenRouteWithServiceName(string serviceNamespace, [CallerMemberName] string serviceName = null, - string loadBalancerType = nameof(LeastConnection)) - => new() + string loadBalancerType = nameof(LeastConnection)) => new() { DownstreamPathTemplate = "/", DownstreamScheme = null, // the scheme should not be defined in service discovery scenarios by default, only ServiceName @@ -281,11 +276,13 @@ private void GivenThereIsAFakeKubernetesProvider(EndpointsV1 endpoints, bool isS await Task.Delay(Random.Shared.Next(1, 10)); // emulate integration delay up to 10 milliseconds if (context.Request.Path.Value == $"/api/v1/namespaces/{namespaces}/endpoints/{serviceName}") { - // Each offlinePerThreads-th request to integrated K8s endpoint should fail + string json; lock (K8sCounterLocker) { _k8sCounter++; var subset = endpoints.Subsets[0]; + + // Each offlinePerThreads-th request to integrated K8s endpoint should fail if (!isStable && _k8sCounter % offlinePerThreads == 0 && _k8sCounter >= offlinePerThreads) { while (offlineServicesNo-- > 0) @@ -299,6 +296,7 @@ private void GivenThereIsAFakeKubernetesProvider(EndpointsV1 endpoints, bool isS } endpoints.Metadata.Generation = _k8sServiceGeneration; + json = JsonConvert.SerializeObject(endpoints); } if (context.Request.Headers.TryGetValue("Authorization", out var values)) @@ -306,7 +304,6 @@ private void GivenThereIsAFakeKubernetesProvider(EndpointsV1 endpoints, bool isS _receivedToken = values.First(); } - var json = JsonConvert.SerializeObject(endpoints); context.Response.Headers.Append("Content-Type", "application/json"); await context.Response.WriteAsync(json); } @@ -324,93 +321,14 @@ private void WithKubernetesAndRoundRobin(IServiceCollection services) => service .RemoveAll().AddSingleton(_clientFactory) .RemoveAll().AddSingleton(); + private int _k8sCounter, _k8sServiceGeneration; + private static readonly object K8sCounterLocker = new(); private RoundRobinAnalyzer _roundRobinAnalyzer; private RoundRobinAnalyzer GetRoundRobinAnalyzer(DownstreamRoute route, IServiceDiscoveryProvider provider) { lock (K8sCounterLocker) { - return _roundRobinAnalyzer ??= new RoundRobinAnalyzer(provider.GetAsync, route.ServiceName); - } - } - - private static readonly object ServiceCountersLocker = new(); - private Dictionary _serviceCounters; - - private static readonly object K8sCounterLocker = new(); - private int _k8sCounter, _k8sServiceGeneration; - - private void GivenK8sProductServiceIsRunning(string url, string response) - { - _serviceHandlers.Add(new()); // allocate single instance - _serviceCounters = new(); // single counter - GivenK8sProductServiceIsRunning(url, response, 0); - _serviceCounters[0] = 0; - } - - private void GivenMultipleK8sProductServicesAreRunning(List urls, List responses) - { - urls.ForEach(_ => _serviceHandlers.Add(new())); // allocate multiple instances - _serviceCounters = new(urls.Count); // multiple counters - for (int i = 0; i < urls.Count; i++) - { - GivenK8sProductServiceIsRunning(urls[i], responses[i], i); - _serviceCounters[i] = 0; - } - } - - private void GivenK8sProductServiceIsRunning(string url, string response, int handlerIndex) - { - var serviceHandler = _serviceHandlers[handlerIndex]; - serviceHandler.GivenThereIsAServiceRunningOn(url, async context => - { - await Task.Delay(Random.Shared.Next(5, 15)); // emulate integration delay up to 15 milliseconds - int count = 0; - lock (ServiceCountersLocker) - { - count = ++_serviceCounters[handlerIndex]; - } - - context.Response.StatusCode = (int)HttpStatusCode.OK; - var threadResponse = string.Concat(count, ':', response); - await context.Response.WriteAsync(threadResponse ?? ((int)HttpStatusCode.OK).ToString()); - }); - } - - private void ThenAllServicesShouldHaveBeenCalledTimes(int expected) - { - var sortedByIndex = _serviceCounters.OrderBy(_ => _.Key).Select(_ => _.Value).ToArray(); - var customMessage = $"All values are [{string.Join(',', sortedByIndex)}]"; - _serviceCounters.Sum(_ => _.Value).ShouldBe(expected, customMessage); - _roundRobinAnalyzer.Events.Count.ShouldBe(expected); - } - - private void ThenAllServicesCalledRealisticAmountOfTimes(int bottom, int top) - { - var sortedByIndex = _serviceCounters.OrderBy(_ => _.Key).Select(_ => _.Value).ToArray(); - var customMessage = $"{nameof(bottom)}: {bottom}\n {nameof(top)}: {top}\n All values are [{string.Join(',', sortedByIndex)}]"; - int sum = 0, totalSum = _serviceCounters.Sum(_ => _.Value); - - // Last services cannot be called at all, zero counters - for (int i = 0; i < _serviceCounters.Count && sum < totalSum; i++) - { - int actual = _serviceCounters[i]; - actual.ShouldBeInRange(bottom, top, customMessage); - sum += actual; - } - } - - private void ThenServiceCountersShouldMatchLeasingCounters(int[] ports) - { - var leasingCounters = _roundRobinAnalyzer.GetHostCounters(); - for (int i = 0; i < ports.Length; i++) - { - var host = leasingCounters.Keys.FirstOrDefault(k => k.DownstreamPort == ports[i]); - if (host != null) // leasing info/counters can be absent because of offline service instance with exact port in unstable scenario - { - int counter1 = _serviceCounters[i]; - int counter2 = leasingCounters[host]; - counter1.ShouldBe(counter2, $"Port: {ports[i]}\n Host: {host}"); - } + return _roundRobinAnalyzer ??= new RoundRobinAnalyzerCreator().Create(route, provider)?.Data as RoundRobinAnalyzer; //??= new RoundRobinAnalyzer(provider.GetAsync, route.ServiceName); } } } @@ -421,7 +339,6 @@ public FakeKubeServiceCreator(IOcelotLoggerFactory factory) : base(factory) { } protected override ServiceHostAndPort GetServiceHostAndPort(KubeRegistryConfiguration configuration, EndpointsV1 endpoint, EndpointSubsetV1 subset, EndpointAddressV1 address) { - //return base.GetServiceHostAndPort(configuration, endpoint, subset, address); var ports = subset.Ports; var index = subset.Addresses.IndexOf(address); var portV1 = ports[index]; @@ -438,109 +355,3 @@ protected override IEnumerable GetServiceTags(KubeRegistryConfiguration return tags; } } - -internal class RoundRobinAnalyzer : RoundRobin, ILoadBalancer -{ - public readonly ConcurrentBag Events = new(); - - public RoundRobinAnalyzer(Func>> services, string serviceName) - : base(services, serviceName) - { - this.Leased += Me_Leased; - } - - private void Me_Leased(object sender, LeaseEventArgs e) => Events.Add(e); - - public const string GenerationPrefix = nameof(EndpointsV1.Metadata.Generation) + ":"; - - public object Analyze() - { - var allGenerations = Events - .Select(e => e.Service.Tags.FirstOrDefault(t => t.StartsWith(GenerationPrefix))) - .Distinct().ToArray(); - var allIndices = Events.Select(e => e.ServiceIndex) - .Distinct().ToArray(); - - Dictionary> eventsPerGeneration = new(); - foreach (var generation in allGenerations) - { - var l = Events.Where(e => e.Service.Tags.Contains(generation)).ToList(); - eventsPerGeneration.Add(generation, l); - } - - Dictionary> generationIndices = new(); - foreach (var generation in allGenerations) - { - var l = eventsPerGeneration[generation].Select(e => e.ServiceIndex).Distinct().ToList(); - generationIndices.Add(generation, l); - } - - Dictionary> generationLeases = new(); - foreach (var generation in allGenerations) - { - var l = eventsPerGeneration[generation].Select(e => e.Lease).ToList(); - generationLeases.Add(generation, l); - } - - Dictionary> generationHosts = new(); - foreach (var generation in allGenerations) - { - var l = eventsPerGeneration[generation].Select(e => e.Lease.HostAndPort).Distinct().ToList(); - generationHosts.Add(generation, l); - } - - Dictionary> generationLeasesWithMaxConnections = new(); - foreach (var generation in allGenerations) - { - List leases = new(); - var uniqueHosts = generationHosts[generation]; - foreach (var host in uniqueHosts) - { - int max = generationLeases[generation].Where(l => l == host).Max(l => l.Connections); - Lease wanted = generationLeases[generation].Find(l => l == host && l.Connections == max); - leases.Add(wanted); - } - - leases = leases.OrderBy(l => l.HostAndPort.DownstreamPort).ToList(); - generationLeasesWithMaxConnections.Add(generation, leases); - } - - return generationLeasesWithMaxConnections; - } - - public bool HasManyServiceGenerations(int maxGeneration) - { - int[] generations = new int[maxGeneration + 1]; - string[] tags = new string[maxGeneration + 1]; - for (int i = 0; i < generations.Length; i++) - { - generations[i] = i; - tags[i] = GenerationPrefix + i; - } - - var all = Events - .Select(e => e.Service.Tags.FirstOrDefault(t => t.StartsWith(GenerationPrefix))) - .Distinct().ToArray(); - return all.All(tags.Contains); - } - - public Dictionary GetHostCounters() - { - var hosts = Events.Select(e => e.Lease.HostAndPort).Distinct().ToList(); - return Events - .GroupBy(e => e.Lease.HostAndPort) - .ToDictionary(g => g.Key, g => g.Max(e => e.Lease.Connections)); - } - - public int BottomOfConnections() - { - var hostCounters = GetHostCounters(); - return hostCounters.Min(_ => _.Value); - } - - public int TopOfConnections() - { - var hostCounters = GetHostCounters(); - return hostCounters.Max(_ => _.Value); - } -} diff --git a/test/Ocelot.AcceptanceTests/ServiceHandler.cs b/test/Ocelot.AcceptanceTests/ServiceHandler.cs index c2e10a819..5996387a3 100644 --- a/test/Ocelot.AcceptanceTests/ServiceHandler.cs +++ b/test/Ocelot.AcceptanceTests/ServiceHandler.cs @@ -12,7 +12,7 @@ public class ServiceHandler : IDisposable { private IWebHost _builder; - public void GivenThereIsAServiceRunningOn(string baseUrl, RequestDelegate del) + public void GivenThereIsAServiceRunningOn(string baseUrl, RequestDelegate handler) { _builder = new WebHostBuilder() .UseUrls(baseUrl) @@ -21,14 +21,14 @@ public void GivenThereIsAServiceRunningOn(string baseUrl, RequestDelegate del) .UseIISIntegration() .Configure(app => { - app.Run(del); + app.Run(handler); }) .Build(); _builder.Start(); } - public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, RequestDelegate del) + public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, RequestDelegate handler) { _builder = new WebHostBuilder() .UseUrls(baseUrl) @@ -38,14 +38,14 @@ public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, Reque .Configure(app => { app.UsePathBase(basePath); - app.Run(del); + app.Run(handler); }) .Build(); _builder.Start(); } - public void GivenThereIsAServiceRunningOnWithKestrelOptions(string baseUrl, string basePath, Action options, RequestDelegate del) + public void GivenThereIsAServiceRunningOnWithKestrelOptions(string baseUrl, string basePath, Action options, RequestDelegate handler) { _builder = new WebHostBuilder() .UseUrls(baseUrl) @@ -56,7 +56,7 @@ public void GivenThereIsAServiceRunningOnWithKestrelOptions(string baseUrl, stri .Configure(app => { app.UsePathBase(basePath); - app.Run(del); + app.Run(handler); }) .Build(); @@ -67,7 +67,7 @@ internal void WithDefaultKestrelServerOptions(KestrelServerOptions options) { } - public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, string fileName, string password, int port, RequestDelegate del) + public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, string fileName, string password, int port, RequestDelegate handler) { _builder = new WebHostBuilder() .UseUrls(baseUrl) @@ -82,7 +82,7 @@ public void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, strin .Configure(app => { app.UsePathBase(basePath); - app.Run(del); + app.Run(handler); }) .Build(); diff --git a/test/Ocelot.AcceptanceTests/Steps.cs b/test/Ocelot.AcceptanceTests/Steps.cs index 501e741d4..d35651046 100644 --- a/test/Ocelot.AcceptanceTests/Steps.cs +++ b/test/Ocelot.AcceptanceTests/Steps.cs @@ -11,24 +11,20 @@ using Newtonsoft.Json; using Ocelot.AcceptanceTests.Caching; using Ocelot.Cache.CacheManager; -using Ocelot.Configuration; using Ocelot.Configuration.ChangeTracking; using Ocelot.Configuration.Creator; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.DependencyInjection; -using Ocelot.LoadBalancer.LoadBalancers; using Ocelot.Logging; using Ocelot.Middleware; using Ocelot.Provider.Consul; using Ocelot.Provider.Eureka; using Ocelot.Provider.Polly; -using Ocelot.ServiceDiscovery.Providers; using Ocelot.Tracing.Butterfly; using Ocelot.Tracing.OpenTracing; using Serilog; using Serilog.Core; -using System.Collections.Concurrent; using System.IO.Compression; using System.Net.Http.Headers; using System.Text; @@ -44,7 +40,6 @@ public class Steps : IDisposable protected TestServer _ocelotServer; protected HttpClient _ocelotClient; protected HttpResponseMessage _response; - protected ConcurrentDictionary _parallelResponses; private HttpContent _postContent; private BearerToken _token; public string RequestIdKey = "OcRequestId"; @@ -63,7 +58,6 @@ public Steps() _ocelotConfigFileName = $"{_testId:N}-{ConfigurationBuilderExtensions.PrimaryConfigFile}"; Files = new() { _ocelotConfigFileName }; Folders = new(); - _parallelResponses = new(); } protected List Files { get; } @@ -281,62 +275,6 @@ protected void StartOcelot(Action _ocelotClient = _ocelotServer.CreateClient(); } - /// - /// This is annoying cos it should be in the constructor but we need to set up the file before calling startup so its a step. - /// - /// The type. - /// The delegate object to load balancer factory. - public void GivenOcelotIsRunningWithCustomLoadBalancer(Func loadBalancerFactoryFunc) - where T : ILoadBalancer - { - _webHostBuilder = new WebHostBuilder() - .ConfigureAppConfiguration((hostingContext, config) => - { - config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); - var env = hostingContext.HostingEnvironment; - config.AddJsonFile("appsettings.json", true, false) - .AddJsonFile($"appsettings.{env.EnvironmentName}.json", true, false); - config.AddJsonFile(_ocelotConfigFileName, false, false); - config.AddEnvironmentVariables(); - }) - .ConfigureServices(s => - { - s.AddOcelot() - .AddCustomLoadBalancer(loadBalancerFactoryFunc); - }) - .Configure(app => { app.UseOcelot().Wait(); }); - - _ocelotServer = new TestServer(_webHostBuilder); - _ocelotClient = _ocelotServer.CreateClient(); - } - - public void GivenOcelotIsRunningWithConsul(params string[] urlsToListenOn) - { - _webHostBuilder = new WebHostBuilder(); - - if (urlsToListenOn?.Length > 0) - { - _webHostBuilder.UseUrls(urlsToListenOn); - } - - _webHostBuilder - .ConfigureAppConfiguration((hostingContext, config) => - { - config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath); - var env = hostingContext.HostingEnvironment; - config.AddJsonFile("appsettings.json", true, false) - .AddJsonFile($"appsettings.{env.EnvironmentName}.json", true, false); - config.AddJsonFile(_ocelotConfigFileName, false, false); - config.AddEnvironmentVariables(); - }) - .ConfigureServices(s => { s.AddOcelot().AddConsul(); }) - .Configure(app => { app.UseOcelot().Wait(); }); - - _ocelotServer = new TestServer(_webHostBuilder); - - _ocelotClient = _ocelotServer.CreateClient(); - } - public void ThenTheTraceHeaderIsSet(string key) { var header = _response.Headers.GetValues(key); @@ -891,34 +829,17 @@ public void GivenIAddAHeader(string key, string value) { _ocelotClient.DefaultRequestHeaders.TryAddWithoutValidation(key, value); } - - public Task[] WhenIGetUrlOnTheApiGatewayMultipleTimes(string url, int times) - { - var tasks = new Task[times]; - _parallelResponses = new(times, times); - for (var i = 0; i < times; i++) - { - tasks[i] = GetParallelResponse(url, i); - _parallelResponses[i] = null; - } - - Task.WaitAll(tasks); - return tasks; - } - - private async Task GetParallelResponse(string url, int threadIndex) - { - var response = await _ocelotClient.GetAsync(url); - //Thread.Sleep(_random.Next(40, 60)); - //var content = await response.Content.ReadAsStringAsync(); - //var counterValue = content.Contains(':') - // ? content.Split(':')[0] // let the first fragment is counter value - // : content; - //int count = int.Parse(counterValue); - //count.ShouldBeGreaterThan(0); - _parallelResponses[threadIndex] = response; - } + public static void WhenIDoActionMultipleTimes(int times, Action action) + { + for (int i = 0; i < times; i++) + action?.Invoke(); + } + public static void WhenIDoActionMultipleTimes(int times, Action action) + { + for (int i = 0; i < times; i++) + action?.Invoke(i); + } public void WhenIGetUrlOnTheApiGatewayMultipleTimesForRateLimit(string url, int times) { @@ -970,10 +891,10 @@ public void GivenThePostHasGzipContent(object input) _postContent = content; } - public void ThenTheResponseBodyShouldBe(string expectedBody) - { - _response.Content.ReadAsStringAsync().Result.ShouldBe(expectedBody); - } + public void ThenTheResponseBodyShouldBe(string expectedBody) + => _response.Content.ReadAsStringAsync().Result.ShouldBe(expectedBody); + public void ThenTheResponseBodyShouldBe(string expectedBody, string customMessage) + => _response.Content.ReadAsStringAsync().Result.ShouldBe(expectedBody, customMessage); public void ThenTheContentLengthIs(int expected) { @@ -982,9 +903,6 @@ public void ThenTheContentLengthIs(int expected) public void ThenTheStatusCodeShouldBe(HttpStatusCode expected) => _response.StatusCode.ShouldBe(expected); - - public void ThenAllStatusCodesShouldBe(HttpStatusCode expected) - => _parallelResponses.ShouldAllBe(response => response.Value.StatusCode == expected); public void ThenTheStatusCodeShouldBe(int expectedHttpStatusCode) { @@ -1195,11 +1113,6 @@ protected virtual void Dispose(bool disposing) _ocelotServer?.Dispose(); _ocelotHost?.Dispose(); _response?.Dispose(); - foreach (var response in _parallelResponses) - { - response.Value?.Dispose(); - } - DeleteFiles(); DeleteFolders(); } diff --git a/test/Ocelot.AcceptanceTests/StickySessionsTests.cs b/test/Ocelot.AcceptanceTests/StickySessionsTests.cs index cc7435e19..9127a02d3 100644 --- a/test/Ocelot.AcceptanceTests/StickySessionsTests.cs +++ b/test/Ocelot.AcceptanceTests/StickySessionsTests.cs @@ -28,6 +28,7 @@ public override void Dispose() } [Fact] + [Trait("Feat", "336")] public void ShouldUseSameDownstreamHost_ForSingleRouteWithHighLoad() { var port1 = PortFinder.GetRandomPort(); @@ -48,6 +49,7 @@ public void ShouldUseSameDownstreamHost_ForSingleRouteWithHighLoad() } [Fact] + [Trait("Feat", "336")] public void ShouldUseDifferentDownstreamHost_ForDoubleRoutesWithDifferentCookies() { var port1 = PortFinder.GetRandomPort(); @@ -71,6 +73,7 @@ public void ShouldUseDifferentDownstreamHost_ForDoubleRoutesWithDifferentCookies } [Fact] + [Trait("Feat", "336")] public void ShouldUseSameDownstreamHost_ForDifferentRoutesWithSameCookie() { var port1 = PortFinder.GetRandomPort(); diff --git a/test/Ocelot.Testing/PortFinder.cs b/test/Ocelot.Testing/PortFinder.cs index 6eb6b64d4..42b904991 100644 --- a/test/Ocelot.Testing/PortFinder.cs +++ b/test/Ocelot.Testing/PortFinder.cs @@ -12,26 +12,42 @@ public static class PortFinder private static readonly ConcurrentBag UsedPorts = new(); /// - /// Gets a pseudo-random port from the range [, ]. + /// Gets a pseudo-random port from the range [, ] for one testing scenario. /// - /// New allocated port for testing scenario. + /// New allocated port. /// Critical situation where available ports range has been exceeded. public static int GetRandomPort() { lock (LockObj) { - if (CurrentPort > EndPortRange) + ExceedingPortRangeException.ThrowIf(CurrentPort > EndPortRange); + return UsePort(CurrentPort++); + } + } + + /// + /// Gets the exact number of ports from the range [, ] for one testing scenario. + /// + /// The number of wanted ports. + /// Array of allocated ports. + /// Critical situation where available ports range has been exceeded. + public static int[] GetPorts(int count) + { + var ports = new int[count]; + lock (LockObj) + { + for (int i = 0; i < count; i++, CurrentPort++) { - throw new ExceedingPortRangeException(); + ExceedingPortRangeException.ThrowIf(CurrentPort > EndPortRange); + ports[i] = UsePort(CurrentPort); } - - return UsePort(CurrentPort++); } + return ports; } private static int UsePort(int port) { - UsedPorts.Add(port); + UsedPorts.Add(port); // TODO Review or remove, now useless var ipe = new IPEndPoint(IPAddress.Loopback, port); @@ -46,4 +62,7 @@ public class ExceedingPortRangeException : Exception { public ExceedingPortRangeException() : base("Cannot find available port to bind to!") { } + + public static void ThrowIf(bool condition) + => _ = condition ? throw new ExceedingPortRangeException() : 0; } diff --git a/test/Ocelot.UnitTests/Consul/ConsulTests.cs b/test/Ocelot.UnitTests/Consul/ConsulTests.cs index b9009d488..fff9eaf91 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulTests.cs @@ -20,6 +20,7 @@ public sealed class ConsulTests : UnitTest, IDisposable private readonly List _consulServiceEntries; private readonly Mock _factory; private readonly Mock _logger; + private readonly Mock _contextAccessor; private IConsulClientFactory _clientFactory; private IConsulServiceBuilder _serviceBuilder; private ConsulRegistryConfiguration _config; @@ -36,6 +37,7 @@ public ConsulTests() _consulServiceEntries = new List(); _factory = new Mock(); _logger = new Mock(); + _contextAccessor = new Mock(); _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); @@ -49,8 +51,11 @@ public void Dispose() private void Arrange([CallerMemberName] string serviceName = null) { _config = new ConsulRegistryConfiguration(_consulScheme, _consulHost, _port, serviceName, null); + var context = new DefaultHttpContext(); + context.Items.Add(nameof(ConsulRegistryConfiguration), _config); + _contextAccessor.SetupGet(x => x.HttpContext).Returns(context); _clientFactory = new ConsulClientFactory(); - _serviceBuilder = new DefaultConsulServiceBuilder(() => _config, _clientFactory, _factory.Object); + _serviceBuilder = new DefaultConsulServiceBuilder(_contextAccessor.Object, _clientFactory, _factory.Object); _provider = new ConsulProvider(_config, _factory.Object, _clientFactory, _serviceBuilder); } diff --git a/test/Ocelot.UnitTests/Consul/DefaultConsulServiceBuilderTests.cs b/test/Ocelot.UnitTests/Consul/DefaultConsulServiceBuilderTests.cs index 25dc8d950..5858efb60 100644 --- a/test/Ocelot.UnitTests/Consul/DefaultConsulServiceBuilderTests.cs +++ b/test/Ocelot.UnitTests/Consul/DefaultConsulServiceBuilderTests.cs @@ -1,28 +1,25 @@ -using Castle.Components.DictionaryAdapter.Xml; -using Consul; +using Consul; +using Microsoft.AspNetCore.Http; using Ocelot.Logging; using Ocelot.Provider.Consul; using Ocelot.Provider.Consul.Interfaces; using System.Reflection; using System.Runtime.CompilerServices; -using System.Xml.Linq; namespace Ocelot.UnitTests.Consul; public sealed class DefaultConsulServiceBuilderTests { private DefaultConsulServiceBuilder sut; - private readonly Func configurationFactory; + private readonly Mock contextAccessor; private readonly Mock clientFactory; private readonly Mock loggerFactory; private readonly Mock logger; private ConsulRegistryConfiguration _configuration; - private ConsulRegistryConfiguration GetConfiguration() => _configuration; - public DefaultConsulServiceBuilderTests() { - configurationFactory = GetConfiguration; + contextAccessor = new(); clientFactory = new(); clientFactory.Setup(x => x.Get(It.IsAny())) .Returns(new ConsulClient()); @@ -35,20 +32,25 @@ public DefaultConsulServiceBuilderTests() private void Arrange([CallerMemberName] string testName = null) { _configuration = new(null, null, 0, testName, null); - sut = new DefaultConsulServiceBuilder(configurationFactory, clientFactory.Object, loggerFactory.Object); + var context = new DefaultHttpContext(); + context.Items.Add(nameof(ConsulRegistryConfiguration), _configuration); + contextAccessor.SetupGet(x => x.HttpContext).Returns(context); + sut = new DefaultConsulServiceBuilder(contextAccessor.Object, clientFactory.Object, loggerFactory.Object); } [Fact] public void Ctor_PrivateMembers_PropertiesAreInitialized() { Arrange(); - var methodClient = sut.GetType().GetProperty("Client", BindingFlags.NonPublic | BindingFlags.Instance); - var methodLogger = sut.GetType().GetProperty("Logger", BindingFlags.NonPublic | BindingFlags.Instance); + var propClient = sut.GetType().GetProperty("Client", BindingFlags.NonPublic | BindingFlags.Instance); + var propLogger = sut.GetType().GetProperty("Logger", BindingFlags.NonPublic | BindingFlags.Instance); + var propConfiguration = sut.GetType().GetProperty("Configuration", BindingFlags.NonPublic | BindingFlags.Instance); // Act - var actualConfiguration = sut.Configuration; - var actualClient = methodClient.GetValue(sut); - var actualLogger = methodLogger.GetValue(sut); + //var actualConfiguration = sut.Configuration; + var actualConfiguration = propConfiguration.GetValue(sut); + var actualClient = propClient.GetValue(sut); + var actualLogger = propLogger.GetValue(sut); // Assert actualConfiguration.ShouldNotBeNull().ShouldBe(_configuration); diff --git a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs index d7b676a23..247af37e9 100644 --- a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs @@ -1,4 +1,5 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Logging; @@ -14,12 +15,20 @@ public class ProviderFactoryTests public ProviderFactoryTests() { - var services = new ServiceCollection(); - var loggerFactory = new Mock(); + var contextAccessor = new Mock(); + var context = new DefaultHttpContext(); + context.Items.Add(nameof(ConsulRegistryConfiguration), new ConsulRegistryConfiguration(null, null, 0, null, null)); + contextAccessor.SetupGet(x => x.HttpContext).Returns(context); + + var loggerFactory = new Mock(); var logger = new Mock(); loggerFactory.Setup(x => x.CreateLogger()).Returns(logger.Object); loggerFactory.Setup(x => x.CreateLogger()).Returns(logger.Object); + var consulFactory = new Mock(); + + var services = new ServiceCollection(); + services.AddSingleton(contextAccessor.Object); services.AddSingleton(consulFactory.Object); services.AddSingleton(loggerFactory.Object); _provider = services.BuildServiceProvider(); diff --git a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs index 262014927..933578ccb 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/OcelotBuilderTests.cs @@ -534,17 +534,13 @@ private void ThenAnExceptionIsntThrown() private class FakeCustomLoadBalancer : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - // Not relevant for these tests - throw new NotImplementedException(); - } + public string Type => nameof(FakeCustomLoadBalancer); - public void Release(ServiceHostAndPort hostAndPort) - { - // Not relevant for these tests - throw new NotImplementedException(); - } + // Not relevant for these tests + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + + // Not relevant for these tests + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } } } diff --git a/test/Ocelot.UnitTests/Kubernetes/KubeTests.cs b/test/Ocelot.UnitTests/Kubernetes/KubeTests.cs index 1b3a3f416..c71444045 100644 --- a/test/Ocelot.UnitTests/Kubernetes/KubeTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/KubeTests.cs @@ -26,6 +26,7 @@ public KubeTests() } [Fact] + [Trait("Feat", "345")] public async Task Should_return_service_from_k8s() { // Arrange diff --git a/test/Ocelot.UnitTests/Kubernetes/OcelotBuilderExtensionsTests.cs b/test/Ocelot.UnitTests/Kubernetes/OcelotBuilderExtensionsTests.cs index d3c0b9967..7e60dd7e2 100644 --- a/test/Ocelot.UnitTests/Kubernetes/OcelotBuilderExtensionsTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/OcelotBuilderExtensionsTests.cs @@ -33,7 +33,8 @@ private static IWebHostEnvironment GetHostingEnvironment() } [Fact] - public void should_set_up_kubernetes() + [Trait("Feat", "345")] + public void Should_set_up_kubernetes() { this.Given(x => WhenISetUpOcelotServices()) .When(x => WhenISetUpKubernetes()) diff --git a/test/Ocelot.UnitTests/Kubernetes/PollKubeTests.cs b/test/Ocelot.UnitTests/Kubernetes/PollKubeTests.cs index 794589bc3..4d6c27483 100644 --- a/test/Ocelot.UnitTests/Kubernetes/PollKubeTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/PollKubeTests.cs @@ -27,7 +27,8 @@ public PollKubeTests() } [Fact] - public void should_return_service_from_kube() + [Trait("Feat", "345")] + public void Should_return_service_from_kube() { var service = new Service(string.Empty, new ServiceHostAndPort(string.Empty, 0), string.Empty, string.Empty, new List()); diff --git a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsTests.cs b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsTests.cs index c13155e46..c2c95f921 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/CookieStickySessionsTests.cs @@ -120,7 +120,7 @@ private void ThenAnErrorIsReturned() private void GivenTheLoadBalancerReturnsError() { _loadBalancer - .Setup(x => x.Lease(It.IsAny())) + .Setup(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(new ErrorResponse(new AnyError())); } @@ -149,14 +149,14 @@ private async Task WhenIMakeTwoRequetsWithDifferentSessionValues([CallerMemberNa contextTwo.Request.Cookies = cookiesTwo; contextTwo.Items.UpsertDownstreamRoute(route); - _firstHostAndPort = await _stickySessions.Lease(contextOne); - _secondHostAndPort = await _stickySessions.Lease(contextTwo); + _firstHostAndPort = await _stickySessions.LeaseAsync(contextOne); + _secondHostAndPort = await _stickySessions.LeaseAsync(contextTwo); } private void GivenTheLoadBalancerReturnsSequence() { _loadBalancer - .SetupSequence(x => x.Lease(It.IsAny())) + .SetupSequence(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(new OkResponse(new ServiceHostAndPort("one", 80))) .ReturnsAsync(new OkResponse(new ServiceHostAndPort("two", 80))); } @@ -169,8 +169,8 @@ private void ThenTheFirstAndSecondResponseAreTheSame() private async Task WhenILeaseTwiceInARow() { - _firstHostAndPort = await _stickySessions.Lease(_httpContext); - _secondHostAndPort = await _stickySessions.Lease(_httpContext); + _firstHostAndPort = await _stickySessions.LeaseAsync(_httpContext); + _secondHostAndPort = await _stickySessions.LeaseAsync(_httpContext); } private void GivenTheDownstreamRequestHasSessionId(string value) @@ -183,13 +183,13 @@ private void GivenTheDownstreamRequestHasSessionId(string value) private void GivenTheLoadBalancerReturns() { _loadBalancer - .Setup(x => x.Lease(It.IsAny())) + .Setup(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(new OkResponse(new ServiceHostAndPort(string.Empty, 80))); } private async Task WhenILease() { - _result = await _stickySessions.Lease(_httpContext); + _result = await _stickySessions.LeaseAsync(_httpContext); } private void ThenTheHostAndPortIsNotNull() diff --git a/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs index deb25a12e..188993807 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/DelegateInvokingLoadBalancerCreatorTests.cs @@ -107,15 +107,9 @@ public FakeLoadBalancer(DownstreamRoute downstreamRoute, IServiceDiscoveryProvid public DownstreamRoute DownstreamRoute { get; } public IServiceDiscoveryProvider ServiceDiscoveryProvider { get; } - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeLoadBalancer); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs index c0ee460ca..6ca090c97 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LeastConnectionTests.cs @@ -58,9 +58,9 @@ public async Task Should_handle_service_returning_to_available() _leastConnection = new LeastConnection(() => Task.FromResult(availableServices), serviceName); - var hostAndPortOne = await _leastConnection.Lease(_httpContext); + var hostAndPortOne = await _leastConnection.LeaseAsync(_httpContext); hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); - var hostAndPortTwo = await _leastConnection.Lease(_httpContext); + var hostAndPortTwo = await _leastConnection.LeaseAsync(_httpContext); hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.2"); _leastConnection.Release(hostAndPortOne.Data); _leastConnection.Release(hostAndPortTwo.Data); @@ -70,9 +70,9 @@ public async Task Should_handle_service_returning_to_available() new(serviceName, new ServiceHostAndPort("127.0.0.1", 80), string.Empty, string.Empty, Array.Empty()), }; - hostAndPortOne = await _leastConnection.Lease(_httpContext); + hostAndPortOne = await _leastConnection.LeaseAsync(_httpContext); hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); - hostAndPortTwo = await _leastConnection.Lease(_httpContext); + hostAndPortTwo = await _leastConnection.LeaseAsync(_httpContext); hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.1"); _leastConnection.Release(hostAndPortOne.Data); _leastConnection.Release(hostAndPortTwo.Data); @@ -83,9 +83,9 @@ public async Task Should_handle_service_returning_to_available() new(serviceName, new ServiceHostAndPort("127.0.0.2", 80), string.Empty, string.Empty, Array.Empty()), }; - hostAndPortOne = await _leastConnection.Lease(_httpContext); + hostAndPortOne = await _leastConnection.LeaseAsync(_httpContext); hostAndPortOne.Data.DownstreamHost.ShouldBe("127.0.0.1"); - hostAndPortTwo = await _leastConnection.Lease(_httpContext); + hostAndPortTwo = await _leastConnection.LeaseAsync(_httpContext); hostAndPortTwo.Data.DownstreamHost.ShouldBe("127.0.0.2"); _leastConnection.Release(hostAndPortOne.Data); _leastConnection.Release(hostAndPortTwo.Data); @@ -93,7 +93,7 @@ public async Task Should_handle_service_returning_to_available() private async Task LeaseDelayAndRelease() { - var hostAndPort = await _leastConnection.Lease(_httpContext); + var hostAndPort = await _leastConnection.LeaseAsync(_httpContext); await Task.Delay(_random.Next(1, 100)); _leastConnection.Release(hostAndPort.Data); } @@ -132,15 +132,15 @@ public async Task Should_serve_from_service_with_least_connections() _services = availableServices; _leastConnection = new LeastConnection(() => Task.FromResult(_services), serviceName); - var response = await _leastConnection.Lease(_httpContext); + var response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[0].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[2].HostAndPort.DownstreamHost); } @@ -159,19 +159,19 @@ public async Task Should_build_connections_per_service() _services = availableServices; _leastConnection = new LeastConnection(() => Task.FromResult(_services), serviceName); - var response = await _leastConnection.Lease(_httpContext); + var response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[0].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[0].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); } @@ -190,26 +190,26 @@ public async Task Should_release_connection() _services = availableServices; _leastConnection = new LeastConnection(() => Task.FromResult(_services), serviceName); - var response = await _leastConnection.Lease(_httpContext); + var response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[0].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[0].HostAndPort.DownstreamHost); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); //release this so 2 should have 1 connection and we should get 2 back as our next host and port _leastConnection.Release(availableServices[1].HostAndPort); - response = await _leastConnection.Lease(_httpContext); + response = await _leastConnection.LeaseAsync(_httpContext); response.Data.DownstreamHost.ShouldBe(availableServices[1].HostAndPort.DownstreamHost); } @@ -260,7 +260,7 @@ private void GivenAHostAndPort(ServiceHostAndPort hostAndPort) private void WhenIGetTheNextHostAndPort() { - _result = _leastConnection.Lease(_httpContext).Result; + _result = _leastConnection.LeaseAsync(_httpContext).Result; } private void ThenTheNextHostAndPortIsReturned() diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs index 616168548..be83ec78c 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerFactoryTests.cs @@ -219,54 +219,30 @@ public Response Create(DownstreamRoute route, IServiceDiscoveryPr private class FakeLoadBalancerOne : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeLoadBalancerOne); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } private class FakeLoadBalancerTwo : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeLoadBalancerTwo); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } private class FakeNoLoadBalancer : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeNoLoadBalancer); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } private class BrokenLoadBalancer : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(BrokenLoadBalancer); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs index fa0b835ff..9a426fe77 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerHouseTests.cs @@ -149,28 +149,16 @@ private void ThenItIsReturned() private class FakeLoadBalancer : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeLoadBalancer); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } private class FakeRoundRobinLoadBalancer : ILoadBalancer { - public Task> Lease(HttpContext httpContext) - { - throw new NotImplementedException(); - } - - public void Release(ServiceHostAndPort hostAndPort) - { - throw new NotImplementedException(); - } + public string Type => nameof(FakeRoundRobinLoadBalancer); + public Task> LeaseAsync(HttpContext httpContext) => throw new NotImplementedException(); + public void Release(ServiceHostAndPort hostAndPort) => throw new NotImplementedException(); } } } diff --git a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs index fd46e9a2a..1a1ab3b02 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/LoadBalancerMiddlewareTests.cs @@ -145,14 +145,14 @@ private void GivenTheLoadBalancerReturnsAnError() { _getHostAndPortError = new ErrorResponse(new List { new ServicesAreNullError("services were null for bah") }); _loadBalancer - .Setup(x => x.Lease(It.IsAny())) + .Setup(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(_getHostAndPortError); } private void GivenTheLoadBalancerReturnsOk() { _loadBalancer - .Setup(x => x.Lease(It.IsAny())) + .Setup(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(new OkResponse(new ServiceHostAndPort("abc", 123, "https"))); } @@ -160,7 +160,7 @@ private void GivenTheLoadBalancerReturns() { _hostAndPort = new ServiceHostAndPort("127.0.0.1", 80); _loadBalancer - .Setup(x => x.Lease(It.IsAny())) + .Setup(x => x.LeaseAsync(It.IsAny())) .ReturnsAsync(new OkResponse(_hostAndPort)); } diff --git a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerTests.cs b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerTests.cs index e1490e898..96494bd0b 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/NoLoadBalancerTests.cs @@ -85,7 +85,7 @@ private void GivenServices(List services) private void WhenIGetTheNextHostAndPort() { - _result = _loadBalancer.Lease(new DefaultHttpContext()).Result; + _result = _loadBalancer.LeaseAsync(new DefaultHttpContext()).Result; } private void ThenTheHostAndPortIs(ServiceHostAndPort expected) diff --git a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinTests.cs b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinTests.cs index 1eb8e9957..545f04403 100644 --- a/test/Ocelot.UnitTests/LoadBalancer/RoundRobinTests.cs +++ b/test/Ocelot.UnitTests/LoadBalancer/RoundRobinTests.cs @@ -142,9 +142,9 @@ private static RoundRobin GivenLoadBalancer(List services, bool immedia } private Response WhenIGetTheNextAddress(RoundRobin roundRobin) - => roundRobin.Lease(_httpContext).Result; + => roundRobin.LeaseAsync(_httpContext).Result; private Task> WhenIGetTheNextAddressAsync(RoundRobin roundRobin) - => roundRobin.Lease(_httpContext); + => roundRobin.LeaseAsync(_httpContext); private static void ThenServicesAreNullErrorIsReturned(Response response) {