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

fix(headless-client): clean up and exit gracefully when on_disconnect called #4785

Merged
merged 7 commits into from
Apr 25, 2024

Conversation

ReactorScram
Copy link
Collaborator

@ReactorScram ReactorScram commented Apr 25, 2024

Calling std::process::exit won't let the DNS deactivation code runs. For some control methods (systemd-resolved) this doesn't matter. For etc-resolvconf and Windows, we are responsible for cleaning up DNS.

Tasks

Edit tasklist title
Beta Give feedback Tasklist Tasks, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Replicate the issue
    Options
  2. Fix it
    Options
  3. Remove the fault injection code
    Options

Closes #4784

Copy link

vercel bot commented Apr 25, 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 Apr 25, 2024 10:34pm

@github-actions github-actions bot added the kind/bug Something isn't working label Apr 25, 2024
Copy link

github-actions bot commented Apr 25, 2024

Terraform Cloud Plan Output

Plan: 15 to add, 14 to change, 36 to destroy.

Terraform Cloud Plan

@ReactorScram ReactorScram self-assigned this Apr 25, 2024
@ReactorScram ReactorScram added the area/linux_client Linux client label Apr 25, 2024
@ReactorScram ReactorScram marked this pull request as ready for review April 25, 2024 17:10
Copy link

github-actions bot commented Apr 25, 2024

Performance Test Results

TCP

Test Name Received/s Sent/s Retransmits
direct-tcp-client2server 242.9 MiB (-1%) 245.4 MiB (-1%) 544 (+79%)
direct-tcp-server2client 240.5 MiB (+0%) 241.9 MiB (+0%) 375 (+50%)
relayed-tcp-client2server 224.2 MiB (-2%) 224.9 MiB (-2%) 239 (-21%)
relayed-tcp-server2client 236.6 MiB (-0%) 237.4 MiB (-0%) 332 (-26%)

UDP

Test Name Total/s Jitter Lost
direct-udp-client2server 50.0 MiB (+0%) 0.08ms (+93%) 0.00% (NaN%)
direct-udp-server2client 50.0 MiB (-0%) 0.01ms (-26%) 0.00% (NaN%)
relayed-udp-client2server 50.0 MiB (+0%) 0.09ms (+48%) 0.00% (NaN%)
relayed-udp-server2client 50.0 MiB (+0%) 0.01ms (-30%) 0.00% (NaN%)

@thomaseizinger
Copy link
Member

Whilst this may fix the DNS control thing, I think the PR title + description (which will be the commit message when merged) should talk about graceful clean-up or something similar.

Why this fixes DNS control settings is actually pretty incidential due to the way how we do this at the moment (on drop) which IMO is worth calling out.

@ReactorScram ReactorScram changed the title fix(headless-client): Revert DNS control when on_disconnect called fix(headless-client): clean up and exit gracefully when on_disconnect called Apr 25, 2024
Copy link
Member

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Thanks!

I think we may also be able to get rid of the reference to the error? That API is very old and I think we now always have ownership over the error that we pass here.

@ReactorScram ReactorScram added this pull request to the merge queue Apr 25, 2024
Merged via the queue into main with commit a011a44 Apr 25, 2024
134 checks passed
@ReactorScram ReactorScram deleted the fix/revert-dns-on-disconnect branch April 25, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/linux_client Linux client kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(headless-client): Headless Client won't revert /etc/resolv.conf when on_disconnect is called
2 participants