Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WaitFor: Add health checks to remaining resources #5645

Open
13 of 28 tasks
mitchdenny opened this issue Sep 10, 2024 · 14 comments · Fixed by #5698 · May be fixed by #5761
Open
13 of 28 tasks

WaitFor: Add health checks to remaining resources #5645

mitchdenny opened this issue Sep 10, 2024 · 14 comments · Fixed by #5698 · May be fixed by #5761
Assignees
@mitchdenny
Labels
area-integrations Issues pertaining to Aspire Integrations packages feature A single user-facing feature. Can be grouped under an epic.
Milestone
9.0

Comments

@mitchdenny
Copy link
Member

mitchdenny commented Sep 10, 2024 •
edited
Loading

With the merge of #5394 and some of the subsequent enhancements around health check integration (#5515) its now time to scale out the support for WaitFor for all of our resources. Essentially each AddXYZ method for a resource needs to register a health check with DI and add a matching HealthCheckAnnotation. Here are two examples:

Redis

aspire/src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs

Lines 36 to 57 in 1b2e640

public static IResourceBuilder<RedisResource> AddRedis(this IDistributedApplicationBuilder builder, string name, int? port = null)
{
ArgumentNullException.ThrowIfNull(builder);
var redis = new RedisResource(name);
string? connectionString = null;
builder.Eventing.Subscribe<ConnectionStringAvailableEvent>(redis, async (@event, ct) =>
{
connectionString = await redis.GetConnectionStringAsync(ct).ConfigureAwait(false);
});
var healthCheckKey = $"{name}_check";
builder.Services.AddHealthChecks().AddRedis(sp => connectionString ?? throw new InvalidOperationException("Connection string is unavailable"), name: healthCheckKey);
return builder.AddResource(redis)
.WithEndpoint(port: port, targetPort: 6379, name: RedisResource.PrimaryEndpointName)
.WithImage(RedisContainerImageTags.Image, RedisContainerImageTags.Tag)
.WithImageRegistry(RedisContainerImageTags.Registry)
.WithHealthCheck(healthCheckKey);
}

Postgres

aspire/src/Aspire.Hosting.PostgreSQL/PostgresBuilderExtensions.cs

Lines 38 to 98 in 1b2e640

public static IResourceBuilder<PostgresServerResource> AddPostgres(this IDistributedApplicationBuilder builder,
string name,
IResourceBuilder<ParameterResource>? userName = null,
IResourceBuilder<ParameterResource>? password = null,
int? port = null)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(name);
var passwordParameter = password?.Resource ?? ParameterResourceBuilderExtensions.CreateDefaultPasswordParameter(builder, $"{name}-password");
var postgresServer = new PostgresServerResource(name, userName?.Resource, passwordParameter);
string? connectionString = null;
builder.Eventing.Subscribe<ConnectionStringAvailableEvent>(postgresServer, async (@event, ct) =>
{
connectionString = await postgresServer.GetConnectionStringAsync(ct).ConfigureAwait(false);
var lookup = builder.Resources.OfType<PostgresDatabaseResource>().ToDictionary(d => d.Name);
foreach (var databaseName in postgresServer.Databases)
{
if (!lookup.TryGetValue(databaseName.Key, out var databaseResource))
{
throw new DistributedApplicationException($"Database resource '{databaseName}' under Postgres server resource '{postgresServer.Name}' not in model.");
}
var connectionStringAvailableEvent = new ConnectionStringAvailableEvent(databaseResource, @event.Services);
await builder.Eventing.PublishAsync<ConnectionStringAvailableEvent>(connectionStringAvailableEvent, ct).ConfigureAwait(false);
var beforeResourceStartedEvent = new BeforeResourceStartedEvent(databaseResource, @event.Services);
await builder.Eventing.PublishAsync(beforeResourceStartedEvent, ct).ConfigureAwait(false);
}
});
var healthCheckKey = $"{name}_check";
builder.Services.AddHealthChecks().AddNpgSql(sp => connectionString ?? throw new InvalidOperationException("Connection string is unavailable"), name: healthCheckKey, configure: (connection) =>
{
// HACK: The Npgsql client defaults to using the username in the connection string if the database is not specified. Here
// we override this default behavior because we are working with a non-database scoped connection string. The Aspirified
// package doesn't have to deal with this because it uses a datasource from DI which doesn't have this issue:
//
// https://github.com/npgsql/npgsql/blob/c3b31c393de66a4b03fba0d45708d46a2acb06d2/src/Npgsql/NpgsqlConnection.cs#L445
//
connection.ConnectionString = connection.ConnectionString + ";Database=postgres;";
});
return builder.AddResource(postgresServer)
.WithEndpoint(port: port, targetPort: 5432, name: PostgresServerResource.PrimaryEndpointName) // Internal port is always 5432.
.WithImage(PostgresContainerImageTags.Image, PostgresContainerImageTags.Tag)
.WithImageRegistry(PostgresContainerImageTags.Registry)
.WithEnvironment("POSTGRES_HOST_AUTH_METHOD", "scram-sha-256")
.WithEnvironment("POSTGRES_INITDB_ARGS", "--auth-host=scram-sha-256 --auth-local=scram-sha-256")
.WithEnvironment(context =>
{
context.EnvironmentVariables[UserEnvVarName] = postgresServer.UserNameReference;
context.EnvironmentVariables[PasswordEnvVarName] = postgresServer.PasswordParameter;
})
.WithHealthCheck(healthCheckKey);
}

Remaining resources

Pri0

P1

P2

  • Azure KeyVault
  • Azure Search
  • Azure Open AI
  • Azure Storage
  • Azure EventHubs
  • Azure WebPubSub
  • Azure SignalR
  • Azure Cosmos DB
  • Keycloak
  • Dapr?
  • AWS?
  • Azure AppConfiguration
  • Azure ApplicationInsights?

Some of these resources don't have health checks and possibly won't need them but we should go through the list and very each one and create issues. We also need test coverage for the WaitFor behavior to verify that it is working as expected for each resource (example):

aspire/tests/Aspire.Hosting.Redis.Tests/RedisFunctionalTests.cs

Lines 21 to 68 in 1b2e640

public async Task VerifyWaitForOnRedisBlocksDependentResources()
{
var cts = new CancellationTokenSource(TimeSpan.FromMinutes(3));
using var builder = TestDistributedApplicationBuilder.CreateWithTestContainerRegistry(testOutputHelper);
// We use the following check added to the Redis resource to block
// dependent reosurces from starting. This means we'll have two checks
// associated with the redis resource ... the built in one and the
// one that we add here. We'll manipulate the TCS to allow us to check
// states at various stages of the execution.
var healthCheckTcs = new TaskCompletionSource<HealthCheckResult>();
builder.Services.AddHealthChecks().AddAsyncCheck("blocking_check", () =>
{
return healthCheckTcs.Task;
});
var redis = builder.AddRedis("redis")
.WithHealthCheck("blocking_check");
var dependentResource = builder.AddRedis("dependentresource")
.WaitFor(redis); // Just using another redis instance as a dependent resource.
using var app = builder.Build();
var pendingStart = app.StartAsync(cts.Token);
var rns = app.Services.GetRequiredService<ResourceNotificationService>();
// What for the Redis server to start.
await rns.WaitForResourceAsync(redis.Resource.Name, KnownResourceStates.Running, cts.Token);
// Wait for the dependent resource to be in the Waiting state.
await rns.WaitForResourceAsync(dependentResource.Resource.Name, KnownResourceStates.Waiting, cts.Token);
// Now unblock the health check.
healthCheckTcs.SetResult(HealthCheckResult.Healthy());
// ... and wait for the resource as a whole to move into the health state.
await rns.WaitForResourceAsync(redis.Resource.Name, (re => re.Snapshot.HealthStatus == HealthStatus.Healthy), cts.Token);
// ... then the dependent resource should be able to move into a running state.
await rns.WaitForResourceAsync(dependentResource.Resource.Name, KnownResourceStates.Running, cts.Token);
await pendingStart; // Startup should now complete.
// ... but we'll shut everything down immediately because we are done.
await app.StopAsync();
}

@mitchdenny mitchdenny added area-integrations Issues pertaining to Aspire Integrations packages and removed area-components labels Sep 10, 2024
@mitchdenny mitchdenny added this to the 9.0 milestone Sep 10, 2024
@mitchdenny mitchdenny self-assigned this Sep 10, 2024
@Alirexaa
Copy link
Contributor

@mitchdenny , I could help you to completing this one.

@davidfowl
Copy link
Member

Lets do the azure resources last.

@mitchdenny
Copy link
Member Author

mitchdenny commented Sep 10, 2024 •
edited
Loading

@Alirexaa of course! Any help is welcome with this. Let's tackle each resource in a different PR and just update the issue with a link to your PR as you work on it.

This was referenced Sep 10, 2024
Merged
Merged
Merged
@Alirexaa
Copy link
Contributor

I will work on Valkey

@mitchdenny mitchdenny mentioned this issue Sep 13, 2024
Merged
16 tasks
@Alirexaa Alirexaa mentioned this issue Sep 13, 2024
Merged
16 tasks
@Alirexaa
Copy link
Contributor

I will work on Milvus.

@davidfowl davidfowl reopened this Sep 13, 2024
@Alirexaa Alirexaa mentioned this issue Sep 13, 2024
Merged
16 tasks
@davidfowl
Copy link
Member

davidfowl commented Sep 13, 2024 •
edited
Loading

Kakfka is high on the priority list.

@davidfowl
Copy link
Member

I sorted the list in priority order

This was referenced Sep 15, 2024
Merged
Merged
@Alirexaa
Copy link
Contributor

I will work on Elasicseach.

@davidfowl davidfowl added the feature A single user-facing feature. Can be grouped under an epic. label Sep 15, 2024
@Alirexaa Alirexaa mentioned this issue Sep 15, 2024
Merged
16 tasks
@Alirexaa
Copy link
Contributor

I will work on Oracle.

@davidfowl davidfowl changed the title WaitFor: Scale out to all resources WaitFor: Add health checks to remaining resources Sep 15, 2024
@mitchdenny mitchdenny mentioned this issue Sep 16, 2024
Merged
16 tasks
@davidfowl
Copy link
Member

Added PgAdmin. That will be useful to add health checks for since its so slow to start. That'll give us a visual indicator in the dashboard that it's healthy.

@Alirexaa Alirexaa mentioned this issue Sep 16, 2024
Merged
16 tasks
@Alirexaa
Copy link
Contributor

I will work in nats.

@mitchdenny mitchdenny mentioned this issue Sep 17, 2024
Open
@mitchdenny
Copy link
Member Author

Added PgAdmin. That will be useful to add health checks for since its so slow to start. That'll give us a visual indicator in the dashboard that it's healthy.

Is the health check primarily for the visual indicator. It's not that common that anyone will do a WaitFor on PGAdmin itself (example of how you would do that):

var builder = DistributedApplication.CreateBuilder(args);
var pgsql = builder.AddPostgres("pgsql");
var db = pgsql.AddDatabase("db");
var app = builder.AddProject<Projects.MyApp>("app")
                 .WithReference(db).WaitFor(db);
pgsql.WithPgAdmin(c => {
  app.WaitFor(c); // Causes app to block loading until pgadmin is running/healthy.
});

@Alirexaa Alirexaa mentioned this issue Sep 17, 2024
Merged
16 tasks
@Alirexaa
Copy link
Contributor

Seq and Qdrant resources are missing.

@Alirexaa
Copy link
Contributor

I will work on Qdrant.

@mitchdenny mitchdenny linked a pull request Sep 18, 2024 that will close this issue
Draft
16 tasks
@Alirexaa Alirexaa mentioned this issue Sep 18, 2024
Open
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mitchdenny mitchdenny

Labels
area-integrations Issues pertaining to Aspire Integrations packages feature A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Milestone
9.0
Development

Successfully merging a pull request may close this issue.

WaitFor support for Garnet. dotnet/aspire
WaitFor for Azure Storage dotnet/aspire
3 participants
@davidfowl @mitchdenny @Alirexaa