Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace health check publisher and scheduler with ResourceHealthCheckService and introduce ResourceReadyEvent. #5870

Merged
merged 24 commits into from
Sep 30, 2024

Conversation

mitchdenny
Copy link
Member

@mitchdenny mitchdenny commented Sep 24, 2024

Description

This PR removes the health check publisher and scheduler and combines it into a single ResourceHealthCheckService which runs in the background doing health checks on a per resource basis.

Fixes #5602
Fixes #5638
Fixes #5871
Fixes #5943

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 (existing tests provide coverage - internal mechanism now relies on this event)
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@mitchdenny mitchdenny self-assigned this Sep 24, 2024
@mitchdenny mitchdenny added this to the 9.0 milestone Sep 24, 2024
@mitchdenny mitchdenny added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Sep 24, 2024
@mitchdenny
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mitchdenny mitchdenny force-pushed the mitchdenny/resource-healthy-event branch from 6393f39 to 6a69d3a Compare September 25, 2024 05:49
@mitchdenny
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mitchdenny
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mitchdenny mitchdenny changed the title ResourceHealthyEvent and suppressing health checks that have become healthy. Replace health check publisher and scheduler with ResourceHealthCheckService and introduce ResourceReadyEvent. Sep 29, 2024
@mitchdenny mitchdenny marked this pull request as ready for review September 30, 2024 03:05
mitchdenny and others added 2 commits September 30, 2024 14:18
Co-authored-by: David Fowler <davidfowl@gmail.com>
Co-authored-by: David Fowler <davidfowl@gmail.com>
Copy link
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

Follow up: Make ResourceHealthCheckService more testable using the time provider and add more logs.

@mitchdenny mitchdenny merged commit 94008ce into main Sep 30, 2024
9 checks passed
@mitchdenny mitchdenny deleted the mitchdenny/resource-healthy-event branch September 30, 2024 07:30
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
2 participants