zabbix-community / helm-zabbix

Helm chart for Zabbix
https://artifacthub.io/packages/helm/zabbix-community/zabbix
Apache License 2.0
83 stars 48 forks source link

Update configmap -completed version #19

Closed seanfariacustomink closed 1 year ago

seanfariacustomink commented 1 year ago

What this PR does / why we need it:

for MySQL and PostgreSQL

Which issue this PR fixes

lack of mysql

sa-ChristianAnton commented 1 year ago

I am actually not too happy with this PR.

I don't see that this PR has been created with some "love" spent to make it good.

Generally, I am not yet sure wether we "want to" support MySQL in this Chart at all...? It's quite some extra effort to maintain.... opionions please. .... @aeciopires what do you think?

aeciopires commented 1 year ago

Hello @seanfariacustomink !

Thank you for your contribution with this helm chart. You've identified a need, opened an issue, and are now submitting a PR. This is very appreciated.

But @sa-ChristianAnton is right. With the merge of this PR, we will have triplicate work with each release of new versions of the helm chart to test the changes in three databases (MySQL, MariaDB and PostgreSQL). For that reason, we'd like to see a few more things in this PR to make our jobs easier and give others clarity on what a great feature this helm chart is getting.

What still needs to be done in this PR is the following:

Could you help us with this? Are you willing to help @sa-ChristianAnton and I with basic tests of running Zabbix every launch of the helm chart?

Perhaps put a disclaimer that this helm chart is primarily tested with PostgreSQL and that problems with other databases may occur and will be fixed on demand. What do you think @sa-ChristianAnton? With these changes does it look good to you?

sa-ChristianAnton commented 1 year ago

I fully agree with @aeciopires with the following additional comments:

I am looking forward to this "project", as it is one, not just a little change...

aeciopires commented 1 year ago

The comment of @seanfariacustomink in the issue #18 is interesting. Maybe we can work together indicating the chart https://github.com/dj-wasabi/helm-zabbix#install-the-helm-chart for people who need to use Zabbix with MySQL.

What do you think @sa-ChristianAnton and @seanfariacustomink?

sa-ChristianAnton commented 1 year ago

The comment of @seanfariacustomink in the issue #18 is interesting. Maybe we can work together indicating the chart https://github.com/dj-wasabi/helm-zabbix#install-the-helm-chart for people who need to use Zabbix with MySQL.

What do you think @sa-ChristianAnton and @seanfariacustomink?

Not sure, as the referenced chart is not being updated for quite some time. I would rather prefer extending this project.

Maybe we should implement automated tests for deployments, upgrades, etc. of Zabbix instances with this helm chart in different scenarios? Like "with HA, with Postgres, with TimescaleDB enabled, with MySQL instead", ....? This would lower the burden of manual testing after every little change and we can support MySQL without problems.

Does Github offer this kind of possibilities with no cost?

aeciopires commented 1 year ago

You're right @sa-ChristianAnton, we can extend this project...

I'll look into the cost of GitHub Actions.

Implementing the automated tests for this helm chart will be a great learning challenge. I've never done this before and I'm willing to learn. But for now we will wait for the response from @seanfariacustomink

sa-ChristianAnton commented 1 year ago

I'll look into the cost of GitHub Actions.

I could imagine that providing a Kubernetes cluster on which to deploy things is not something Github Actions will provide for free. In that case I can provide one running in my home lab, or somewhere else, so that only executing the tests would have to be triggered from Github somehow.

seanfariacustomink commented 1 year ago

Hi all,

After playing with this over the Christmas break I now know this entire PR will have to change. I don't think it is required to support MYSQL DB within helm but rather the ability to point to a (MySQL, Maria, etc) database and disable Postgresql DB creation within the cluster. This approach would be less work and be perfect for anyone with an existing Zabbix application to move to the helm version. I will most likely create another PR for that.