fix: issue with docker config and volume setup #159
Conversation
Extheoisah
commented
Nov 16, 2023
Extheoisah
commented
- modifies the volume setup script for sim.json config
- adds --data-dir to CMD args (default is /data in volume)
- fix entrypoint.sh script
Extheoisah
force-pushed
the
fix-docker
branch
from
0fab47d to
e01198b
Compare
| @@ -15,14 +16,15 @@ help: | |||
| @echo "stop Stops the Docker container." | |||
| @echo "" | |||
| @echo "Variables:" | |||
| @echo "SIMFILE_PATH Path to the sim.json file." | |||
| @echo "SIMFILE_PATH Path to the sim.json file." | |||
There was a problem hiding this comment.
nit: this line and the one for stop three lines above end with a ., while the rest do not.
|
|
||
| run-docker: | ||
| docker run -d --rm --name sim-ln --init -v simln-data:/data -e SIMFILE_PATH=/data/sim.json -e LOG_LEVEL=$(LOG_LEVEL) -e HELP=${HELP} -e PRINT_BATCH_SIZE=${PRINT_BATCH_SIZE} -e TOTAL_TIME=${TOTAL_TIME} sim-ln | ||
| docker run -d --rm --name sim-ln --init -v simln-data:/data -e SIMFILE_PATH=/data/sim.json -e DATA_DIR=${DATA_DIR} -e LOG_LEVEL=$(LOG_LEVEL) -e HELP=${HELP} -e PRINT_BATCH_SIZE=${PRINT_BATCH_SIZE} -e TOTAL_TIME=${TOTAL_TIME} sim-ln | ||
|
|
||
| run-interactive: |
There was a problem hiding this comment.
I didn't catch this when it was added, but I'm wondering now:
Don't we want to also pass the env variables when running interactively?
There was a problem hiding this comment.
hmm... we should. I'll fix that.
There was a problem hiding this comment.
fixed here -> 85082b3
| @@ -1,7 +1,12 @@ | |||
| #!/bin/sh | |||
|
|
|||
| # Define the start command | |||
| START_COMMAND="/usr/local/bin/sim-cli $SIMFILE_PATH" | |||
| START_COMMAND="/usr/local/bin/sim-cli --sim-file $SIMFILE_PATH" | |||
There was a problem hiding this comment.
Shouldn't this only be passed now if $SIMFILE_PATH is defined? If not passed, this will default to pick it from data_dir/sim.json
There was a problem hiding this comment.
We still need it because the cli looks for the config JSON in the root by default. whereas in docker, we're mounting a volume that hosts the JSON. So we need to point it to the docker volume where the sim.json is hosted and use that as a default if no SIMFILE_PATH is specified
Makefile
Outdated
|
|
||
| run-docker: | ||
| docker run -d --rm --name sim-ln --init -v simln-data:/data -e SIMFILE_PATH=/data/sim.json -e LOG_LEVEL=$(LOG_LEVEL) -e HELP=${HELP} -e PRINT_BATCH_SIZE=${PRINT_BATCH_SIZE} -e TOTAL_TIME=${TOTAL_TIME} sim-ln | ||
| docker run -d --rm --name sim-ln --init -v simln-data:/data -e SIMFILE_PATH=/data/sim.json -e DATA_DIR=${DATA_DIR} -e LOG_LEVEL=$(LOG_LEVEL) -e HELP=${HELP} -e PRINT_BATCH_SIZE=${PRINT_BATCH_SIZE} -e TOTAL_TIME=${TOTAL_TIME} sim-ln |
There was a problem hiding this comment.
-e SIMFILE_PATH=/data/sim.json should be -e SIMFILE_PATH=${SIMFILE_PATH} now right? Given the parameter is not required anymore plus we want the user to be able to re-define the location if desired
Extheoisah
force-pushed
the
fix-docker
branch
from
e01198b to
85082b3
Compare
|
Looks straightforward enough. utACK 85082b3 |
There was a problem hiding this comment.
Came back to test and ran into issues with make docker build and make run-interactive, I think we've made some breaking changes since you last updated :')
docker run -it --rm --name sim-ln --init -v simln-data:/data_dir -e SIMFILE_PATH=/data_dir/sim.json -e DATA_DIR= -e LOG_LEVEL=info -e HELP= -e PRINT_BATCH_SIZE= -e TOTAL_TIME= sim-ln
error: unexpected argument '--sim-file' found
tip: to pass '--sim-file' as a value, use '-- --sim-file'
Hmm...maybe. I haven't run it since. I'll look into it |
This commit updates the Dockerfile to clean up the apt-get cache after installing clang and llvm. This helps reduce the size of the Docker image.
Extheoisah
force-pushed
the
fix-docker
branch
from
85082b3 to
bbda19f
Compare
@carlaKC So I tried running locally with the new changes and the only error I had was due to limitations in my docker disk size (not related to sim-ln) for which I added a small clean-up here -> bbda19f |
