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

build: docker instructions and fixes #6465

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

eatyourgreens
Copy link
Contributor

@eatyourgreens eatyourgreens commented Nov 14, 2024

  • replace the outdated docker-compose command with docker compose in the READMEs.
  • fix AS in Dockerfile.
  • update the Docker instructions in the READMEs.
  • make sure that environment variables are set for local development in the docker-compose.yml files.

Please request review from @zooniverse/frontend team or an individual member of that team.

Linked Issue and/or Talk Post

How to Review

To build and run the monorepo packages from the root directory:

docker compose build
docker compose up -d

That runs production builds of the project app and root app in a container:

Shut down the running services and test the project docker compose config:

docker compose down
cd packages/app-project
docker compose up -d

That runs yarn dev for the project app and starts a development storybook. In theory, local changes to the project app should trigger hot module reloading in the running container:

Shut down the project services:

docker compose down

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
  • FEM works in a mobile browser

- replace the outdated `docker-compose` command with `docker compose` in the READMEs.
- fix `AS` in Dockerfile.
- update the Docker instructions in the READMEs.
- make sure that environment variables are set for local development in the `docker-compose.yml` files.
@coveralls
Copy link

coveralls commented Nov 14, 2024

Coverage Status

coverage: 77.843% (-0.03%) from 77.873%
when pulling 6da098e on eatyourgreens:docker-fixes
into fbab1d8 on zooniverse:master.

@eatyourgreens eatyourgreens mentioned this pull request Nov 14, 2024
9 tasks
@eatyourgreens
Copy link
Contributor Author

eatyourgreens commented Nov 14, 2024

This PR gets the project Docker builds working on a MacBook, but if no one uses Docker to run yarn dev, it might just be best to delete packages/app-project/docker-compose.yml, to be honest.

The top-level /docker-compose.yml allows you to test a production deploy on your local machine before actually deploying to Kubernetes (or to debug a failed production deploy locally, which is probably how Docker will be used locally in practice.)

@eatyourgreens
Copy link
Contributor Author

If you're new to Docker, here's a description of images vs. containers:
https://stackoverflow.com/questions/23735149/what-is-the-difference-between-a-docker-image-and-a-container/26960888#26960888

@eatyourgreens
Copy link
Contributor Author

eatyourgreens commented Nov 14, 2024

Working on this, I saw the the production monorepo image has become really big again. You might need to do another pass on removing unnecessary files from the image. front-end-monorepo_prod here is a copy of the image that gets deployed weekly, and it's grown to over 1GB again. You can inspect image sizes in Docker Desktop, or by running docker images from a terminal.

Sizes for the monorepo dev and production images. Dev is 2.07GB. Production is 1.42GB.

Here's the per-directory sizes from inside a running production container.

 ✔ Network front-end-monorepo_default  Created                                                                                                                  0.0s 
/usr/src # du -sh *
1.6G	node_modules
4.0K	package.json
641.8M	packages
824.0K	yarn.lock
/usr/src # du -sh packages/*
8.0K	packages/app-content-pages
327.3M	packages/app-project
302.3M	packages/app-root
48.0K	packages/lib-async-states
6.5M	packages/lib-classifier
660.0K	packages/lib-content
60.0K	packages/lib-grommet-theme
344.0K	packages/lib-panoptes-js
1.7M	packages/lib-react-components
236.0K	packages/lib-subject-viewers
2.6M	packages/lib-user
48.0K	packages/tools-standard
/usr/src # 

node_modules includes development dependencies, despite the --production flag on the yarn install. That might be a yarn bug.

Copy link
Contributor

@goplayoutside3 goplayoutside3 left a comment

Choose a reason for hiding this comment

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

This is looking good. Thank you for your help in getting Docker sorted out!

I'm able to use docker compose at the root and app-project. I'll push some updates to the docker-compose.yml for app-root too (and its Readme).

I'm also going to run a test deploy to the fe-project-branch just to be safe before approval/merge.

Copy link
Contributor

@goplayoutside3 goplayoutside3 left a comment

Choose a reason for hiding this comment

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

Success 👍 Build run

@eatyourgreens
Copy link
Contributor Author

Success 👍 Build run

🎉

@goplayoutside3 goplayoutside3 merged commit 958e806 into zooniverse:master Nov 14, 2024
8 checks passed
@eatyourgreens eatyourgreens deleted the docker-fixes branch November 14, 2024 17:41
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.

3 participants