Skip to content
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

test(connlib): add RoamClient transition to state machine tests #5060

Closed
wants to merge 1 commit into from

Conversation

thomaseizinger
Copy link
Member

@thomaseizinger thomaseizinger commented May 21, 2024

Firezone needs to work seemlessly despite the user roaming networks. We can incorporate this property into our state machine tests with a RoamClient transition. This updates the client's IPv4 and IPv6 socket to a new pair.

In these tests, we treat all unhandled packets as a bug. Unfortunately, a client cannot immediately detect that it roamed networks (we use BINDING requests to all our relays for that). Prior to it detecting which interfaces are now available and which are gone, it may still send outdated srflx candidates to the gateway which will prompt the gateway to send packets to that endpoint. Thus, we have to add an exception to the "all unhandled packets are a bug"-check and simply discard packets to a client's old sockets.

Depends-On: #5080.
Depends-On: #5077.
Depends-On: #5079.
Depends-On: #5049.

Copy link

vercel bot commented May 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
firezone ⬜️ Ignored (Inspect) Visit Preview May 22, 2024 11:31pm

@github-actions github-actions bot added the kind/test PRs that are focused on increasing test coverage label May 21, 2024
Copy link

github-actions bot commented May 21, 2024

Terraform Cloud Plan Output

Plan: 16 to add, 16 to change, 1 to destroy.

Terraform Cloud Plan

Copy link

github-actions bot commented May 21, 2024

Performance Test Results

TCP

Test Name Received/s Sent/s Retransmits
direct-tcp-client2server 239.1 MiB (-1%) 240.5 MiB (-1%) 433 (+113%)
direct-tcp-server2client 239.9 MiB (+1%) 241.3 MiB (+1%) 435 (+378%)
relayed-tcp-client2server 226.0 MiB (+6%) 226.8 MiB (+6%) 343 (+27%)
relayed-tcp-server2client 235.2 MiB (-1%) 236.1 MiB (-1%) 453 (-25%)

UDP

Test Name Total/s Jitter Lost
direct-udp-client2server 500.0 MiB (+0%) 0.03ms (+36%) 45.11% (+12%)
direct-udp-server2client 500.0 MiB (+0%) 0.01ms (-63%) 22.38% (+16%)
relayed-udp-client2server 500.0 MiB (+0%) 0.03ms (+62%) 56.05% (+1%)
relayed-udp-server2client 500.0 MiB (+0%) 0.12ms (+2167%) 41.37% (+28%)

Base automatically changed from chore/connlib/tests-send-transmit to main May 22, 2024 23:23
github-merge-queue bot pushed a commit that referenced this pull request Jun 25, 2024
Currently, `snownet` tries to be very clever in how it roams
connections. This is/was necessary because we associated DNS-specific
state with a connection. More specifically, the assigned proxy IPs for a
DNS resource are stored as part of a connection with the gateway.

As a result, DNS resources would always break if the underlying
connection in `snownet` failed. This is quite error prone and means,
`snownet` must be very careful to never-ever fail a connection
erroneously. With #5049, we no longer store any important state with a
connection and thus, can implement roaming in much simpler way: Drop all
connections and let the incoming packets create new ones. This is much
more robust as we don't have to "patch" existing state in `snownet` as
part of roaming.

We test this new functionality by adding a `RoamClient` transition to
`tunnel_test`. This ensures roaming works in a lot of scenarios,
including relayed and non-relayed situations as well as roaming between
either of them. As a result, we can delete several of the more specific
test cases of `snownet`.

Depends-On: #5049.
Replaces: #5060.
Resolves: #5080.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/test PRs that are focused on increasing test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant