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: Improved the Dockerfile to make the project less hostile to use with docker #113

Merged
merged 4 commits into from
Dec 15, 2021

Conversation

GCastilho
Copy link
Contributor

Problem: Some behaviors of the project are hostile to use in a docker environment, that includes:

  • The user being node instead of the default root, which limits mountpoints with write access
  • The configuration files not being in a single folder, requiring individual mounts for each file
  • The config files needing to the mapped using environment variables to a location where the node user has write permissions to allow the use with volumes - this happens because the default behavior when the target does not exists for volume creation is to create a folder, which breaks the script since it expects the .json to be a file
  • The database can't be mapped to a different folder - this one is the motivation for the PR: for the reasons described in the previous point, we can't create a mountpoint for the db.json, in which case it will stay in the container filesystem and lost upon restart, which is unacceptable

Solution: Refactor the Dockerfile to change default locations for the .json files and add support for a different location for the db.json

I've refactored the Dockerfile to set ENVs changing the default location for all the config files (read using readConfigPathsFromENV) to be in the /root folder (root's home), this allow easy mapping of the /root folder to a volume

The user for the docker process was also removed, it's now the default root user - A different user for the docker process is only useful in very specific cases, in general, it's not a problem to run the process as root since the container is already an isolated environment; and, since it was causing permission problems for the volume mountpoints, I've removed it

I've also refactored the Dockerfile to use two stages, one for build and other for the live image, that should reduce the image size and reduce the code shipped in the image. The behavior is the same


The only change in program code was the addition of the CONFIG_DB_PATH to allow specifying a different path for the db and the db initialization to use that config instead of the hardcoded 'db.json'. All the other changes were in the Dockerfile and should only affect docker images

-- Added volume /root on Dockerfile
-- Added ENVs for all common settings to map them inside /root
-- Removed node as user
-- Split build and deploy in two different stages
-- Replaced 'npm install' for 'npm ci' and 'npm ci --production'
-- The path for the db.json now can be specified using the env CONFIG_DB_PATH and defaults to db.json
-- Added ENV on the Dockerfile specifying the default location for the db.json file to be inside the /root folder
-- Add a dockerignore file with the same contents of the .gitignore
@Joohansson
Copy link
Owner

Sounds good. I'm curious about the mapping of json files. Is this change breaking in any way, such that people running the container will have to move their config files?

@GCastilho
Copy link
Contributor Author

By default the files will be mapped to the volume in /root, but these locations can be overwritten passing an environment variable to the container

@Joohansson
Copy link
Owner

@GCastilho This is how my docker composer config looks today. So with this change, I would only have to make one mapping if I got it right. That is from the folder where I keep the config files to /root?

Something like this?
./:/root

image

@GCastilho
Copy link
Contributor Author

@Joohansson Correct, your volumes: may look something like this:

 - ./myLocalConfigFolder:/root

and that will map all your JSON files to 'myLocalConfigFolder'

@Joohansson Joohansson merged commit 5a5e0f0 into Joohansson:master Dec 15, 2021
@GCastilho GCastilho deleted the build/less-docker-hostile branch December 15, 2021 21:40
@aspic
Copy link
Collaborator

aspic commented Dec 16, 2021

Awesome improvement!

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