Skip to content

Treat FailedToStart executables as stopped in DcpExecutor.StopResourceAsync - #21441

Closed
David Fowler (davidfowl) wants to merge 1 commit into
mainfrom
agent/fix-stop-failedtostart-executable
Closed

David Fowler (davidfowl) wants to merge 1 commit into
mainfrom
agent/fix-stop-failedtostart-executable

Conversation

@davidfowl

Copy link
Copy Markdown
Collaborator

Description

Fixes the "Failed to stop resource" failure mode of ResourceCommandServiceTests.ExecuteCommandAsync_RebuildCommand_ReturnsBuildOutput on Hosting-2 (windows-latest). This is a product bug in DcpExecutor.StopResourceAsync.

Root cause. The first rebuild command restarts the project, which deletes and re-creates its DCP Executable. The test then immediately runs the second rebuild, whose first step stops the resource. In the failing runs, the stop patch (Spec.Stop = true) reached the new Executable before DCP had started its process. DCP then reported the Executable as FailedToStart, a terminal state in which nothing is running. StopResourceAsync treats only Finished or Terminated as stopped. It kept polling through the delete retry pipeline (about 15s) and then threw InvalidOperationException: Failed to stop resource '...', so the rebuild command failed and Assert.True(secondResult.Success) failed.

Sequence from job 123561387182 (run 40905807668):

  • 15:58:32 First rebuild: Build succeeded. Restarting resource..., then Starting Executable 'myProject-jgbacpwt'
  • 15:58:33.03 Second rebuild: Stopping resource 'myProject-jgbacpwt'...
  • 15:58:33 Resource myProject/myProject-jgbacpwt changed state: FailedToStart
  • 15:58:33–15:58:50 Executable 'myProject-jgbacpwt' is still running; trying again to stop it... × 10, while the watcher reports no further changes to the Executable
  • 15:58:50 Error executing command 'rebuild'. System.InvalidOperationException: Failed to stop resource 'myProject-jgbacpwt'.

Job 123745958478 shows the same Starting → FailedToStart transition followed by the same exception.

Fix. StopResourceAsync treats an Executable in FailedToStart as stopped. The test fake TestKubernetesService used to force every stopped Executable to Finished. It now leaves a FailedToStart Executable as is, matching DCP. Container stop handling is unchanged, because there is no evidence for that path.

Regression test. DcpExecutorTests.StopResourceAsync_ExecutableThatFailedToStart_IsTreatedAsStopped puts an Executable in FailedToStart and stops it.

  • Before the fix (local, Linux, with a temporary 30s guard): InvalidOperationException: Failed to stop resource 'CoolProgram-…' at DcpExecutor.cs:1232 after 19s. This is the same exception as CI. With the committed default guard, it fails with a TimeoutException.
  • After the fix: 3 of 3 runs passed. All 366 DcpExecutorTests and ResourceCommandServiceTests passed.

Recurrence. The window is aspire main CI, Oct 23–30, first attempts only, out of 51 failed runs. Hosting-2 (windows-latest) failed in 7 of those runs:

Related to #20662 (#21427 fixes the build-count mode of the same test).

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No

🤖 Posted by AspireAgent (AI Agent)

…eAsync

When a stop request reaches an Executable before DCP has started its process,
DCP reports the Executable as FailedToStart. StopResourceAsync only accepted
Finished or Terminated, so it retried for about 15 seconds and then threw
'Failed to stop resource', which made the rebuild command fail intermittently
on Windows (ExecuteCommandAsync_RebuildCommand_ReturnsBuildOutput).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@aspire-repo-bot
aspire-repo-bot Bot requested a balanced review from Copilot October 30, 2026 09:29
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 21441

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 21441"

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Tests selector

52 / 100 PR test projects · 4 PR jobs, from 3 changed files.

Selected PR test projects (52 / 100)

Aspire.Cli.EndToEnd.Tests, Aspire.Hosting.Analyzers.Tests, Aspire.Hosting.Azure.EntraId.Tests, Aspire.Hosting.Azure.Kubernetes.Tests, Aspire.Hosting.Azure.Kusto.Tests, Aspire.Hosting.Azure.Provisioning.Tests, Aspire.Hosting.Azure.Tests, Aspire.Hosting.Blazor.Tests, Aspire.Hosting.Browsers.Tests, Aspire.Hosting.CodeGeneration.Go.Tests, Aspire.Hosting.CodeGeneration.Java.Tests, Aspire.Hosting.CodeGeneration.Python.Tests, Aspire.Hosting.CodeGeneration.Rust.Tests, Aspire.Hosting.CodeGeneration.TypeScript.Tests, Aspire.Hosting.Containers.Tests, Aspire.Hosting.DevTunnels.Tests, Aspire.Hosting.Docker.Tests, Aspire.Hosting.Dotnet.Tests, Aspire.Hosting.DotnetTool.Tests, Aspire.Hosting.EntityFrameworkCore.Tests, Aspire.Hosting.Foundry.Tests, Aspire.Hosting.Garnet.Tests, Aspire.Hosting.Go.Tests, Aspire.Hosting.Java.Tests, Aspire.Hosting.JavaScript.Tests, Aspire.Hosting.Kafka.Tests, Aspire.Hosting.Keycloak.Tests, Aspire.Hosting.Kubernetes.Tests, Aspire.Hosting.Maui.Tests, Aspire.Hosting.Milvus.Tests, Aspire.Hosting.MongoDB.Tests, Aspire.Hosting.MySql.Tests, Aspire.Hosting.Nats.Tests, Aspire.Hosting.OpenAI.Tests, Aspire.Hosting.Oracle.Tests, Aspire.Hosting.Orleans.Tests, Aspire.Hosting.PostgreSQL.Tests, Aspire.Hosting.Python.Tests, Aspire.Hosting.Qdrant.Tests, Aspire.Hosting.RabbitMQ.Tests, Aspire.Hosting.Radius.Tests, Aspire.Hosting.Redis.Tests, Aspire.Hosting.RemoteHost.Tests, Aspire.Hosting.Rust.Tests, Aspire.Hosting.Seq.Tests, Aspire.Hosting.SqlServer.Tests, Aspire.Hosting.Testing.Tests, Aspire.Hosting.Tests, Aspire.Hosting.Valkey.Tests, Aspire.Hosting.Yarp.Tests, Aspire.Playground.Tests, Aspire.Templates.Tests

Selected PR jobs (4)

cli-starter-validation, extension-e2e, polyglot, typescript-api-compat


How these were chosen — grouped by what changed

⚠️ 46 of the 52 selected test projects come from a single change — src/Aspire.Hosting/Dcp/DcpExecutor.cs.

🔧 src/Aspire.Hosting/Dcp/DcpExecutor.cs (changed source)
→ 46 via the project graph

show 46

Aspire.Hosting.Analyzers.Tests (2 hops), Aspire.Hosting.Azure.EntraId.Tests (2 hops), Aspire.Hosting.Azure.Kubernetes.Tests (2 hops), Aspire.Hosting.Azure.Kusto.Tests (2 hops), Aspire.Hosting.Azure.Provisioning.Tests (3 hops), Aspire.Hosting.Azure.Tests, Aspire.Hosting.Browsers.Tests (2 hops), Aspire.Hosting.CodeGeneration.Go.Tests, Aspire.Hosting.CodeGeneration.Java.Tests, Aspire.Hosting.CodeGeneration.Python.Tests, Aspire.Hosting.CodeGeneration.Rust.Tests, Aspire.Hosting.CodeGeneration.TypeScript.Tests, Aspire.Hosting.Containers.Tests (2 hops), Aspire.Hosting.DevTunnels.Tests (2 hops), Aspire.Hosting.Docker.Tests (2 hops), Aspire.Hosting.DotnetTool.Tests (2 hops), Aspire.Hosting.EntityFrameworkCore.Tests (2 hops), Aspire.Hosting.Foundry.Tests (2 hops), Aspire.Hosting.Garnet.Tests (2 hops), Aspire.Hosting.Go.Tests (2 hops), Aspire.Hosting.Java.Tests (2 hops), Aspire.Hosting.JavaScript.Tests (2 hops), Aspire.Hosting.Kafka.Tests (2 hops), Aspire.Hosting.Keycloak.Tests (2 hops), Aspire.Hosting.Kubernetes.Tests (2 hops), Aspire.Hosting.Maui.Tests, Aspire.Hosting.Milvus.Tests (2 hops), Aspire.Hosting.MongoDB.Tests (2 hops), Aspire.Hosting.MySql.Tests (2 hops), Aspire.Hosting.Nats.Tests (2 hops), Aspire.Hosting.OpenAI.Tests (2 hops), Aspire.Hosting.Oracle.Tests (2 hops), Aspire.Hosting.Orleans.Tests (2 hops), Aspire.Hosting.PostgreSQL.Tests (2 hops), Aspire.Hosting.Python.Tests (2 hops), Aspire.Hosting.Qdrant.Tests (2 hops), Aspire.Hosting.RabbitMQ.Tests (2 hops), Aspire.Hosting.Redis.Tests (2 hops), Aspire.Hosting.RemoteHost.Tests, Aspire.Hosting.Rust.Tests (2 hops), Aspire.Hosting.Seq.Tests (2 hops), Aspire.Hosting.SqlServer.Tests (2 hops), Aspire.Hosting.Testing.Tests (2 hops), Aspire.Hosting.Valkey.Tests (2 hops), Aspire.Hosting.Yarp.Tests (2 hops), Aspire.Playground.Tests

🧪 tests/Aspire.Hosting.Tests/Dcp/DcpExecutorTests.cs (changed test)
→ 1 directly: Aspire.Hosting.Tests
→ 3 via the project graph: Aspire.Hosting.Blazor.Tests, Aspire.Hosting.Dotnet.Tests, Aspire.Hosting.Radius.Tests

📦 affected project Aspire.Hosting.Dotnet
→ 2 tests: Aspire.Cli.EndToEnd.Tests, Aspire.Templates.Tests

📦 affected project Aspire.Hosting
→ 1 test: Aspire.Cli.EndToEnd.Tests

🧪 tests/Aspire.Hosting.Tests/Dcp/TestKubernetesService.cs (changed test)
→ 1 directly: Aspire.Hosting.Tests

Job reasons

Job Triggered by
cli-starter-validation affected project Aspire.Hosting.PostgreSQL
extension-e2e • src/Aspire.Hosting/Dcp/DcpExecutor.cs
• affected project Aspire.Hosting
polyglot • affected project Aspire.Hosting
• affected project Aspire.Hosting.Azure.Provisioning.KeyVault
typescript-api-compat affected project Aspire.Hosting

Selection computed for commit 3f0bc44.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain with the change to stop handling for failed-to-start executables.

No open issues 🎉

What changed in this PR

Fixes executable stop handling in Aspire hosting so rebuild commands do not fail when an executable enters FailedToStart. This addresses the stop failure separately from the build-count issue handled by #21427.

Changes:

  • Treats FailedToStart executables as stopped; container handling remains unchanged.
  • Updates the Kubernetes test fake to preserve that terminal state.
  • Adds a regression test verifying that stopping a failed-to-start executable succeeds.
File Description
tests/​Aspire.Hosting.Tests/​Dcp/​TestKubernetesService.cs Preserves FailedToStart when an executable is stopped.
tests/​Aspire.Hosting.Tests/​Dcp/​DcpExecutorTests.cs Adds regression coverage for stopping a failed-to-start executable.
src/​Aspire.Hosting/​Dcp/​DcpExecutor.cs Recognizes FailedToStart as a completed stop condition.

Review effort: Balanced

Copy link
Copy Markdown
Collaborator Author

The first CI attempt at head 3f0bc44b has three failures. None of them touch the changed code path (DcpExecutor.StopResourceAsync for Executables):

  • Hosting-1 (windows-latest) (job 123990775524): DistributedApplicationTests.BeforeResourceStartedEvent_NotFiredForExplicitStartOnInitialCreation times out in app.StopAsync (TaskCanceledException). The job log has no Stopping resource lines, so StopResourceAsync never ran. This is a pre-existing main flake: on main between Oct 23 and Oct 30 it caused 17 of the 20 Hosting-1 (windows-latest) failures across 51 failed first-attempt runs. One example is job 123897944326.
  • Hosting.MongoDB (ubuntu-latest) (job 123990783084): MongoDbReplicaSetFunctionalTests replica-set initialization failed with MongoNodeIsRecoveringException (InterruptedDueToReplStateChange).
  • Cli E2E KubernetesDeployWithHelmChartTests.DeployK8sWithExternalHelmChart (job 123995477969): a Hex1b WaitUntil timed out during the Kubernetes Helm deploy.

I'm keeping this PR as a draft until the required CI is green.

🤖 Posted by AspireAgent (AI Agent)

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

Copy link
Copy Markdown
Collaborator Author

Closing this as superseded. Its fix landed on main in #21597 (1b26403): DcpExecutor.StopResourceAsync now treats ExecutableState.FailedToStart as stopped, and a regression test covers it. #21597 also handles ContainerState.FailedToStart.

🤖 Posted by AspireAgent (AI Agent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants