viper0131 / check_mk

Check_MK docker version
21 stars 20 forks source link

No proper Volume support for container re-creation #24

Closed DJGummikuh closed 6 years ago

DJGummikuh commented 6 years ago

This docker image unfortunately does not provide volume support so that the container can be deleted and re-created without losing the configured services. Can you please update the image accordingly?

mcgege commented 6 years ago

You can always add volumes to the container, even if we don't predefine them ... the challenge here is to find all the needed directories that should be preserved and what should be overwritten by an update. If you find a good solution please provide a PR! I solved this for myself with the backup feature (daily export) ...

DJGummikuh commented 6 years ago

hehehe

the challenge here is to find all the needed directories that should be preserved and what should be overwritten by an update.

That's somewhat the reason why I hoped you would be better suited to do that than me :-D

VIl1aIn commented 6 years ago

I have resolution about this problem. Need modify bootstrap and recreate image. My example bootstrap and cmk.sh for run and recreate container w/o data lost. Dockerfile add 'which' packet, it need for build... DockCMK.zip

mcgege commented 6 years ago

@VIl1aIn Thanks for your idea ... can you provide a PR? You should also include a documentation update, and it should still work if the persistent dirs (/opt/omd/*) are not mounted ...

VIl1aIn commented 6 years ago

Hi Michael, That is PR? Problem Resolution, I believe. What should it look like? I can write more details about the resolution. If dirs are not mounted, container will be created with "/opt" dirs in to inside and part "else" of the bootstrap scripts not working. If do start/stop action under the container, then "bootstrap" not running again.  05.09.2018, 13:45, "Michael Geiger" notifications@github.com:@VIl1aIn Thanks for your idea ... can you provide a PR? You should also include a documentation update, and it should still work if the persistent dirs (/opt/omd/*) are not mounted ...—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

DJGummikuh commented 6 years ago

@VIl1aIn PR is "Pull Request" it means that you fork the repository, apply the change you did in git, commit it to your fork then request the changed fork to be merged back into this repository.

mcgege commented 6 years ago

Should work now, thanks!

DJGummikuh commented 6 years ago

Quick question (even though the issue is already closed). Is there a reason not to define /opt/backup and /opt/omd/sites as VOLUME in the Dockerfile now? That would allow for recreation of the container even without explicitly providing the volumes via docker run or whatever mechanism.

VIl1aIn commented 6 years ago

Hi,Running a container without an external volume can be used for testing if you do not plan on working with it for a long time. Check_mk will accumulate data in the /omd/sites folder and the container will grow in size. If you recreate the container, all data in this case will be lost. An external volume allows you to solve this. Hardcoding of a volume in a Dockerfile is not recommended, since it may be necessary to start the container in different ways, depending on the requirements.17.09.2018, 14:20, "Johannes" notifications@github.com:Quick question (even though the issue is already closed). Is there a reason not to define /opt/backup and /opt/omd/sites as VOLUME in the Dockerfile now? That would allow for recreation of the container even without explicitly providing the volumes via docker run or whatever mechanism.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

DJGummikuh commented 6 years ago

@VIl1aIn I disagree in that defining the VOLUMEs in the Dockerfile does prevent you from using it any less than now. On the contrary, by defining the VOLUME fields in the Dockerfile you switch from "default is to loose everything on deletion" to "default is to keep everything". Nobody prevents you from docker rm -fv'ing the container, which automatically also deletes every automatically created volume for that container. OTOH if you don't think too much about it and just create the container without any explicit volume definition, you'll be in for a surprise if you recreate the container only to see that everything is deleted (and also the username/password is resetted to a publicly available combination!). In my experience it's better to play it safe.

One last thought: adding volumes in the Dockerfile also serves as some kind of documentation as it allows a user to create a container naked, then do docker inspect on the container and directly see on-site the requirements the image has regarding ports, volumes etc.

mcgege commented 6 years ago

(reopen issue as the discussion for #37 is going on here)

mcgege commented 6 years ago

@DJGummikuh @VIl1aIn First of all: Thanks for your contribution, this really helps to make this little project better!

As the declaration of these mointpoints in Dockerfile doesn't make them mandatory (correct me if I'm wrong), I say: It doesn't hurt ;-)

Maybe it's also a good idea to enhance the README, so that this topic is more clear to the user? And how about providing a sample init script? What do you think?

DJGummikuh commented 6 years ago

Well they are made "mandatory" in a sense that docker always creates volumes now, somewhere in its belly in case you don't explicitly define them. However, this is actually how docker is supposed to run (not trashing would-be persistent data into the container filesystem) and poses no issue as you can easily delete the volumes with docker rm -fv or by doing docker system prune.

VIl1aIn commented 6 years ago

Hi, to Johannes. About VOLUME in Dockerfile. I agree that the container will not grow, it's my fault, I did not quite understand the previous message. Using VOLUME is good in special cases, when the user understands why he does it. In general, an ordinary user running a container expects that after the system stops, the system will return to its original state (disk space, and so on). And when using VOLUME, the volume will remain and it will have to be deleted by hand, if it is not required. My opinion is, if VOLUME is in the Dockerfile, then you should specially emphasize it and give instructions on its use and removal, if the need for a container and volume has disappeared. In addition, the specifics of check_mk is that not everything can be configured through WATO, sometimes you need to make corrections "by hand" in scripts and configurations, and it's more convenient to do it on an external volume. VOLUME has an opaque path in the file system. Thanks, and all that I wrote exclusively my opinion and not a guide to action :)

DJGummikuh commented 6 years ago

@VIl1aIn you are right about the opaque filesystem path in the host filesystem, however you always have the possibility of using docker cp to copy files in and out of the container from somewhere, completely regardless where the files are actually residing. Enter frameworks like kubernetes or openshift, this sometimes is the only way for you to access the filesystem anyways as it's behind a SAN or what not. In these cases, volumes are usually handled differently from in-container storage (larger quotas, for example) which further stresses (in my p.o.v.) that defining volumes where we expect considerable amount of data to be generated/stored is a good idea. I ammended the README.md in my pull request to accommodate for the information of the user.

@mcgege @viper0131 since the readme is (as far as I know) not automatically reflected to the docker hub page please don't forget to ammend it there as well (if you merge my pull request in that is)

VIl1aIn commented 6 years ago

to Michael. Hello. And thanks for you support and project. :) On the use of an external volume, it seems to me that there is nothing to add to README. If a user uses an external volume, he understands why he can manage it as he needs. My refinement does not change anything in using the original image. to Johannes. I did not say that VOLUME is bad. Simply, both approaches have their pros and cons. But in this case, including VOLUME in the Dockerfile, we force the user to use it, and this does not suit everyone. In the end, you can simply describe this possibility and, in addition, nobody forbids creating your Dockerfile with the necessary properties. :)

mcgege commented 6 years ago

Well, I thought about this the last days, and I also see the pros and cons here. This discussion brought me to cleanup my local development system - and I found out that there were many orphan volumes that weren't removed with docker system prune -a (see this article).

I fear that predefining (and not using) those volumes leads to more and more of those orphaned volumes, so I vote for leaving it the way it is.

viper0131 commented 6 years ago

so I vote for leaving it the way it is

My personal opinion on this subject is that I don't like the VOLUMES inside the Dockerfile because from that moment on you see the data as persistent. I like the docker concept als 'throw away architecture'. To make things persistent you can mount every possible directory or even a file when starting a container and that makes very clear where your persistent data is stored. The volume definitions just cluther the /var/lib/docker directory....

VIl1aIn commented 6 years ago

I vote not to include VOLUMES in Dockerfile. Who needs it, he can do it himself, while he knows what he is doing and why he needs it.

mcgege commented 6 years ago

Ok, thank you all for your feedback ... we‘ll keep the volume definition out.