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

Feature Request: Populate data directory if using docker bind mounts #226

Open
max-tet opened this issue Oct 6, 2023 · 6 comments
Open
Labels
enhancement New feature or request

Comments

@max-tet
Copy link

max-tet commented Oct 6, 2023

Hi.

Problem

I was trying to run grocy in docker using the docker-compose.yml here and it works great. However, I prefer bind mounts over volume mounts and so I changed this line slightly to read - ./app-db:/var/www/data. That would mount the app-db directory instead of a volume.

As a result, grocy doesn't work anymore. I see the following error returned: Unable to run Grocy: config.php in data directory (/var/www/public/../data) not found. Have you copied config-dist.php to the data directory and renamed it to config.php?

Probable Cause

This seems to be to be caused by the different behaviour between volume mounts and bind mounts with respect to preexisting data.

My guess is that the config.php is preexisting and therefore available on startup when using a volume but missing when using a bind mount.

Proposed Solution

A practice that I have seen often is to explicitly copy any preexisting data on application startup into the mounted directory if it is empty instead of relying on docker to do it. That way, volume mounts and bind mounts would behave identical.

It might also be feasable to not mount the whole of /var/www/data but only relevant subdirectories like /var/www/data/db and /var/www/data/storage. That would mean to just keep the default config. Any config changes could be done using environment variables. Would persisting those two directories be enough or should others be mounted as well?

Thanks and have a good day!

@max-tet max-tet added the enhancement New feature or request label Oct 6, 2023
@berrnd

This comment was marked as off-topic.

@berrnd berrnd transferred this issue from grocy/grocy Oct 6, 2023
@jayaddison
Copy link
Contributor

Hi @max-tet - thank you for the problem report, and for offering possible solutions.

My guess is that the config.php is preexisting and therefore available on startup when using a volume but missing when using a bind mount.

I believe that's true, yes - in fact the backend container build includes a step that creates the config.php file by copying the default grocy-distributed one. As things are today, users may then update that within the volume that it is contained on.

Proposed Solution

A practice that I have seen often is to explicitly copy any preexisting data on application startup into the mounted directory if it is empty instead of relying on docker to do it. That way, volume mounts and bind mounts would behave identical.

This could be OK, although as I understand it: this might require some co-ordination between the PHP application and the container? (so far, @berrnd and I basically maintain each component without any coordination, and from my point of view as an unpredictably-available maintainer, that's quite convenient)

It might also be feasable to not mount the whole of /var/www/data but only relevant subdirectories like /var/www/data/db and /var/www/data/storage. That would mean to just keep the default config. Any config changes could be done using environment variables. Would persisting those two directories be enough or should others be mounted as well?

That would be my preferred approach; in fact, I like two things about it:

  • It should reduce the amount of read-write state required in the container; if I understand this correctly, the default config-dist.php file (renamed to config.php) would not require any user modification, while users could still use the full range of configuration options.
  • It seems to follow the 12-Factor app config guideline around preferring environment variables for container configuration.

I do have one concern: backwards-compatibility. I'm not sure how to achieve the single-volume to multi-volume migration in a way that would be straightforward and safe for users.

@max-tet
Copy link
Author

max-tet commented Nov 5, 2023

Hi. Thanks for your reply.

This could be OK, although as I understand it: this might require some co-ordination between the PHP application and the container?

I am not sure about that. The step in the Dockerfile that copies the config file should be removed but that can happen later. Since grocy would copy the config to the mounted dir only if it does not exist, almost every case would behave just like it does now. The one exception is the case described above where instead of failing, it would start normally.

However, you are right about the second approach being more 12-factory-appy. And if those are the only two relevant directories, then mounting them should be fine. I suggest documenting that approach, perhaps even as the reccommended one.

You are right about backward compatibility. But would existing users need to migrate at all? They could keep the single-volume "legacy" approach and only new users would go multi-volume.

@jayaddison
Copy link
Contributor

I am not sure about that. The step in the Dockerfile that copies the config file should be removed but that can happen later. Since grocy would copy the config to the mounted dir only if it does not exist, almost every case would behave just like it does now. The one exception is the case described above where instead of failing, it would start normally.

Ok; I think I follow your reasoning here. I'll do some experimentation locally, removing the config-dist.php copy step and checking the results.

You are right about backward compatibility. But would existing users need to migrate at all? They could keep the single-volume "legacy" approach and only new users would go multi-volume.

Maybe true, but that would also make it more difficult to respond to support questions. I'm already not great at doing that, and having different volume configurations possible on user deployments would add more scenarios to consider. If a feature already isn't well-supported, then I don't like the idea of making it more complicated.

Does Docker support a way to run a migration to split/consolidate volumes during an upgrade? (I don't think so, but if there is a way, then that'd be good to learn about)

@max-tet
Copy link
Author

max-tet commented Dec 12, 2023

Maybe true, but that would also make it more difficult to respond to support questions.

Ok, true. I get that.

Does Docker support a way to run a migration to split/consolidate volumes during an upgrade?

Not that I know of. The only way I can think of is using a special container for that where you mount the source volume to /a and the target volumes to /b/1, /b/2, etc. and then cp -r /a /b. So nothing automatic that just works.

@arkanoid87
Copy link

I had a very bad experience with grocy on my first install due to this. There should be no difference at containel level if bind mount is used instead of volume

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

4 participants