Fix Redis persistent lifetime startup#17827
Conversation
Use Redis endpoint target ports for TLS startup arguments so container command-line evaluation does not wait for allocated public ports before the container exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17827Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17827" |
There was a problem hiding this comment.
Pull request overview
Fixes a deadlock when a Redis container with TLS configured is started with ContainerLifetime.Persistent. The TLS startup arguments (--tls-port/--port) were resolving EndpointProperty.Port (the host-side allocated port), which is unavailable before the container exists in proxyless persistent mode, causing endpoint allocation to wait forever. Since these arguments configure ports inside the container, they must use EndpointProperty.TargetPort instead.
Changes:
- Switch Redis TLS startup arg port references from
EndpointProperty.PorttoEndpointProperty.TargetPortfor both primary (TLS) and secondary (non-TLS) endpoints. - Add a regression test (
RedisWithCertificateUsesTargetPortsForCommandLineArgs) asserting the args resolve to the container target ports 6379/6380.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Aspire.Hosting.Redis/RedisBuilderExtensions.cs | Use EndpointProperty.TargetPort for --tls-port and --port Redis args. |
| tests/Aspire.Hosting.Redis.Tests/AddRedisTests.cs | Adds test verifying TLS args use target ports under persistent lifetime. |
Use a bounded wait around the Redis argument evaluation regression test so the test fails promptly if endpoint resolution deadlocks again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
❓ CLI E2E Tests unknown — 110 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26801455791 |
|
/backport to release/13.4 |
|
Started backporting to |
Description
Redis persistent containers could deadlock before container creation when Redis TLS startup arguments resolved allocated endpoint ports. Those arguments configure the ports Redis listens on inside the container, so they should use endpoint target ports instead of public/allocated ports, which may differ and may not be available before a proxyless persistent container exists.
This updates Redis TLS startup arguments to use target-port bindings for both the primary TLS endpoint and secondary non-TLS endpoint. With this change, the reported AppHost shape starts Redis instead of waiting forever on endpoint allocation:
Fixes: #17822
Validation:
dotnet test --project tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj --no-launch-profile -- --filter-method "*.RedisWithCertificateUsesTargetPortsForCommandLineArgs" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet test --project tests/Aspire.Hosting.Redis.Tests/Aspire.Hosting.Redis.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Checklist
<remarks />and<code />elements on your triple slash comments?