-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
🌱 Add integration test to avoid manager.Start deadlocks #2418
🌱 Add integration test to avoid manager.Start deadlocks #2418
Conversation
745e5e2
to
3c9b982
Compare
// Read the unexported healthProbeListener field of the manager to get the listener address. | ||
// This is a hack but it's better than using a hard-coded port. |
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.
Is the listening address not exposed in any other way? Could we avoid relying on reflection to get a private value?
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.
I couldn't find another way. The problem is that I don't have access to private fields.
I couldn't put this test in the manager package because of cyclic dependencies (and it's probably also better as an integration test if it's outside of the package).
Happy to change it if there is an alternative. I couldn't find any apart from exporting something in the Manager that we probably don't want to export
7278e60
to
dc04cc1
Compare
dc04cc1
to
3ecb530
Compare
Should be all addressed/answered now. I moved the test below internal. While the _test.go didn't "leak" any exported types the v1/v2 packages did. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer, vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
unrelated flake /retest |
For more context on the deadlock we had in the past:
Fixes #2322