Improve initialization of userdata

Blizzard26 commented 2 years ago

Summary

Currently the entrypoint checks if the userdata directory is completely empty and only in that case copies over the etc/ directory (and empty logs and tmp directories) from dist. IMHO this feels overly strict and has some issues along with. For example in my setup I have a second docker container container logviewer that mounts the logs directory inside userdata. If that docker container is started at the same time as openhab (e.g., in the same docker-compose script), the entrypoint will not initialize the userdata directory on a fresh install as it already contains an empty logs directory. Moreover, I push my configuration to a private git repository. For this purpose I have a .gitignore in my userdata directory to exclude some files. This also prevents the entrypoint from initializing the userdata directory.

Possible Solution

Instead of checking if the userdata directory is completely empty, in my opinion it would be better to check if the etc directory exists, and if not copy over the contents from dist. logs and tmp could either be directly created by the script or copied the same way if there is some reason for it.

As a workaround for myself I added the following script to cont-init.d, which does exactly that:

#!/bin/bash

volume="${OPENHAB_USERDATA}/etc"
source="${OPENHAB_HOME}/dist/userdata/etc"

if [ -z "$(ls -A "$volume")" ]; then
    echo "Initializing empty volume ${volume} ..."
    cp -av "${source}/." "${volume}/"
fi

Image

openHAB version: OpenHab 3.2
Image tag used: 3.2.0-debian

wborn commented 2 years ago

I think it is best to stick to the default way how Docker determines if volumes should be initialized (completely empty) so it works just like most other Docker containers in the world, see https://github.com/openhab/openhab-docker/issues/249#issuecomment-541320732.

Using some other fuzzy logic will make it less obvious when volumes are initialized and it can also create new issues where volumes are reinitialized unexpectedly causing data loss.

Blizzard26 commented 2 years ago

Hi Wouter, I don't fully agree. But I guess the actual topic is a different one: userdata shouldn't be a volume in the first place. userdata/jsondb should be a volume, so should be userdata/persistence. userdata/logs should not be part of a volume at all - there is no data in there worth persisting across different container starts. To access the log files you can still mount the directory. userdata/etc is debatable. In most docker containers configuration is not a volume. Either it's configured through env Variables or it's just mounted to a host file/directory. Personally I would not make userdata/etc a volume. It just system configuration and almost never changed. Same goes for userdate/cache and userdata/tmp.

The problem with that is that it's not clear what of userdata should be a volume as some addons create additional files or directories which should actually be volume, but we don't know which.

But I guess this won't be changed any time soon, so I'll stick with my workaround or maybe just mount the directory I need...

Just my 5 cents ;)

Best, Andreas

openhab / openhab-docker