- 
                Notifications
    You must be signed in to change notification settings 
- Fork 715
Harden Docker container runtime health check #10402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR strengthens the container runtime health check by verifying that the Docker daemon is running before checking Docker Buildx availability.
- Adds CheckDockerDaemonAsyncto probe the Docker daemon viadocker container ls -n 1
- Adds CheckDockerBuildxAsyncto validate that Buildx is installed and functioning
- Updates CheckIfRunningAsyncto chain both checks sequentially
Comments suppressed due to low confidence (1)
src/Aspire.Hosting/Publishing/DockerContainerRuntime.cs:134
- Add unit tests for the new CheckDockerDaemonAsyncandCheckDockerBuildxAsyncmethods, covering both successful and failure paths to ensure the health check logic is fully validated.
    public async Task<bool> CheckIfRunningAsync(CancellationToken cancellationToken)
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker buildx version (stderr): {Error}", error); | ||
| logger.LogInformation("docker container ls (stderr): {Error}", error); | 
    
      
    
      Copilot
AI
    
    
    
      Jul 15, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using LogWarning or LogError for stderr output to better reflect that this data represents an error stream rather than informational messages.
| logger.LogInformation("docker container ls (stderr): {Error}", error); | |
| logger.LogWarning("docker container ls (stderr): {Error}", error); | 
| OnOutputData = output => | ||
| { | ||
| logger.LogInformation("docker buildx version (stdout): {Output}", output); | ||
| logger.LogInformation("docker container ls (stdout): {Output}", output); | ||
| }, | ||
| OnErrorData = error => | ||
| { | ||
| logger.LogInformation("docker buildx version (stderr): {Error}", error); | ||
| logger.LogInformation("docker container ls (stderr): {Error}", error); | ||
| }, | 
    
      
    
      Copilot
AI
    
    
    
      Jul 15, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The OnOutputData and OnErrorData logging closures are duplicated in both health-check methods; consider extracting a shared helper to reduce duplication and improve maintainability.
| OnOutputData = output => | |
| { | |
| logger.LogInformation("docker buildx version (stdout): {Output}", output); | |
| logger.LogInformation("docker container ls (stdout): {Output}", output); | |
| }, | |
| OnErrorData = error => | |
| { | |
| logger.LogInformation("docker buildx version (stderr): {Error}", error); | |
| logger.LogInformation("docker container ls (stderr): {Error}", error); | |
| }, | |
| OnOutputData = output => LogProcessOutput("stdout", output), | |
| OnErrorData = error => LogProcessOutput("stderr", error), | 
| Do the same for podman? | 
| /backport to release/9.4 | 
| Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16283710455 | 
Closes #10391.