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

Add Docker support and make expressjs listen on ipv4 and ipv6 by default #47

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

SunRed
Copy link

@SunRed SunRed commented Feb 12, 2021

This uses a Dockerfile derived from the parent repo this one is forked off. For a smaller image it also uses the alpine image like @niekcandaele suggested already. I added the storage system dependencies as comments as they should only be installed on demand to save some space. As such, I removed the storage dependencies from the package.json as well because these should only be installed afterwards on demand and otherwise these would be installed by default for the Docker image on build as well. This of course needs documentation so I think I may create an issue for this.
I added a simple docker-compose.yml I am using myself (with postgres) that builds the image and starts the container with a runnable configuration if run with docker-compose up -d. (You may only need to create the config.js before starting the container or simply comment it out).

Lastly, the hostname should be set to null by default to make expressjs listen on both ipv4 and ipv6 to make it also more suitable to be run with Docker. :)

I merged the changes from @niekcandaele's branch, so this closes #8 of course and I suggest closing #9 for this.

niekcandaele and others added 6 commits September 22, 2020 20:18
* Add Dockerfile
* Add docker-compose.yml
* Set default host address to null to make expressjs listen on both ipv4 and ipv6
These packages should be installed on demand and not by default
* Add .dockerignore
* Add docker-compose.yml version number
* Use docker-managed volume instead of directory for files

Co-authored-by: niekcandaele <[email protected]>
@SunRed
Copy link
Author

SunRed commented Feb 12, 2021

I also just pushed some images to https://hub.docker.com/r/sunred/haste-server that can be pulled and run directly.

…deps

* Moving the storage dependencies again into package.json but to the optionalDependencies
* Add build script that excludes the optional dependencies for manual installation of these
* Add a comment in Dockerfile on how to use the minimal build script
@SunRed
Copy link
Author

SunRed commented Feb 13, 2021

Going through the commit history I noticed you did add the document storage client dependencies again at some point to allow for an easier default installation process. I think it is indeed a better practice to install all dependencies by default and allow for an optional "minimal" build to be run and install the optional dependencies manually. Thus, I added a minimal build script, added the deps as optional that are installed by default and also added a comment about this in the Dockerfile. :)

* Exclude a bunch of files and directories not needed for Docker image
* Exclude dev dependencies for Docker image build by introducing a production build script
* Using a different alpine base image and multi-stage build reduces the Docker image by roughly 35 MB alone
* Move babel to dev dependencies
* Fix highlight.min.js accidentally being ignored
Forgot to change it after moving the directory from /usr/src/app to /app in Dockerfile
* Memcached version number was set wrong accidentally
* Installation of individual document storage client dependencies in Dockerfile made npm read entire package-lock again and install all dependencies
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.

Docker support
2 participants