dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.83k stars 457 forks source link

Should DB show `Waiting` status rather than `Starting` whilst Postgress image is being pulled #5620

Open afscrome opened 2 months ago

afscrome commented 2 months ago

Is there an existing issue for this?

Describe the bug

With the new Waiting state being introduced with WaitFor, would it make more sense to use the Waiting state for the Postgres database resource whilst the Postgres server image is being pulled?

image

(Same issue applies to other resources using this pattern e.g. SQL Server)

Expected Behavior

Since the database is waiting for the server's docker pull to complete, Waiting seems like a more appropiate status than Starting.

Steps To Reproduce

  1. Ensure you don't have the postgress image locally (docker image rm docker.io/library/postgres:16.4 -f)
  2. Use the following app
    
    var builder = DistributedApplication.CreateBuilder(args);

var server = builder.AddPostgres("server"); var db = server.AddDatabase("db");

var migrator = builder.AddExecutable("migrator", "pwsh", ".", "-NoProfile", "-NoLogo", "-NonInteractive", "-Command", "Write-Host 'Starting Migration'; Start-Sleep -Seconds 10; Write-Host 'Done'") .WaitFor(db);

var foo = builder.AddExecutable("temp", "fake", ".") .WaitFor(server) .WaitForCompletion(migrator);

builder.Build().Run();


### Exceptions (if any)

_No response_

### .NET Version info

    <PackageReference Include="Aspire.Hosting.PostgreSQL" Version="9.0.0-preview.4.24459.3" />

.NET SDK: Version: 8.0.401 Commit: 811edcc344 Workload version: 8.0.400-manifests.fa3104af MSBuild version: 17.11.4+37eb419ad

Runtime Environment: OS Name: Windows OS Version: 10.0.22631 OS Platform: Windows RID: win-x64 Base Path: C:\Program Files\dotnet\sdk\8.0.401\

.NET workloads installed: Configured to use loose manifests when installing new manifests. [aspire] Installation Source: SDK 8.0.400, VS 17.10.35201.131 Manifest Version: 9.0.0-preview.4.24454.4/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\9.0.0-preview.4.24454.4\WorkloadManifest.json Install Type: FileBased

Host: Version: 8.0.8 Architecture: x64 Commit: 08338fcaa5

.NET SDKs installed: 8.0.304 [C:\Program Files\dotnet\sdk] 8.0.401 [C:\Program Files\dotnet\sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found: x86 [C:\Program Files (x86)\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables: Not set

global.json file: Not found

Learn more: https://aka.ms/dotnet/info

Download .NET: https://aka.ms/dotnet/download



### Anything else?

_No response_
davidfowl commented 2 months ago

We can't tell when an image is being pulled, this isn't status that we get from the orchestrator. Maybe a good future suggestion, moving this to the backlog.

afscrome commented 2 months ago

I guess there are actually two issues here

  1. Container Resources should show waiting
  2. The Database resource should show Waiting until the parent database resource has started

Whilst 1 will require more information from the orchestrator, I think we have enough information to do 2. I can make this work locally by adding a WaitFor to the database resource.

var builder = DistributedApplication.CreateBuilder(args);

var pg = builder.AddPostgres("postgres");

var db = pg.AddDatabase("test")
    .WaitFor(pg); // The fix

builder.Build().Run();

Although there may be some caveats with this approach around a) Remote (e.g. Azure) resources and b) Health Check failures.

afscrome commented 2 months ago

On second thoughts, a counter argument to this whole idea is that it'll start to introduce differences between statuses in local dev, and in deployment. WaitFor as I understand it is local dev only. So whilst using the Waiting status brings some nice symmetry with other local waits, it may lead to some deviation where the same action shows Starting when deployed, but Waiting locally.