Possible fix for port exhaustion during integration tests#1719
Closed
WhitWaldo wants to merge 3 commits intodapr:masterfrom
Closed
Possible fix for port exhaustion during integration tests#1719WhitWaldo wants to merge 3 commits intodapr:masterfrom
WhitWaldo wants to merge 3 commits intodapr:masterfrom
Conversation
Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
There was a problem hiding this comment.
Pull request overview
This PR attempts to reduce flaky CI integration test failures caused by host-port collisions (and resulting gRPC connectivity issues) by introducing TCP port reservations when pre-assigning Dapr sidecar ports in the app-first startup flow.
Changes:
- Add
PortReservation+PortUtilities.ReserveTcpPort()to temporarily reserve host TCP ports. - Update app-first startup (
DaprTestApplicationBuilder.BuildAndStartAsync) to reserve HTTP/gRPC ports before configuring the harness/app, then release reservations prior to initializing the harness. - Re-implement
GetAvailablePort()on top ofReserveTcpPort().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Dapr.Testcontainers/Common/Testing/DaprTestApplicationBuilder.cs | Uses port reservations when pre-assigning Dapr HTTP/gRPC ports in app-first mode; adds cleanup logic. |
| src/Dapr.Testcontainers/Common/PortUtilities.cs | Introduces reservable TCP ports and updates GetAvailablePort() to use the reservation mechanism. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+156
to
162
| // Release port reservations just before daprd starts to minimize collisions. | ||
| httpReservation.Dispose(); | ||
| grpcReservation.Dispose(); | ||
| httpReservation = null; | ||
| grpcReservation = null; | ||
|
|
||
| await harness.InitializeAsync(); |
Comment on lines
+131
to
+133
| // Pre-assign ports so the app knows where Dapr will be (avoid collisions) | ||
| httpReservation = PortUtilities.ReserveTcpPort(); | ||
| do |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Whit Waldo <whit.waldo@innovian.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Frequently, integration tests fail during CICD in GitHub due to ports not being assigned exclusively, leading to the tests failing because of a lack of gRPC connectivity. This is an attempt to remedy that issue.
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #[issue number]
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: