-
Notifications
You must be signed in to change notification settings - Fork 57
feat(zabbix-proxy): add support for persistent volume #73
Conversation
Will update later currently getting error - cannot open database file "/var/lib/zabbix/db_data/zabbix-proxy.sqlite": [2] No such file or directory |
Zabbix process didn't have access to the volume as it had root ownership by default. Updating ownership to zabbix user id 1997 fixed the problem. |
Hi, @tomsozolins! Thank you very much for your contribution. I really appreciate this initiative. Sorry for the delay in reviewing your Pull Request. I was on vacation. Tomorrow I will take the time to review and test the changes. Have a good week. CC: @sa-ChristianAnton for your information. |
Hi @tomsozolins! I understood your contribution, but I think that parameters have same result and are more simple. Do you agree?
If works, I think this Pull Request can be closed without merge. The same idea works to zabbixserver, zabbixagent, zabbixweb. |
Hi! @aeciopires what about the volumeClaimTemplates? Can it be specified in extra fields? Also noticed that security context is global value, but we only need to change it for zabbix proxy when mounting persistent volume as zabbix user. |
Hi @tomsozolins ! I agree with the idea of securityContext having the option to be localized and with the idea of changing the implementation to support extraVolumeClaimTemplate... but looking again at values.yaml... we have the extraPodSpecs parameter which makes the code generic enough to add any settings. Does this work for you?
The same idea works to zabbixserver, zabbixservice, zabbixweb. If it doesn't work, could you change the PR to add securityContext and extraVolumeClaimTemplate support for zabbixProxy, zabbixServer, zabbixWeb, zabbixWebservice? |
Hi @tomsozolins! I'm sorry! The extraPodSpecs won't work for you because it is at the pod level and the extraVolumeClaim would go into the Spec at the StateFulSet level (one level higher). Also volumeClaimTemplates only works for statefulset... it doesn't work for deployment [1] [2]. In this case, you can only change it in zabbixProxy. Could you change the PR implementation to consider the extraVolumeClaimTemplate correctly (making it very generic similar to the other extra parameters) and leave the securityContext localized for each component, overriding the global? Example: if the localized securityContext exists, use it, otherwise use the global. |
@aeciopires Ok, i will look into this later. |
Cleanup volume mount templates - extra sections is enough for use case
Cleanup redundant values - extra values is enough for use case
@aeciopires Hi! I removed all the extra stuff i added previously and added only extraVolumeClaimTemplate. extraVolumeMounts:
- mountPath: /var/lib/zabbix/db_data
name: zabbixproxy-data
extraPodSpecs:
securityContext:
fsGroup: 1997
extraVolumeClaimTemplate:
- metadata:
name: zabbixproxy-data
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: 1Gi
storageClassName: gp3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tomsozolins!
Great job! Thanks for your contribution.
I wroten some suggestions.
Please, do you can make other changes?
-
In values.yaml file change ubuntu-6.0.7 to ubuntu-6.0.8
-
In docs/example/kind/values.yaml file change alpine-6.2.0 to alpine-6.2.2
-
Run the helm-docs command following the instructions of CONTRIBUTING.md file to update the README.md.
Have a nice day.
CC: @sa-ChristianAnton do you can review this pull request too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @tomsozolins!
Thank you again for your contribution.
@aeciopires Found an issue with the addition to VolumeClaimTemplate now there is error: |
Should it be like this? This works without errors. volumeClaimTemplates:
{{- toYaml . | nindent 4 }} |
I understood the problem @tomsozolins. Works with the new config? |
Great. Thanks for hotfix. I will test in the next days and will make the merge and publish the version. Feel free for new improvements. |
Add persistent volume support for zabbix-proxy