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.

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

refactor(headless-client): remove "linux-client" alias #4933

Merged
merged 34 commits into from
May 10, 2024
Merged

refactor(headless-client): remove "linux-client" alias #4933

merged 34 commits into from
May 10, 2024

Conversation

ReactorScram
Copy link
Collaborator

@ReactorScram ReactorScram commented May 9, 2024 •
edited

Is this worth it?

Before merging

Edit tasklist title
Beta Give feedback Tasklist Before merging, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Double-check docs and ask Jamil to review
    Options
  2. Would need Brian to review the terraform thing
    Options
  3. Make sure Docker compat isn't broken for existing users (shouldn't be, the image is still just client)
    Options
  4. Decide whether compatibility tests need to pass (if something breaks after merge we can revert this)
    Options

@ReactorScram
proof of concept for installing a system service with WiX
e742f80
@ReactorScram
this one gets the service to run, but it doesn't respond, so Windows …
c83174f 
…gets confused
@ReactorScram
Merge remote-tracking branch 'origin/main' into chore/windows-service…
6d36d31 
…-poc
@ReactorScram
fmt
abc9953
@ReactorScram
this one starts and stops properly
e615f93
@ReactorScram
fix Linux build
b8a8670
@ReactorScram
Merge remote-tracking branch 'origin/main' into chore/windows-service…
4011ca3 
…-poc
@ReactorScram
refactor(gui-client): move known_dirs module into headless-client
618c941 
I'm going to want a well-known dir that the Windows IPC service writes logs to,
and that the Windows GUI can pick them up from.

I don't know how I did this for Linux last week, but it should probably be in
here too.
@ReactorScram
fix Linux build
442e01c
@ReactorScram
Merge branch 'refactor/move-known-dirs' into chore/windows-service-poc
7e17b56
@ReactorScram
Merge branch 'chore/windows-service-poc' of github.com:firezone/firez…
8585e2d 
…one into chore/windows-service-poc
@ReactorScram
DRY log dirs
679a9da
@ReactorScram
typo
606469f
@ReactorScram
fix logging
12edbc1
@ReactorScram
fix stopping, remove debug file
0c77e34
@ReactorScram
opt in to receive shutdown signals
9bc5d5c
@ReactorScram
do a test install of the MSI
6083819
@ReactorScram
fix(windows-client): clear Firezone-specific DNS rules at startup
83f807d 
This has a known gap where theoretically the GUI could sign in while the
service is hung in startup, and then the service would wipe out the GUI's
DNS rules.

The workaround for that would be to restart the GUI, but in practice I think
this is almost impossible, Windows would have to give the service no CPU time
while the user was signing in, then the user would have to immediately open
Firezone before the service got running.

Closes #4899
@ReactorScram
/passive wasn't working in CI
0a35424
@ReactorScram
try to fix CI
d5f58a6
@ReactorScram
you gotta escape those deadly forward slashes
900737b
@ReactorScram
fix test checklist
0f9de54
@ReactorScram
Merge branch 'chore/windows-service-poc' into fix/windows-dns-4899
c654e35
@ReactorScram
Merge remote-tracking branch 'origin/main' into chore/windows-service…
470eb1c 
…-poc
@ReactorScram
Merge branch 'chore/windows-service-poc' into fix/windows-dns-4899
ed5ee6a
@ReactorScram
refactor(headless-client): remove "linux-client" alias
2c62195
@ReactorScram ReactorScram added the area/linux_client Linux client label May 9, 2024
@ReactorScram ReactorScram self-assigned this May 9, 2024
@vercel Vercel
Copy link

vercel bot commented May 9, 2024 •
edited

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 10, 2024 3:27pm

@github-actions github-actions bot added the kind/refactor Code refactoring label May 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024 •
edited

Terraform Cloud Plan Output

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

Terraform Cloud Plan

@github-actions GitHub Actions
Copy link

github-actions bot commented May 9, 2024 •
edited

Performance Test Results

TCP

Test Name Received/s Sent/s Retransmits
direct-tcp-client2server 242.4 MiB (+4%) 244.0 MiB (+3%) 317 (-13%)
direct-tcp-server2client 244.6 MiB (+0%) 245.9 MiB (+0%) 279 (-4%)
relayed-tcp-client2server 226.5 MiB (+0%) 227.2 MiB (+0%) 194 (-35%)
relayed-tcp-server2client 235.6 MiB (+1%) 236.2 MiB (+1%) 312 (-27%)

UDP

Test Name Total/s Jitter Lost
direct-udp-client2server 499.9 MiB (-0%) 0.04ms (-8%) 41.13% (-11%)
direct-udp-server2client 500.0 MiB (+0%) 0.01ms (-74%) 21.70% (+2%)
relayed-udp-client2server 500.0 MiB (+0%) 0.03ms (-43%) 55.85% (-3%)
relayed-udp-server2client 500.0 MiB (+0%) 0.01ms (+3%) 40.30% (-1%)

@ReactorScram ReactorScram requested a review from jamilbk May 9, 2024 15:07
@ReactorScram ReactorScram marked this pull request as ready for review May 9, 2024 15:07
@ReactorScram
Copy link
Collaborator Author

@bmanifold Would this PR break your monitoring VM?

Since there will be a headless Client for Windows in the near future, I'm trying to slowly remove the platform name from the exes. "linux-client" is currently just another name for "headless-client".

@bmanifold
Copy link
Collaborator

@bmanifold Would this PR break your monitoring VM?

Since there will be a headless Client for Windows in the near future, I'm trying to slowly remove the platform name from the exes. "linux-client" is currently just another name for "headless-client".

It looks like you've already updated the place that would need to change for the monitoring VM. As long as the new binary runs in the same way the firezone-linux-client binary ran, I think it should be ok, unless there's something I'm overlooking.

@ReactorScram
Copy link
Collaborator Author

Okay. The thing I said about the explicit subcommand is actually for a different PR I had mixed up.

So this is fine, it's blocking on #4903

@ReactorScram ReactorScram changed the base branch from fix/windows-dns-4899 to main May 9, 2024 17:16
@ReactorScram
Copy link
Collaborator Author

Actually it's not blocked on that PR. Just rebased on main.

@ReactorScram
Copy link
Collaborator Author

@jamilbk should we force this even though the compat tests didn't all pass? I don't think I have that permission

@jamilbk
Copy link
Member

jamilbk commented May 10, 2024

@jamilbk should we force this even though the compat tests didn't all pass? I don't think I have that permission

Yeah if they're expected to fail, and main is expected to stay green, we can merge. Added permission for you

@ReactorScram
Copy link
Collaborator Author

Yeah should be good. I think they're failing because my new code is looking for firezone-headless-client and the old Docker images are built with firezone-linux-client. But it should not affect Docker users because they just call the image client.

If I'm wrong and main breaks I'll open a PR to revert this.

@ReactorScram ReactorScram added this pull request to the merge queue May 10, 2024
Merged via the queue into main with commit 0ad72f0 May 10, 2024
135 checks passed
@ReactorScram ReactorScram deleted the refactor/remove-linux-client branch May 10, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamilbk jamilbk jamilbk approved these changes

@bmanifold bmanifold Awaiting requested review from bmanifold

Assignees

@ReactorScram ReactorScram

Labels
area/linux_client Linux client kind/refactor Code refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ReactorScram @bmanifold @jamilbk