search

OCSInventory-NG / OCSInventory-Docker-Image

Docker image for OCSInventory Server
GNU General Public License v3.0
78 stars 93 forks source link

Addressing some issues #100

Open dorpauli opened 1 year ago

dorpauli commented 1 year ago

Must read before submitting

Please, take a look to our contributing guidelines before submitting your pull request. There's some simple rules that will help us to speed up the review process and avoid any misunderstanding

Contributors GuideLines

Status

READY

Description

In our organization we like to use OCS-Inventory with docker. So we tested a lot and encounter some things that could be improved. Internally we now build our own image, but i want to share the main things we invented. First of all we changed the sed commands in the entrypoints with perl, so that complex passwords with special characters are no problem anymore. Secondly we implemented the opportunity to use the install.php script in the image, if it is necessary. The last thing is, that we shrink the size of the image while using docker multistage build. Now the image is around 327 MB. Before the change it was 547 MB. The trick is to not have build-essential in one of the image layers.

I hope you can make use of it and that the addressed issues can be solved with that PR. Please comment if you have questions and sth. is unclear.

Related Issues

https://github.com/OCSInventory-NG/OCSInventory-Docker-Image/issues/99 https://github.com/OCSInventory-NG/OCSInventory-Docker-Image/issues/98 https://github.com/OCSInventory-NG/OCSInventory-Docker-Image/issues/81

Todos

Test environment

I did some tests with it, but didn't use nginx.

General informations

Docker host's operating system : Debian 11 and Ubuntu 22.04 (testet both) Mysql Server version : MariaDB 10.11

Docker informations

Docker compose version : 1.25.0 Docker version : 20.10.5

Deploy Notes

Notes regarding deployment the contained body of work. These should note any dependencies changes, logical changes, etc.

1.

Impacted Areas in Application

List general components of the application that this PR will affect:

dorpauli commented 1 year ago

By the way, there is some strange behavior with mysql as you use it with docker-compose. After restart of the images mysql has some errors when an agent pushes information. He gets a http 500 in this case. Sometimes after a login in the dashboard, this issue was gone. With mariadb everything works great!

dorpauli commented 1 year ago

I reverted back to solution without multi staged build, but use one RUN now.

nroach44 commented 1 year ago

I can confirm that this works fine, no issues so far when using it in lieu of the published image.

tom-ph commented 1 year ago

Hi, since you're editing the Dockerfile can you apt-get install cron to also address https://github.com/OCSInventory-NG/OCSInventory-Docker-Image/issues/97? Or can I ask you how did you manage to make the cron_all_software.php cronjob work (if you did) in order to update the software installed on the servers?

Thanks, Tommaso

dorpauli commented 1 year ago

@tom-ph Yes i will do so. But i don't think, this is enough to activate cron for ocsinventory correctly. As far as i can see, the admin have to manually set the cronjobs in the container. Even if cron is installed. Every time a new image is used for the container, the cronjob is gone. So sth. in the entrypoint of the container with new parameters would be better i think. Or maybe it is possible to mount the needed cronjobs in /etc/cron.daily?

EDIT: You also cannot edit crontab, if there is no editor like nano or vim...

tom-ph commented 1 year ago

Thanks! My idea was to add a script that copies the cron_all_software.php into the cron.daily folder to the docker-entrypoint.d folder. Of course this can only be done if cron is installed. Maybe the script could be added to the repo, looking for an env variable that defaults to false. Maybe I'll do a pull request after yours get accepted.

dorpauli commented 1 year ago

Another idea that orients on the ocs documentation came in my mind. Will push it tomorrow, if i have time for that. :)

dorpauli commented 1 year ago

@tom-ph have you read https://wiki.ocsinventory-ng.org/13.Docker-documentation/Using-the-docker-image/#crontab-implementation ? Implementing cron in the container seems unnecessary. You can just use cron on the server, where docker is running, and "docker exec" the cron scripts inside the container.

tom-ph commented 1 year ago

Actually I didn't, but I don't like it for a number of reasons. The first is conceptual: I don't think that the host should implement an application logic. The second is practical: we are planning to migrate OCSInventory to our kubernetes cluster and this would be a overhead (and in my opinion this is because the conceptual choice is not the best).

I think that the minimum requirement should be having cron inside the container to give the flexibility to implement the crontabs based on each one's requirements.

The ideal solution would be to give the possibility to activate each of the cronjobs described here: https://wiki.ocsinventory-ng.org/03.Basic-documentation/Understanding-OCS-Crontab/#understanding-ocs-crontab just using environment variables that are false by default.

dorpauli commented 1 year ago

Good point @tom-ph ! I have been thinking of another solution and would like to hear your opinion.

To ensure admins can set up the cronjobs exactly as they need, I would provide the ability to use a cron file. This file can be included as volume in /etc/cron/docker-crontab. If the file exists, the entrypoint script runs crontab /etc/cron/docker-crontab and activates cron with cron -n. With this solution I do not have to define fixed times and commands.

tom-ph commented 1 year ago

I think this could be a great solution. If well documented, it could be an easy way to setup crontab with all the needed flexibility.

tom-ph commented 1 year ago

Hey, let me know if I can help! By the way, I think nano can be removed from packages with your proposed solution.

dorpauli commented 1 year ago

Hey, let me know if I can help! By the way, I think nano can be removed from packages with your proposed solution.

Hey, you can test it of course. I've build a new image here: https://hub.docker.com/r/dorpauli/ocsinventory-docker-image Or you just build it yourself. It's up to you.

Of course, the documentation will need to be updated if my changes are used. At this point I would like to help the developers. They just have to give me the hint where to write down what.

nicolashlightvfx commented 7 months ago

any news about this pull request being merge ? it's almost a year now of course there's a test image here but officially we still have to run the cron manually inside the docker, to refresh the list of software detected on machines.