Skip to content

Fix frontend and web docker environments #2162

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
+2 −3
Open

Fix frontend and web docker environments #2162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions frontend/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ services:
container_name: dev-nginx
restart: always
volumes:
- ./:/usr/src/frontend
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? That way you are missing all other assets

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assets are used by the frontend container to compile the final js and wasm files. The web container lacks the dependencies to re-compile so it doesn't make much sense having them there.

nginx isn't using them neither so I figured it would be better to remove that. Does the UI access that path in any scenario? Haven't noticed any issues while testing it

- ../web:/usr/src/web
- ./static/frontend:/usr/src/web/static/frontend
- ../web/nginx.conf:/etc/nginx/nginx.conf
- ../web/coordinators/:/etc/nginx/conf.d/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/etc/nginx/conf.d/ doesn't exist in the container, it just created an empty folder in the host.

ports:
- 8888:80
2 changes: 1 addition & 1 deletion web/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ services:
restart: always
volumes:
- ./:/usr/src/web
- ../frontend/static/frontend:/usr/src/web/static/frontend
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running npm run dev should create the specific files required for web in /web, and thatś important because every index.html file is different: https://github.com/RoboSats/robosats/blob/main/frontend/webpack.config.ts#L122

Copy link
Contributor Author

@aftermath2 aftermath2 Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that be done in the Dockerfile instead of requiring installing dependencies on the host and manual compilation? I would expect the docker environment to set everything up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's inside the /usr/src/web folder after running the web docker compose with no pre-existing volumes

/usr/src/robosats # ls -la /usr/src/web/static/
total 60
drwxr-xr-x    7 root     root          4096 Aug 11 20:48 .
drwxr-xr-x    4 1000     1000          4096 Aug 11 20:48 ..
drwxr-xr-x    6 root     root          4096 Aug 11 20:48 assets
drwxr-xr-x    3 root     root          4096 Aug 11 20:48 css
drwxr-xr-x    2 root     root          4096 Aug 11 20:48 css_pro
drwxr-xr-x    4 root     root          4096 Aug 11 20:48 federation
-rw-r--r--    1 root     root         21333 Aug 11 20:48 federation.json
-rw-r--r--    1 root     root           485 Aug 11 20:48 lnproxies.json
drwxr-xr-x    3 root     root          4096 Aug 11 20:48 locales
-rw-r--r--    1 root     root          1882 Aug 11 20:48 thirdparties.json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just read this comment on the web Dockerfile

robosats/web/Dockerfile

Lines 5 to 6 in ef4b96c

# Needs a copy or symlink of /frontend/static in /nodeapp/static
# Github client release workflow copies /frontend/static here

Which is essentially the same as the bind mount I added. I think a bind mount is better because it is handled by Docker and requires no host changes

Copy link
Member

@KoalaSat KoalaSat Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I think we need to rethink the whole thing. Maybe increase the picture and spot a definitive solution, I think that organically we have been overlapping functionalities with time

Copy link
Contributor Author

@aftermath2 aftermath2 Aug 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I would leave the mainnet environment (root docker-compose.yml) aside for now and focus on testing environments (mainly docker-tests.yml), to not mess up with anyone running the former in production.

All the other docker-compose files aren't necessary in my opinion, except perhaps a frontend-only environment to spin up the UI easily. If someone needs a custom setup he could create it using the individual component images.

- ./nginx.conf:/etc/nginx/nginx.conf
- ./coordinators/:/etc/nginx/conf.d/
ports:
- 8080:80