overhangio / tutor

The Docker-based Open edX distribution designed for peace of mind
https://docs.tutor.overhang.io/
GNU Affero General Public License v3.0
936 stars 442 forks source link

Allow caddy container to connect to multiple networks through patches #935

Open ghassanmas opened 1 year ago

ghassanmas commented 1 year ago

Is your feature request related to a problem? Please describe.

We have a tutor/Open edX installtion on a server which is also as well sharing (services/ docker compose project(s)), we would want the caddy container to be proxy server for all other projects, but however when creating extending the Caddyfile through caddyfile file patch as e.g.

name: caddy_proxy
version: 0.1.0
patches:
  caddyfile: |
    myservice.mydomain.com {
    reverse_proxy  myservice
    }

in order for the above to work something like the following needs to be run:

docker network connect myservice_network tutor_local-caddy-1.

And ecahtime the caddy container would be recreated the above command needs to run, unless we create a patch somwhere here: https://github.com/overhangio/tutor/blob/3b2373f6f14de595200a41966cd27c27369368b5/tutor/templates/local/docker-compose.prod.yml#L19-L27

Probably it would like this: where we add extra patch, called local-docker-compose-caddy-networks

    {% if not ENABLE_HTTPS %}
    networks:
      default:
        # These aliases are for internal communication between containers when running locally
        # with *.local.overhang.io hostnames.
        aliases:
          - "{{ LMS_HOST }}"
          {{ patch("local-docker-compose-caddy-aliases")|indent(10) }}
        {{ patch("local-docker-compose-caddy-networks")|indent(8) }}
    {% endif %}

Would such change be accepted, if yes; (I would open a PR accordingly), if not can you suggest an alternative if possible.

Thank you!.

regisb commented 1 year ago

:thinking: Tutor was really not designed to share services between different Compose projects. I'm not sure I want to open that can of worms by supporting this feature.

Did you consider running another Caddy container outside of Tutor? That second Caddy container would act as a proxy for the other containers. There would be very little performance overhead, as far as I understand.

ghassanmas commented 1 year ago

Yes and acutally that was our first approach (to make tutor caddy run as a proxy only for tutro related services and to run it behind another main proxy that manges all the services).

But we got a weird a bug that is only occurs in chrome evey now and then; The error in dev console was/is Infinite Redirect ERR_TOO_MANY_REDIRECTS . We tried many things to resolve it, and it was uneasy to debug because it's really hard to reproduce.
And if you would do hard refresh in browser; ctrl + Shift R it will resolve it, but then it would come after few days. That's why it was hard to debug.

And we were able to resolve it, by reverting to default tutor proxy mode, that is we let tutor take over the proxy.

regisb commented 1 year ago

I understand. Can we try to resolve this bug that you are facing instead of adding a complex feature to Tutor?

ghassanmas commented 1 year ago

Yes, If you generally don't consdir adding patch for caddy caddy networks, then we have to look for other alternatives.

I would say if possible let's keep it open for now and I would post about it in discuss.openedx.org and see either if other folks have had encountred the same bug or they would find the flexbility of extra networks patches for any reason useful.

regisb commented 1 year ago

An alternative solution which you can start using today is to add your changes to a docker-compose.override.yml file. Did you consider that option?

ghassanmas commented 1 year ago

I just looked into it and here is what I did in order for it to work,

cp docker-compose.prod.yml docker-compose.override.yml

And then inside the new docker-compose.override.yml

... 
services:
   caddy;
     image: someething
     netowrks:
         - my_spesfic_services

networks:  
  default
  my_services:
     external: true

And it worked as expected, let me know if you consdier the approch valid.

Thank you!

regisb commented 1 year ago

Yep, that's pretty much what I had in mind. The only downside that I can see is that the override file is going to cause docker compose to crash in dev mode (tutor dev start) because the caddy service does not exist in dev mode. To address that issue, we should support the existence of a docker-compose.prod.override.yml file. If that's an issue for you, please let me know -- or better: open a PR.

regisb commented 10 months ago

@ghassanmas can we close this, or would you like to open a PR to create a new "docker-compose.prod.override.yml" file?

ghassanmas commented 9 months ago

Yes, I will open a PR for closing this.