From 1cdd275dc99a472154bbf65415d0528c27fcc1b9 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Thu, 1 May 2025 13:05:48 +1000 Subject: [PATCH 1/3] =?UTF-8?q?Revert=20"Revert=20"Wait=20for=20dashboard?= =?UTF-8?q?=20to=20be=20healthy=20before=20returning=20URL=20via=20RPC?= =?UTF-8?q?=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 935f06b15acaa8068375d5507423aacdaaf1de52. --- src/Aspire.Hosting/Aspire.Hosting.csproj | 1 + .../Backchannel/AppHostRpcTarget.cs | 26 ++++++++++++++++--- .../Dashboard/DashboardLifecycleHook.cs | 2 +- .../DistributedApplicationBuilder.cs | 15 +++++++++++ src/Shared/KnownHealthCheckNames.cs | 12 +++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 src/Shared/KnownHealthCheckNames.cs diff --git a/src/Aspire.Hosting/Aspire.Hosting.csproj b/src/Aspire.Hosting/Aspire.Hosting.csproj index 5b47ff4cdc4..76868939098 100644 --- a/src/Aspire.Hosting/Aspire.Hosting.csproj +++ b/src/Aspire.Hosting/Aspire.Hosting.csproj @@ -21,6 +21,7 @@ + diff --git a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs index fe50ca47efb..c3102597dba 100644 --- a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs +++ b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs @@ -21,7 +21,8 @@ internal class AppHostRpcTarget( IServiceProvider serviceProvider, IDistributedApplicationEventing eventing, PublishingActivityProgressReporter activityReporter, - IHostApplicationLifetime lifetime + IHostApplicationLifetime lifetime, + DistributedApplicationOptions options ) { public async IAsyncEnumerable<(string Id, string StatusText, bool IsComplete, bool IsError)> GetPublishingActivitiesAsync([EnumeratorCancellation]CancellationToken cancellationToken) @@ -101,6 +102,25 @@ public Task PingAsync(long timestamp, CancellationToken cancellationToken) public Task<(string BaseUrlWithLoginToken, string? CodespacesUrlWithLoginToken)> GetDashboardUrlsAsync() { + return GetDashboardUrlsAsync(CancellationToken.None); + } + + public async Task<(string BaseUrlWithLoginToken, string? CodespacesUrlWithLoginToken)> GetDashboardUrlsAsync(CancellationToken cancellationToken) + { + if (!options.DashboardEnabled) + { + logger.LogError("Dashboard URL requested but dashboard is disabled."); + throw new InvalidOperationException("Dashboard URL requested but dashboard is disabled."); + } + + // Wait for the dashboard to be healthy before we return the URL. This is to avoid + // a race condition when using Codespaces or devcontainers where the dashboard URL + // is displayed before the dashboard port forwarding is actually configured. It is + // also a point of friction to show the URL before the dashboard is ready to be used + // when using Devcontainers/Codespaces because people think that something isn't working + // when in fact they just need to refresh the page. + await resourceNotificationService.WaitForResourceHealthyAsync(KnownResourceNames.AspireDashboard, cancellationToken).ConfigureAwait(false); + var dashboardOptions = serviceProvider.GetService>(); if (dashboardOptions is null) @@ -122,11 +142,11 @@ public Task PingAsync(long timestamp, CancellationToken cancellationToken) if (baseUrlWithLoginToken == codespacesUrlWithLoginToken) { - return Task.FromResult<(string, string?)>((baseUrlWithLoginToken, null)); + return (baseUrlWithLoginToken, null); } else { - return Task.FromResult((baseUrlWithLoginToken, codespacesUrlWithLoginToken)); + return (baseUrlWithLoginToken, codespacesUrlWithLoginToken); } } diff --git a/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs b/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs index 2d22a5db2d1..bd9e337b06f 100644 --- a/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs +++ b/src/Aspire.Hosting/Dashboard/DashboardLifecycleHook.cs @@ -127,7 +127,6 @@ private void AddDashboardResource(DistributedApplicationModel model) nameGenerator.EnsureDcpInstancesPopulated(dashboardResource); ConfigureAspireDashboardResource(dashboardResource); - // Make the dashboard first in the list so it starts as fast as possible. model.Resources.Insert(0, dashboardResource); } @@ -179,6 +178,7 @@ private void ConfigureAspireDashboardResource(IResource dashboardResource) dashboardResource.Annotations.Add(new ResourceSnapshotAnnotation(snapshot)); dashboardResource.Annotations.Add(new EnvironmentCallbackAnnotation(ConfigureEnvironmentVariables)); + dashboardResource.Annotations.Add(new HealthCheckAnnotation(KnownHealthCheckNames.DasboardHealthCheck)); } internal async Task ConfigureEnvironmentVariables(EnvironmentCallbackContext context) diff --git a/src/Aspire.Hosting/DistributedApplicationBuilder.cs b/src/Aspire.Hosting/DistributedApplicationBuilder.cs index 9eb7c04ebc4..fcab487bad2 100644 --- a/src/Aspire.Hosting/DistributedApplicationBuilder.cs +++ b/src/Aspire.Hosting/DistributedApplicationBuilder.cs @@ -19,6 +19,7 @@ using Aspire.Hosting.Lifecycle; using Aspire.Hosting.Orchestrator; using Aspire.Hosting.Publishing; +using Aspire.Hosting.Utils; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -331,6 +332,20 @@ public DistributedApplicationBuilder(DistributedApplicationOptions options) _innerBuilder.Services.AddLifecycleHook(); _innerBuilder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ConfigureDefaultDashboardOptions>()); _innerBuilder.Services.TryAddEnumerable(ServiceDescriptor.Singleton, ValidateDashboardOptions>()); + + // Dashboard health check. + _innerBuilder.Services.AddHealthChecks().AddUrlGroup(sp => { + + var dashboardOptions = sp.GetRequiredService>().Value; + if (StringUtils.TryGetUriFromDelimitedString(dashboardOptions.DashboardUrl, ";", out var firstDashboardUrl)) + { + return firstDashboardUrl; + } + else + { + throw new DistributedApplicationException($"The dashboard resource '{KnownResourceNames.AspireDashboard}' does not have endpoints."); + } + }, KnownHealthCheckNames.DasboardHealthCheck); } if (options.EnableResourceLogging) diff --git a/src/Shared/KnownHealthCheckNames.cs b/src/Shared/KnownHealthCheckNames.cs new file mode 100644 index 00000000000..5bf0276584c --- /dev/null +++ b/src/Shared/KnownHealthCheckNames.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Aspire; + +internal static class KnownHealthCheckNames +{ + /// + /// Common name for dashboard health check. + /// + public const string DasboardHealthCheck = "aspire_dashboard_check"; +} From e02d5168c37a9eb3fc201f1a87e17e5247d33ce4 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Thu, 1 May 2025 14:28:10 +0000 Subject: [PATCH 2/3] Workaround. --- .../Backchannel/AppHostRpcTarget.cs | 16 +++++++++------- .../Health/ResourceHealthCheckService.cs | 5 ++++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs index c3102597dba..8d7af405556 100644 --- a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs +++ b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs @@ -9,6 +9,7 @@ using Aspire.Hosting.Publishing; using Aspire.Hosting.Utils; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -113,13 +114,14 @@ public Task PingAsync(long timestamp, CancellationToken cancellationToken) throw new InvalidOperationException("Dashboard URL requested but dashboard is disabled."); } - // Wait for the dashboard to be healthy before we return the URL. This is to avoid - // a race condition when using Codespaces or devcontainers where the dashboard URL - // is displayed before the dashboard port forwarding is actually configured. It is - // also a point of friction to show the URL before the dashboard is ready to be used - // when using Devcontainers/Codespaces because people think that something isn't working - // when in fact they just need to refresh the page. - await resourceNotificationService.WaitForResourceHealthyAsync(KnownResourceNames.AspireDashboard, cancellationToken).ConfigureAwait(false); + // Wait for the dashboard to be healthy before returning the URL. This next statement has several + // layers of hacks. Some to work around devcontainer/codespaces port forwarding behavior, and one to + // temporarily work around the fact that resource events abuse the state to mark the resource as + // hidden instead of having another field. + await resourceNotificationService.WaitForResourceAsync( + KnownResourceNames.AspireDashboard, + re => re.Snapshot.HealthReports.All(h => h.Status == HealthStatus.Healthy), + cancellationToken).ConfigureAwait(false); var dashboardOptions = serviceProvider.GetService>(); diff --git a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs index bf831314053..da7da1d6780 100644 --- a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs +++ b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs @@ -39,7 +39,10 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } } - if (resourceEvent.Snapshot.State?.Text == KnownResourceStates.Running) + // HACK: We are special casing the Aspire dashboard here until we address this issue. This is so that + // AppHostRpcTarget can hold until the resource is in a healthy state (due to health check) even + // though it is a hidden resource. + if (resourceEvent.Snapshot.State?.Text == KnownResourceStates.Running || resourceEvent.Resource.Name == KnownResourceNames.AspireDashboard) { if (state == null) { From 70ccf2474fa95c2758f823d6c08eb00367e864d6 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Fri, 2 May 2025 01:07:58 +0000 Subject: [PATCH 3/3] Add comments. --- src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs | 5 ++++- src/Aspire.Hosting/Health/ResourceHealthCheckService.cs | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs index 8d7af405556..b481e2861f0 100644 --- a/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs +++ b/src/Aspire.Hosting/Backchannel/AppHostRpcTarget.cs @@ -117,7 +117,10 @@ public Task PingAsync(long timestamp, CancellationToken cancellationToken) // Wait for the dashboard to be healthy before returning the URL. This next statement has several // layers of hacks. Some to work around devcontainer/codespaces port forwarding behavior, and one to // temporarily work around the fact that resource events abuse the state to mark the resource as - // hidden instead of having another field. + // hidden instead of having another field. There is a corresponding modification in the ResourceHealthService + // which allows the dashboard resource to trigger health reports even though it never enters + // the Running state. This is a hack. The reason we can't just check HealthStatus is because + // the current implementation of HealthStatus depends on the state of the resource as well. await resourceNotificationService.WaitForResourceAsync( KnownResourceNames.AspireDashboard, re => re.Snapshot.HealthReports.All(h => h.Status == HealthStatus.Healthy), diff --git a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs index da7da1d6780..cefe7f32102 100644 --- a/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs +++ b/src/Aspire.Hosting/Health/ResourceHealthCheckService.cs @@ -39,9 +39,9 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } } - // HACK: We are special casing the Aspire dashboard here until we address this issue. This is so that - // AppHostRpcTarget can hold until the resource is in a healthy state (due to health check) even - // though it is a hidden resource. + // HACK: We are special casing the Aspire dashboard here until we address the issue of the Hidden state + // making it impossible to determine whether a hidden resource is running or not. When that change + // is made we can remove the special case logic here for the dashboard. if (resourceEvent.Snapshot.State?.Text == KnownResourceStates.Running || resourceEvent.Resource.Name == KnownResourceNames.AspireDashboard) { if (state == null)