microsoft / microsoft/aspire

Assign `DcpInstancesAnnotation` after `BeforeStartEvent`?

Open
#10,149 1 comment 0 reactions 0 assignees View on GitHub
area-app-model area-app-testing
Dominant language
C#
Stars
6.3k
Forks
991
Avg merge
2d 15h
Merged PRs (30d)
196

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Describe the bug

I've been trying to troubleshoot why some of my tests were unexpectedly hitting race conditions with each other. In my main apphost, these resources are marked as `.WithLifetime(ContainerLifetime.Persistent)`, but for my tests I wanted to turn off all container persistence to ensure the tests are isoalted from each other.

I was attemptign to do this through a `BeforeStartEvent` subscription, but on further investigation it appears that the `DcpInstancesAnnotation` has been already been assigned to the resource by the time my event handler runs. So even if I remove the Lifetime annotation at this point, it is a bit late as DCP has already processed it.

Should this annotation be assigned to resources after `BeforeStartEvent`s have been processed to give consumers a chance to modify these values before DCP locks in it's naming decision - https://github.com/dotnet/aspire/blob/cb9d231f82f7d8a6971954c21b4808259388ab91/src/Aspire.Hosting/DistributedApplicationBuilder.cs#L371 .

![Image](https://github.com/user-attachments/assets/e8b1054a-aa92-49cf-8200-afe2d11f64f2)

### Expected Behavior

I should be able to change whether a container is persistent or now in `BeforeStartEvent`.

### Steps To Reproduce

I'd expect the following to start two containers, not for both tests to re-use the same container.

```cs
// NUnit
[assembly: Parallelizable(ParallelScope.All)]

public class WebTests
{
[TestCase(1)]
[TestCase(2)]
public async Task Test(int instance)
{
var builder = DistributedApplicationTestingBuilder.Create();
DisablePersistentContainers(builder);

var sql = builder.AddSqlServer("sql")
.WithLifetime(ContainerLifetime.Persistent);

var app = builder.Build();

await app.StartAsync();
await Task.Delay(120_000);
}

private static IDistributedApplicationBuilder DisablePersistentContainers(IDistributedApplicationBuilder builder)
{
builder.Eventing.Subscribe((evt, ct) =>
{
foreach (var resource in evt.Model.Resources.OfType())
{
foreach (var lifetimeAnnotation in resource.Annotations.OfType().ToArray())
{
resource.Annotations.Remove(lifetimeAnnotation);
}
foreach (var proxySupport in resource.Annotations.OfType().ToArray())
{
resource.Annotations.Remove(proxySupport);
}
}
return Task.CompletedTask;
});

return builder;
}

}
return builder;
}
```

### Exceptions (if any)

_No response_

### .NET Version info

Reproed on both 9.3.0 and 9.4.0-preview.1.25329.2

### Anything else?

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.