monitoringartist / dockbix-xxl

:whale: Dockerized Zabbix - server, web, proxy, java gateway, snmpd with additional extensions
https://hub.docker.com/u/monitoringartist/
GNU General Public License v2.0
377 stars 136 forks source link

VOLUME definitions prevent creating inherited images with alerts & external scripts #114

Closed kompa3 closed 6 years ago

kompa3 commented 6 years ago

Currently, Dockerfile contains VOLUME definitions for directories such as /usr/local/share/zabbix/alertscripts and /usr/local/share/zabbix/externalscripts.

This effectively prevents creating an inherited image from dockbix-xxl that would contain an alert script or an external script because volume contents cannot be changed after the volume has been defined (volume creates a snapshot and any changes to the volume contents in build steps after that will be discarded).

The VOLUME definitions are actually unnecessary because if you want to mount the script directory from a host, docker run -v creates the volume.

According to Best practices for writing Dockerfiles VOLUME should be used for mutable parts of the image. Script directories will not be changed by the zabbix application so they are not mutable in that sense - on the contrary, they should be static in a sense that they are defined before running the container either by mounting a host directory or inheriting the image.

jangaraj commented 6 years ago

Hi Tommi, thx for issue&PR.

Script directories will not be changed by the zabbix application so they are not mutable in that sense - on the contrary, they should be static in a sense that they are defined before running the container either by mounting a host directory or inheriting the image.

Yes, I agree. We can remove VOLUME from the Dockerfile completely. What do you think?

kompa3 commented 6 years ago

Yes, I discussed this with my colleagues and we agree that the current VOLUME definitions could be removed completely. The only mutable data that could be made persistent using volumes is Zabbix database which anyways resides totally elsewhere.

jangaraj commented 6 years ago

Yes. Could you remove VOLUME definition in your PR, please?

kompa3 commented 6 years ago

Removed

jangaraj commented 6 years ago

👍 Thank you, Tommi.