-
Notifications
You must be signed in to change notification settings - Fork 506
[Docker] Fix networking and startup issues with Docker Compose scripts. Update scripts to align with best practices #4987
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
Conversation
74794ad to
72a13f5
Compare
|
This PR is now fully working for me. I've removed the On Monday, I plan to also look into whether we can add some automatic deployment / basic testing for these Angular Docker images, as that'd help ensure they don't break unexpectedly again. But, this PR could be reviewed without those changes (as those automated deployment/tests won't touch our code, they'd just touch the GitHub actions). NOTE: While I'd like to backport some of these changes to all branches, this PR will only cleanly backport to |
tinsch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! I tested the "Try out DSpace 9" manual on main and can confirm that the frontend did not start on my Mac with Docker-Desktop. With your fix everything started and ran smoothly, both the dev and production version.
I just have some minor questions/comments you could check. Everything else looks good 👍
9593a1e to
0333a10
Compare
950df4a to
b498cad
Compare
1e49c8d to
d3b3d4f
Compare
|
This PR has been updated with a new This PR is now ready to merge. As it's already had another test (besides myself) and has new automated tests, I plan to merge it tomorrow and backport it to all active maintenance branches. |
…mpose files with backend codebase.
…e comments. Use Dockerfile best practices for ENTRYPOINT vs CMD. Switch to "npm ci" which is recommended for Docker environments.
…d and update to "npm ci" (recommended for Docker). Use best practices for ENTRYPOINT vs CMD
…. This allows production mode to fully work again.
…e files. This change in 'docker-compose.yml' fixes the bug with startup of the UI in dev mode. I also tested all other scripts and there's no change in behavior when removing these settings.
…ing the `--poll` flag to serve command.
…cker images/scripts are working properly.
d3b3d4f to
06feb29
Compare
|
Merging with one approval. I'll work on backporting this to all active branches (most of those likely will need to be done manually, as minor changes will be needed for each branch). |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-9_x
git worktree add -d .worktree/backport-4987-to-dspace-9_x origin/dspace-9_x
cd .worktree/backport-4987-to-dspace-9_x
git switch --create backport-4987-to-dspace-9_x
git cherry-pick -x 24329251da035dbac5ed1cb1d492e474894ae20d 4065f06f43c1128ac449ee5e61ba9c4188958116 6ca170f7deca013dd5dafb6a81ae1bb9fc0994e5 3e11b3c048d836a935f5f5edee7f9be157494950 ffc53c33453c25ee58ec14340fd8ae2d61c7ec83 334e5a2f102b9c742b483ec7c1da547f4c02a1a7 d6f3f8f89e56e025dadd2da1aa54f724e0e09e63 cf3f01dbb63b5d535c008ba45c5c3da47bc7464c 06feb29bd71fec38951b9c356d83ff6be9600d46 |
References
Description
This PR updates the existing Dockerfiles and Docker Compose scripts in the following ways:
Dockerfilenpm installwithnpm ciin containers.npm ci= "clean install" and is recommended for Docker imagesUpdated to use(reverted this because it caused errors when testing in GitHub actions)nodeuser instead ofrootENTRYPOINTandCMDper Docker best practices.Dockerfile.distnpm installwithnpm ciin containers.npm ci= "clean install" and is recommended for Docker imagesmax_old_space_size=4096to increase memory to 4GB for build process. (This helped solve out of memory errors when rebuilding this container locally)ENTRYPOINTandCMDper Docker best practices.ttyandstdin_openconfigurations from scripts. This fixed the bug with starting up the UI in developer mode exactly as suggested in this comment. See ffc53c3maindocker-deploystep to our GitHub actions which automatically tests both the production and development images to verify they startup properly. See 06feb29Instructions for Reviewers
Verify it's now possible to run the UI and backend via Docker in either Production mode or Developer Mode.
How to run in Developer Mode
How to run in Production Mode
docker-compose-dist.ymlscript is used here, which is the production mode script)