From e36554ce669f351ccd7c1eb829aae0b3bbd77d19 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi Date: Tue, 20 Jun 2023 16:17:59 +0200 Subject: [PATCH 01/23] fixing some issues in poll consul: - Timer is not thread safe, avoiding usage of it - No Ressources are returned for first call - Using a providers pool, instead of creating a new provider instance --- src/Ocelot.Provider.Consul/Consul.cs | 25 ++---- .../ConsulProviderFactory.cs | 71 +++++++++------ src/Ocelot.Provider.Consul/PollConsul.cs | 90 +++++++++++-------- .../ConsulFileConfigurationRepositoryTests.cs | 32 +++---- ...lingConsulServiceDiscoveryProviderTests.cs | 2 +- 5 files changed, 126 insertions(+), 94 deletions(-) diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index 2e73f5271..138aac52b 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -1,16 +1,11 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; - -using global::Consul; - +using Consul; using Ocelot.Infrastructure.Extensions; - using Ocelot.Logging; - using Ocelot.ServiceDiscovery.Providers; - +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; using Ocelot.Values; namespace Ocelot.Provider.Consul @@ -71,12 +66,10 @@ private static Service BuildService(ServiceEntry serviceEntry, Node serviceNode) private static bool IsValid(ServiceEntry serviceEntry) { - if (string.IsNullOrEmpty(serviceEntry.Service.Address) || serviceEntry.Service.Address.Contains("http://") || serviceEntry.Service.Address.Contains("https://") || serviceEntry.Service.Port <= 0) - { - return false; - } - - return true; + return !string.IsNullOrEmpty(serviceEntry.Service.Address) + && !serviceEntry.Service.Address.Contains("http://") + && !serviceEntry.Service.Address.Contains("https://") + && serviceEntry.Service.Port > 0; } private static string GetVersionFromStrings(IEnumerable strings) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 9392f461b..7ab3c871d 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -1,29 +1,48 @@ -using Ocelot.Logging; - +using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.Extensions.DependencyInjection; +using Ocelot.Logging; +using Ocelot.ServiceDiscovery; + +namespace Ocelot.Provider.Consul; + +public static class ConsulProviderFactory +{ + private static readonly List ServiceDiscoveryProviders = new(); + private static readonly object LockObject = new(); + +#pragma warning disable CA2211 // Non-constant fields should not be visible + public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => +#pragma warning restore CA2211 // Non-constant fields should not be visible + { + var factory = provider.GetService(); + var consulFactory = provider.GetService(); + var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, + route.ServiceName, config.Token); + var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); + + if (string.Compare(config.Type, "PollConsul", StringComparison.OrdinalIgnoreCase) != 0) + { + return consulServiceDiscoveryProvider; + } + + lock (LockObject) + { + var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); + + if (discoveryProvider != null) + { + return discoveryProvider; + } + + discoveryProvider = new PollConsul( + config.PollingInterval, route.ServiceName, factory, + consulServiceDiscoveryProvider); + + ServiceDiscoveryProviders.Add(discoveryProvider); -using Ocelot.ServiceDiscovery; - -namespace Ocelot.Provider.Consul -{ - public static class ConsulProviderFactory - { - public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => - { - var factory = provider.GetService(); - - var consulFactory = provider.GetService(); - - var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); - - var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - - if (config.Type?.ToLower() == "pollconsul") - { - return new PollConsul(config.PollingInterval, factory, consulServiceDiscoveryProvider); - } - - return consulServiceDiscoveryProvider; - }; - } + return discoveryProvider; + } + }; } diff --git a/src/Ocelot.Provider.Consul/PollConsul.cs b/src/Ocelot.Provider.Consul/PollConsul.cs index b5ac65d86..0db469383 100644 --- a/src/Ocelot.Provider.Consul/PollConsul.cs +++ b/src/Ocelot.Provider.Consul/PollConsul.cs @@ -1,53 +1,73 @@ using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; -using Ocelot.Values; using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; +using Ocelot.Values; -namespace Ocelot.Provider.Consul; - -public sealed class PollConsul : IServiceDiscoveryProvider, IDisposable +namespace Ocelot.Provider.Consul { - private readonly IOcelotLogger _logger; - private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; - private Timer _timer; - private bool _polling; - private List _services; - - public PollConsul(int pollingInterval, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) + public sealed class PollConsul : IServiceDiscoveryProvider, IDisposable { - _logger = factory.CreateLogger(); - _consulServiceDiscoveryProvider = consulServiceDiscoveryProvider; - _services = new List(); + private readonly IOcelotLogger _logger; + private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; + + private readonly int _pollingInterval; + private DateTime _lastUpdateTime; + + private List _services; + private readonly SemaphoreSlim _semaphore = new(1, 1); - _timer = new Timer(async x => + public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) { - if (_polling) + _logger = factory.CreateLogger(); + _consulServiceDiscoveryProvider = consulServiceDiscoveryProvider; + _pollingInterval = pollingInterval; + ServiceName = serviceName; + _services = new List(); + + //initializing with update time = DateTime.MinValue + //polling will occur then. + _lastUpdateTime = DateTime.MinValue; + } + + public string ServiceName { get; } + + /// + /// Getting the services, but, if first call, + /// retrieving the services and then starting the timer. + /// + /// the service list. + public async Task> Get() + { + await _semaphore.WaitAsync(); + try { - return; - } + var refreshTime = _lastUpdateTime.AddMilliseconds(_pollingInterval); - _polling = true; - await Poll(); - _polling = false; - }, null, pollingInterval, pollingInterval); - } + //checking if any services available + if (refreshTime >= DateTime.UtcNow && _services.Any()) + { + return _services; + } - public void Dispose() - { - _timer?.Dispose(); - _timer = null; - } + _logger.LogInformation($"Retrieving new client information for service: {ServiceName}"); + _services = await _consulServiceDiscoveryProvider.Get(); + _lastUpdateTime = DateTime.UtcNow; - public Task> Get() - { - return Task.FromResult(_services); - } + return _services; + } + finally + { + _semaphore.Release(); + } + } - private async Task Poll() - { - _services = await _consulServiceDiscoveryProvider.Get(); + public void Dispose() + { + _semaphore.Dispose(); + } } } diff --git a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs index 9d4d29bad..85235e9e4 100644 --- a/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ConsulFileConfigurationRepositoryTests.cs @@ -1,19 +1,19 @@ -using global::Consul; -using Microsoft.Extensions.Options; -using Moq; -using Newtonsoft.Json; -using Ocelot.Cache; -using Ocelot.Configuration.File; -using Ocelot.Logging; -using Ocelot.Provider.Consul; -using Ocelot.Responses; -using Shouldly; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using TestStack.BDDfy; +using global::Consul; +using Microsoft.Extensions.Options; +using Moq; +using Newtonsoft.Json; +using Ocelot.Cache; +using Ocelot.Configuration.File; +using Ocelot.Logging; +using Ocelot.Provider.Consul; +using Ocelot.Responses; +using Shouldly; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using TestStack.BDDfy; using Xunit; namespace Ocelot.UnitTests.Consul diff --git a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs index b3780fd21..b11802288 100644 --- a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs @@ -55,7 +55,7 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices(int expected) { - using (var provider = new PollConsul(_delay, _factory.Object, _consulServiceDiscoveryProvider.Object)) + using (var provider = new PollConsul(_delay, "test", _factory.Object, _consulServiceDiscoveryProvider.Object)) { var result = Wait.WaitFor(3000).Until(() => { From c3ea2db6cf33ca1142d8e26acf46f81fce7c949b Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi Date: Tue, 20 Jun 2023 16:18:53 +0200 Subject: [PATCH 02/23] line endings --- .../ConsulProviderFactory.cs | 94 +++++++++---------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 7ab3c871d..382527606 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -1,48 +1,48 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.Extensions.DependencyInjection; -using Ocelot.Logging; -using Ocelot.ServiceDiscovery; - -namespace Ocelot.Provider.Consul; - -public static class ConsulProviderFactory -{ - private static readonly List ServiceDiscoveryProviders = new(); - private static readonly object LockObject = new(); - -#pragma warning disable CA2211 // Non-constant fields should not be visible - public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => -#pragma warning restore CA2211 // Non-constant fields should not be visible - { - var factory = provider.GetService(); - var consulFactory = provider.GetService(); - var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, - route.ServiceName, config.Token); - var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - - if (string.Compare(config.Type, "PollConsul", StringComparison.OrdinalIgnoreCase) != 0) - { - return consulServiceDiscoveryProvider; - } - - lock (LockObject) - { - var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); - - if (discoveryProvider != null) - { - return discoveryProvider; - } - - discoveryProvider = new PollConsul( - config.PollingInterval, route.ServiceName, factory, - consulServiceDiscoveryProvider); - - ServiceDiscoveryProviders.Add(discoveryProvider); - - return discoveryProvider; - } - }; +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.DependencyInjection; +using Ocelot.Logging; +using Ocelot.ServiceDiscovery; + +namespace Ocelot.Provider.Consul; + +public static class ConsulProviderFactory +{ + private static readonly List ServiceDiscoveryProviders = new(); + private static readonly object LockObject = new(); + +#pragma warning disable CA2211 // Non-constant fields should not be visible + public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => +#pragma warning restore CA2211 // Non-constant fields should not be visible + { + var factory = provider.GetService(); + var consulFactory = provider.GetService(); + var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, + route.ServiceName, config.Token); + var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); + + if (string.Compare(config.Type, "PollConsul", StringComparison.OrdinalIgnoreCase) != 0) + { + return consulServiceDiscoveryProvider; + } + + lock (LockObject) + { + var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); + + if (discoveryProvider != null) + { + return discoveryProvider; + } + + discoveryProvider = new PollConsul( + config.PollingInterval, route.ServiceName, factory, + consulServiceDiscoveryProvider); + + ServiceDiscoveryProviders.Add(discoveryProvider); + + return discoveryProvider; + } + }; } From 6984d1dc9aa6460fd6de85baf9c2736027eb906a Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi Date: Tue, 20 Jun 2023 21:45:15 +0200 Subject: [PATCH 03/23] adding some test cases --- ...lingConsulServiceDiscoveryProviderTests.cs | 55 ++++-- .../Consul/ProviderFactoryTests.cs | 168 +++++++++++++----- 2 files changed, 157 insertions(+), 66 deletions(-) diff --git a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs index b11802288..1065d3cbb 100644 --- a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs @@ -42,6 +42,17 @@ public void should_return_service_from_consul() .BDDfy(); } + [Fact] + public void should_return_service_from_consul_without_delay() + { + var service = new Service(string.Empty, new ServiceHostAndPort(string.Empty, 0), string.Empty, string.Empty, new List()); + + this.Given(x => GivenConsulReturns(service)) + .When(x => WhenIGetTheServicesWithoutDelay(1)) + .Then(x => ThenTheCountIs(1)) + .BDDfy(); + } + private void GivenConsulReturns(Service service) { _services.Add(service); @@ -55,28 +66,38 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices(int expected) { - using (var provider = new PollConsul(_delay, "test", _factory.Object, _consulServiceDiscoveryProvider.Object)) + using var provider = new PollConsul(_delay, "test", _factory.Object, _consulServiceDiscoveryProvider.Object); + var result = Wait.WaitFor(3000).Until(() => { - var result = Wait.WaitFor(3000).Until(() => + try + { + _result = provider.Get().GetAwaiter().GetResult(); + return _result.Count == expected; + } + catch (Exception) { - try - { - _result = provider.Get().GetAwaiter().GetResult(); - if (_result.Count == expected) - { - return true; - } + return false; + } + }); - return false; - } - catch (Exception) - { - return false; - } - }); + result.ShouldBeTrue(); + } - result.ShouldBeTrue(); + private void WhenIGetTheServicesWithoutDelay(int expected) + { + using var provider = new PollConsul(_delay, "test2", _factory.Object, _consulServiceDiscoveryProvider.Object); + bool result; + try + { + _result = provider.Get().GetAwaiter().GetResult(); + result = _result.Count == expected; + } + catch (Exception) + { + result = false; } + + result.ShouldBeTrue(); } } } diff --git a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs index 7b3085c53..30e3371e8 100644 --- a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs @@ -1,56 +1,126 @@ -using Microsoft.Extensions.DependencyInjection; +using System; +using System.Linq; +using Microsoft.Extensions.DependencyInjection; using Moq; using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.Logging; using Ocelot.Provider.Consul; +using Ocelot.ServiceDiscovery.Providers; using Shouldly; -using System; -using Xunit; - -namespace Ocelot.UnitTests.Consul -{ - public class ProviderFactoryTests - { - private readonly IServiceProvider _provider; - - public ProviderFactoryTests() - { - var services = new ServiceCollection(); - 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(); - services.AddSingleton(consulFactory.Object); - services.AddSingleton(loggerFactory.Object); - _provider = services.BuildServiceProvider(); - } - - [Fact] - public void should_return_ConsulServiceDiscoveryProvider() - { - var route = new DownstreamRouteBuilder() - .WithServiceName(string.Empty) - .Build(); - - var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration(string.Empty, string.Empty, string.Empty, 1, string.Empty, string.Empty, 1), route); - provider.ShouldBeOfType(); - } - - [Fact] - public void should_return_PollingConsulServiceDiscoveryProvider() - { - var stopsPollerFromPolling = 10000; - - var route = new DownstreamRouteBuilder() - .WithServiceName(string.Empty) - .Build(); - - var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration("pollconsul", "http", string.Empty, 1, string.Empty, string.Empty, stopsPollerFromPolling), route); - var pollProvider = provider as PollConsul; - pollProvider.ShouldNotBeNull(); - pollProvider.Dispose(); - } - } +using Xunit; + +namespace Ocelot.UnitTests.Consul; + +public class ProviderFactoryTests +{ + private readonly IServiceProvider _provider; + + public ProviderFactoryTests() + { + var services = new ServiceCollection(); + 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(); + services.AddSingleton(consulFactory.Object); + services.AddSingleton(loggerFactory.Object); + _provider = services.BuildServiceProvider(); + } + + [Fact] + public void should_return_ConsulServiceDiscoveryProvider() + { + var route = new DownstreamRouteBuilder() + .WithServiceName(string.Empty) + .Build(); + + var provider = ConsulProviderFactory.Get(_provider, + new ServiceProviderConfiguration(string.Empty, string.Empty, string.Empty, 1, string.Empty, string.Empty, + 1), route); + provider.ShouldBeOfType(); + } + + [Fact] + public void should_return_PollingConsulServiceDiscoveryProvider() + { + var provider = DummyPollingConsulServiceFactory(string.Empty); + var pollProvider = provider as PollConsul; + pollProvider.ShouldNotBeNull(); + pollProvider.Dispose(); + } + + [Fact] + public void should_return_SameProviderForGivenServiceName() + { + var provider = DummyPollingConsulServiceFactory("test"); + var provider2 = DummyPollingConsulServiceFactory("test"); + + provider.ShouldBeEquivalentTo(provider2); + + var pollProvider = provider as PollConsul; + pollProvider.ShouldNotBeNull(); + + var pollProvider2 = provider2 as PollConsul; + pollProvider2.ShouldNotBeNull(); + + pollProvider.ServiceName.ShouldBeEquivalentTo(pollProvider2.ServiceName); + + pollProvider.Dispose(); + pollProvider2.Dispose(); + } + + [Theory] + [InlineData(new object[] { new[] { "service1", "service2", "service3", "service4" } })] + public void should_return_ProviderAccordingToServiceName(string[] serviceNames) + { + var providersList = serviceNames.Select(DummyPollingConsulServiceFactory).ToList(); + + foreach (var serviceName in serviceNames) + { + var currentProvider = DummyPollingConsulServiceFactory(serviceName); + providersList.ShouldContain(currentProvider); + } + + var convertedProvidersList = providersList.Select(x => x as PollConsul).ToList(); + + foreach (var convertedProvider in convertedProvidersList) + { + convertedProvider.ShouldNotBeNull(); + } + + foreach (var serviceName in serviceNames) + { + var cProvider = DummyPollingConsulServiceFactory(serviceName); + var convertedCProvider = cProvider as PollConsul; + + convertedCProvider.ShouldNotBeNull(); + + var matchingProviders = convertedProvidersList.Where(x => x.ServiceName == convertedCProvider.ServiceName) + .ToList(); + matchingProviders.ShouldHaveSingleItem(); + + matchingProviders.First().ShouldNotBeNull(); + matchingProviders.First().ServiceName.ShouldBeEquivalentTo(convertedCProvider.ServiceName); + } + + foreach (var convertedProvider in convertedProvidersList) + { + convertedProvider.Dispose(); + } + } + + private IServiceDiscoveryProvider DummyPollingConsulServiceFactory(string serviceName) + { + var stopsFromPolling = 10000; + + var route = new DownstreamRouteBuilder() + .WithServiceName(serviceName) + .Build(); + + return ConsulProviderFactory.Get(_provider, + new ServiceProviderConfiguration("pollconsul", "http", string.Empty, 1, string.Empty, string.Empty, + stopsFromPolling), route); + } } From 76a1ded0714be0aef11a5dcd4559467aaef2f458 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi Date: Mon, 26 Jun 2023 20:04:08 +0200 Subject: [PATCH 04/23] Using a lock instead of SemaphoreSlim --- .../ConsulProviderFactory.cs | 2 -- src/Ocelot.Provider.Consul/PollConsul.cs | 25 ++++++------------- ...lingConsulServiceDiscoveryProviderTests.cs | 4 +-- .../Consul/ProviderFactoryTests.cs | 9 ------- 4 files changed, 9 insertions(+), 31 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 382527606..4c7d406e8 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -12,9 +12,7 @@ public static class ConsulProviderFactory private static readonly List ServiceDiscoveryProviders = new(); private static readonly object LockObject = new(); -#pragma warning disable CA2211 // Non-constant fields should not be visible public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => -#pragma warning restore CA2211 // Non-constant fields should not be visible { var factory = provider.GetService(); var consulFactory = provider.GetService(); diff --git a/src/Ocelot.Provider.Consul/PollConsul.cs b/src/Ocelot.Provider.Consul/PollConsul.cs index 0db469383..d42edbd9d 100644 --- a/src/Ocelot.Provider.Consul/PollConsul.cs +++ b/src/Ocelot.Provider.Consul/PollConsul.cs @@ -3,22 +3,21 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading; using System.Threading.Tasks; using Ocelot.Values; namespace Ocelot.Provider.Consul { - public sealed class PollConsul : IServiceDiscoveryProvider, IDisposable + public sealed class PollConsul : IServiceDiscoveryProvider { private readonly IOcelotLogger _logger; private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; private readonly int _pollingInterval; private DateTime _lastUpdateTime; + private readonly object _lockObject = new(); private List _services; - private readonly SemaphoreSlim _semaphore = new(1, 1); public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) { @@ -40,34 +39,24 @@ public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory /// retrieving the services and then starting the timer. /// /// the service list. - public async Task> Get() + public Task> Get() { - await _semaphore.WaitAsync(); - try + lock (_lockObject) { var refreshTime = _lastUpdateTime.AddMilliseconds(_pollingInterval); //checking if any services available if (refreshTime >= DateTime.UtcNow && _services.Any()) { - return _services; + return Task.FromResult(_services); } _logger.LogInformation($"Retrieving new client information for service: {ServiceName}"); - _services = await _consulServiceDiscoveryProvider.Get(); + _services = _consulServiceDiscoveryProvider.Get().Result; _lastUpdateTime = DateTime.UtcNow; - return _services; + return Task.FromResult(_services); } - finally - { - _semaphore.Release(); - } - } - - public void Dispose() - { - _semaphore.Dispose(); } } } diff --git a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs index 1065d3cbb..8eaf2d028 100644 --- a/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Consul/PollingConsulServiceDiscoveryProviderTests.cs @@ -66,7 +66,7 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices(int expected) { - using var provider = new PollConsul(_delay, "test", _factory.Object, _consulServiceDiscoveryProvider.Object); + var provider = new PollConsul(_delay, "test", _factory.Object, _consulServiceDiscoveryProvider.Object); var result = Wait.WaitFor(3000).Until(() => { try @@ -85,7 +85,7 @@ private void WhenIGetTheServices(int expected) private void WhenIGetTheServicesWithoutDelay(int expected) { - using var provider = new PollConsul(_delay, "test2", _factory.Object, _consulServiceDiscoveryProvider.Object); + var provider = new PollConsul(_delay, "test2", _factory.Object, _consulServiceDiscoveryProvider.Object); bool result; try { diff --git a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs index 30e3371e8..f4265b5b7 100644 --- a/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs @@ -48,7 +48,6 @@ public void should_return_PollingConsulServiceDiscoveryProvider() var provider = DummyPollingConsulServiceFactory(string.Empty); var pollProvider = provider as PollConsul; pollProvider.ShouldNotBeNull(); - pollProvider.Dispose(); } [Fact] @@ -66,9 +65,6 @@ public void should_return_SameProviderForGivenServiceName() pollProvider2.ShouldNotBeNull(); pollProvider.ServiceName.ShouldBeEquivalentTo(pollProvider2.ServiceName); - - pollProvider.Dispose(); - pollProvider2.Dispose(); } [Theory] @@ -104,11 +100,6 @@ public void should_return_ProviderAccordingToServiceName(string[] serviceNames) matchingProviders.First().ShouldNotBeNull(); matchingProviders.First().ServiceName.ShouldBeEquivalentTo(convertedCProvider.ServiceName); } - - foreach (var convertedProvider in convertedProvidersList) - { - convertedProvider.Dispose(); - } } private IServiceDiscoveryProvider DummyPollingConsulServiceFactory(string serviceName) From 410a2efe181e34e124920bff2866d7d20534a310 Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 27 Jun 2023 17:50:53 +0300 Subject: [PATCH 05/23] Improve code readability --- .../ConsulProviderFactory.cs | 69 ++++++++-------- src/Ocelot.Provider.Consul/PollConsul.cs | 82 +++++++++---------- 2 files changed, 77 insertions(+), 74 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 4c7d406e8..600037bfe 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -1,9 +1,11 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; +using Ocelot.Configuration; using Ocelot.Logging; using Ocelot.ServiceDiscovery; +using Ocelot.ServiceDiscovery.Providers; +using System; +using System.Collections.Generic; +using System.Linq; namespace Ocelot.Provider.Consul; @@ -11,36 +13,37 @@ public static class ConsulProviderFactory { private static readonly List ServiceDiscoveryProviders = new(); private static readonly object LockObject = new(); + + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; - public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => + private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { - var factory = provider.GetService(); + var factory = provider.GetService(); var consulFactory = provider.GetService(); - var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Scheme, config.Host, config.Port, - route.ServiceName, config.Token); - var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - - if (string.Compare(config.Type, "PollConsul", StringComparison.OrdinalIgnoreCase) != 0) - { - return consulServiceDiscoveryProvider; - } - - lock (LockObject) - { - var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); - - if (discoveryProvider != null) - { - return discoveryProvider; - } - - discoveryProvider = new PollConsul( - config.PollingInterval, route.ServiceName, factory, - consulServiceDiscoveryProvider); - - ServiceDiscoveryProviders.Add(discoveryProvider); - - return discoveryProvider; - } - }; + + var consulRegistryConfiguration = new ConsulRegistryConfiguration( + config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); + + var consulProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); + + if ("PollConsul".Equals(config.Type, StringComparison.OrdinalIgnoreCase)) + { + lock (LockObject) + { + var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); + if (discoveryProvider != null) + { + return discoveryProvider; + } + + discoveryProvider = new PollConsul( + config.PollingInterval, route.ServiceName, factory, consulProvider); + ServiceDiscoveryProviders.Add(discoveryProvider); + + return discoveryProvider; + } + } + + return consulProvider; + } } diff --git a/src/Ocelot.Provider.Consul/PollConsul.cs b/src/Ocelot.Provider.Consul/PollConsul.cs index d42edbd9d..5aa71f807 100644 --- a/src/Ocelot.Provider.Consul/PollConsul.cs +++ b/src/Ocelot.Provider.Consul/PollConsul.cs @@ -1,62 +1,62 @@ using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; +using Ocelot.Values; using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Ocelot.Values; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public sealed class PollConsul : IServiceDiscoveryProvider { - public sealed class PollConsul : IServiceDiscoveryProvider - { - private readonly IOcelotLogger _logger; - private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; + private readonly IOcelotLogger _logger; + private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; - private readonly int _pollingInterval; - private DateTime _lastUpdateTime; - private readonly object _lockObject = new(); + private readonly int _pollingInterval; + private readonly object _lockObject = new(); - private List _services; + private DateTime _lastUpdateTime; + private List _services; - public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) - { - _logger = factory.CreateLogger(); - _consulServiceDiscoveryProvider = consulServiceDiscoveryProvider; - _pollingInterval = pollingInterval; - ServiceName = serviceName; - _services = new List(); - - //initializing with update time = DateTime.MinValue - //polling will occur then. - _lastUpdateTime = DateTime.MinValue; - } + public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) + { + _logger = factory.CreateLogger(); + _consulServiceDiscoveryProvider = consulServiceDiscoveryProvider; + _pollingInterval = pollingInterval; - public string ServiceName { get; } + // Initialize by DateTime.MinValue as lowest value. + // Polling will occur immediately during the first call + _lastUpdateTime = DateTime.MinValue; - /// - /// Getting the services, but, if first call, - /// retrieving the services and then starting the timer. - /// - /// the service list. - public Task> Get() - { - lock (_lockObject) - { - var refreshTime = _lastUpdateTime.AddMilliseconds(_pollingInterval); + _services = new List(); + ServiceName = serviceName; + } - //checking if any services available - if (refreshTime >= DateTime.UtcNow && _services.Any()) - { - return Task.FromResult(_services); - } + public string ServiceName { get; } - _logger.LogInformation($"Retrieving new client information for service: {ServiceName}"); - _services = _consulServiceDiscoveryProvider.Get().Result; - _lastUpdateTime = DateTime.UtcNow; + /// + /// Get the services. + /// If the first call, retrieve the services and then start the timer. + /// + /// A with a result of . + public Task> Get() + { + lock (_lockObject) + { + var refreshTime = _lastUpdateTime.AddMilliseconds(_pollingInterval); + // Check if any services available + if (refreshTime >= DateTime.UtcNow && _services.Any()) + { return Task.FromResult(_services); } + + _logger.LogInformation($"Retrieving new client information for service: {ServiceName}."); + _services = _consulServiceDiscoveryProvider.Get().Result; + _lastUpdateTime = DateTime.UtcNow; + + return Task.FromResult(_services); } } } From ed9a49abdbac165e211ad3f76a8acb173683b3ee Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 27 Jun 2023 17:56:16 +0300 Subject: [PATCH 06/23] CA2211: Non-constant fields should not be visible --- .../EurekaProviderFactory.cs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs index a1d4f522a..066c370b5 100644 --- a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs +++ b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs @@ -1,23 +1,25 @@ using Microsoft.Extensions.DependencyInjection; - +using Ocelot.Configuration; using Ocelot.ServiceDiscovery; - +using Ocelot.ServiceDiscovery.Providers; using Steeltoe.Discovery; +using System; + +namespace Ocelot.Provider.Eureka; -namespace Ocelot.Provider.Eureka +public static class EurekaProviderFactory { - public static class EurekaProviderFactory + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; + + private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { - public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => - { - var client = provider.GetService(); + var client = provider.GetService(); - if (config.Type?.ToLower() == "eureka" && client != null) - { - return new Eureka(route.ServiceName, client); - } + if (config.Type?.ToLower() == "eureka" && client != null) + { + return new Eureka(route.ServiceName, client); + } - return null; - }; + return null; } } From d8c4e689f64fb9773b316a47f0af67d30fac57ef Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 27 Jun 2023 20:36:09 +0300 Subject: [PATCH 07/23] Use IOcelotLogger to remove warnings & messages of static code analysis (aka IDE0052) --- .../ServiceDiscoveryProviderFactory.cs | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs index ed04c3532..252dbb82d 100644 --- a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs @@ -1,39 +1,33 @@ -using System; -using System.Collections.Generic; - -using Ocelot.ServiceDiscovery.Configuration; - -using Ocelot.Logging; - using Microsoft.Extensions.DependencyInjection; - using Ocelot.Configuration; - -using Ocelot.ServiceDiscovery.Providers; - +using Ocelot.Logging; using Ocelot.Responses; - +using Ocelot.ServiceDiscovery.Configuration; +using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; +using System; +using System.Collections.Generic; namespace Ocelot.ServiceDiscovery { public class ServiceDiscoveryProviderFactory : IServiceDiscoveryProviderFactory { - private readonly IOcelotLoggerFactory _factory; private readonly ServiceDiscoveryFinderDelegate _delegates; - private readonly IServiceProvider _provider; + private readonly IServiceProvider _provider; + private readonly IOcelotLogger _logger; public ServiceDiscoveryProviderFactory(IOcelotLoggerFactory factory, IServiceProvider provider) { - _factory = factory; _provider = provider; - _delegates = provider.GetService(); + _delegates = provider.GetService(); + _logger = factory.CreateLogger(); } public Response Get(ServiceProviderConfiguration serviceConfig, DownstreamRoute route) { if (route.UseServiceDiscovery) - { + { + _logger.LogInformation($"The {nameof(DownstreamRoute.UseServiceDiscovery)} mode of the route '{route.UpstreamPathTemplate}' is enabled."); return GetServiceDiscoveryProvider(serviceConfig, route); } @@ -50,7 +44,9 @@ public Response Get(ServiceProviderConfiguration serv } private Response GetServiceDiscoveryProvider(ServiceProviderConfiguration config, DownstreamRoute route) - { + { + _logger.LogInformation($"Getting service discovery provider of {nameof(config.Type)} '{config.Type}'..."); + if (config.Type?.ToLower() == "servicefabric") { var sfConfig = new ServiceFabricConfiguration(config.Host, config.Port, route.ServiceName); @@ -66,8 +62,11 @@ private Response GetServiceDiscoveryProvider(ServiceP return new OkResponse(provider); } } - - return new ErrorResponse(new UnableToFindServiceDiscoveryProviderError($"Unable to find service discovery provider for type: {config.Type}")); + + var message = $"Unable to find service discovery provider for {nameof(config.Type)}: '{config.Type}'!"; + _logger.LogWarning(message); + + return new ErrorResponse(new UnableToFindServiceDiscoveryProviderError(message)); } } } From 0018a46b7b6eb9334ad5e3859c159be4158e6ee3 Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 27 Jun 2023 20:39:52 +0300 Subject: [PATCH 08/23] Fix errors with unit tests discovery. Remove legacy life hacks of discovering tests on .NET Core --- .../Ocelot.AcceptanceTests.csproj | 11 +++-------- .../Ocelot.IntegrationTests.csproj | 17 +++++++---------- test/Ocelot.UnitTests/Ocelot.UnitTests.csproj | 19 ++++--------------- 3 files changed, 14 insertions(+), 33 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj index 572cccd4a..87c9d40b3 100644 --- a/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj +++ b/test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj @@ -3,9 +3,9 @@ 0.0.0-dev net7.0 Ocelot.AcceptanceTests - Exe - Ocelot.AcceptanceTests - true + false + true + true osx.10.11-x64;osx.10.12-x64;win7-x64;win10-x64 false false @@ -36,9 +36,6 @@ - - - @@ -70,8 +67,6 @@ - - diff --git a/test/Ocelot.IntegrationTests/Ocelot.IntegrationTests.csproj b/test/Ocelot.IntegrationTests/Ocelot.IntegrationTests.csproj index 3dfcaf800..3be82c9e9 100644 --- a/test/Ocelot.IntegrationTests/Ocelot.IntegrationTests.csproj +++ b/test/Ocelot.IntegrationTests/Ocelot.IntegrationTests.csproj @@ -3,9 +3,9 @@ 0.0.0-dev net7.0 Ocelot.IntegrationTests - Exe - Ocelot.IntegrationTests - true + false + true + true win10-x64;osx.10.11-x64;osx.10.12-x64;win7-x64 false false @@ -14,15 +14,14 @@ True 1591 - + + PreserveNewest - - - - + + @@ -53,8 +52,6 @@ - - diff --git a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj index bee0691fe..477f79f84 100644 --- a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj +++ b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj @@ -4,10 +4,10 @@ 0.0.0-dev net7.0 Ocelot.UnitTests - Ocelot.UnitTests - Exe - true - osx.10.11-x64;osx.10.12-x64;win7-x64;win10-x64 + false + true + true + osx.10.11-x64;osx.10.12-x64;win7-x64;win10-x64 false false false @@ -36,10 +36,6 @@ - - - - PreserveNewest @@ -80,13 +76,6 @@ - - - - - - - all From bfa7d64643931de8939693b2538d5ab132b4bc86 Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 27 Jun 2023 20:41:55 +0300 Subject: [PATCH 09/23] Update unit tests --- .../ServiceDiscoveryProviderFactory.cs | 3 +- .../ServiceDiscoveryProviderFactoryTests.cs | 33 ++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs index 252dbb82d..c58bd1b9f 100644 --- a/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs +++ b/src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs @@ -27,7 +27,8 @@ public Response Get(ServiceProviderConfiguration serv { if (route.UseServiceDiscovery) { - _logger.LogInformation($"The {nameof(DownstreamRoute.UseServiceDiscovery)} mode of the route '{route.UpstreamPathTemplate}' is enabled."); + var routeName = route.UpstreamPathTemplate?.Template ?? route.ServiceName ?? string.Empty; + _logger.LogInformation($"The {nameof(DownstreamRoute.UseServiceDiscovery)} mode of the route '{routeName}' is enabled."); return GetServiceDiscoveryProvider(serviceConfig, route); } diff --git a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs index e414019f6..4f6c85054 100644 --- a/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs +++ b/test/Ocelot.UnitTests/ServiceDiscovery/ServiceDiscoveryProviderFactoryTests.cs @@ -31,7 +31,7 @@ public class ServiceDiscoveryProviderFactoryTests private ServiceDiscoveryProviderFactory _factory; private DownstreamRoute _route; private readonly Mock _loggerFactory; - private Mock _logger; + private readonly Mock _logger; private IServiceProvider _provider; private readonly IServiceCollection _collection; @@ -41,7 +41,10 @@ public ServiceDiscoveryProviderFactoryTests() _logger = new Mock(); _collection = new ServiceCollection(); _provider = _collection.BuildServiceProvider(); - _factory = new ServiceDiscoveryProviderFactory(_loggerFactory.Object, _provider); + _factory = new ServiceDiscoveryProviderFactory(_loggerFactory.Object, _provider); + + _loggerFactory.Setup(x => x.CreateLogger()) + .Returns(_logger.Object); } [Fact] @@ -129,7 +132,8 @@ public void should_return_service_fabric_provider() .WithType("ServiceFabric") .Build(); - this.Given(x => x.GivenTheRoute(serviceConfig, route)) + this.Given(x => x.GivenTheRoute(serviceConfig, route)) + .And(x => GivenAFakeDelegate()) .When(x => x.WhenIGetTheServiceProvider()) .Then(x => x.ThenTheServiceProviderIs()) .BDDfy(); @@ -158,7 +162,18 @@ private void ThenTheDelegateIsCalled() private void ThenTheResultIsError() { - _result.IsError.ShouldBeTrue(); + _result.IsError.ShouldBeTrue(); + _result.Errors.Count.ShouldBe(1); + + _logInformationMessages.ShouldNotBeNull() + .Count.ShouldBe(2); + _logger.Verify(x => x.LogInformation(It.IsAny()), + Times.Exactly(2)); + + _logWarningMessages.ShouldNotBeNull() + .Count.ShouldBe(1); + _logger.Verify(x => x.LogWarning(It.IsAny()), + Times.Once()); } private void ThenTheFollowingServicesAreReturned(List downstreamAddresses) @@ -181,9 +196,17 @@ private void GivenTheRoute(ServiceProviderConfiguration serviceConfig, Downstrea _serviceConfig = serviceConfig; _route = route; } + + private List _logInformationMessages = new(); + private List _logWarningMessages = new(); private void WhenIGetTheServiceProvider() - { + { + _logger.Setup(x => x.LogInformation(It.IsAny())) + .Callback(message => _logInformationMessages.Add(message)); + _logger.Setup(x => x.LogWarning(It.IsAny())) + .Callback(message => _logWarningMessages.Add(message)); + _result = _factory.Get(_serviceConfig, _route); } From 8028783b3db674b7b039550cc9478f9038917af9 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Tue, 27 Jun 2023 20:49:28 +0200 Subject: [PATCH 10/23] Also refactoring the kubernetes provider factory (like consul and eureka) --- .../KubernetesProviderFactory.cs | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 0cfc2a23e..709c8c2eb 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -12,30 +12,28 @@ namespace Ocelot.Provider.Kubernetes { public static class KubernetesProviderFactory { - public static ServiceDiscoveryFinderDelegate Get = (provider, config, route) => - { - var factory = provider.GetService(); - return GetKubeProvider(provider, config, route, factory); - }; + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; + private const string PollKube = "pollkube"; - private static ServiceDiscovery.Providers.IServiceDiscoveryProvider GetKubeProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route, IOcelotLoggerFactory factory) + private static ServiceDiscovery.Providers.IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { + var factory = provider.GetService(); var kubeClient = provider.GetService(); - var k8sRegistryConfiguration = new KubeRegistryConfiguration + var k8SRegistryConfiguration = new KubeRegistryConfiguration { KeyOfServiceInK8s = route.ServiceName, KubeNamespace = string.IsNullOrEmpty(route.ServiceNamespace) ? config.Namespace : route.ServiceNamespace, }; - var k8sServiceDiscoveryProvider = new KubernetesServiceDiscoveryProvider(k8sRegistryConfiguration, factory, kubeClient); + var k8SServiceDiscoveryProvider = new KubernetesServiceDiscoveryProvider(k8SRegistryConfiguration, factory, kubeClient); - if (config.Type?.ToLower() == "pollkube") + if (config.Type?.ToLower() == PollKube) { - return new PollKubernetes(config.PollingInterval, factory, k8sServiceDiscoveryProvider); + return new PollKubernetes(config.PollingInterval, factory, k8SServiceDiscoveryProvider); } - return k8sServiceDiscoveryProvider; + return k8SServiceDiscoveryProvider; } } } From 69651c269be30412b7203ec245f030618ef254ae Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Tue, 27 Jun 2023 20:53:39 +0200 Subject: [PATCH 11/23] shorten references... --- .../KubernetesProviderFactory.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 709c8c2eb..8eac267fd 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -1,12 +1,13 @@ using System; - + using KubeClient; - + using Microsoft.Extensions.DependencyInjection; - + using Ocelot.Configuration; using Ocelot.Logging; using Ocelot.ServiceDiscovery; +using Ocelot.ServiceDiscovery.Providers; namespace Ocelot.Provider.Kubernetes { @@ -15,7 +16,7 @@ public static class KubernetesProviderFactory public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; private const string PollKube = "pollkube"; - private static ServiceDiscovery.Providers.IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) + private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var factory = provider.GetService(); var kubeClient = provider.GetService(); From e05bd929b90d9fb3a75f958456c7c8bed9e1a97d Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Tue, 27 Jun 2023 20:54:53 +0200 Subject: [PATCH 12/23] const before... --- src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 8eac267fd..4c4ca96dd 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -13,9 +13,8 @@ namespace Ocelot.Provider.Kubernetes { public static class KubernetesProviderFactory { + private const string PollKube = "pollkube"; public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; - private const string PollKube = "pollkube"; - private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var factory = provider.GetService(); From 0959ef8739e463fbbf33d3ad4155343ecb0c214e Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Thu, 29 Jun 2023 09:08:28 +0200 Subject: [PATCH 13/23] Some minor fixes, using Equals Ordinal ignore case and a string constant for provider type definition instead of string litterals. Fixing usings. --- .../ConsulProviderFactory.cs | 10 +++++++--- .../EurekaProviderFactory.cs | 6 +++++- .../KubernetesProviderFactory.cs | 19 ++++++++++--------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 600037bfe..18844e828 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -9,8 +9,12 @@ namespace Ocelot.Provider.Consul; -public static class ConsulProviderFactory -{ +public static class ConsulProviderFactory +{ + /// + /// String constant used for provider type definition. + /// + public const string PollConsul = "PollConsul"; private static readonly List ServiceDiscoveryProviders = new(); private static readonly object LockObject = new(); @@ -26,7 +30,7 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide var consulProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - if ("PollConsul".Equals(config.Type, StringComparison.OrdinalIgnoreCase)) + if (PollConsul.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) { lock (LockObject) { diff --git a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs index 066c370b5..a51f46ec9 100644 --- a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs +++ b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs @@ -9,13 +9,17 @@ namespace Ocelot.Provider.Eureka; public static class EurekaProviderFactory { + /// + /// String constant used for provider type definition. + /// + public const string Eureka = "Eureka"; public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var client = provider.GetService(); - if (config.Type?.ToLower() == "eureka" && client != null) + if (Eureka.Equals(config.Type, StringComparison.OrdinalIgnoreCase) && client != null) { return new Eureka(route.ServiceName, client); } diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 4c4ca96dd..c0c9f7a23 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -1,20 +1,21 @@ -using System; - -using KubeClient; - +using KubeClient; using Microsoft.Extensions.DependencyInjection; - using Ocelot.Configuration; using Ocelot.Logging; using Ocelot.ServiceDiscovery; using Ocelot.ServiceDiscovery.Providers; +using System; namespace Ocelot.Provider.Kubernetes { public static class KubernetesProviderFactory - { - private const string PollKube = "pollkube"; - public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; + { + /// + /// String constant used for provider type definition. + /// + public const string PollKube = "PollKube"; + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; + private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) { var factory = provider.GetService(); @@ -28,7 +29,7 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide var k8SServiceDiscoveryProvider = new KubernetesServiceDiscoveryProvider(k8SRegistryConfiguration, factory, kubeClient); - if (config.Type?.ToLower() == PollKube) + if (PollKube.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) { return new PollKubernetes(config.PollingInterval, factory, k8SServiceDiscoveryProvider); } From 944ffd7087c91b1c99f0b804ce48478f8c99e00b Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Sat, 16 Sep 2023 16:24:05 +0200 Subject: [PATCH 14/23] waiting a bit longer then? --- test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs index 9b673be9d..36baa690e 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs @@ -42,7 +42,7 @@ public void should_reload_config_on_change() this.Given(x => _steps.GivenThereIsAConfiguration(_initialConfig)) .And(x => _steps.GivenOcelotIsRunningReloadingConfig(true)) .And(x => _steps.GivenThereIsAConfiguration(_anotherConfig)) - .And(x => _steps.GivenIWait(5000)) + .And(x => _steps.GivenIWait(7500)) .And(x => _steps.ThenConfigShouldBe(_anotherConfig)) .BDDfy(); } From 9f5cbaf8b960336f467ac76d75576005297d7572 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 18 Sep 2023 16:27:30 +0300 Subject: [PATCH 15/23] @RaynaldM code review --- src/Ocelot.Provider.Consul/ConsulProviderFactory.cs | 3 ++- src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs | 3 ++- src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index 18844e828..a0f4b49cb 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -14,7 +14,8 @@ public static class ConsulProviderFactory /// /// String constant used for provider type definition. /// - public const string PollConsul = "PollConsul"; + public const string PollConsul = nameof(Provider.Consul.PollConsul); + private static readonly List ServiceDiscoveryProviders = new(); private static readonly object LockObject = new(); diff --git a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs index a51f46ec9..c45d0b540 100644 --- a/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs +++ b/src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs @@ -12,7 +12,8 @@ public static class EurekaProviderFactory /// /// String constant used for provider type definition. /// - public const string Eureka = "Eureka"; + public const string Eureka = nameof(Provider.Eureka.Eureka); + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index c0c9f7a23..1b3774dd3 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -14,6 +14,7 @@ public static class KubernetesProviderFactory /// String constant used for provider type definition. /// public const string PollKube = "PollKube"; + public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamRoute route) From 29dbba7a8216c165a39773938c2807a5b8512dee Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Mon, 18 Sep 2023 21:20:33 +0200 Subject: [PATCH 16/23] renaming PollKubernetes to PollKube --- .../KubernetesProviderFactory.cs | 4 ++-- .../{PollKubernetes.cs => PollKube.cs} | 15 +++++++-------- .../PollingKubeServiceDiscoveryProviderTests.cs | 6 +++--- 3 files changed, 12 insertions(+), 13 deletions(-) rename src/Ocelot.Provider.Kubernetes/{PollKubernetes.cs => PollKube.cs} (77%) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 1b3774dd3..85fe930fb 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -13,7 +13,7 @@ public static class KubernetesProviderFactory /// /// String constant used for provider type definition. /// - public const string PollKube = "PollKube"; + public const string PollKube = nameof(Provider.Kubernetes.PollKube); public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; @@ -32,7 +32,7 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide if (PollKube.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) { - return new PollKubernetes(config.PollingInterval, factory, k8SServiceDiscoveryProvider); + return new PollKube(config.PollingInterval, factory, k8SServiceDiscoveryProvider); } return k8SServiceDiscoveryProvider; diff --git a/src/Ocelot.Provider.Kubernetes/PollKubernetes.cs b/src/Ocelot.Provider.Kubernetes/PollKube.cs similarity index 77% rename from src/Ocelot.Provider.Kubernetes/PollKubernetes.cs rename to src/Ocelot.Provider.Kubernetes/PollKube.cs index cd0e854f4..31d42a56b 100644 --- a/src/Ocelot.Provider.Kubernetes/PollKubernetes.cs +++ b/src/Ocelot.Provider.Kubernetes/PollKube.cs @@ -1,14 +1,13 @@ -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; - -using Ocelot.Logging; +using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; namespace Ocelot.Provider.Kubernetes { - public class PollKubernetes : IServiceDiscoveryProvider + public class PollKube : IServiceDiscoveryProvider { private readonly IOcelotLogger _logger; private readonly IServiceDiscoveryProvider _kubeServiceDiscoveryProvider; @@ -16,9 +15,9 @@ public class PollKubernetes : IServiceDiscoveryProvider private bool _polling; private List _services; - public PollKubernetes(int pollingInterval, IOcelotLoggerFactory factory, IServiceDiscoveryProvider kubeServiceDiscoveryProvider) + public PollKube(int pollingInterval, IOcelotLoggerFactory factory, IServiceDiscoveryProvider kubeServiceDiscoveryProvider) { - _logger = factory.CreateLogger(); + _logger = factory.CreateLogger(); _kubeServiceDiscoveryProvider = kubeServiceDiscoveryProvider; _services = new List(); diff --git a/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs b/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs index 0d80adc89..09b86bce3 100644 --- a/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs +++ b/test/Ocelot.UnitTests/Kubernetes/PollingKubeServiceDiscoveryProviderTests.cs @@ -20,7 +20,7 @@ namespace Ocelot.UnitTests.Kubernetes public class PollingKubeServiceDiscoveryProviderTests { private readonly int _delay; - private PollKubernetes _provider; + private PollKube _provider; private readonly List _services; private readonly Mock _factory; private readonly Mock _logger; @@ -33,7 +33,7 @@ public PollingKubeServiceDiscoveryProviderTests() _delay = 1; _factory = new Mock(); _logger = new Mock(); - _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); + _factory.Setup(x => x.CreateLogger()).Returns(_logger.Object); _kubeServiceDiscoveryProvider = new Mock(); } @@ -61,7 +61,7 @@ private void ThenTheCountIs(int count) private void WhenIGetTheServices(int expected) { - _provider = new PollKubernetes(_delay, _factory.Object, _kubeServiceDiscoveryProvider.Object); + _provider = new PollKube(_delay, _factory.Object, _kubeServiceDiscoveryProvider.Object); var result = Wait.WaitFor(3000).Until(() => { From b81d33e3876d84d174705293a7e78ea3425a6355 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Mon, 18 Sep 2023 21:27:33 +0200 Subject: [PATCH 17/23] ... odd... --- test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs index 36baa690e..a355be353 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs @@ -83,7 +83,7 @@ public void should_not_trigger_change_token_with_no_change() .BDDfy(); } - private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 1000) - 100; + private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 1200) - 100; public void Dispose() { From ef8992550bdb44596cfb970e4728bdd0ada7ae93 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Mon, 18 Sep 2023 21:41:27 +0200 Subject: [PATCH 18/23] ... very odd, we have an issue with configuration update duration... --- test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs index a355be353..fd2ac262b 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs @@ -42,7 +42,7 @@ public void should_reload_config_on_change() this.Given(x => _steps.GivenThereIsAConfiguration(_initialConfig)) .And(x => _steps.GivenOcelotIsRunningReloadingConfig(true)) .And(x => _steps.GivenThereIsAConfiguration(_anotherConfig)) - .And(x => _steps.GivenIWait(7500)) + .And(x => _steps.GivenIWait(10000)) .And(x => _steps.ThenConfigShouldBe(_anotherConfig)) .BDDfy(); } @@ -83,7 +83,7 @@ public void should_not_trigger_change_token_with_no_change() .BDDfy(); } - private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 1200) - 100; + private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 2000) - 100; public void Dispose() { From d8955718a6f5f6cb427e292a2cbabbefb47ce5ef Mon Sep 17 00:00:00 2001 From: raman-m Date: Tue, 19 Sep 2023 13:49:47 +0300 Subject: [PATCH 19/23] IDE0002: Name can be simplified --- src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs index 85fe930fb..5d9fae016 100644 --- a/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs +++ b/src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs @@ -13,7 +13,7 @@ public static class KubernetesProviderFactory /// /// String constant used for provider type definition. /// - public const string PollKube = nameof(Provider.Kubernetes.PollKube); + public const string PollKube = nameof(Kubernetes.PollKube); public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; From 2161db089c9106c41c1d318eee9d2d8486afae30 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Tue, 19 Sep 2023 14:20:58 +0200 Subject: [PATCH 20/23] All tests passing locally, hopefully it works online --- .../ConfigurationReloadTests.cs | 38 ++++++++----------- .../SequentialTestsCollection.cs | 8 ++++ 2 files changed, 23 insertions(+), 23 deletions(-) create mode 100644 test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs diff --git a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs index fd2ac262b..5b098089d 100644 --- a/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs +++ b/test/Ocelot.AcceptanceTests/ConfigurationReloadTests.cs @@ -9,32 +9,24 @@ namespace Ocelot.AcceptanceTests { + [Collection("Sequential")] public class ConfigurationReloadTests : IDisposable { - private readonly FileConfiguration _initialConfig; - private readonly FileConfiguration _anotherConfig; - private readonly Steps _steps; - - public ConfigurationReloadTests() + private readonly FileConfiguration _initialConfig = new() { - _steps = new Steps(); - - _initialConfig = new FileConfiguration + GlobalConfiguration = new FileGlobalConfiguration { - GlobalConfiguration = new FileGlobalConfiguration - { - RequestIdKey = "initialKey", - }, - }; - - _anotherConfig = new FileConfiguration + RequestIdKey = "initialKey", + }, + }; + private readonly FileConfiguration _anotherConfig = new() + { + GlobalConfiguration = new FileGlobalConfiguration { - GlobalConfiguration = new FileGlobalConfiguration - { - RequestIdKey = "someOtherKey", - }, - }; - } + RequestIdKey = "someOtherKey", + }, + }; + private readonly Steps _steps = new(); [Fact] public void should_reload_config_on_change() @@ -42,7 +34,7 @@ public void should_reload_config_on_change() this.Given(x => _steps.GivenThereIsAConfiguration(_initialConfig)) .And(x => _steps.GivenOcelotIsRunningReloadingConfig(true)) .And(x => _steps.GivenThereIsAConfiguration(_anotherConfig)) - .And(x => _steps.GivenIWait(10000)) + .And(x => _steps.GivenIWait(7500)) .And(x => _steps.ThenConfigShouldBe(_anotherConfig)) .BDDfy(); } @@ -83,7 +75,7 @@ public void should_not_trigger_change_token_with_no_change() .BDDfy(); } - private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 2000) - 100; + private const int MillisecondsToWaitForChangeToken = (int)(OcelotConfigurationChangeToken.PollingIntervalSeconds * 1000) - 100; public void Dispose() { diff --git a/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs b/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs new file mode 100644 index 000000000..2b9c8d474 --- /dev/null +++ b/test/Ocelot.AcceptanceTests/SequentialTestsCollection.cs @@ -0,0 +1,8 @@ +using Xunit; + +namespace Ocelot.AcceptanceTests; + +[CollectionDefinition("Sequential", DisableParallelization = true)] +public class SequentialTestsCollection +{ +} From 24f2091cb814a811df3fc83dc7457d988c29d577 Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Tue, 26 Sep 2023 17:37:55 +0200 Subject: [PATCH 21/23] just a bit of cleanup --- src/Ocelot.Provider.Consul/Consul.cs | 116 +++++++++--------- .../ConsulClientFactory.cs | 20 ++- .../ConsulFileConfigurationRepository.cs | 109 ++++++++-------- .../ConsulMiddlewareConfigurationProvider.cs | 114 ++++++++--------- .../ConsulProviderFactory.cs | 53 ++++---- .../ConsulRegistryConfiguration.cs | 31 +++-- .../IConsulClientFactory.cs | 9 +- .../OcelotBuilderExtensions.cs | 33 +++-- src/Ocelot.Provider.Consul/PollConsul.cs | 17 ++- .../UnableToSetConfigInConsulError.cs | 11 +- src/Ocelot.Provider.Consul/Usings.cs | 5 +- 11 files changed, 238 insertions(+), 280 deletions(-) diff --git a/src/Ocelot.Provider.Consul/Consul.cs b/src/Ocelot.Provider.Consul/Consul.cs index 138aac52b..ecb61b2ab 100644 --- a/src/Ocelot.Provider.Consul/Consul.cs +++ b/src/Ocelot.Provider.Consul/Consul.cs @@ -1,82 +1,78 @@ -using Consul; -using Ocelot.Infrastructure.Extensions; +using Ocelot.Infrastructure.Extensions; using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; using Ocelot.Values; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public class Consul : IServiceDiscoveryProvider { - public class Consul : IServiceDiscoveryProvider - { - private readonly ConsulRegistryConfiguration _config; - private readonly IOcelotLogger _logger; - private readonly IConsulClient _consul; - private const string VersionPrefix = "version-"; + private const string VersionPrefix = "version-"; + private readonly ConsulRegistryConfiguration _config; + private readonly IConsulClient _consul; + private readonly IOcelotLogger _logger; - public Consul(ConsulRegistryConfiguration config, IOcelotLoggerFactory factory, IConsulClientFactory clientFactory) - { - _logger = factory.CreateLogger(); - _config = config; - _consul = clientFactory.Get(_config); - } + public Consul(ConsulRegistryConfiguration config, IOcelotLoggerFactory factory, IConsulClientFactory clientFactory) + { + _logger = factory.CreateLogger(); + _config = config; + _consul = clientFactory.Get(_config); + } - public async Task> Get() - { - var queryResult = await _consul.Health.Service(_config.KeyOfServiceInConsul, string.Empty, true); + public async Task> Get() + { + var queryResult = await _consul.Health.Service(_config.KeyOfServiceInConsul, string.Empty, true); - var services = new List(); + var services = new List(); - foreach (var serviceEntry in queryResult.Response) + foreach (var serviceEntry in queryResult.Response) + { + if (IsValid(serviceEntry)) { - if (IsValid(serviceEntry)) + var nodes = await _consul.Catalog.Nodes(); + if (nodes.Response == null) { - var nodes = await _consul.Catalog.Nodes(); - if (nodes.Response == null) - { - services.Add(BuildService(serviceEntry, null)); - } - else - { - var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == serviceEntry.Service.Address); - services.Add(BuildService(serviceEntry, serviceNode)); - } + services.Add(BuildService(serviceEntry, null)); } else { - _logger.LogWarning($"Unable to use service Address: {serviceEntry.Service.Address} and Port: {serviceEntry.Service.Port} as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"); + var serviceNode = nodes.Response.FirstOrDefault(n => n.Address == serviceEntry.Service.Address); + services.Add(BuildService(serviceEntry, serviceNode)); } } - - return services.ToList(); + else + { + _logger.LogWarning( + $"Unable to use service Address: {serviceEntry.Service.Address} and Port: {serviceEntry.Service.Port} as it is invalid. Address must contain host only e.g. localhost and port must be greater than 0"); + } } - private static Service BuildService(ServiceEntry serviceEntry, Node serviceNode) - { - return new Service( - serviceEntry.Service.Service, - new ServiceHostAndPort(serviceNode == null ? serviceEntry.Service.Address : serviceNode.Name, serviceEntry.Service.Port), - serviceEntry.Service.ID, - GetVersionFromStrings(serviceEntry.Service.Tags), - serviceEntry.Service.Tags ?? Enumerable.Empty()); - } + return services.ToList(); + } - private static bool IsValid(ServiceEntry serviceEntry) - { - return !string.IsNullOrEmpty(serviceEntry.Service.Address) - && !serviceEntry.Service.Address.Contains("http://") - && !serviceEntry.Service.Address.Contains("https://") - && serviceEntry.Service.Port > 0; - } + private static Service BuildService(ServiceEntry serviceEntry, Node serviceNode) + { + return new Service( + serviceEntry.Service.Service, + new ServiceHostAndPort(serviceNode == null ? serviceEntry.Service.Address : serviceNode.Name, + serviceEntry.Service.Port), + serviceEntry.Service.ID, + GetVersionFromStrings(serviceEntry.Service.Tags), + serviceEntry.Service.Tags ?? Enumerable.Empty()); + } - private static string GetVersionFromStrings(IEnumerable strings) - { - return strings - ?.FirstOrDefault(x => x.StartsWith(VersionPrefix, StringComparison.Ordinal)) - .TrimStart(VersionPrefix); - } + private static bool IsValid(ServiceEntry serviceEntry) + { + return !string.IsNullOrEmpty(serviceEntry.Service.Address) + && !serviceEntry.Service.Address.Contains("http://") + && !serviceEntry.Service.Address.Contains("https://") + && serviceEntry.Service.Port > 0; + } + + private static string GetVersionFromStrings(IEnumerable strings) + { + return strings + ?.FirstOrDefault(x => x.StartsWith(VersionPrefix, StringComparison.Ordinal)) + .TrimStart(VersionPrefix); } } diff --git a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs index 583d81d78..5f5009d9b 100644 --- a/src/Ocelot.Provider.Consul/ConsulClientFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulClientFactory.cs @@ -1,20 +1,14 @@ -using Consul; +namespace Ocelot.Provider.Consul; -namespace Ocelot.Provider.Consul +public class ConsulClientFactory : IConsulClientFactory { - public class ConsulClientFactory : IConsulClientFactory + public IConsulClient Get(ConsulRegistryConfiguration config) { - public IConsulClient Get(ConsulRegistryConfiguration config) + return new ConsulClient(c => { - return new ConsulClient(c => - { - c.Address = new Uri($"{config.Scheme}://{config.Host}:{config.Port}"); + c.Address = new Uri($"{config.Scheme}://{config.Host}:{config.Port}"); - if (!string.IsNullOrEmpty(config?.Token)) - { - c.Token = config.Token; - } - }); - } + if (!string.IsNullOrEmpty(config?.Token)) c.Token = config.Token; + }); } } diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index 353b217ae..8eea73f25 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -1,86 +1,81 @@ -using Consul; +using System.Text; using Microsoft.Extensions.Options; using Newtonsoft.Json; +using Ocelot.Cache; using Ocelot.Configuration.File; using Ocelot.Configuration.Repository; using Ocelot.Logging; using Ocelot.Responses; -using System.Text; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public class ConsulFileConfigurationRepository : IFileConfigurationRepository { - public class ConsulFileConfigurationRepository : IFileConfigurationRepository + private readonly IOcelotCache _cache; + private readonly string _configurationKey; + private readonly IConsulClient _consul; + private readonly IOcelotLogger _logger; + + public ConsulFileConfigurationRepository( + IOptions fileConfiguration, + IOcelotCache cache, + IConsulClientFactory factory, + IOcelotLoggerFactory loggerFactory) { - private readonly IConsulClient _consul; - private readonly string _configurationKey; - private readonly Cache.IOcelotCache _cache; - private readonly IOcelotLogger _logger; - - public ConsulFileConfigurationRepository( - IOptions fileConfiguration, - Cache.IOcelotCache cache, - IConsulClientFactory factory, - IOcelotLoggerFactory loggerFactory) - { - _logger = loggerFactory.CreateLogger(); - _cache = cache; + _logger = loggerFactory.CreateLogger(); + _cache = cache; - var serviceDiscoveryProvider = fileConfiguration.Value.GlobalConfiguration.ServiceDiscoveryProvider; - _configurationKey = string.IsNullOrWhiteSpace(serviceDiscoveryProvider.ConfigurationKey) ? "InternalConfiguration" : - serviceDiscoveryProvider.ConfigurationKey; + var serviceDiscoveryProvider = fileConfiguration.Value.GlobalConfiguration.ServiceDiscoveryProvider; + _configurationKey = string.IsNullOrWhiteSpace(serviceDiscoveryProvider.ConfigurationKey) + ? "InternalConfiguration" + : serviceDiscoveryProvider.ConfigurationKey; - var config = new ConsulRegistryConfiguration(serviceDiscoveryProvider.Scheme, serviceDiscoveryProvider.Host, - serviceDiscoveryProvider.Port, _configurationKey, serviceDiscoveryProvider.Token); - - _consul = factory.Get(config); - } + var config = new ConsulRegistryConfiguration(serviceDiscoveryProvider.Scheme, serviceDiscoveryProvider.Host, + serviceDiscoveryProvider.Port, _configurationKey, serviceDiscoveryProvider.Token); - public async Task> Get() - { - var config = _cache.Get(_configurationKey, _configurationKey); + _consul = factory.Get(config); + } - if (config != null) - { - return new OkResponse(config); - } + public async Task> Get() + { + var config = _cache.Get(_configurationKey, _configurationKey); - var queryResult = await _consul.KV.Get(_configurationKey); + if (config != null) return new OkResponse(config); - if (queryResult.Response == null) - { - return new OkResponse(null); - } + var queryResult = await _consul.KV.Get(_configurationKey); - var bytes = queryResult.Response.Value; + if (queryResult.Response == null) return new OkResponse(null); - var json = Encoding.UTF8.GetString(bytes); + var bytes = queryResult.Response.Value; - var consulConfig = JsonConvert.DeserializeObject(json); + var json = Encoding.UTF8.GetString(bytes); - return new OkResponse(consulConfig); - } + var consulConfig = JsonConvert.DeserializeObject(json); - public async Task Set(FileConfiguration ocelotConfiguration) - { - var json = JsonConvert.SerializeObject(ocelotConfiguration, Formatting.Indented); + return new OkResponse(consulConfig); + } - var bytes = Encoding.UTF8.GetBytes(json); + public async Task Set(FileConfiguration ocelotConfiguration) + { + var json = JsonConvert.SerializeObject(ocelotConfiguration, Formatting.Indented); - var kvPair = new KVPair(_configurationKey) - { - Value = bytes, - }; + var bytes = Encoding.UTF8.GetBytes(json); - var result = await _consul.KV.Put(kvPair); + var kvPair = new KVPair(_configurationKey) + { + Value = bytes + }; - if (result.Response) - { - _cache.AddAndDelete(_configurationKey, ocelotConfiguration, TimeSpan.FromSeconds(3), _configurationKey); + var result = await _consul.KV.Put(kvPair); - return new OkResponse(); - } + if (result.Response) + { + _cache.AddAndDelete(_configurationKey, ocelotConfiguration, TimeSpan.FromSeconds(3), _configurationKey); - return new ErrorResponse(new UnableToSetConfigInConsulError($"Unable to set FileConfiguration in consul, response status code from consul was {result.StatusCode}")); + return new OkResponse(); } + + return new ErrorResponse(new UnableToSetConfigInConsulError( + $"Unable to set FileConfiguration in consul, response status code from consul was {result.StatusCode}")); } } diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index b7cb22b88..924dd3dfe 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -7,84 +7,76 @@ using Ocelot.Middleware; using Ocelot.Responses; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public static class ConsulMiddlewareConfigurationProvider { - public static class ConsulMiddlewareConfigurationProvider + public static OcelotMiddlewareConfigurationDelegate Get = async builder => { - public static OcelotMiddlewareConfigurationDelegate Get = async builder => - { - var fileConfigRepo = builder.ApplicationServices.GetService(); - var fileConfig = builder.ApplicationServices.GetService>(); - var internalConfigCreator = builder.ApplicationServices.GetService(); - var internalConfigRepo = builder.ApplicationServices.GetService(); + var fileConfigRepo = builder.ApplicationServices.GetService(); + var fileConfig = builder.ApplicationServices.GetService>(); + var internalConfigCreator = builder.ApplicationServices.GetService(); + var internalConfigRepo = builder.ApplicationServices.GetService(); - if (UsingConsul(fileConfigRepo)) - { - await SetFileConfigInConsul(builder, fileConfigRepo, fileConfig, internalConfigCreator, internalConfigRepo); - } - }; + if (UsingConsul(fileConfigRepo)) + await SetFileConfigInConsul(builder, fileConfigRepo, fileConfig, internalConfigCreator, internalConfigRepo); + }; - private static bool UsingConsul(IFileConfigurationRepository fileConfigRepo) + private static bool UsingConsul(IFileConfigurationRepository fileConfigRepo) + { + return fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository); + } + + private static async Task SetFileConfigInConsul(IApplicationBuilder builder, + IFileConfigurationRepository fileConfigRepo, IOptionsMonitor fileConfig, + IInternalConfigurationCreator internalConfigCreator, IInternalConfigurationRepository internalConfigRepo) + { + // get the config from consul. + var fileConfigFromConsul = await fileConfigRepo.Get(); + + if (IsError(fileConfigFromConsul)) { - return fileConfigRepo.GetType() == typeof(ConsulFileConfigurationRepository); + ThrowToStopOcelotStarting(fileConfigFromConsul); } - - private static async Task SetFileConfigInConsul(IApplicationBuilder builder, - IFileConfigurationRepository fileConfigRepo, IOptionsMonitor fileConfig, - IInternalConfigurationCreator internalConfigCreator, IInternalConfigurationRepository internalConfigRepo) + else if (ConfigNotStoredInConsul(fileConfigFromConsul)) + { + //there was no config in consul set the file in config in consul + await fileConfigRepo.Set(fileConfig.CurrentValue); + } + else { - // get the config from consul. - var fileConfigFromConsul = await fileConfigRepo.Get(); + // create the internal config from consul data + var internalConfig = await internalConfigCreator.Create(fileConfigFromConsul.Data); - if (IsError(fileConfigFromConsul)) + if (IsError(internalConfig)) { - ThrowToStopOcelotStarting(fileConfigFromConsul); - } - else if (ConfigNotStoredInConsul(fileConfigFromConsul)) - { - //there was no config in consul set the file in config in consul - await fileConfigRepo.Set(fileConfig.CurrentValue); + ThrowToStopOcelotStarting(internalConfig); } else { - // create the internal config from consul data - var internalConfig = await internalConfigCreator.Create(fileConfigFromConsul.Data); - - if (IsError(internalConfig)) - { - ThrowToStopOcelotStarting(internalConfig); - } - else - { - // add the internal config to the internal repo - var response = internalConfigRepo.AddOrReplace(internalConfig.Data); + // add the internal config to the internal repo + var response = internalConfigRepo.AddOrReplace(internalConfig.Data); - if (IsError(response)) - { - ThrowToStopOcelotStarting(response); - } - } - - if (IsError(internalConfig)) - { - ThrowToStopOcelotStarting(internalConfig); - } + if (IsError(response)) ThrowToStopOcelotStarting(response); } - } - private static void ThrowToStopOcelotStarting(Response config) - { - throw new Exception($"Unable to start Ocelot, errors are: {string.Join(',', config.Errors.Select(x => x.ToString()))}"); + if (IsError(internalConfig)) ThrowToStopOcelotStarting(internalConfig); } + } - private static bool IsError(Response response) - { - return response == null || response.IsError; - } + private static void ThrowToStopOcelotStarting(Response config) + { + throw new Exception( + $"Unable to start Ocelot, errors are: {string.Join(',', config.Errors.Select(x => x.ToString()))}"); + } - private static bool ConfigNotStoredInConsul(Response fileConfigFromConsul) - { - return fileConfigFromConsul.Data == null; - } + private static bool IsError(Response response) + { + return response == null || response.IsError; + } + + private static bool ConfigNotStoredInConsul(Response fileConfigFromConsul) + { + return fileConfigFromConsul.Data == null; } } diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index a0f4b49cb..afa231dd3 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -1,45 +1,39 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using Ocelot.Configuration; -using Ocelot.Logging; -using Ocelot.ServiceDiscovery; +using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; -using System; -using System.Collections.Generic; -using System.Linq; - -namespace Ocelot.Provider.Consul; - + +namespace Ocelot.Provider.Consul; + public static class ConsulProviderFactory { /// - /// String constant used for provider type definition. + /// String constant used for provider type definition. /// - public const string PollConsul = nameof(Provider.Consul.PollConsul); + 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(); + private static readonly object LockObject = new(); public static ServiceDiscoveryFinderDelegate Get { get; } = CreateProvider; - - 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 consulFactory = provider.GetService(); var consulRegistryConfiguration = new ConsulRegistryConfiguration( - config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); + config.Scheme, config.Host, config.Port, route.ServiceName, config.Token); + + var consulProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - var consulProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); - if (PollConsul.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) - { lock (LockObject) { - var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); - if (discoveryProvider != null) - { - return discoveryProvider; - } + var discoveryProvider = + ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); + if (discoveryProvider != null) return discoveryProvider; discoveryProvider = new PollConsul( config.PollingInterval, route.ServiceName, factory, consulProvider); @@ -47,8 +41,7 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide return discoveryProvider; } - } - return consulProvider; - } -} + return consulProvider; + } +} diff --git a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs index 38bf515fa..52fba7893 100644 --- a/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs +++ b/src/Ocelot.Provider.Consul/ConsulRegistryConfiguration.cs @@ -1,20 +1,19 @@ -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public class ConsulRegistryConfiguration { - public class ConsulRegistryConfiguration + public ConsulRegistryConfiguration(string scheme, string host, int port, string keyOfServiceInConsul, string token) { - public ConsulRegistryConfiguration(string scheme, string host, int port, string keyOfServiceInConsul, string token) - { - Host = string.IsNullOrEmpty(host) ? "localhost" : host; - Port = port > 0 ? port : 8500; - Scheme = string.IsNullOrEmpty(scheme) ? "http" : scheme; - KeyOfServiceInConsul = keyOfServiceInConsul; - Token = token; - } - - public string KeyOfServiceInConsul { get; } - public string Scheme { get; } - public string Host { get; } - public int Port { get; } - public string Token { get; } + Host = string.IsNullOrEmpty(host) ? "localhost" : host; + Port = port > 0 ? port : 8500; + Scheme = string.IsNullOrEmpty(scheme) ? "http" : scheme; + KeyOfServiceInConsul = keyOfServiceInConsul; + Token = token; } + + public string KeyOfServiceInConsul { get; } + public string Scheme { get; } + public string Host { get; } + public int Port { get; } + public string Token { get; } } diff --git a/src/Ocelot.Provider.Consul/IConsulClientFactory.cs b/src/Ocelot.Provider.Consul/IConsulClientFactory.cs index 2f9f3058e..3ee3a2b25 100644 --- a/src/Ocelot.Provider.Consul/IConsulClientFactory.cs +++ b/src/Ocelot.Provider.Consul/IConsulClientFactory.cs @@ -1,9 +1,6 @@ -using Consul; +namespace Ocelot.Provider.Consul; -namespace Ocelot.Provider.Consul +public interface IConsulClientFactory { - public interface IConsulClientFactory - { - IConsulClient Get(ConsulRegistryConfiguration config); - } + IConsulClient Get(ConsulRegistryConfiguration config); } diff --git a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs index cc3ec264b..48bce6f6e 100644 --- a/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs +++ b/src/Ocelot.Provider.Consul/OcelotBuilderExtensions.cs @@ -3,25 +3,24 @@ using Ocelot.Configuration.Repository; using Ocelot.DependencyInjection; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public static class OcelotBuilderExtensions { - public static class OcelotBuilderExtensions + public static IOcelotBuilder AddConsul(this IOcelotBuilder builder) { - public static IOcelotBuilder AddConsul(this IOcelotBuilder builder) - { - builder.Services.AddSingleton(ConsulProviderFactory.Get); - builder.Services.AddSingleton(); - builder.Services.RemoveAll(typeof(IFileConfigurationPollerOptions)); - builder.Services.AddSingleton(); - return builder; - } + builder.Services.AddSingleton(ConsulProviderFactory.Get); + builder.Services.AddSingleton(); + builder.Services.RemoveAll(typeof(IFileConfigurationPollerOptions)); + builder.Services.AddSingleton(); + return builder; + } - public static IOcelotBuilder AddConfigStoredInConsul(this IOcelotBuilder builder) - { - builder.Services.AddSingleton(ConsulMiddlewareConfigurationProvider.Get); - builder.Services.AddHostedService(); - builder.Services.AddSingleton(); - return builder; - } + public static IOcelotBuilder AddConfigStoredInConsul(this IOcelotBuilder builder) + { + builder.Services.AddSingleton(ConsulMiddlewareConfigurationProvider.Get); + builder.Services.AddHostedService(); + builder.Services.AddSingleton(); + return builder; } } diff --git a/src/Ocelot.Provider.Consul/PollConsul.cs b/src/Ocelot.Provider.Consul/PollConsul.cs index 5aa71f807..295a17e15 100644 --- a/src/Ocelot.Provider.Consul/PollConsul.cs +++ b/src/Ocelot.Provider.Consul/PollConsul.cs @@ -1,25 +1,22 @@ using Ocelot.Logging; using Ocelot.ServiceDiscovery.Providers; using Ocelot.Values; -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; namespace Ocelot.Provider.Consul; public sealed class PollConsul : IServiceDiscoveryProvider { - private readonly IOcelotLogger _logger; private readonly IServiceDiscoveryProvider _consulServiceDiscoveryProvider; + private readonly object _lockObject = new(); + private readonly IOcelotLogger _logger; private readonly int _pollingInterval; - private readonly object _lockObject = new(); private DateTime _lastUpdateTime; private List _services; - public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, IServiceDiscoveryProvider consulServiceDiscoveryProvider) + public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory factory, + IServiceDiscoveryProvider consulServiceDiscoveryProvider) { _logger = factory.CreateLogger(); _consulServiceDiscoveryProvider = consulServiceDiscoveryProvider; @@ -36,10 +33,10 @@ public PollConsul(int pollingInterval, string serviceName, IOcelotLoggerFactory public string ServiceName { get; } /// - /// Get the services. - /// If the first call, retrieve the services and then start the timer. + /// Get the services. + /// If the first call, retrieve the services and then start the timer. /// - /// A with a result of . + /// A with a result of . public Task> Get() { lock (_lockObject) diff --git a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs index abf763735..0e6481aad 100644 --- a/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs +++ b/src/Ocelot.Provider.Consul/UnableToSetConfigInConsulError.cs @@ -1,12 +1,11 @@ using Ocelot.Errors; -namespace Ocelot.Provider.Consul +namespace Ocelot.Provider.Consul; + +public class UnableToSetConfigInConsulError : Error { - public class UnableToSetConfigInConsulError : Error + public UnableToSetConfigInConsulError(string s) + : base(s, OcelotErrorCode.UnknownError, 404) { - public UnableToSetConfigInConsulError(string s) - : base(s, OcelotErrorCode.UnknownError, 404) - { - } } } diff --git a/src/Ocelot.Provider.Consul/Usings.cs b/src/Ocelot.Provider.Consul/Usings.cs index 8dee5b38a..777ae303e 100644 --- a/src/Ocelot.Provider.Consul/Usings.cs +++ b/src/Ocelot.Provider.Consul/Usings.cs @@ -1,13 +1,10 @@ // Default Microsoft.NET.Sdk namespaces + global using System; global using System.Collections.Generic; -global using System.IO; global using System.Linq; -global using System.Net.Http; -global using System.Threading; global using System.Threading.Tasks; // Project extra global namespaces global using Consul; -global using Ocelot; global using Ocelot.ServiceDiscovery; From 77bbaf04020c99e52036cf3f2b78daa9ae25504c Mon Sep 17 00:00:00 2001 From: Guillaume Gnaegi <58469901+ggnaegi@users.noreply.github.com> Date: Fri, 29 Sep 2023 13:52:35 +0200 Subject: [PATCH 22/23] Some missing braces and commas --- .../ConsulFileConfigurationRepository.cs | 12 +++++++++--- .../ConsulMiddlewareConfigurationProvider.cs | 12 ++++++++++-- src/Ocelot.Provider.Consul/ConsulProviderFactory.cs | 7 ++++++- src/Ocelot.Provider.Eureka/Eureka.cs | 3 +-- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs index 8eea73f25..7ac7612aa 100644 --- a/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs +++ b/src/Ocelot.Provider.Consul/ConsulFileConfigurationRepository.cs @@ -40,11 +40,17 @@ public async Task> Get() { var config = _cache.Get(_configurationKey, _configurationKey); - if (config != null) return new OkResponse(config); + if (config != null) + { + return new OkResponse(config); + } var queryResult = await _consul.KV.Get(_configurationKey); - if (queryResult.Response == null) return new OkResponse(null); + if (queryResult.Response == null) + { + return new OkResponse(null); + } var bytes = queryResult.Response.Value; @@ -63,7 +69,7 @@ public async Task Set(FileConfiguration ocelotConfiguration) var kvPair = new KVPair(_configurationKey) { - Value = bytes + Value = bytes, }; var result = await _consul.KV.Put(kvPair); diff --git a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs index 924dd3dfe..f6b644199 100644 --- a/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs +++ b/src/Ocelot.Provider.Consul/ConsulMiddlewareConfigurationProvider.cs @@ -19,7 +19,9 @@ public static class ConsulMiddlewareConfigurationProvider var internalConfigRepo = builder.ApplicationServices.GetService(); if (UsingConsul(fileConfigRepo)) + { await SetFileConfigInConsul(builder, fileConfigRepo, fileConfig, internalConfigCreator, internalConfigRepo); + } }; private static bool UsingConsul(IFileConfigurationRepository fileConfigRepo) @@ -57,10 +59,16 @@ private static async Task SetFileConfigInConsul(IApplicationBuilder builder, // add the internal config to the internal repo var response = internalConfigRepo.AddOrReplace(internalConfig.Data); - if (IsError(response)) ThrowToStopOcelotStarting(response); + if (IsError(response)) + { + ThrowToStopOcelotStarting(response); + } } - if (IsError(internalConfig)) ThrowToStopOcelotStarting(internalConfig); + if (IsError(internalConfig)) + { + ThrowToStopOcelotStarting(internalConfig); + } } } diff --git a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs index afa231dd3..724c6fb9d 100644 --- a/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs +++ b/src/Ocelot.Provider.Consul/ConsulProviderFactory.cs @@ -29,11 +29,15 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide var consulProvider = new Consul(consulRegistryConfiguration, factory, consulFactory); if (PollConsul.Equals(config.Type, StringComparison.OrdinalIgnoreCase)) + { lock (LockObject) { var discoveryProvider = ServiceDiscoveryProviders.FirstOrDefault(x => x.ServiceName == route.ServiceName); - if (discoveryProvider != null) return discoveryProvider; + if (discoveryProvider != null) + { + return discoveryProvider; + } discoveryProvider = new PollConsul( config.PollingInterval, route.ServiceName, factory, consulProvider); @@ -41,6 +45,7 @@ private static IServiceDiscoveryProvider CreateProvider(IServiceProvider provide return discoveryProvider; } + } return consulProvider; } diff --git a/src/Ocelot.Provider.Eureka/Eureka.cs b/src/Ocelot.Provider.Eureka/Eureka.cs index 49756e9d2..cbb73785f 100644 --- a/src/Ocelot.Provider.Eureka/Eureka.cs +++ b/src/Ocelot.Provider.Eureka/Eureka.cs @@ -1,6 +1,5 @@ using Ocelot.ServiceDiscovery.Providers; -using Ocelot.Values; -using Steeltoe.Discovery; +using Ocelot.Values; namespace Ocelot.Provider.Eureka { From 11110714d51b50db9d792dd0139ebc8b648922d6 Mon Sep 17 00:00:00 2001 From: Raman Maksimchuk Date: Fri, 29 Sep 2023 18:10:25 +0300 Subject: [PATCH 23/23] Update servicediscovery.rst: Review and update "Consul" section --- docs/features/servicediscovery.rst | 82 +++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/docs/features/servicediscovery.rst b/docs/features/servicediscovery.rst index e32e7cd0e..504ecbfb9 100644 --- a/docs/features/servicediscovery.rst +++ b/docs/features/servicediscovery.rst @@ -3,29 +3,45 @@ Service Discovery ================= -Ocelot allows you to specify a service discovery provider and will use this to find the host and port for the downstream service Ocelot is forwarding a request to. At the moment this is only supported in the -GlobalConfiguration section which means the same service discovery provider will be used for all Routes you specify a ServiceName for at Route level. +Ocelot allows you to specify a *service discovery* provider and will use this to find the host and port for the downstream service to which Ocelot forwards the request. +At the moment this is only supported in the **GlobalConfiguration** section, which means the same *service discovery* provider will be used for all Routes for which you specify a ``ServiceName`` at Route level. Consul ------ -The first thing you need to do is install the NuGet package that provides Consul support in Ocelot. + | **Namespace**: `Ocelot.Provider.Consul `_ + +The first thing you need to do is install `the NuGet package `_ that provides `Consul `_ support in Ocelot. .. code-block:: powershell Install-Package Ocelot.Provider.Consul -Then add the following to your ConfigureServices method. +Then add the following to your ``ConfigureServices`` method: .. code-block:: csharp - s.AddOcelot() - .AddConsul(); + ConfigureServices(services => + { + services.AddOcelot() + .AddConsul(); + }); + +Currently there are 2 types of Consul *service discovery* providers: ``Consul`` and ``PollConsul``. +The default provider is ``Consul``, which means that if ``ConsulProviderFactory`` cannot read, understand, or parse the **Type** property of the ``ServiceProviderConfiguration`` object, then a ``Consul`` provider instance is created by the factory. + +Explore these types of providers and understand the differences in the subsections below. + +Consul Provider Type +^^^^^^^^^^^^^^^^^^^^ + + | **Class**: `Ocelot.Provider.Consul.Consul `_ -The following is required in the GlobalConfiguration. The Provider is required and if you do not specify a host and port the Consul default -will be used. +The following is required in the `GlobalConfiguration `_. +The **ServiceDiscoveryProvider** property is required, and if you do not specify a host and port, the Consul default ones will be used. -Please note the Scheme option defaults to HTTP. It was added in this `PR `_. It defaults to HTTP to not introduce a breaking change. +Please note the `Scheme `_ option defaults to HTTP. +It was added in `PR 1154 `_. It defaults to HTTP to not introduce a breaking change. .. code-block:: json @@ -38,7 +54,10 @@ Please note the Scheme option defaults to HTTP. It was added in this `PR `_ +and `LeastConnection `_ algorithm you can use. +If no load balancer is specified Ocelot will not load balance requests. .. code-block:: json @@ -53,9 +72,15 @@ In order to tell Ocelot a Route is to use the service discovery provider for its }, } -When this is set up Ocelot will lookup the downstream host and port from the service discover provider and load balance requests across any available services. +When this is set up Ocelot will lookup the downstream host and port from the *service discovery* provider and load balance requests across any available services. -A lot of people have asked me to implement a feature where Ocelot polls Consul for latest service information rather than per request. If you want to poll Consul for the latest services rather than per request (default behaviour) then you need to set the following configuration. +PollConsul Provider Type +^^^^^^^^^^^^^^^^^^^^^^^^ + + | **Class**: `Ocelot.Provider.Consul.PollConsul `_ + +A lot of people have asked me to implement a feature where Ocelot *polls Consul* for latest service information rather than per request. +If you want to *poll Consul* for the latest services rather than per request (default behaviour) then you need to set the following configuration: .. code-block:: json @@ -68,11 +93,19 @@ A lot of people have asked me to implement a feature where Ocelot polls Consul f The polling interval is in milliseconds and tells Ocelot how often to call Consul for changes in service configuration. -Please note there are tradeoffs here. If you poll Consul it is possible Ocelot will not know if a service is down depending on your polling interval and you might get more errors than if you get the latest services per request. This really depends on how volatile your services are. I doubt it will matter for most people and polling may give a tiny performance improvement over calling Consul per request (as sidecar agent). If you are calling a remote Consul agent then polling will be a good performance improvement. +Please note there are tradeoffs here. If you *poll Consul* it is possible Ocelot will not know if a service is down depending on your polling interval and you might get more errors than if you get the latest services per request. This really depends on how volatile your services are. I doubt it will matter for most people and polling may give a tiny performance improvement over calling Consul per request (as sidecar agent). If you are calling a remote Consul agent then polling will be a good performance improvement. -Your services need to be added to Consul something like below (C# style but hopefully this make sense)...The only important thing to note is not to add http or https to the Address field. I have been contacted before about not accepting scheme in Address and accepting scheme in address. After reading `this `_ I don't think the scheme should be in there. +Service Definition +^^^^^^^^^^^^^^^^^^ -.. code-block: csharp +Your services need to be added to Consul something like below (C# style but hopefully this make sense)... +The only important thing to note is not to add ``http`` or ``https`` to the Address field. +I have been contacted before about not accepting scheme in Address and accepting scheme in address. +After reading `this `_ I don't think the scheme should be in there. + +In C# + +.. code-block:: csharp new AgentService() { @@ -82,21 +115,22 @@ Your services need to be added to Consul something like below (C# style but hope ID = "some-id", } -Or +Or, in JSON .. code-block:: json - "Service": { - "ID": "some-id", - "Service": "some-service-name", - "Address": "localhost", - "Port": 8080 - } + "Service": { + "ID": "some-id", + "Service": "some-service-name", + "Address": "localhost", + "Port": 8080 + } ACL Token ^^^^^^^^^ -If you are using ACL with Consul Ocelot supports adding the X-Consul-Token header. In order so this to work you must add the additional property below. +If you are using `ACL `_ with Consul, Ocelot supports adding the "X-Consul-Token" header. +In order so this to work you must add the additional property below: .. code-block:: json @@ -248,7 +282,7 @@ This configuration means that if you have a request come into Ocelot on /product Please take a look through all of the docs to understand these options. Custom Providers ----------------------------------- +---------------- Ocelot also allows you to create your own ServiceDiscovery implementation. This is done by implementing the ``IServiceDiscoveryProvider`` interface, as shown in the following example: