Fix duplicate resource stopped events during restart - #20279
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20279Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20279" |
Tests selector50 / 99 PR test projects · 4 PR jobs, from 6 changed files. Selected PR test projects (50 / 99)
Selected PR jobs (4)
How these were chosen — grouped by what changed
🔧 show 45
🧪 📦 affected project 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The transition logic is consistent across watcher and orchestrator paths and has focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Prevents duplicate resource-stopped callbacks by tracking transitions from the DCP watcher’s cached state.
Changes:
- Adds
PreviousStateto resource-change events. - Suppresses repeated terminal-state notifications while preserving later stop cycles.
- Adds regression coverage for refreshes, replacements, and replicas.
| File | Description |
|---|---|
src/Aspire.Hosting/Dcp/DcpExecutor.cs |
Supplies previous-state metadata for initial updates. |
src/Aspire.Hosting/Dcp/DcpExecutorEvents.cs |
Extends the internal change context. |
src/Aspire.Hosting/Dcp/DcpResourceWatcher.cs |
Derives previous state from cached DCP objects. |
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs |
Detects terminal transitions using DCP states. |
tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs |
Covers state tracking, refreshes, replacements, and replicas. |
tests/Aspire.Hosting.Tests/Orchestrator/ApplicationOrchestratorTests.cs |
Covers snapshot mutation and repeated stop notifications. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
32db91d
into
main
Documents microsoft/aspire#20279 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1705
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1705 targeting Added a clarifying note to the "Resource stopped event" section of
Note This draft PR needs human review before merging. |
|
The CI build failed due to test failure(s) that appear unrelated to the PR changes. These may be flaky tests. Suspected flaky test(s):
Suggested actions:
You can re-run the failed jobs from the workflow run page. |
Description
Prevent duplicate
OnResourceStoppedcallbacks when a handler initiates a restart. Restart preparation can change the published resource snapshot toWaitingorStartingwhile DCP still reports the same terminal state. Comparing against that snapshot incorrectly treats a later notification as another stop.Carry
PreviousStateonOnResourceChangedContext, using the DCP watcher's existing resource cache.ApplicationOrchestratorcompares the supplied DCP states without maintaining its own tracking collection. Endpoint/service refreshes report no state transition, and replacement DCP objects have no previous state.Fixes #17745
User-facing scenario
An
OnResourceStoppedhandler requests a restart. Another notification for the same stopped process should not invoke the handler again; a subsequent real run/stop cycle should invoke it once more. No AppHost API changes are required.Validation
Checklist
<remarks />and<code />elements on your triple slash comments?