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

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

Open
winstonma opened this issue Sep 18, 2020 · 13 comments
Open

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

winstonma opened this issue Sep 18, 2020 · 13 comments

Comments

@winstonma
Copy link

winstonma commented Sep 18, 2020

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.

@winstonma winstonma changed the title docker-entrypoint.sh didn't copy the config file docker-entrypoint.sh didn't copy the config file Sep 18, 2020
@Legion2
Copy link
Contributor

Legion2 commented Oct 1, 2020

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

@winstonma
Copy link
Author

winstonma commented Oct 2, 2020

@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
Copy link
Contributor

Legion2 commented Oct 2, 2020

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

@winstonma
Copy link
Author

winstonma commented Oct 2, 2020

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
Copy link
Contributor

Legion2 commented Oct 2, 2020

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
Copy link
Author

Yes

May I highlight several things

  • 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.
  • MagicMirror would not start if config.js doesn't exist. I would suggest if the script check existence of the file config.js

@Legion2
Copy link
Contributor

Legion2 commented Oct 2, 2020

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
Copy link
Contributor

khassel commented Oct 2, 2020

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
Copy link
Author

winstonma commented Oct 2, 2020

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
Copy link
Contributor

khassel commented Oct 2, 2020

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
Copy link
Author

@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
Copy link
Contributor

khassel commented Oct 2, 2020

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.

  • 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)

@winstonma
Copy link
Author

  • 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)

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

No branches or pull requests

3 participants