-
Notifications
You must be signed in to change notification settings - Fork 62
Use COMPLEMENT_HOSTNAME_RUNNING_COMPLEMENT config for ExtraHosts in Docker
#825
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
Conversation
| // Note: this feature of docker landed in Docker 20.10, | ||
| // see https://github.com/moby/moby/pull/40007 | ||
| extraHosts = []string{"host.docker.internal:host-gateway"} | ||
| extraHosts = []string{fmt.Sprintf("%s:host-gateway", cfg.HostnameRunningComplement)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change.
Using the value from our config instead of hard-coded.
Reference: COMPLEMENT_HOSTNAME_RUNNING_COMPLEMENT -> HostnameRunningComplement
| HostnameRunningDocker = "localhost" | ||
| // HostnameRunningComplement is the hostname of Complement from the perspective of a Homeserver. | ||
| HostnameRunningComplement = "host.docker.internal" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing as these seem unused
| // Note: this feature of docker landed in Docker 20.10, | ||
| // see https://github.com/moby/moby/pull/40007 | ||
| extraHosts = []string{"host.docker.internal:host-gateway"} | ||
| extraHosts = []string{fmt.Sprintf("%s:host-gateway", cfg.HostnameRunningComplement)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this regressed in #389
(previously, we we're using HostnameRunningComplement here)
devonh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me. Hopefully this is transparent to people and nobody sees any breakages in their own setups.
|
Thanks for the review @devonh 🐊 |
Use
COMPLEMENT_HOSTNAME_RUNNING_COMPLEMENTconfig forExtraHostsin DockerThis isn't spawning from any problem (and I'm not making this change to fix anything specifically although probably fixes podman support). Internally, at Element, we're playing around with adding our own self-hosted GitHub runners and this piece of code was pointed out. I noticed it hard-coded
host.docker.internalinstead of using the config (fromCOMPLEMENT_HOSTNAME_RUNNING_COMPLEMENT) which seemed a bit suspect.It looks like this regressed in #389 (previously, we we're using
HostnameRunningComplementhere)Pull Request Checklist