Skip to content

Commit

Permalink
Feature/removed consul and its deps to other package (ThreeMammals#539)
Browse files Browse the repository at this point in the history
* ThreeMammals#529 removed consul deps and introduced delegate to find service discovery provider

* +semver: breaking moved consul configuration to package..introduced mechanism for packages to configure Ocelot pipeline
  • Loading branch information
TomPallister authored Aug 12, 2018
1 parent a91235b commit 87348e5
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 92 deletions.
9 changes: 7 additions & 2 deletions docs/features/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,18 @@ At the moment there is no validation at this stage it only happens when Ocelot v
Store configuration in consul
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If you add the following when you register your services Ocelot will attempt to store and retrieve its configuration in consul KV store.
The first thing you need to do is install the NuGet package that provides Consul support in Ocelot.

``Install-Package Ocelot.Provider.Consul``

Then you add the following when you register your services Ocelot will attempt to store and retrieve its configuration in consul KV store.

.. code-block:: csharp
services
.AddOcelot()
.AddStoreOcelotConfigurationInConsul();
.AddConsul()
.AddConfigStoredInConsul();
You also need to add the following to your ocelot.json. This is how Ocelot
finds your Consul agent and interacts to load and store the configuration from Consul.
Expand Down
11 changes: 11 additions & 0 deletions docs/features/servicediscovery.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ you specify a ServiceName for at ReRoute level.
Consul
^^^^^^

The first thing you need to do is install the NuGet package that provides Consul support in Ocelot.

``Install-Package Ocelot.Provider.Consul``

Then add the following to your ConfigureServices method.

.. code-block:: csharp
s.AddOcelot()
.AddConsul();
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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public Task StopAsync(CancellationToken cancellationToken)

private async Task Poll()
{
_logger.LogInformation("Started polling consul");
_logger.LogInformation("Started polling");

var fileConfig = await _repo.Get();

Expand All @@ -91,7 +91,7 @@ private async Task Poll()
_previousAsJson = asJson;
}

_logger.LogInformation("Finished polling consul");
_logger.LogInformation("Finished polling");
}

/// <summary>
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion src/Ocelot/Errors/OcelotErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public enum OcelotErrorCode
UnableToFindLoadBalancerError,
RequestTimedOutError,
UnableToFindQoSProviderError,
UnableToSetConfigInConsulError,
UnmappableRequestError,
RateLimitOptionsError,
PathTemplateDoesntStartWithForwardSlash,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Ocelot.Middleware
{
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;

public delegate Task OcelotMiddlewareConfigurationDelegate(IApplicationBuilder builder);
}
67 changes: 7 additions & 60 deletions src/Ocelot/Middleware/OcelotMiddlewareExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,16 +134,17 @@ private static async Task<IInternalConfiguration> CreateConfiguration(IApplicati
internalConfigRepo.AddOrReplace(newInternalConfig.Data);
});

var fileConfigRepo = builder.ApplicationServices.GetService<IFileConfigurationRepository>();

var adminPath = builder.ApplicationServices.GetService<IAdministrationPath>();

if (UsingConsul(fileConfigRepo))
var configurations = builder.ApplicationServices.GetServices<OcelotMiddlewareConfigurationDelegate>();

// Todo - this has just been added for consul so far...will there be an ordering problem in the future? Should refactor all config into this pattern?
foreach (var configuration in configurations)
{
//Lots of jazz happens in here..check it out if you are using consul to store your config.
await SetFileConfigInConsul(builder, fileConfigRepo, fileConfig, internalConfigCreator, internalConfigRepo);
await configuration(builder);
}
else if(AdministrationApiInUse(adminPath))

if(AdministrationApiInUse(adminPath))
{
//We have to make sure the file config is set for the ocelot.env.json and ocelot.json so that if we pull it from the
//admin api it works...boy this is getting a spit spags boll.
Expand All @@ -160,49 +161,6 @@ private static bool AdministrationApiInUse(IAdministrationPath adminPath)
return adminPath != null;
}

private static async Task SetFileConfigInConsul(IApplicationBuilder builder,
IFileConfigurationRepository fileConfigRepo, IOptionsMonitor<FileConfiguration> fileConfig,
IInternalConfigurationCreator internalConfigCreator, IInternalConfigurationRepository internalConfigRepo)
{
// get the config from consul.
var fileConfigFromConsul = await fileConfigRepo.Get();

if (IsError(fileConfigFromConsul))
{
ThrowToStopOcelotStarting(fileConfigFromConsul);
}
else if (ConfigNotStoredInConsul(fileConfigFromConsul))
{
//there was no config in consul set the file in config in consul
await fileConfigRepo.Set(fileConfig.CurrentValue);
}
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);

if (IsError(response))
{
ThrowToStopOcelotStarting(response);
}
}

if (IsError(internalConfig))
{
ThrowToStopOcelotStarting(internalConfig);
}
}
}

private static async Task SetFileConfig(IFileConfigurationSetter fileConfigSetter, IOptionsMonitor<FileConfiguration> fileConfig)
{
var response = await fileConfigSetter.Set(fileConfig.CurrentValue);
Expand All @@ -213,11 +171,6 @@ private static async Task SetFileConfig(IFileConfigurationSetter fileConfigSette
}
}

private static bool ConfigNotStoredInConsul(Responses.Response<FileConfiguration> fileConfigFromConsul)
{
return fileConfigFromConsul.Data == null;
}

private static bool IsError(Response response)
{
return response == null || response.IsError;
Expand All @@ -240,12 +193,6 @@ 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 UsingConsul(IFileConfigurationRepository fileConfigRepo)
{
//todo - remove coupling by string
return fileConfigRepo.GetType().Name == "ConsulFileConfigurationRepository";
}

private static void CreateAdministrationArea(IApplicationBuilder builder, IInternalConfiguration configuration)
{
if (!string.IsNullOrEmpty(configuration.AdministrationPath))
Expand Down
8 changes: 8 additions & 0 deletions src/Ocelot/ServiceDiscovery/ServiceDiscoveryFinderDelegate.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace Ocelot.ServiceDiscovery
{
using System;
using Ocelot.Configuration;
using Providers;

public delegate IServiceDiscoveryProvider ServiceDiscoveryFinderDelegate(IServiceProvider provider, ServiceProviderConfiguration config, string key);
}
16 changes: 9 additions & 7 deletions src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,16 @@ private IServiceDiscoveryProvider GetServiceDiscoveryProvider(ServiceProviderCon
return new EurekaServiceDiscoveryProvider(key, _eurekaClient);
}

// Todo - dont just hardcode this...only expect Consul at the momement so works.
var finderDelegate = _delegates.FirstOrDefault();

var provider = finderDelegate?.Invoke(_provider, config, key);
foreach (var serviceDiscoveryFinderDelegate in _delegates)
{
var provider = serviceDiscoveryFinderDelegate?.Invoke(_provider, config, key);
if (provider != null)
{
return provider;
}
}

return provider;
return null;
}
}

public delegate IServiceDiscoveryProvider ServiceDiscoveryFinderDelegate(IServiceProvider provider, ServiceProviderConfiguration config, string key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void should_call_setter_when_gets_new_config()
}
};

this.Given(x => WhenTheConfigIsChangedInConsul(newConfig, 0))
this.Given(x => WhenTheConfigIsChanged(newConfig, 0))
.Then(x => ThenTheSetterIsCalledAtLeast(newConfig, 1))
.BDDfy();
}
Expand All @@ -96,13 +96,13 @@ public void should_not_poll_if_already_polling()
}
};

this.Given(x => WhenTheConfigIsChangedInConsul(newConfig, 10))
this.Given(x => WhenTheConfigIsChanged(newConfig, 10))
.Then(x => ThenTheSetterIsCalled(newConfig, 1))
.BDDfy();
}

[Fact]
public void should_do_nothing_if_call_to_consul_fails()
public void should_do_nothing_if_call_to_provider_fails()
{
var newConfig = new FileConfiguration
{
Expand All @@ -121,19 +121,19 @@ public void should_do_nothing_if_call_to_consul_fails()
}
};

this.Given(x => WhenConsulErrors())
this.Given(x => WhenProviderErrors())
.Then(x => ThenTheSetterIsCalled(newConfig, 0))
.BDDfy();
}

private void WhenConsulErrors()
private void WhenProviderErrors()
{
_repo
.Setup(x => x.Get())
.ReturnsAsync(new ErrorResponse<FileConfiguration>(new AnyError()));
}

private void WhenTheConfigIsChangedInConsul(FileConfiguration newConfig, int delay)
private void WhenTheConfigIsChanged(FileConfiguration newConfig, int delay)
{
_repo
.Setup(x => x.Get())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ public void should_return_internal_server_error(OcelotErrorCode errorCode)
[InlineData(OcelotErrorCode.UnableToFindLoadBalancerError)]
[InlineData(OcelotErrorCode.UnableToFindServiceDiscoveryProviderError)]
[InlineData(OcelotErrorCode.UnableToFindQoSProviderError)]
[InlineData(OcelotErrorCode.UnableToSetConfigInConsulError)]
[InlineData(OcelotErrorCode.UnknownError)]
[InlineData(OcelotErrorCode.UnmappableRequestError)]
[InlineData(OcelotErrorCode.UnsupportedAuthenticationProviderError)]
Expand Down Expand Up @@ -126,7 +125,7 @@ public void check_we_have_considered_all_errors_in_these_tests()
// If this test fails then it's because the number of error codes has changed.
// You should make the appropriate changes to the test cases here to ensure
// they cover all the error codes, and then modify this assertion.
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(35, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(34, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
}

private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)
Expand Down

0 comments on commit 87348e5

Please sign in to comment.