Custom resource with `WaitFor` behaves very differently if it implements `IResourceWithParent`

[afscrome]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

If I have a custom IResourceWithParent, it seems to get set to Ready once it's parent is ready, ignoring any other WaitFor I've specified on the resource, and even though like the other custom resource I don't have any other code indicating it's initial state.

Expected Behavior

Given that custom1 has a WaitFor(migrator) and migrator is still waiting, I'd expect custom1 to still be in the Waiting state.

Also since it's a custom resource and I haven't implemented anything to mark the resource as ready, I'd expect the state after it's finished Waiting to be Unknown like custom2 rather than Ready. (Or for custom2 to default it's initial state to Ready like custom1 does.

I can "fix" these issues by not implementing IResourceWithParent on MigratedDatabase, but it seemed surprising to me that the presence of this caused such different behaviour.

Steps To Reproduce

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

var db = sql.AddDatabase("whatever")
    .WaitFor(migrator);

var custom1 = builder.AddResource(new MigratedDatabase("custom1", db))
    .WaitFor(migrator);

var custom2 = builder.AddResource(new CustomResource("custom2"))
    .WaitFor(migrator);

public class MigratedDatabase(string name, IResourceBuilder<IResourceWithConnectionString> inner)
    : Resource(name), IResourceWithParent<IResourceWithConnectionString>, IResourceWithConnectionString
{
    public IResourceWithConnectionString Parent => inner.Resource;
    public ReferenceExpression ConnectionStringExpression => Parent.ConnectionStringExpression;
}


public class CustomResource(string name) : Resource(name)
{
}

Exceptions (if any)

No response

.NET Version info

No response

Anything else?

This example is based on how I've done DB migrations in 8.x with https://github.com/davidfowl/WaitForDependenciesAspire.

[DamianEdwards]

FYI @mitchdenny

[davidfowl]

We’re going to all child resource inherit parent state

[mitchdenny]

@afscrome we've made a change in main whereby child resources no longer have their own health checks by default. This means that when you do a WaitFor(database) its really only waiting for the server to start up. Because of this what you would probably want to do is have your migrator now WaitFor(database) which will start as soon as the server is healthy. Presumably your migrator can then create the database.

What you are starting to touch on however is the fact that custom resources don't really have a "default driver" which pushes them through the lifecycle from Starting to Running and beyond.

This is something that we won't solve for 9.0 I don't believe but it is something that I hope we can tackle in 9.x. In the meantime if you are building custom resources you also need to handle publishing resource events (via ResourceNotificationService).

For child resources we automatically copy the state, and health status (if the resource is in a running state) (this just merged in with this change #5870)

[mitchdenny]

Putting this into the backlog for 9.x.

[afscrome]

@mitchdenny Do you have any examples of driving a custom resource through?

I've got the following:

public static IResourceBuilder<MigratedDatabase> AddCentralDb3(this IResourceBuilder<SqlServerServerResource> sql, string name)
{
    var appBuilder = sql.ApplicationBuilder;
    var db = sql.AddDatabase($"{name}-inner");

    var migrator = sql
        .AddMigrator(name)
        .WaitFor(sql);

    var migratedDb = appBuilder
        .AddResource(new MigratedDatabase(name, db))
        .WaitForCompletion(migrator)
        .WithInitialState(new CustomResourceSnapshot()
        {
            Properties = [],
            ResourceType = "MigratedDb",
            State = new(KnownResourceStates.Running, KnownResourceStateStyles.Success),
        });

    // Forward connection string available event from the inner resource
    appBuilder.Eventing.Subscribe<ConnectionStringAvailableEvent>(db.Resource, async (e, c) =>
    {
        var connectionStringEvent = new ConnectionStringAvailableEvent(migratedDb.Resource, e.Services);
        await appBuilder.Eventing.PublishAsync(connectionStringEvent, c);
    });

    return migratedDb;
}

private static IResourceBuilder<ExecutableResource> AddMigrator(this IResourceBuilder<SqlServerServerResource> sql, string name)
{
    return sql.ApplicationBuilder
        .AddExecutable($"{name}-migrator", "pwsh", ".",
            "-NoProfile",
            "-NoLogo",
            "-NonInteractive",
            "-Command",
            "Write-Host 'Starting Migration'; Start-Sleep -Seconds 3; Write-Host 'Done'"
        );
}

In order to get the Waiting for resource / Finished waiting for resource messages to show up in console logs, added

    appBuilder.Eventing.Subscribe<BeforeStartEvent>(async (e, c) =>
    {
        var startEvent = new BeforeResourceStartedEvent(migratedDb.Resource, e.Services);
        // Be very careful here - if you don't do NonBlocking, app will deadlock
        await appBuilder.Eventing.PublishAsync(startEvent, EventDispatchBehavior.NonBlockingConcurrent, c);
    });

However my app state remains stuck in Waiting and never progresses to Succeeded, even though console logs show the Finished waiting for resource 'centralDb-migrator'.

I assume I'm missing an event in which I need to call resourceNotificationService.PublishUpdateAsync but I'm not sure what I'm missing.

I have been able to get it to work by doing the waiting myself in BeforeResourceStartedEvent

appBuilder.Eventing.Subscribe<BeforeResourceStartedEvent>(migratedDb.Resource, (e, c) =>
{
    _ = Task.Run(async () =>
    {
        var resourceNotificationService = e.Services.GetRequiredService<ResourceNotificationService>();
        await resourceNotificationService.WaitForDependenciesAsync(migratedDb.Resource, c);

        await resourceNotificationService.PublishUpdateAsync(migratedDb.Resource, x => x with
        {
            State = new(KnownResourceStates.Running, KnownResourceStateStyles.Success),
        });
    }, c);

    return Task.CompletedTask;
});

However this results in the WaitFor messages being duplicated in the console logs:

2024-10-04T20:42:10 Waiting for resource 'centralDb-migrator' to complete. 2024-10-04T20:42:10 Waiting for resource 'centralDb-migrator' to complete. 2024-10-04T20:42:16 Finished waiting for resource 'centralDb-migrator'. 2024-10-04T20:42:16 Finished waiting for resource 'centralDb-migrator'.

[mitchdenny]

OK I've been looking at this. Ultimately the orchestrator and operators idea will be the thing that makes this work more seamlessly. What I was thinking was that we could add a health check that would cause things to block on the custom resource like this (does not work):

using System.Security.Cryptography;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Diagnostics.HealthChecks;

var builder = DistributedApplication.CreateBuilder(args);

var custom = builder.AddCustomResource("custom");

var db = builder.AddPostgres("pg").AddDatabase("db")
                .WaitFor(custom);

var apiService = builder.AddProject<Projects.CustomResources_ApiService>("apiservice")
                        .WithReference(db);

builder.AddProject<Projects.CustomResources_Web>("webfrontend")
    .WithExternalHttpEndpoints()
    .WithReference(apiService);


builder.Build().Run();

public class CustomResource(string name) : Resource(name)
{

}

public static class CustomResourceExtensions
{
    public static IResourceBuilder<CustomResource> AddCustomResource(this IDistributedApplicationBuilder builder, string name)
    {
        var resource = new CustomResource(name);

        // This is just to simulate some background process that is doing something with
        // this custom resource. in this case we just wait for 20 seconds
        bool customResourceIsHealthy = false;
        builder.Eventing.Subscribe<BeforeStartEvent>((@e, ct) => {
            _ = Task.Run(async () => {
                var rns = @e.Services.GetRequiredService<ResourceNotificationService>();

                await rns.PublishUpdateAsync(resource, s => s with {
                    State = "Starting"
                });

                await Task.Delay(20000);

                await rns.PublishUpdateAsync(resource, s => s with {
                    State = "Running"
                });

                customResourceIsHealthy = true;
            });
            return Task.CompletedTask;
        });

        var healthCheckKey = $"{name}_check";
        builder.Services.AddHealthChecks().AddCheck(healthCheckKey, (r) => customResourceIsHealthy ? HealthCheckResult.Healthy() : HealthCheckResult.Unhealthy());
        
        return builder.AddResource(resource)
                      .WithHealthCheck(healthCheckKey);
    }
}

The reason it doesn't work is that ResourceNotification.WaitForDependenciesAsync(...) only waits for its own health checks to return healthy - not its dependencies. This works for the server -> database relationship because we have special case handling for IResourceWithParent<T> inside the ResourceHealthCheckService.

I'm having a think about what we can do here tactically for 9.x to make this scenario possible without going the whole orcehstrator/operator thing.

[mitchdenny]

@afscrome ... @davidfowl and I had an extended conversation about this. Once we fully understood the problem we decided to further constraint the WaitFor(...) and WaitForCompletion(...) methods to resources that we know can support them.

For now this excludes most (maybe all) resources that implement IResourceWithParent and whose lifecycle is owned by ApplicationExecutor.

This is probably not the solution you are looking for but based on where we are at with 9.0 we think its the right thing to do to avoid creating confusion with folks. In the future we are hoping that work related to #6040 can unlock WaitFor(...) for child resources as well.

[afscrome]

That's fair.

I have also worked out how to get my custom resource to properly work with WaitFor - need to spawn a new Task tnside the BeforeStartEvent that awaits publishing the BeforeResourceStartedEvent (which in turn does the waiting) before making the resource state healthy. Without spawning a new task, publishing the BeforeResourceStartedEvent was waiting for BeforeStartEvent to complete, which in turn was waiting for the BeforeResourceStartedEvent publish to finish. Making the publish non blocking wasn't an option as I needed to wait on that to mark the resource as running.

        appBuilder.Eventing.Subscribe<BeforeStartEvent>((e, c) =>
        {
            var resourceNotificationService = e.Services.GetRequiredService<ResourceNotificationService>();

            // Have to run this in a non blocking task, otherwise the call to publish
            // BeforeResourceStartedEvent deadlocks with the BeforeStartEvent subscription
            _ = Task.Run(async () =>
            {
                var startEvent = new BeforeResourceStartedEvent(migratedDb.Resource, e.Services);
                // Be very careful here - if you don't do NonBlocking, app will deadlock
                await appBuilder.Eventing.PublishAsync(startEvent, c);
                await resourceNotificationService.PublishUpdateAsync(migratedDb.Resource, x => x with
                {
                    State = new(KnownResourceStates.Running, KnownResourceStateStyles.Success),
                });
            }, c);
            return Task.CompletedTask;
        });

https://github.com/dotnet/aspire/discussions/6117 has a bit more context around what I'm trying to do, as well as a fuller code example.

[davidfowl]

@afscrome yes this is not formalized today but @mitchdenny wants to spend more time (in https://github.com/dotnet/aspire/issues/6040) coming up with a lifecycle model to manage resources. What you have there looks correct. Waiting happens in the BeforeResourceStartedEvent, we debated making this an explicit wait event but landed here so far. As long as your resource fires and blocks here you can implement IResourceWithWaitSupport you'll be able to add WaitForAnnotations to your resource.