-
Notifications
You must be signed in to change notification settings - Fork 704
Update health check to ensure blob containers created at right time #9159
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.
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
Changes from all commits
4ba607c
ec3a2df
6aa390f
e993433
eddebef
5bd0c61
48e2d5d
9737260
80ce587
4ebf0d7
a72f514
c21f230
53fac90
3242c5c
ba67f13
273233b
30641bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,9 +135,10 @@ public async Task VerifyAzureStorageEmulatorResource() | |
|
||
[Fact] | ||
[RequiresDocker] | ||
[QuarantinedTest("https://github.com/dotnet/aspire/issues/9139")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure that this can be dropped? For quarantined tests we want to take it out after it has been green for a certain number of runs (~100 right now). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add it back then. What else? Reopening the issues? Is tracking automatic or is there a process to follow to unquarantine like for aspnet? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, re-open the issue. And it will be tracked automatically. And I will take care of taking it out of quarantine for now. It will get semi-automated in medium term. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! |
||
public async Task VerifyAzureStorageEmulator_blobcontainer_auto_created() | ||
{ | ||
var cts = new CancellationTokenSource(TimeSpan.FromMinutes(3)); | ||
|
||
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(testOutputHelper); | ||
var storage = builder.AddAzureStorage("storage").RunAsEmulator(); | ||
var blobs = storage.AddBlobs("BlobConnection"); | ||
|
@@ -146,16 +147,16 @@ public async Task VerifyAzureStorageEmulator_blobcontainer_auto_created() | |
using var app = builder.Build(); | ||
await app.StartAsync(); | ||
|
||
var rns = app.Services.GetRequiredService<ResourceNotificationService>(); | ||
await rns.WaitForResourceHealthyAsync(blobContainer.Resource.Name, cancellationToken: cts.Token); | ||
|
||
var hb = Host.CreateApplicationBuilder(); | ||
hb.Configuration["ConnectionStrings:BlobConnection"] = await blobs.Resource.ConnectionStringExpression.GetValueAsync(CancellationToken.None); | ||
hb.AddAzureBlobClient("BlobConnection"); | ||
|
||
using var host = hb.Build(); | ||
await host.StartAsync(); | ||
|
||
var rns = app.Services.GetRequiredService<ResourceNotificationService>(); | ||
await rns.WaitForResourceHealthyAsync(blobContainer.Resource.Name, CancellationToken.None); | ||
|
||
var serviceClient = host.Services.GetRequiredService<BlobServiceClient>(); | ||
var blobContainerClient = serviceClient.GetBlobContainerClient("testblobcontainer"); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to duplicate the HC here? Or why do we need to keep the HC on lines:160-167?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it's on the Blobs resource it. Line 160 is on the Emulator resource. Doing it on the storage is not sufficient as the
WaitForHealthyAsync
doesn't bubble up to the parent resources.If it were just for the existing tests we could probably not have this specific one. But it's more consistent to keep it if we do it for containers.