rstrouse / nodejs-poolController-dashPanel

A dashboard panel for tagyoureit/nodejs-poolController
33 stars 20 forks source link

Docker container fails to start without config.json #58

Open orrious opened 2 years ago

orrious commented 2 years ago

Without config.json: 1.) Docker container fails to start 2.) Logging does NOT show the reason the container failed to start.

Please add logic to: 1.) Provide logging showing that config.json doesn't exist. 2.) Create config.json with the defaults.

Change your current entrypoint in dockerfile to a CMD and add entrypoint.sh as your entrypoint. This will run entrypoint.sh and then the CMD when the container starts. It's a nice way to inject an init script of sorts.

entrypoint.sh

if [ ! -e /app/config.json ] 
then
    echo "config.json didn't exist, creating one!" 
    cp /app/defaultConfig.json /app/config.json 
fi

exec "$@"
tagyoureit commented 2 years ago

I'm not a docker expert... but the app takes care of copying the defaultConfig.json to config.json. Have you used the template docker config files from the wiki? These map the config.json file to a data directory. You don't want the data files being kept inside the container, anyway. This allows you to blow away/update the container but still keep your data files.

volumes:
      - /data/poolcontroller/config.json:/app/config.json
      - /data/poolcontroller/data:/app/data
      - /data/poolcontroller/logs:/app/logs

cc: @emes @wurmr

orrious commented 2 years ago

Yes, I used the provided docker-compose. My experience was when I first started the container, it created config.json as a directory rather than a file. Docker has no idea if it's supposed to be a file or a directory. So I deleted the directory and touched the file.

In my experience it's not a good idea to mount a file as a volume, rather mount the directory. When you do mount a directory, if the same one exists in the container, it will be mounted over top of by the one from the host. If the directory doesn't exist on the host, it will be created then mounted, but will still be empty. At that point you would need to copy a base config file into the new directory.

If you would like to replicate, start the container with no volumes defined. It will work as it's using the config.json inside the container. Not what we are looking for.

Then start the container with the volumes defined, but no config,json file present on the host. It should create a directory called config.json.

tagyoureit commented 2 years ago

From my little bit of research, it does seem like mounting a file in the container that does not exist will create a directory on the host system (SO1, SO2).

The right technical answer would be to move the config.json (but not defaultConfig.json) to the data directory. It would be doable, but changing the entry script like you have above is much easier and would require much less rework on our end. My understanding is that once the file exists, and docker maps the container file to a local file, everything else should be ok. I was trying to think about a scenario where we might add a new key to defaultConfig.json but so long as config.json is mount on the container mount I don't think that will ever be a problem.

I'll wait for others to chime in, but so long as the fix can be your script, I think that is the cleanest path forward.

orrious commented 2 years ago

I wouldn't move it to the /data dir. Create a new dir called /config and put it there.

If you ever needed to add a new required key, you can search config.json for the key and if not found add it. You do that with sed and awk in the same entrypoint.sh script.

Btw, this is my docker-compose.yaml

services:
  poolcontroller-dashpanel:
    restart: always
    container_name: poolcontroller-dashpanel
    ports:
      - "5150:5150"
    volumes:
      - ./config.json:/app/config.json
      - ./data:/app/data
      - ./logs:/app/logs
    image: msmi/nodejs-poolcontroller-dashpanel

I would change: - ./config:/app/config

tagyoureit commented 2 years ago

Adding new keys is exactly what the defaultConfig.json is for. During initialization we copy over any new keys. Yes, a separate config directory would be another way of doing that. But it would still be the same amount of work, and we'd want to do it across all 3 projects so it would touch a lot of moving parts.

orrious commented 2 years ago

So it's not a breaking change, in the Dockerfile, you can create a symbolic link from /app/config.json to /app/config/config.json. Then you can point at either location for a couple releases.

orrious commented 2 years ago

Has there been thought into only releasing containers in the future? You can make whatever changes you like in the container from one release to another.

orrious commented 2 years ago

Also, if defaultConfig.json is for adding new keys, why didn't it fill everything in with a blank file? When I touched config.json it still crashed. If we can get it to take a blank file and fill it in, that would be much easier

tagyoureit commented 2 years ago

The breaking change will be for us, the developers. We are constantly reading and writing to that file in the initialization, any config changes, shutdown, backup/restore, etc and that's across three apps. It's not impossible, just a lot of extra work.

And no, we have not thought about only releasing on containers. Containers are great, but they aren't the only method of deploying.

so, if defaultConfig.json is for adding new keys, why didn't it fill everything in with a blank file? When I touched config.json it still crashed.

Because we are looking for a json structure. If you create the file with {} as the contents it won't break.

orrious commented 2 years ago

With a symbolic link ln -s /app/config/config.json /app/config.json, it doesn't matter which you access, it's the same object that is being accessed / modified. How would this break the application?

orrious commented 2 years ago

And no, we have not thought about only releasing on containers. Containers are great, but they aren't the only method of deploying.

They could be the only deployment method though. It would make things easier for the developers, IMO. You will always know exactly what is running inside the container, regardless of where it's running or what version it is. Each developer will also develop in the exact same container version, node version, taking those additional variables out of the equation.

From the consumer stance, the user need only keep docker up to date for their HW/OS platform and pull the latest published containers. What changes inside the container from one version to another is irrelevant and obfuscated from the end user.