Fix resource service port collision in --isolated mode#18332
Merged
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18332Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18332" |
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a single CLI end-to-end test, IsolatedInstancesOtelLogsTests, that validates two isolated Aspire instances produce distinct OpenTelemetry log output. It scaffolds a Starter app (no Redis), clones it into instance1/ and instance2/, starts both AppHosts with aspire start --isolated --apphost …, waits for apiservice to be running in each, captures aspire otel logs --format json from each, and asserts both contain resourceLogs and differ. The test follows the established Hex1b automator patterns already used by OtelLogsTests, StartStopTests, and WaitCommandTests, and the --apphost/--isolated/wait/otel logs/start/stop surface it exercises all exist in the CLI.
Changes:
- Adds
IsolatedInstancesOtelLogsTests.TwoIsolatedInstancesProduceDifferentOtelLogs, a new CLI E2E test for per-instance telemetry isolation. - Drives two concurrent isolated AppHosts targeted via
--apphostand asserts the captured OTLP JSON differs.
When running multiple instances with --isolated, the AppHost's dashboard, resource service, and OTLP endpoints all tried to bind to the same fixed ports from the launch profile/user secrets. Fix by clearing port-bearing env vars (ASPNETCORE_URLS, ASPIRE_RESOURCE_SERVICE_ENDPOINT_URL, and ASPIRE_DASHBOARD_OTLP_*_ENDPOINT_URL) in ConfigureIsolatedModeAsync so the AppHost falls back to OS-assigned random ports. Also suppress the launch profile in the dotnet-run fallback path when isolated, so the fixed ports from the profile don't leak through.
…env vars" This reverts commit 4dc855d.
This comment has been minimized.
This comment has been minimized.
JamesNK
added
the
area-app-model
This comment has been minimized.
This comment has been minimized.
JamesNK
force-pushed
the
cli-e2e-isolated-instances-otel-logs
branch
from
8be2133 to
cfd6ee3
Compare
JamesNK
force-pushed
the
cli-e2e-isolated-instances-otel-logs
branch
from
cfd6ee3 to
9d97225
Compare
This comment has been minimized.
This comment has been minimized.
JamesNK
force-pushed
the
cli-e2e-isolated-instances-otel-logs
branch
from
9d97225 to
fed34da
Compare
JamesNK
force-pushed
the
cli-e2e-isolated-instances-otel-logs
branch
from
fed34da to
a302808
Compare
JamesNK
changed the title
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tests/Aspire.Cli.EndToEnd.Tests/IsolatedInstancesOtelLogsTests.cs:109
- These assertions don't validate what they claim.
WaitUntilTextAsync("instance1")/"instance2"succeed immediately because those literals are already on screen from the directory names (mv IsolatedApp instance1,cp -r instance1 instance2) and the echoed commands themselves (otel_instance1.json,otel_instance2.json) — independent of whethergrepactually extracted a "Content root path" line. As written, the test passes even if the captured JSON contains no content-root path at all, so it does not prove the two isolated instances produce distinct telemetry (the stated purpose). Note this also diverges from the PR description, which says it asserts the payloads differ viaservice.instance.id; neitherservice.instance.idnor any payload comparison is performed. Follow the sentinel-marker convention used just above for theresourceLogschecks: print a marker only whengrepmatches the instance-specific path, and wait for that marker.
// Stop both instances
await auto.AspireStopAsync(counter, apphost: appHost1);
await auto.AspireStopAsync(counter, apphost: appHost2);
}
}
This comment has been minimized.
This comment has been minimized.
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. Test projects (46 / 97)
Jobs (5)
Selection computed for commit |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Member
Author
PR Testing ReportHead Commit: Changes Analyzed
Test Scenarios Executed
Notes
Overall Result✅ All 27 tests passed — PR VERIFIED |
karolz-ms
approved these changes
Jun 19, 2026
Contributor
|
✅ No documentation update needed. Decision: Triggered signals (1): This PR fixes a port collision bug in Evidence that the triggered signal is already documented:
The docs already correctly describe the intended behavior that this PR restores. No new user-facing surface was introduced — all 4 remaining changed files are under |
Contributor
|
/backport to release/13.4 |
Contributor
|
Started backporting to |
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When running multiple AppHost instances with
--isolated, the resource service endpoint inDashboardServiceHostuses the fixed port fromASPIRE_RESOURCE_SERVICE_ENDPOINT_URL(e.g. port 22147 from launch profiles/user secrets). This causes an "address already in use" error when the second instance tries to bind to the same port.Fix
In
DashboardServiceHost.ConfigureKestrel, whenDcpOptions.RandomizePortsis true (set by--isolatedmode), always bind to port 0 on loopback instead of using the configured port. This lets the OS assign a unique port for each instance.Changes
DashboardServiceHost.cs: InjectIOptions<DcpOptions>and checkRandomizePorts. When true, bypass the configured URI and listen onIPAddress.Loopbackwith port 0.CliE2EAutomatorHelpers.cs: Addapphostparameter toAspireStartAsyncandAspireStopAsynchelpers.IsolatedInstancesOtelLogsTests.cs: New E2E test that creates two copies of a Starter project, starts both with--isolated --apphost, and verifies each instance'sContent root pathin OTLP structured logs points at the correct instance directory.