Re-connecting should be part of DCP request retry - #18121
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18121Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18121" |
There was a problem hiding this comment.
Pull request overview
This PR is a manual forward-port of PR #18096 into main. It moves the DCP API server connection establishment (EnsureKubernetesAsync) inside the retry loop of ExecuteWithRetry, so that failures caused by a missing or partially-written kubeconfig file are properly retried. This addresses timing issues where DCP hasn't finished writing its kubeconfig when Aspire first tries to read it (issues #18050 and #18041).
Changes:
- Moved
EnsureKubernetesAsyncfrom before the retry pipeline into theresiliencePipeline.ExecuteAsynclambda, allowing kubeconfig read failures to be retried by the outer resilience pipeline. Removed the now-redundant dedicated timeout pipeline wrapper insideEnsureKubernetesAsyncand switched to explicitFileStreamwithFileShare.ReadWrite | FileShare.Deletefor non-exclusive kubeconfig reads. - Added a new test class
KubernetesServiceTestswith two integration tests verifying the retry behavior when the kubeconfig is initially missing or partially written, using a lightweight fake DCP API server backed by Kestrel on a random port.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Aspire.Hosting/Dcp/KubernetesService.cs |
Moves EnsureKubernetesAsync inside the retry loop in ExecuteWithRetry; removes the inner timeout pipeline from EnsureKubernetesAsync; opens the kubeconfig file with FileShare.ReadWrite | FileShare.Delete to avoid interfering with DCP writes. |
tests/Aspire.Hosting.Tests/Dcp/KubernetesServiceTests.cs |
New test file with two tests for the retry-on-kubeconfig-unavailable behavior, along with test helpers (TestFileSystemService, TestDcpApiServer). |
PR Testing ReportPR Information
Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
What the change does
Test Scenarios ExecutedScenario 1: Check out PR branch + buildObjective: Confirm the PR source restores and builds cleanly. Steps:
Observations:
Scenario 2: Run the 2 new KubernetesServiceTestsObjective: Directly validate the fix — connection is established/re-established inside the retry loop. Steps:
Tests run:
Observations:
Scenario 3: DCP namespace regression sweepObjective: Ensure the Steps:
Observations:
Notes
Summary
Overall Result✅ PR VERIFIED The new tests directly validate the reconnect-inside-retry behavior, and the full DCP test namespace passes with no regressions. Build is clean. Recommendations
|
|
Re-running the failed jobs in the CI workflow for this pull request because 3 jobs were identified as retry-safe transient failures in the CI run attempt.
|
|
❌ CLI E2E Tests failed — 114 passed, 1 failed, 2 unknown (commit ❌ Failed Tests
View all recordings
📹 Recordings uploaded automatically from CI run #27367459470 |
2d41a8f
into
main
|
✅ No documentation update needed. docs_optional → No documentation update is needed for this PR. Triggered signals: none ( Allowlist justification: The two changed files are:
No new public types, methods, options, configuration keys, CLI flags, resource strings, container image tags, or user-visible default values were introduced. The PR fixes issues #18050 and #18041 where Aspire could fail to connect to DCP if it tried to read the kubeconfig before DCP finished writing it. The intended behavior — that Aspire connects to DCP successfully on startup — was already the documented, expected behavior; this change makes the implementation match that expectation more reliably under timing pressure. |
Manual backport (forward-port? 😄) of #18096 into
mainbranch