bastilimbach / docker-MagicMirror

Docker image for the Magic Mirror 2 project by Michael Teeuw.
https://hub.docker.com/r/bastilimbach/docker-magicmirror/
MIT License
185 stars 54 forks source link

docker-entrypoint.sh didn't copy the config file #49

Open winstonma opened 4 years ago

winstonma commented 4 years ago

In docker-entrypoint.sh there is line that place the config file before the MagicMirror starts

if [ ! "$(ls -A /opt/magic_mirror/config)" ]; then
    cp /opt/magic_mirror/mm-docker-config.js /opt/magic_mirror/config/config.js
fi

But when the docker image is built config.js didn't appear. Therefore I guess the docker-entrypoint.sh didn't work as intended.

Legion2 commented 4 years ago

docker-entrypoint.sh is executed when the container is started not when the image is built.

winstonma commented 4 years ago

@Legion2 maybe I am wrong

But the problem here is, the script end up didn't copy the mm-docker-config.js to /opt/magic_mirror/config because the checking condition always return false (because the config folder exist in MagicMirror repository). Should the statement check this existance of config/config.js instead?

Legion2 commented 4 years ago

I think it checks if there are files in the config directory by using ls

winstonma commented 4 years ago

Exactly! But MagicMirror repository already created the folder, therefore this checking will always return false (because ls -A /opt/magic_mirror/config returns true because when you clone MagicMirror, the config folder would be created)

Legion2 commented 4 years ago

ls -A /opt/magic_mirror/config returns an empty string (prints nothing to stdout) because the directory is empty, it does not return a boolean, there are no boolean types in sh/bash. [ ! "" ] returns the exit code 0 which means a true value and the conditional expression is executed.

winstonma commented 4 years ago

Yes

May I highlight several things

Legion2 commented 4 years ago

MagicMirror repository by default have a file config.js.sample sits inside the config folder (started 4 months ago). Therefore the statement would not return empty string and eventually it would not execute the cp command.

Ok I didn't know that. This breaks the check you can open a pr and propose a different method to check. But I don't know if it will be merged soon (see #48).

khassel commented 4 years ago

MagicMirror repository by default have a file config.js.sample sits inside the config folder (started 4 months ago). Therefore the statement would not return empty string and eventually it would not execute the cp command.

yes, but if you start the container with

docker run  -d \
    --publish 80:8080 \
    --restart always \
    --volume ~/magic_mirror/config:/opt/magic_mirror/config \
    --volume ~/magic_mirror/modules:/opt/magic_mirror/modules \
    --volume /etc/localtime:/etc/localtime:ro \
    --name magic_mirror \
    bastilimbach/docker-magicmirror

this will map ~/magic_mirror/config:/opt/magic_mirror/config and everything inside the container in /opt/magic_mirror/config is replaced with the files from the host directory. If you start with an emtpy host dir, the directory inside the container is also empty (config.js.sample is removed in this case) and the copy is executed.

But if you have any file in your host-dir, the copy will not run. It would be better to check if config.js exists.

winstonma commented 4 years ago

Also in docker-entrypoint.sh have other two checks, I am not sure if it would be better if it check the existence of the file rather than the emptiness of the folder.

khassel commented 4 years ago

No.

The first check copies the default-modules if the mapped directory from the host is empty, that's o.k. The last check use a config.js.template to create a config.js (only if config.js.template exists), so here is already checked on a file.

winstonma commented 4 years ago

@khassel Yeah you are right 🤔 I didn't start docker in that way. I just copied the dockerfile into my environment start the docker.

But I still wonder why the script docker-entrypoint.sh is needed if it map the file in the docker argument?

khassel commented 4 years ago

The docker run ... statement maps volumes from the host into the container. After this mapping docker-entrypoint.sh is executed and this script tries to correct some special cases.

winstonma commented 4 years ago
  • It makes no sense to start without default-modules (if host-dir empty)
  • It makes no sense to start without config.js (if not exists in host-dir)

Thanks

However the checking is still invalid because the config folder exist in the original repository (it is for the special case)