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

readme: add support for docker-compose #13

Closed
wants to merge 1 commit into from
Closed

readme: add support for docker-compose #13

wants to merge 1 commit into from

Conversation

reeseovine
Copy link
Contributor

Fixes #6

@vercel
Copy link

vercel bot commented Mar 9, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @MaxLeiter on Vercel.

@MaxLeiter first needs to authorize it.

@MaxLeiter
Copy link
Owner

@katacarbix what if you add .env.local to the .dockerignore in server? Do you think that could fix the problem?

@reeseovine
Copy link
Contributor Author

The build fails with:

$ next build
undefined/:path*
warn  - You have enabled experimental feature(s).
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

`destination` does not start with `/`, `http://`, or `https://` for route {"source":"/api/:path*","destination":"undefined/:path*"}


Error: Invalid rewrite found
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The command '/bin/sh -c yarn build' returned a non-zero code: 1
ERROR: Service 'client' failed to build : Build failed

because obviously 'undefined' isn't a valid backend URL :)

I've been mulling over it and I think the simplest solution would be to, instead of rewriting the URL on build, make a really simple server which serves the frontend for all requests except for ones to /api/* which would be forwarded to the backend.

Maybe I'm overcomplicating it? Or maybe Next has a feature that does exactly that, I don't know.

@MaxLeiter
Copy link
Owner

MaxLeiter commented Mar 10, 2022

Next has support for a custom server (so you can use with it with e.g. express), but I prefer the front-end being completely separate so it can be served via providers like Vercel and Netlify. We could go with .env.docker, and use the dotenv package to parse it only if another env var is set (by docker)?

@reeseovine
Copy link
Contributor Author

So I finally have a working solution which involves a separate nginx container. It assumes api links have been rewritten to /api/... by setting API_URL= (empty value).

I also commented the CORS header setting just for testing; I think in production we should use a trusted_proxies header instead.

@azlee91
Copy link

azlee91 commented Mar 12, 2022

Can you try with API_URL=http://server:3000 in your .env.local? I was able to sign up in my own instance using that without changing the CORS header and I see communication with the backend happening just fine . I use nginx proxy manager so I didn't spin up a separate nginx with this docker-compose

@reeseovine
Copy link
Contributor Author

reeseovine commented Mar 12, 2022

@azlee91 Thank you for the suggestion, but that doesn't work for me. Is there anything you changed outside of .env.local?

I think the issue is that when the client sends a request to http://server:3000, it won't work if the browser is outside of the network that the server is in so you need a way to expose it to the wider internet.

@azlee91
Copy link

azlee91 commented Mar 12, 2022

@katacarbix I just copied your docker-compose and changed the .env.local to use the http://server:3000 after I saw your last comment, docker-compose up and saw it working fine. But also, like I said, I did use nginxproxymanager to proxy my subdomain to the exposed port.

Here's my docker-compose.yml, I removed a couple things I thought were not needed

version: '3.8'

services:
  server:
    build: ./server
    restart: unless-stopped
    user: 1000:1000
    environment:
      - "PORT=3000" # probably not needed
      - "JWT_SECRET=change_me!"
  client:
    build: ./client
    restart: unless-stopped
    user: 1000:1000
    depends_on:
      - server
    ports:
      - "3748:3001"

@reeseovine
Copy link
Contributor Author

reeseovine commented Mar 12, 2022

Ohh I see what you mean. So you're using nginx proxy manager to fill the role that the proxy container currently plays?

I use NPM too but I feel like it's easier to leave roles like this up to dedicated containers, even if it does consume slightly more resources :)

I also think it makes more sense to provide an all-in-one solution in this repo that works out of the box, rather than expecting admins to know how to connect the two themselves.

@MaxLeiter
Copy link
Owner

MaxLeiter commented Mar 12, 2022

After some research, I guess this issue may be inaccurately titled. @katacarbix @azlee91, should a docker compose even be in the repo? Or should I just provide two docker images and let the sysadmin figure out connecting them? (Eventually, there will be docs to guide them). Docker-composes seem more system specific than I thought

@reeseovine
Copy link
Contributor Author

@MaxLeiter I suppose not, it's just a template to help you get started, and I think most of the time just putting it in the readme is sufficient enough.

I can go ahead and do that if you'd like!

@azlee91
Copy link

azlee91 commented Mar 12, 2022

@MaxLeiter I agree with @katacarbix, a minimum working example in say the "getting started" section would help people who want to just get it up and running to try it out

@MaxLeiter
Copy link
Owner

@katacarbix that’d be great 🙏

@reeseovine
Copy link
Contributor Author

Done! There is still the client env variables issue but that's a problem for another day.

@MaxLeiter
Copy link
Owner

Sorry for the delay @reeseovine — i wanted to solve the env vars before merging but I’m now on vacation. I’ll get to this PR first thing when I’m back next week. Thanks again for your contribution 🙏

@reeseovine
Copy link
Contributor Author

@MaxLeiter No worries! Enjoy your vacation 😄

@MaxLeiter MaxLeiter changed the title Add support for docker-compose readme: add support for docker-compose Mar 19, 2022
@MaxLeiter
Copy link
Owner

@reeseovine I'm finally taking a swing at this, but am running into a problem: the client relies on the server to be started in order to build. Is it possible to have the server build AND run before client builds?

@reeseovine
Copy link
Contributor Author

@MaxLeiter There's the depends_on property but I don't think it would affect the build order. I'll take a look when I get home.

@MaxLeiter
Copy link
Owner

Yeah that’s what I’ve found, its just for starting afaict

@MaxLeiter
Copy link
Owner

fwiw this is what I have

version: '3.8'
services:
  server:
    build: ./server
    restart: unless-stopped
    user: 1000:1000
    environment:
      - "JWT_SECRET=change_me!" # use `openssl rand -hex 32` to generate a strong secret
      - "SECRET=secret"
    expose:
      - 3000
  client:
    build: ./client
    restart: unless-stopped
    user: 1000:1000
    environment:
      - "API_URL=http://localhost:3000"
      - "SECRET=secret"
    expose:
      - 3001
    depends_on:
      server:
        condition: service_completed_successfully

@MaxLeiter
Copy link
Owner

new docker-compose, but still running into issues with networking during build time:

version: '3.4'
services:
  server:
    build: ./server
    restart: unless-stopped
    user: 1000:1000
    environment:
      - JWT_SECRET=change_me! # use `openssl rand -hex 32` to generate a strong secret
      - SECRET_KEY=secret
    expose:
      - 3000
    networks:
      - general
    healthcheck:
      test: [ "CMD", "curl",  "-s", "localhost:3000"]
      interval: 10s
      timeout: 45s
      retries: 10
    container_name: server
  client:
    build:
      context: ./client
      args:
        API_URL: http://server:3000
    restart: unless-stopped
    user: 1000:1000
    environment:
      - API_URL=http://server:3000
      - SECRET_KEY=secret
    ports:
      - "3001:3001"
    networks:
      - general
    depends_on:
      server:
        condition: service_healthy

networks:
  general:
    driver: bridge

@reeseovine
Copy link
Contributor Author

reeseovine commented Mar 31, 2022

Ok so I've made some progress but I still haven't solved the big issue of build/start order. I've brought back the Nginx proxy since the client needs to know the internet-accessible URL and path to the server and this takes care of some of the headache of that.

docker-compose.yml:

version: '3.4'

services:
  server:
    build:
      context: ./server
      network: host
    restart: unless-stopped
    user: 1000:1000
    environment:
      - JWT_SECRET=change_me! # use `openssl rand -hex 64` to generate a strong secret
      - SECRET_KEY=secret
    expose:
      - 3000
    healthcheck:
      test: [ "CMD", "curl",  "-s", "127.0.0.1:3000"]
      interval: 10s
      timeout: 45s
      retries: 10

  client:
    build:
      context: ./client
      network: host
      args:
        API_URL: http://192.168.0.17/server-api
        SECRET_KEY: secret
    restart: unless-stopped
    user: 1000:1000
    environment:
      - API_URL=http://192.168.0.17/server-api
      - SECRET_KEY=secret
    expose:
      - 3001
    depends_on:
      server:
        condition: service_healthy
      proxy:

  proxy:
    image: nginx:alpine
    restart: unless-stopped
    volumes:
      - "./nginx.conf:/etc/nginx/nginx.conf:ro"
    ports:
      - "80:80"

The Nginx proxy needs to have the ip addresses of the server and client because it can't resolve their hostnames when they're down and throws an error. You can get these with docker network inspect drift_default but the stack has to be started beforehand so it's kind of a chicken-and-egg problem.

nginx.conf:

events {}
http {
	server {
		listen 80;
		location / {
			proxy_pass http://172.20.0.3:3001/;
		}
		location /server-api/ {
			proxy_pass http://172.20.0.2:3000/;
		}
	}
}

Here are the commands I've come up with to start this hacky setup:

touch client/.env server/.env
docker-compose up -d server proxy
docker-compose up -d client

Updating requires a bit more:

git pull
docker-compose build server
docker-compose up -d server proxy
docker-compose build client
docker-compose up -d client

Now I'm really not sure this is a shippable solution. I think Docker-compose is just not designed for this kind of setup. I understand Docker is maybe not the primary target and you'd like to have first-class support for Vercel, I get it, but I think we'd need to do some more work on the client-server communication before it's Docker ready.

@MaxLeiter
Copy link
Owner

MaxLeiter commented Mar 31, 2022

@reeseovine I think the solution is just to admit defeat and provide two docker-composes, and treat the client and server as two independent services: drift-client and drift-server.

@MaxLeiter
Copy link
Owner

Closing due to #75

Thanks a lot for your efforts here, @reeseovine! I'm excited to finally have this on Docker

@MaxLeiter MaxLeiter closed this Apr 13, 2022
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.

Create a docker-compose
3 participants