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

Docker Setup very incomplete #1262

Open
2 tasks done
darioackermann opened this issue Oct 27, 2023 · 3 comments
Open
2 tasks done

Docker Setup very incomplete #1262

darioackermann opened this issue Oct 27, 2023 · 3 comments

Comments

@darioackermann
Copy link

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

(irrelevant for this issue)

Expected Behavior

  • When setting up the local client docker, when browsing the docker IP, the client should be displayed
  • The client should connect to a (through env variables) specificable API

Actual Behavior

  • When browsing the exposed client, the default NGINX "it works" page is displayed
    • NGINX points by default to /usr/share/, although frontend files are in /var/www,
  • The client blindly tries to connect /api/ on the same host.

To Reproduce

  • Run the client docker image without any additonal settings and expose port 80
  • Browse port 80 in your browser: You will see an "nginx - it works!" page

Currently applied workaround

Easy fix:

docker-compose.yml

  carbon-api:
    image: cloudcarbonfootprint/api
    restart: unless-stopped
    env_file:
      - services/ccf/.env

  carbon-client:
    image: xxx.dkr.ecr.eu-central-1.amazonaws.com/xxx
    build:
      context: ./services/ccf/
    restart: unless-stopped
    environment:
      - VIRTUAL_HOST=ccf.mydomain.com

ccf/Dockerfile

FROM cloudcarbonfootprint/client
COPY default.conf /etc/nginx/conf.d/default.conf

default.conf

server {
  listen        80;
  listen   [::]:80;
  location / {
    root    /var/www/;
    index   index.html;
  }
  location /api {
    rewrite     ^(.*) $1 break;
    proxy_pass  http://carbon-api:4000/api/;
  }
}

I believe that this could easily be fixed as right now the docker images "as-is" are unusable.
A simple rewriting done by nginx (as provided above) could be easily included in the docker images.

@darioackermann
Copy link
Author

Also, we should make an effort to upgrade to node18, as it seems to fix the graph bug: #1069

@4upz
Copy link
Member

4upz commented Nov 6, 2023

Hi @darioackermann, thanks for opening this issue and sorry that you're running into issues with the Docker containers.

Can you confirm if you were running into the nginx error after cloning the repo and using the default Docker compose files? Or were you using a different method to spin up the containers?

Additionally, would you be open to creating a PR with your proposed changes?

For added context, the default docker-compose file and the corresponding dockerfiles are meant to serve as templates, that should be enough to run out of the box but will require some customization if you wish to tailor it to for specific use. In some of your suggestions, I see there are specific references to what looks like an ECR link for the client image (which should be hosted on Docker Hub) and a custom domain name, which may not be the best to include within the default file.

@4upz
Copy link
Member

4upz commented Nov 6, 2023

Also, we should make an effort to upgrade to node18, as it seems to fix the graph bug: #1069

As for the Node upgrade, we are currently testing and working on this

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

No branches or pull requests

2 participants