search

cetic / helm-zabbix

Helm Chart For Zabbix
https://artifacthub.io/packages/helm/cetic/zabbix
Apache License 2.0
56 stars 57 forks source link

allow specific image tags #63

Closed scoopex closed 2 years ago

scoopex commented 2 years ago

introduce specific image tags

aeciopires commented 2 years ago

Hi, @scoopex!

I understand your necessity. The PR is cool for me. Do you agree @sa-ChristianAnton?

aeciopires commented 2 years ago

By default specific tags is not defined, but with this PR is possible to use.

scoopex commented 2 years ago

Hi @scoopex!

Sorry. I reviewed and approved quickly. Talking with @sa-ChristianAnton I saw that the parameters you are setting as follows.

          image: "{{ .Values.zabbixserver.image.repository }}:{{ .Values.zabbixserver_image_tag }}"
          image: "{{ .Values.zabbixagent.image.repository }}:{{ .Values.zabbixagent_image_tag }}"
          image: "{{ .Values.zabbixweb.image.repository }}:{{ .Values.zabbixweb_image_tag }}"
          image: "{{ .Values.zabbixwebservice.image.repository }}:{{ .Values.zabbixwebservice_image_tag }}"

We think it would be more interesting to define it as follows.

          image: "{{ .Values.zabbixserver.image.repository }}:{{ .Values.zabbixserver.image.tag }}"
          image: "{{ .Values.zabbixagent.image.repository }}:{{ .Values.zabbixagent.image.tag }}"
          image: "{{ .Values.zabbixweb.image.repository }}:{{ .Values.zabbixweb.image.tag }}"
          image: "{{ .Values.zabbixwebservice.image.repository }}:{{ .Values.zabbixwebservice.image.tag }}"

The same idea applies to snippets with the if statement. Example:

Before:

{{- if .Values.zabbixagent_image_tag }}

After:

{{- if .Values.zabbixagent.image.tag }}

Another question: could you put these specific parameters in the values.yaml file (even if commented) with the same value as the tag? This makes it more intuitive for other people to use.

Yes of course, i changed that.

scoopex commented 2 years ago

Pushed "make gen-docs" update.