-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Volumes should not be defined in base images #404
Comments
Setting |
Yes, but a pointless volume is still created. |
@huggla is right. This is maybe ok if you use docker run and just have a few volumes. But if you use docker-compose or maybe even swarm than there are unaccounted volumes on your docker host, which are connected to the container and thus could not be removed. And even worse is that these containers are not named, they have a random id. Just to show you. I run an application deployed with docker-compose.
DRIVER VOLUME NAME There are two volumes for the postgres container, as you can validate with docker inspect
|
I ran into the same problem and spent a few hours trying to understand why random volumes were being created in docker-compose even though I'd set one for |
I can add another view why not to use the VOLUME: We use automated tests with Postgres Image pre-filled with data during build time. This way the image starts a lot faster which saves computing time. Now imagine running these tests on every commit and pull request. You make hundreds of empty volumes with that process. Currently we use our own Dockerfile, copy-pasted from official repo, only with the VOLUME line commented out. |
This is also causing me issues on kubernetes, the behavior you use is forbidden in kubernetes for production, https://kubernetes.io/docs/concepts/storage/persistent-volumes/ :
|
2018-12-05 23:46:19 (39.5 MB/s) - '/usr/local/bin/gosu.asc' saved [543/543]
how did you get it to build i always run into the same issue? |
|
I'm not trying to argue for or against the |
Without the volume call, if you are using it for testing purposes it will write data into the container and that data will be lost upon container deletion. But even with the volume, every time you create a container it just spawns a new anonymous volume, so you get the exact same behavior, but you leave volumes all over the place. And the workaround is horrendous. Manually forking every version of postgres and changing one line. |
It would be great if the project maintained current behavior and a no volume branch while deprecating over a few version the current behavior. |
@workmaster2n I don't think this is best practice really, it is just quicker to get something working when you don't know what you are doing. Best practice is to know your tools reasonably well. |
@workmaster2n see docker-library/official-images#2437 (comment) for a decent summary of when we (the Official Images maintainers) recommend that image maintainers include a |
So this Say docker run --rm -it \
-v /srv/data/postgresql:/var/lib/postgresql \
postgres psql -U postgres and try to find a sliver of data. No such luck 😰 I actually had my data in docker run --rm -it \
-v /srv/data/postgresql:/var/lib/postgresql \
-e PGDATA=/srv/data/postgresql/11 \
postgres psql -U postgres and that worked fine. Anyway, I think I'll stick with using |
You can still have docker run --rm -it \
-v /srv/data/postgresql/11/:/var/lib/postgresql/data/ \
postgres:11 |
Thanks for the suggestion. |
Base images should avoid setting VOLUME since it is currently impossible to unset in child images:
moby/moby#3465
The text was updated successfully, but these errors were encountered: