Skip to content

test: Refactor disconnect handling integration tests #424

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

test: Refactor disconnect handling integration tests #424

wants to merge 2 commits into from

Conversation

@dave-tucker
Copy link
Collaborator

@dave-tucker dave-tucker commented Jul 25, 2025 •
edited
Loading

DRY up the disconnect handling test logic.
Fix a data race in the MonitorCondition tests.

@dave-tucker dave-tucker requested a review from jcaamano July 25, 2025 15:55
@dave-tucker
Copy link
Collaborator Author

dave-tucker commented Jul 25, 2025 •
edited
Loading

The refactor for to DRY up the tests and fix a data race could be removed if required - but this has affected CI stability and the ability to iterate on this locally.

This is related to #368 but with less code churn.
I don't think we need to set socket options on the TCP socket or set a Deadline on the underlying connection - use of the context Deadline should hopefully be sufficient in this case.

I think we can revisit #368 afterwards to extract any other relevant changes to the inactivity handling logic.

EDIT: It was pointed out to me that:
a. The context deadline won't work - as discussed in #368
b. That this doesn't fix the issue with echo waiting on the rpcMutex as recently reported in #391

This PR is now just the test refactor.

@dave-tucker
test: Enable TestWithReconnect
0ea2461 
Signed-off-by: Dave Tucker <[email protected]>
@dave-tucker
test: refactor common reconnect tests
cb991fa 
The TestWithInactivity and TestWithReconnect logic is pretty much
identical. Move the common code into testReconnect. This also
removes the 2 ovsClients. Only a single client is used now.
A test may change the options at the beginning of the test.

Adds a TestWithFastInactivity variant with an aggressive timer
to try and reproduce ovn-kubernetes#391.

This also fixes data races in the MonitorCondition tests.

Signed-off-by: Dave Tucker <[email protected]>
@dave-tucker dave-tucker changed the title Attempt to fix deadlock test: Refactor disconnect handling integration tests Jul 25, 2025
@dave-tucker dave-tucker mentioned this pull request Aug 15, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcaamano jcaamano Awaiting requested review from jcaamano

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dave-tucker