Restructure backend/endpoint config, config reload #42
Conversation
Tratcher
force-pushed
the
tratcher/reloadconfig
branch
from
29bbb5d to
94912b0
Compare
|
bump |
| // You can then change appsettings.json on disk and we will apply the new configs without a restart. | ||
| services.Configure<ProxyConfigRoot>(_configuration.GetSection("ReverseProxy")); | ||
| services.AddHostedService<ProxyConfigApplier>(); | ||
| services.AddReverseProxy().LoadFromConfig(_configuration.GetSection("ReverseProxy"), reloadOnChange: true); |
There was a problem hiding this comment.
Other places tend to do this by accepting an IConfiguration in the AddXYZ call. Is that not appropriate here?
There was a problem hiding this comment.
I wanted to clarify the separation between "I'll set some of the proxies options from config" and "I'm loading routes and endpoints from config". I expect this is where you'd add similar extensions for LoadFromServiceFabric, LoadFromDatabase, etc..
There was a problem hiding this comment.
This brings up a question for services like Antares, we probably don't want to fetch the entire route table from the db, but instead have a pull and cache for each hostname model. Is that basically a replacememt router?
There was a problem hiding this comment.
This brings up a question for services like Antares, we probably don't want to fetch the entire route table from the db, but instead have a pull and cache for each hostname model. Is that basically a replacememt router?
Hmm, that doesn't fit well into the current model. I'll file a separate issue to evaluate that.
There was a problem hiding this comment.
@samsp-msft well done, you broke it. I've filed #46 for that scenario. I think supporting it would require a much different architecture.
src/ReverseProxy.Core/Configuration/ProxyConfigLoaderOptions.cs
Outdated
Show resolved
Hide resolved
| { | ||
| _subscription = _proxyConfig.OnChange((newConfig, name) => _ = ApplyAsync(newConfig)); | ||
| } | ||
| return ApplyAsync(_proxyConfig.CurrentValue); |
There was a problem hiding this comment.
I know this isn't new, but could there be overlapping calls to ApplyAsync? Should we use a semaphore or something?
There was a problem hiding this comment.
In theory there could be multiple calls, but everything ApplyAsync calls is already heavily guarded. The config objects all deep clone, the route and backeds repos lock on set and get, and the various collections used by ApplyConfigurationsAsync also lock. It's not an efficient process, but hopefully your config isn't changing that frequently.
#9 Restructure backend/endpoint config as discussed in https://github.com/microsoft/reverse-proxy/blob/master/docs/config.md#backend-configuration, nesting endpoints under their backends, and moving from list to object/dictionary formats. After Consulting with David I also took the opportunity to simplify the internal object model to match.
Also moved the config reload logic into the product assemblies.