Skip to content

Non-Docker Host #1100

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Non-Docker Host #1100

wants to merge 3 commits into from

Conversation

CWempe
Copy link

@CWempe CWempe commented Mar 18, 2018

This PR adds support to generate the config (and certificates) for hosts that are not docker containers.

Useful if you want to proxy to another physical machine.

Just start a dummy container with the new UPSTREAM_NAME environment variable, because nginx-proxy and docker-gen cannot detect the correct server address if it is not a connected container.

I tried the local test before I created this PR, but it failed completely. 😞

2 passed, 12 xfailed, 113 error in 57.40 seconds

However I don't think this is because of my changes, since it faild with the master branch, too.

@pho3nixf1re
Copy link

I currently need this functionality. Integrating a patch for now.

@pho3nixf1re
Copy link

pho3nixf1re commented Mar 18, 2018

Any idea how this could be used in conjunction with the Let's Encrypt companion integration? Or would it JustWork™? Or I could just RTFM 😊

@Taznc
Copy link

Taznc commented Apr 20, 2018

This works great. I manually patched my template file but this gets overwritten when updated. Is there any way to do this automatically or is this going to be merged into the build? Thank you!

@pho3nixf1re
Copy link

I commented out the line that automatically pulls the latest template. I think it would be good to provide that url as an ENV variable so that way we can override it with our own. I've got a couple other customizations I've made to it as well.

@bkraul
Copy link

bkraul commented Jul 24, 2018

This is much needed. Please merge!

@bkraul
Copy link

bkraul commented Jul 24, 2018

In the meantime, this project does the job. However, I still think this should be a feature of jwilder/nginx-proxy.

If it could be so that we don't have to use a dummy container, that would be great (by maybe using an array of domains=>upstreams in an ENV var, or even a file somewhere. Just a thought ;).

@Taznc
Copy link

Taznc commented Nov 13, 2018

Is there any way to get this merged or a feature like this added? I have set this manually multiple times but it gets overwritten. Thank you! @jwilder

@jolly-jump
Copy link

Please, please, I'm all for merging this one

@jolly-jump jolly-jump mentioned this pull request Jan 4, 2019
@jolly-jump
Copy link

Hi @CWempe

BTW, if the merge won't be attractive because it seems you needed a dummy-container, you could point out, that any container would do. e.g. I reuse a "python:3-slim" container that I use anyway and added your "command: tail -f /dev/null" to the docker-compose file. This seems to do the job just as well.
Thanks for your solution anyway.

@BNolet
Copy link

BNolet commented Jan 19, 2019

Did this ever get merged? Will it?

@hanscees
Copy link

please merge this one?

@sjdrc
Copy link

sjdrc commented Oct 18, 2019

Where is this PR currently at? This functionality is super useful and will allow us to just use this one proxy instead of having to maintain this and apache2

@radoslav-stefanov
Copy link

I am looking forward this feature too. Currently I use a workaround by editing nginx.tmpl to be able to generate my list of upstreams.

@bkraul
Copy link

bkraul commented Jan 23, 2020

Hey man is it possible you could share an example for your tmpl workaround? Also I doubt jwilder will be doing anything about this. Been waiting on this PR for ages

@radoslav-stefanov
Copy link

radoslav-stefanov commented Jan 24, 2020

Hey man is it possible you could share an example for your tmpl workaround? Also I doubt jwilder will be doing anything about this. Been waiting on this PR for ages

It is very ugly lol, but does the job for what I want.

Basically I used @CWempe's template and I just added another UPSTREAM_NAME variable and then parse it. Should be around line 20.

{{ if .Container.Env.UPSTREAM_NAME }}
                        {{ if .Container.Env.VIRTUAL_PORT }}
                                server {{ .Container.Env.UPSTREAM_NAME }}:{{ .Container.Env.VIRTUAL_PORT }};
                        {{ else }}
                                server {{ .Container.Env.UPSTREAM_NAME }};
                                server {{ .Container.Env.UPSTREAM_NAME2 }};
                        {{ end }}
{{ else if .Network.IP }}

I am too lazy to learn how Go templating works. Maybe someone can add a check for UPSTREAM_NAMES if it is defined (similar to what we have now) and loop over it. Separated by space or comma.

In bash that would be something like:

UPSTREAMS="192.168.10.2 192.168.10.3"

for upstream in $UPSTREAMS; do
  server $upstream\;
done

@mfenniak
Copy link

mfenniak commented Jul 25, 2020

This PR looks really handy for this situation. But in the absence of it, here's a workaround... create a docker container that forwards the network traffic to your off-docker service host. I've accomplished this work a socat container like this:

docker create \
  --name=someservicename-forward \
  -p 4455:4455 \
  -e VIRTUAL_HOST=... \
  -e VIRTUAL_PORT=4455 \
  -e LETSENCRYPT_EMAIL=... \
  -e LETSENCRYPT_HOST=... \
  -e LETSENCRYPT_ACCOUNT_ALIAS=... \
  alpine/socat tcp-listen:4455,fork,reuseaddr tcp-connect:10.1.1.156:8123

nginx-proxy then treats this container as an upstream, which actually sends all network traffic to 10.1.1.156:8123 by a socat command.

In the absence of a merge of this PR, or similar functionality, this seemed like a nice 'compatible' way to get everything working without much hacking.

@f403 f403 mentioned this pull request Aug 9, 2020
@spfuture
Copy link

@mfenniak Your solution pretty, but have overhead at CPU load, app (alpine/socat) container, and traffic load.
Your traffic way scheme: nginx-proxy -> alpine/socat -> real_host_with_app

Solution by @CWempe nginx-proxy -> real_host_with_app its much better.

@gustavogmoraes
Copy link

In the meantime, this project does the job. However, I still think this should be a feature of jwilder/nginx-proxy.

If it could be so that we don't have to use a dummy container, that would be great (by maybe using an array of domains=>upstreams in an ENV var, or even a file somewhere. Just a thought ;).

Tried using the mikesplain/nginx-proxy-pass-docker but when I try to connect to another physical machine on my local network I get "*378 no live upstreams while connecting to upstream", any ideas?

@bkraul
Copy link

bkraul commented Jun 19, 2021

@gustavogmoraes Are you putting the appropriate port to listen to the physical machine?

i.e.

docker run -d -p 80:80 -e TARGET_SERVER=192.168.8.15:8080 mikesplain/nginx-proxy-pass

Also I believe you have to use IP addressing to reference the remote machine. Maybe I'm wrong tho...been a while. I use traefik now, which does this painlessly.

@buchdag
Copy link
Member

buchdag commented Jun 19, 2021

Could the questions about mikesplain/nginx-proxy-pass-docker could be asked on this project repository rather than here ?

@bkraul
Copy link

bkraul commented Jun 19, 2021

@buchdag sure. Or this PR could have been merged and we wouldn't need to speak about the other project repository 🙂 ...

@buchdag
Copy link
Member

buchdag commented Jun 20, 2021

@bkraul Both docker-gen and nginx-proxy stated scope was always to generated config file from / proxy to Docker container.

While I understand this is a useful and wanted feature, it won't be implemented using zombie containers.

@buchdag buchdag added kind/feature-request Issue requesting a new feature status/pr-needs-tests This PR needs new or additional test(s) labels Jun 20, 2021
@jolly-jump
Copy link

jolly-jump commented Nov 26, 2022

@bkraul Both docker-gen and nginx-proxy stated scope was always to generated config file from / proxy to Docker container.

While I understand this is a useful and wanted feature, it won't be implemented using zombie containers.

Hi @buchdag ,
I don't understand why you call it "zombie containers"? You can use any container, it runs, maybe uses up some CPU-cycles initially and then does nothing. So what?
The upside is: I can use acme-companion to have valid certificates in no time and still forward any traffic to the not-so-intelligent service upstream.

Yes, if I just wanted the traffic to work, I guess that what @amosipov said: #998 (comment) would suffice. But then I don't have the comfort of automatic certificates...

Please reconsider!

BTW: The current patch reduces (as far as I can see) to:

--- a/nginx.tmpl
+++ b/nginx.tmpl
@@ -120,6 +120,15 @@ upstream {{ .Upstream }} {
         # {{ $container.Name }}
         server {{ $containerNetwork.IP }}:{{ $address.Port }};
                         {{ end }}
+                   {{ else if $container.Env.UPSTREAM_NAME }}
+                        {{ $server_found = "true" }}
+                       {{ if $container.Env.VIRTUAL_PORT }}
+        # non-docker: {{ $container.Name }}
+       server {{ $container.Env.UPSTREAM_NAME }}:{{ $container.Env.VIRTUAL_PORT }};
+                       {{ else }}
+        # non-docker: {{ $container.Name }}
+       server {{ $container.Env.UPSTREAM_NAME }};
+                       {{ end }}
                     {{ else if $containerNetwork }}
         # {{ $container.Name }}
                         {{ if $containerNetwork.IP }}

@buchdag buchdag added the scope/configurable-upstream Issue or PR related to non dockerized / configurable upstream label Dec 2, 2022
@buchdag buchdag added type/feat PR for a new feature and removed kind/feature-request Issue requesting a new feature labels Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope/configurable-upstream Issue or PR related to non dockerized / configurable upstream status/pr-needs-tests This PR needs new or additional test(s) type/feat PR for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.