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

Revert and fix Unix socket location #1417

Merged
merged 3 commits into from
May 10, 2023
Merged

Conversation

juanfont
Copy link
Owner

@juanfont juanfont commented May 3, 2023

Reverts #1414

@juanfont juanfont requested a review from kradalby as a code owner May 3, 2023 20:26
@juanfont juanfont changed the title Revert "Revert unix_socket to default value" Revert and fix Unix socket location May 3, 2023
@ohdearaugustin
Copy link
Collaborator

This whole thing broke the installation for most people who use docker. Just changing the default unix_socket for packing with debian. We probably should fix the debian package and not just change the unix_socket location. This is a breaking change.

@juanfont
Copy link
Owner Author

juanfont commented May 4, 2023

This whole thing broke the installation for most people who use docker. Just changing the default unix_socket for packing with debian. We probably should fix the debian package and not just change the unix_socket location. This is a breaking change.

Any idea how to fix both things?

@ohdearaugustin
Copy link
Collaborator

I guess there are three options:

  1. We change the the deb package build process, so it will also create the socket in /var/run/headscale.socket. I would still need to check if that is possible. This folder is created in the postinstall.sh of the package.

  2. We add the /var/run/headscale/ folder directly in the docker container.

  3. We only update the documentation for the install with docker that people need to change the unix_socket path in their setup to /var/run/headscale.socket

I personally prefer the first option as we would revert an unintentional breaking change of a default value. @kradalby Do you have any opinion on that? Or was this change necessary and I don't see why it was necessary?

@ohdearaugustin ohdearaugustin mentioned this pull request May 6, 2023
6 tasks
config.go Outdated Show resolved Hide resolved
@juanfont juanfont force-pushed the revert-1414-topic/fix_unix_socket branch from f5899dc to f90ebb8 Compare May 10, 2023 10:16
@juanfont juanfont merged commit 9f38125 into main May 10, 2023
@juanfont
Copy link
Owner Author

@ohdearaugustin we are going for option 2, as it seems the easiest and most compatible.

Milokita added a commit to Milokita/headscale that referenced this pull request May 11, 2023
update the example config to match juanfont#1417
@Milokita Milokita mentioned this pull request May 11, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants