Skip to content

Commit b75cce0

Browse files
bart-vmwarebart-vmware
authored
Follow-up changes for new IDiscoveryClient.InstancesFetched event (#1677)
* Fixed: Adapt EurekaDiscoveryClient.InstancesFetched event args match what's returned by GetInstancesAsync: - Keyed by VIP addresses instead of app names - Take ReturnUpInstancesOnly into account - Don't return an empty collection when no instances are found * Adapt load balancers to ignore duplicate URIs
1 parent 5d88bb2 commit b75cce0

5 files changed

Lines changed: 232 additions & 46 deletions

File tree

‎src/Discovery/src/Eureka/EurekaDiscoveryClient.cs‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -373,20 +373,19 @@ internal async Task FetchRegistryAsync(bool doFullUpdate, CancellationToken canc
373373

374374
private static ReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>> ToServiceInstanceMap(ApplicationInfoCollection apps)
375375
{
376-
// @formatter:wrap_chained_method_calls chop_always
377-
// @formatter:wrap_before_first_method_call true
378-
379-
return apps
380-
.SelectMany(app => app.Instances)
381-
.GroupBy(instance => instance.AppName, StringComparer.OrdinalIgnoreCase)
382-
.ToDictionary(grouping => grouping.Key, grouping => (IReadOnlyList<IServiceInstance>)grouping
383-
.Select(instance => instance.ToServiceInstance())
384-
.ToList()
385-
.AsReadOnly(), StringComparer.OrdinalIgnoreCase)
386-
.AsReadOnly();
387-
388-
// @formatter:wrap_before_first_method_call restore
389-
// @formatter:wrap_chained_method_calls restore
376+
var dictionary = new Dictionary<string, IReadOnlyList<IServiceInstance>>(StringComparer.OrdinalIgnoreCase);
377+
378+
foreach (string vipAddress in apps.VipInstanceMap.Keys.ToArray())
379+
{
380+
ReadOnlyCollection<InstanceInfo> instancesByVipAddress = apps.GetInstancesByVipAddress(vipAddress);
381+
382+
if (instancesByVipAddress.Count > 0)
383+
{
384+
dictionary[vipAddress] = instancesByVipAddress.Select(instance => instance.ToServiceInstance()).ToList().AsReadOnly();
385+
}
386+
}
387+
388+
return new ReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>>(dictionary);
390389
}
391390

392391
private void RaiseFetchEvents(ApplicationsFetchedEventArgs? applicationsEventArgs, DiscoveryInstancesFetchedEventArgs? instancesEventArgs)

‎src/Discovery/src/HttpClients/LoadBalancers/ServiceInstancesResolver.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ public async Task<IList<IServiceInstance>> ResolveInstancesAsync(string serviceI
8383

8484
if (instancesFromCache != null)
8585
{
86+
instancesFromCache = RemoveDuplicatesByUri(instancesFromCache);
8687
LogReturningInstancesFromCache(instancesFromCache.Count);
8788
return instancesFromCache;
8889
}
@@ -103,6 +104,8 @@ public async Task<IList<IServiceInstance>> ResolveInstancesAsync(string serviceI
103104
}
104105
}
105106

107+
instances = RemoveDuplicatesByUri(instances);
108+
106109
if (_distributedCache != null)
107110
{
108111
byte[] cacheValue = ToCacheValue(instances);
@@ -112,6 +115,22 @@ public async Task<IList<IServiceInstance>> ResolveInstancesAsync(string serviceI
112115
return instances;
113116
}
114117

118+
private static List<IServiceInstance> RemoveDuplicatesByUri(List<IServiceInstance> instances)
119+
{
120+
var seenUris = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
121+
var result = new List<IServiceInstance>();
122+
123+
foreach (IServiceInstance instance in instances)
124+
{
125+
if (seenUris.Add(instance.Uri.AbsoluteUri))
126+
{
127+
result.Add(instance);
128+
}
129+
}
130+
131+
return result;
132+
}
133+
115134
private static List<IServiceInstance>? FromCacheValue(byte[]? cacheValue)
116135
{
117136
if (cacheValue is { Length: > 0 })

‎src/Discovery/test/Configuration.Test/ConfigurationDiscoveryClientTest.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,13 @@ public async Task InstancesFetched_event_is_raised_after_configuration_change()
229229
await using WebApplication webApplication = builder.Build();
230230

231231
ConfigurationDiscoveryClient discoveryClient = webApplication.Services.GetServices<IDiscoveryClient>().OfType<ConfigurationDiscoveryClient>().Single();
232-
int eventCount = 0;
233232
DiscoveryInstancesFetchedEventArgs? eventArgs = null;
233+
int eventCount = 0;
234234

235235
discoveryClient.InstancesFetched += (_, args) =>
236236
{
237-
eventCount++;
238237
eventArgs = args;
238+
Interlocked.Increment(ref eventCount);
239239
};
240240

241241
fileProvider.ReplaceAppSettingsJsonFile("""

‎src/Discovery/test/Eureka.Test/EurekaDiscoveryClientTest.cs‎

Lines changed: 100 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Globalization;
56
using System.Net;
67
using FluentAssertions.Extensions;
78
using Microsoft.AspNetCore.Builder;
@@ -655,38 +656,127 @@ public async Task ApplicationEventsFireAfterFetch()
655656
webApplication.Services.GetRequiredService<HttpClientHandlerFactory>().Using(handler);
656657

657658
var discoveryClient = webApplication.Services.GetRequiredService<EurekaDiscoveryClient>();
658-
int applicationsEventCount = 0;
659659
ApplicationsFetchedEventArgs? applicationsEventArgs = null;
660-
int instancesEventCount = 0;
660+
int applicationsEventCount = 0;
661661
DiscoveryInstancesFetchedEventArgs? instancesEventArgs = null;
662+
int instancesEventCount = 0;
662663

663664
discoveryClient.ApplicationsFetched += (_, args) =>
664665
{
665-
applicationsEventCount++;
666666
applicationsEventArgs = args;
667+
Interlocked.Increment(ref applicationsEventCount);
667668
};
668669

669670
discoveryClient.InstancesFetched += (_, args) =>
670671
{
671-
instancesEventCount++;
672672
instancesEventArgs = args;
673+
Interlocked.Increment(ref instancesEventCount);
673674
};
674675

675676
await discoveryClient.FetchRegistryAsync(true, TestContext.Current.CancellationToken);
676677
SpinWait.SpinUntil(() => applicationsEventCount == 1 && instancesEventCount == 1, 5.Seconds()).Should().BeTrue();
677678

679+
applicationsEventArgs.Should().NotBeNull();
680+
InstanceInfo oldInstanceFromAppEvent = applicationsEventArgs.Applications.Should().ContainSingle().Which.Instances.Should().ContainSingle().Which;
681+
oldInstanceFromAppEvent.ActionType.Should().Be(ActionType.Added);
682+
683+
instancesEventArgs.Should().NotBeNull();
684+
IServiceInstance oldInstanceFromEvent = instancesEventArgs.InstancesByServiceId.Should().ContainKey("foo").WhoseValue.Should().ContainSingle().Which;
685+
oldInstanceFromEvent.Uri.ToString().Should().Be("http://localhost:8080/");
686+
687+
IList<IServiceInstance> oldInstancesFromGet = await discoveryClient.GetInstancesAsync("foo", TestContext.Current.CancellationToken);
688+
oldInstancesFromGet.Should().ContainSingle().Which.Uri.Should().Be(oldInstanceFromEvent.Uri);
689+
678690
await discoveryClient.FetchRegistryAsync(false, TestContext.Current.CancellationToken);
679691
SpinWait.SpinUntil(() => applicationsEventCount == 2 && instancesEventCount == 2, 5.Seconds()).Should().BeTrue();
680692

693+
InstanceInfo newInstanceFromAppEvent = applicationsEventArgs.Applications.Should().ContainSingle().Which.Instances.Should().ContainSingle().Which;
694+
newInstanceFromAppEvent.ActionType.Should().Be(ActionType.Modified);
695+
696+
IServiceInstance newInstanceFromEvent = instancesEventArgs.InstancesByServiceId.Should().ContainKey("foo").WhoseValue.Should().ContainSingle().Which;
697+
newInstanceFromEvent.Uri.ToString().Should().Be("http://modified-host:8080/");
698+
699+
IList<IServiceInstance> newInstancesFromGet = await discoveryClient.GetInstancesAsync("foo", TestContext.Current.CancellationToken);
700+
newInstancesFromGet.Should().ContainSingle().Which.Uri.Should().Be(newInstanceFromEvent.Uri);
701+
681702
handler.Mock.VerifyNoOutstandingExpectation();
703+
}
682704

683-
applicationsEventArgs.Should().NotBeNull();
684-
InstanceInfo newInstanceInfo = applicationsEventArgs.Applications.Should().ContainSingle().Which.Instances.Should().ContainSingle().Which;
685-
newInstanceInfo.ActionType.Should().Be(ActionType.Modified);
705+
[Theory]
706+
[InlineData(false)]
707+
[InlineData(true)]
708+
public async Task InstancesFetched_returns_same_data_as_GetInstancesAsync(bool filterOnlyUpInstances)
709+
{
710+
const string registryJson = """
711+
{
712+
"applications": {
713+
"application": [
714+
{
715+
"name": "ignored",
716+
"instance": [
717+
{
718+
"instanceId": "id1",
719+
"hostName": "h1",
720+
"app": "app1",
721+
"ipAddr": "10.0.0.1",
722+
"status": "UP",
723+
"dataCenterInfo": {
724+
"@class": "com.netflix.appinfo.InstanceInfo$DefaultDataCenterInfo",
725+
"name": "MyOwn"
726+
},
727+
"vipAddress": "vapp1"
728+
},
729+
{
730+
"instanceId": "id2",
731+
"hostName": "h2",
732+
"app": "app1",
733+
"ipAddr": "10.0.0.2",
734+
"status": "DOWN",
735+
"dataCenterInfo": {
736+
"@class": "com.netflix.appinfo.InstanceInfo$DefaultDataCenterInfo",
737+
"name": "MyOwn"
738+
},
739+
"vipAddress": "vapp1"
740+
}
741+
]
742+
}
743+
]
744+
}
745+
}
746+
""";
686747

687-
instancesEventArgs.Should().NotBeNull();
688-
IServiceInstance newServiceInstance = instancesEventArgs.InstancesByServiceId.Should().ContainKey("foo").WhoseValue.Should().ContainSingle().Which;
689-
newServiceInstance.Uri.ToString().Should().Be("http://modified-host:8080/");
748+
var appSettings = new Dictionary<string, string?>
749+
{
750+
["Eureka:Client:ShouldFetchRegistry"] = "false",
751+
["Eureka:Client:ShouldRegisterWithEureka"] = "false",
752+
["Eureka:Client:ShouldFilterOnlyUpInstances"] = filterOnlyUpInstances.ToString(CultureInfo.InvariantCulture)
753+
};
754+
755+
WebApplicationBuilder builder = TestWebApplicationBuilderFactory.Create();
756+
builder.Configuration.AddInMemoryCollection(appSettings);
757+
builder.Services.AddEurekaDiscoveryClient();
758+
759+
var handler = new DelegateToMockHttpClientHandler();
760+
handler.Mock.Expect(HttpMethod.Get, "http://localhost:8761/eureka/apps").Respond("application/json", registryJson);
761+
762+
await using WebApplication webApplication = builder.Build();
763+
webApplication.Services.GetRequiredService<HttpClientHandlerFactory>().Using(handler);
764+
765+
var discoveryClient = webApplication.Services.GetRequiredService<EurekaDiscoveryClient>();
766+
DiscoveryInstancesFetchedEventArgs? eventArgs = null;
767+
discoveryClient.InstancesFetched += (_, args) => eventArgs = args;
768+
769+
await discoveryClient.FetchRegistryAsync(true, TestContext.Current.CancellationToken);
770+
SpinWait.SpinUntil(() => eventArgs != null, 5.Seconds()).Should().BeTrue();
771+
772+
eventArgs.Should().NotBeNull();
773+
774+
IList<IServiceInstance> instancesFromGet = await discoveryClient.GetInstancesAsync("vapp1", TestContext.Current.CancellationToken);
775+
IReadOnlyList<IServiceInstance> instancesFromEvent = eventArgs.InstancesByServiceId.Should().ContainKey("vapp1").WhoseValue;
776+
777+
instancesFromEvent.Should().BeEquivalentTo(instancesFromGet);
778+
779+
handler.Mock.VerifyNoOutstandingExpectation();
690780
}
691781

692782
private sealed class ExtraRequestHeadersDelegatingHandler : DelegatingHandler

0 commit comments

Comments
 (0)