diff --git a/src/Aspire.Hosting/Dcp/DcpExecutor.cs b/src/Aspire.Hosting/Dcp/DcpExecutor.cs index 5a76c0441d..eb57bcee4c 100644 --- a/src/Aspire.Hosting/Dcp/DcpExecutor.cs +++ b/src/Aspire.Hosting/Dcp/DcpExecutor.cs @@ -56,7 +56,7 @@ internal sealed class DcpExecutor : IDcpExecutor, IAsyncDisposable private readonly ResourceSnapshotBuilder _snapshotBuilder; // Internal for testing. - internal ResiliencePipeline DeleteResourceRetryPipeline { get; set; } + internal ResiliencePipeline DeleteResourceRetryPipeline { get; set; } internal ResiliencePipeline CreateServiceRetryPipeline { get; set; } internal ResiliencePipeline WatchResourceRetryPipeline { get; set; } @@ -1485,18 +1485,24 @@ async Task EnsureResourceDeletedAsync(string resourceName) where T : CustomRe // before resorting to more extreme measures. if (!resourceNotFound) { - await DeleteResourceRetryPipeline.ExecuteAsync(async (state, attemptCancellationToken) => + var result = await DeleteResourceRetryPipeline.ExecuteAsync(async (state, attemptCancellationToken) => { try { await _kubernetesService.GetAsync(state, cancellationToken: attemptCancellationToken).ConfigureAwait(false); - throw new DistributedApplicationException($"Failed to delete '{state}' successfully before restart."); + return false; } catch (HttpOperationException ex) when (ex.Response.StatusCode == System.Net.HttpStatusCode.NotFound) { // Success. + return true; } }, resourceName, cancellationToken).ConfigureAwait(false); + + if (!result) + { + throw new DistributedApplicationException($"Failed to delete '{resourceName}' successfully before restart."); + } } } } diff --git a/src/Aspire.Hosting/Dcp/DcpPipelineBuilder.cs b/src/Aspire.Hosting/Dcp/DcpPipelineBuilder.cs index 30bc644a04..a8c50e3ebc 100644 --- a/src/Aspire.Hosting/Dcp/DcpPipelineBuilder.cs +++ b/src/Aspire.Hosting/Dcp/DcpPipelineBuilder.cs @@ -10,16 +10,16 @@ namespace Aspire.Hosting.Dcp; internal static class DcpPipelineBuilder { - public static ResiliencePipeline BuildDeleteRetryPipeline(ILogger logger) + public static ResiliencePipeline BuildDeleteRetryPipeline(ILogger logger) { - var ensureDeleteRetryStrategy = new RetryStrategyOptions() + var ensureDeleteRetryStrategy = new RetryStrategyOptions() { BackoffType = DelayBackoffType.Exponential, Delay = TimeSpan.FromMilliseconds(200), UseJitter = true, MaxRetryAttempts = 10, // Cumulative time for all attempts amounts to about 15 seconds MaxDelay = TimeSpan.FromSeconds(3), - ShouldHandle = new PredicateBuilder().Handle(), + ShouldHandle = args => ValueTask.FromResult(!args.Outcome.Result), OnRetry = (retry) => { logger.LogDebug("Retrying check for deleted resource. Attempt: {Attempt}. Error message: {ErrorMessage}", retry.AttemptNumber, retry.Outcome.Exception?.Message); @@ -27,7 +27,7 @@ public static ResiliencePipeline BuildDeleteRetryPipeline(ILogger logger) } }; - var execution = new ResiliencePipelineBuilder().AddRetry(ensureDeleteRetryStrategy).Build(); + var execution = new ResiliencePipelineBuilder().AddRetry(ensureDeleteRetryStrategy).Build(); return execution; } diff --git a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs index a6e661d398..272d58e59e 100644 --- a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs +++ b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs @@ -293,7 +293,6 @@ internal class ResourceMonitorState private readonly object _lock = new object(); private readonly string _resourceName; private TaskCompletionSource? _delayInterruptTcs; - private CancellationTokenSource? _delayCts; public ResourceMonitorState(ILogger logger, ResourceEvent initialEvent, CancellationToken serviceStoppingToken) { @@ -334,6 +333,7 @@ public void SetLatestEvent(ResourceEvent resourceEvent) internal async Task DelayAsync(ResourceEvent? currentEvent, TimeSpan delay, CancellationToken cancellationToken) { + Task delayInterruptedTask; lock (_lock) { // The event might have changed before delay was called. Interrupt immediately if required. @@ -342,26 +342,16 @@ internal async Task DelayAsync(ResourceEvent? currentEvent, TimeSpan delay _logger.LogTrace("Health monitoring delay interrupted for resource '{Resource}'.", _resourceName); return true; } - if (_delayCts == null || !_delayCts.TryReset()) - { - _delayCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - } _delayInterruptTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + delayInterruptedTask = _delayInterruptTcs.Task; } - var completedTask = await Task.WhenAny(Task.Delay(delay, _delayCts.Token), _delayInterruptTcs.Task).ConfigureAwait(false); + // Don't throw to avoid writing the thrown exception to the debug console. + // See https://github.com/dotnet/aspire/issues/7486 + await delayInterruptedTask.WaitAsync(delay, cancellationToken).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing); + var delayInterrupted = delayInterruptedTask.IsCompletedSuccessfully == true; - if (completedTask != _delayInterruptTcs.Task) - { - // Task.Delay won. - return false; - } - else - { - // Delay was interrupted. Cancel the delay task so it doesn't hang around when not needed. - _delayCts.Cancel(); - return true; - } + return delayInterrupted; } private static bool ShouldInterrupt(ResourceEvent currentEvent, ResourceEvent previousEvent) diff --git a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs index ff0327f96e..bebfc17ec0 100644 --- a/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs +++ b/tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs @@ -1106,7 +1106,7 @@ public async Task ErrorIfResourceNotDeletedBeforeRestart() var appExecutor = CreateAppExecutor(distributedAppModel, kubernetesService: kubernetesService, events: dcpEvents); // Set a custom pipeline without retries or delays to avoid waiting. - appExecutor.DeleteResourceRetryPipeline = new ResiliencePipelineBuilder().Build(); + appExecutor.DeleteResourceRetryPipeline = new ResiliencePipelineBuilder().Build(); await appExecutor.RunApplicationAsync();