diff --git a/src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs b/src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs index 07ea40bb665..f08f3482946 100644 --- a/src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs +++ b/src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Globalization; +using System.Net.Http.Json; using System.Text; using System.Text.Json; +using System.Text.Json.Serialization; using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Redis; using Aspire.Hosting.Utils; @@ -197,20 +199,50 @@ public static IResourceBuilder WithRedisInsight(this IResourceBui return builder; } - static async Task ImportRedisDatabases(ILogger resourceLogger, IEnumerable redisInstances, HttpClient client, CancellationToken ct) + static async Task ImportRedisDatabases(ILogger resourceLogger, IEnumerable redisInstances, HttpClient client, CancellationToken cancellationToken) { + var databasesPath = "/api/databases"; + + var pipeline = new ResiliencePipelineBuilder().AddRetry(new Polly.Retry.RetryStrategyOptions + { + Delay = TimeSpan.FromSeconds(2), + MaxRetryAttempts = 5, + }).Build(); + using (var stream = new MemoryStream()) { + // As part of configuring RedisInsight we need to factor in the possibility that the + // container resource is being run with persistence turned on. In this case we need + // to get the list of existing databases because we might need to delete some. + var lookup = await pipeline.ExecuteAsync(async (ctx) => + { + var getDatabasesResponse = await client.GetFromJsonAsync(databasesPath, cancellationToken).ConfigureAwait(false); + return getDatabasesResponse?.ToLookup( + i => i.Name ?? throw new InvalidDataException("Database name is missing."), + i => i.Id ?? throw new InvalidDataException("Database ID is missing.")); + }, cancellationToken).ConfigureAwait(false); + + var databasesToDelete = new List(); + using var writer = new Utf8JsonWriter(stream); writer.WriteStartArray(); foreach (var redisResource in redisInstances) { + if (lookup is { } && lookup.Contains(redisResource.Name)) + { + // It is possible that there are multiple databases with + // a conflicting name so we delete them all. This just keeps + // track of the specific ID that we need to delete. + databasesToDelete.AddRange(lookup[redisResource.Name]); + } + if (redisResource.PrimaryEndpoint.IsAllocated) { var endpoint = redisResource.PrimaryEndpoint; writer.WriteStartObject(); + writer.WriteString("host", redisResource.Name); writer.WriteNumber("port", endpoint.TargetPort!.Value); writer.WriteString("name", redisResource.Name); @@ -223,7 +255,7 @@ static async Task ImportRedisDatabases(ILogger resourceLogger, IEnumerable + { + // Create a DELETE request to send to the existing instance of + // RedisInsight with the IDs of the database to delete. + var deleteContent = JsonContent.Create(new + { + ids = databasesToDelete + }); + + var deleteRequest = new HttpRequestMessage(HttpMethod.Delete, databasesPath) + { + Content = deleteContent + }; + + var deleteResponse = await client.SendAsync(deleteRequest, cancellationToken).ConfigureAwait(false); + deleteResponse.EnsureSuccessStatusCode(); + + }, cancellationToken).ConfigureAwait(false); + } + await pipeline.ExecuteAsync(async (ctx) => { var response = await client.PostAsync(apiUrl, content, ctx) .ConfigureAwait(false); response.EnsureSuccessStatusCode(); - }, ct).ConfigureAwait(false); + }, cancellationToken).ConfigureAwait(false); } catch (Exception ex) @@ -259,6 +307,15 @@ await pipeline.ExecuteAsync(async (ctx) => } } + private class RedisDatabaseDto + { + [JsonPropertyName("id")] + public Guid? Id { get; set; } + + [JsonPropertyName("name")] + public string? Name { get; set; } + } + /// /// Configures the host port that the Redis Commander resource is exposed on instead of using randomly assigned port. /// diff --git a/tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs b/tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs index ce9520d99c3..ff6e91d44cc 100644 --- a/tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs +++ b/tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs @@ -15,6 +15,7 @@ using StackExchange.Redis; using Xunit; using Xunit.Abstractions; +using Aspire.Hosting.Tests.Dcp; namespace Aspire.Hosting.Redis.Tests; @@ -120,6 +121,111 @@ public async Task VerifyRedisResource() Assert.Equal("value", value); } + [Fact] + [RequiresDocker] + public async Task VerifyDatabasesAreNotDuplicatedForPersistentRedisInsightContainer() + { + var randomResourceSuffix = Random.Shared.Next(10000).ToString(); + var cts = new CancellationTokenSource(TimeSpan.FromMinutes(5)); + + var configure = (DistributedApplicationOptions options) => + { + options.ContainerRegistryOverride = TestConstants.AspireTestContainerRegistry; + }; + + using var builder1 = TestDistributedApplicationBuilder.Create(configure, testOutputHelper); + builder1.Configuration[$"DcpPublisher:ResourceNameSuffix"] = randomResourceSuffix; + + IResourceBuilder? redisInsightBuilder = null; + var redis1 = builder1.AddRedis("redisForInsightPersistence") + .WithRedisInsight(c => + { + redisInsightBuilder = c; + c.WithLifetime(ContainerLifetime.Persistent); + }); + + // Wire up an additional event subcription to ResourceReadyEvent on the RedisInsightResource + // instance. This works because the ResourceReadyEvent fires non-blocking sequential so the + // wire-up that WithRedisInsight does is guaranteed to execute before this one does. So we then + // use this to block pulling the list of databases until we know they've been updated. This + // will repeated below for the second app. + // + // Issue: https://github.com/dotnet/aspire/issues/6455 + Assert.NotNull(redisInsightBuilder); + var redisInsightsReady = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + builder1.Eventing.Subscribe(redisInsightBuilder.Resource, (evt, ct) => + { + redisInsightsReady.TrySetResult(); + return Task.CompletedTask; + }); + + using var app1 = builder1.Build(); + + await app1.StartAsync(cts.Token); + + await redisInsightsReady.Task.WaitAsync(cts.Token); + + using var client1 = app1.CreateHttpClient($"{redis1.Resource.Name}-insight", "http"); + var firstRunDatabases = await client1.GetFromJsonAsync("/api/databases", cts.Token); + + Assert.NotNull(firstRunDatabases); + Assert.Single(firstRunDatabases); + Assert.Equal($"{redis1.Resource.Name}", firstRunDatabases[0].Name); + + await app1.StopAsync(cts.Token); + + using var builder2 = TestDistributedApplicationBuilder.Create(configure, testOutputHelper); + builder2.Configuration[$"DcpPublisher:ResourceNameSuffix"] = randomResourceSuffix; + + var redis2 = builder2.AddRedis("redisForInsightPersistence") + .WithRedisInsight(c => + { + redisInsightBuilder = c; + c.WithLifetime(ContainerLifetime.Persistent); + }); + + // Wire up an additional event subcription to ResourceReadyEvent on the RedisInsightResource + // instance. This works because the ResourceReadyEvent fires non-blocking sequential so the + // wire-up that WithRedisInsight does is guaranteed to execute before this one does. So we then + // use this to block pulling the list of databases until we know they've been updated. This + // will repeated below for the second app. + Assert.NotNull(redisInsightBuilder); + redisInsightsReady = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + builder2.Eventing.Subscribe(redisInsightBuilder.Resource, (evt, ct) => + { + redisInsightsReady.TrySetResult(); + return Task.CompletedTask; + }); + + using var app2 = builder2.Build(); + await app2.StartAsync(cts.Token); + + await redisInsightsReady.Task.WaitAsync(cts.Token); + + using var client2 = app2.CreateHttpClient($"{redisInsightBuilder.Resource.Name}", "http"); + var secondRunDatabases = await client2.GetFromJsonAsync("/api/databases", cts.Token); + + Assert.NotNull(secondRunDatabases); + Assert.Single(secondRunDatabases); + Assert.Equal($"{redis2.Resource.Name}", secondRunDatabases[0].Name); + Assert.NotEqual(secondRunDatabases.Single().Id, firstRunDatabases.Single().Id); + + // HACK: This is a workaround for the fact that ApplicationExecutor is not a public type. What I have + // done here is I get the latest event from RNS for the insights instance which gives me the resource + // name as known from a DCP perspective. I then use the ApplicationExecutorProxy (introduced with this + // change to call the ApplicationExecutor stop method. The proxy is a public type with an internal + // constructor inside the Aspire.Hosting.Tests package. This is a short term solution for 9.0 to + // make sure that we have good test coverage for WithRedisInsight behavior, but we need a better + // long term solution in 9.x for folks that will want to do things like execute commands against + // resources to stop specific containers. + var rns = app2.Services.GetRequiredService(); + var latestEvent = await rns.WaitForResourceHealthyAsync(redisInsightBuilder.Resource.Name, cts.Token); + var executorProxy = app2.Services.GetRequiredService(); + await executorProxy.StopResourceAsync(latestEvent.ResourceId, cts.Token); + + await app2.StopAsync(cts.Token); + } + [Fact] [RequiresDocker] public async Task VerifyWithRedisInsightImportDatabases() diff --git a/tests/Aspire.Hosting.Tests/Dcp/ApplicationExecutorProxy.cs b/tests/Aspire.Hosting.Tests/Dcp/ApplicationExecutorProxy.cs new file mode 100644 index 00000000000..543ae16d3dc --- /dev/null +++ b/tests/Aspire.Hosting.Tests/Dcp/ApplicationExecutorProxy.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Aspire.Hosting.Dcp; + +namespace Aspire.Hosting.Tests.Dcp; + +public class ApplicationExecutorProxy +{ + internal ApplicationExecutorProxy(ApplicationExecutor executor) + { + _executor = executor; + } + + private readonly ApplicationExecutor _executor; + + public Task StartResourceAsync(string resourceName, CancellationToken cancellationToken) => _executor.StartResourceAsync(resourceName, cancellationToken); + + public Task StopResourceAsync(string resourceName, CancellationToken cancellationToken) => _executor.StopResourceAsync(resourceName, cancellationToken); +} diff --git a/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs b/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs index c04a4ddb44f..aed5c6891f8 100644 --- a/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs +++ b/tests/Aspire.Hosting.Tests/Utils/TestDistributedApplicationBuilder.cs @@ -6,8 +6,10 @@ using System.Reflection; using Aspire.Components.Common.Tests; using Aspire.Hosting.Dashboard; +using Aspire.Hosting.Dcp; using Aspire.Hosting.Eventing; using Aspire.Hosting.Testing; +using Aspire.Hosting.Tests.Dcp; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -76,6 +78,8 @@ private TestDistributedApplicationBuilder(Action? o.OtlpGrpcEndpointUrl ??= "http://localhost:4317"; }); + _innerBuilder.Services.AddSingleton(sp => new ApplicationExecutorProxy(sp.GetRequiredService())); + _innerBuilder.Services.AddHttpClient(); _innerBuilder.Services.ConfigureHttpClientDefaults(http => http.AddStandardResilienceHandler()); if (testOutputHelper is not null)