feat: add helm chart #161
feat: add helm chart #161
Conversation
heliapb
force-pushed
the
feat/helm_chart
branch
2 times, most recently
from
4f1ac25 to
5921f90
Compare
|
This issue could be a good way to test the full helm chart and installing custom resources as well, in a separate PR |
|
@heliapb Do you think your could load the current perses-operator docker image into kind and use that to test this scenario? |
ibakshay
left a comment
ibakshay
left a comment
There was a problem hiding this comment.
Thank you very much for this PR! Left few comments :)
| ##@ Helm | ||
| .PHONY: helm-chart | ||
| helm-chart: manifests | ||
| kubebuilder edit --plugins=helm/v1-alpha |
There was a problem hiding this comment.
It will be great if the kubebuilder binary can be installed locally instead of installing it globally by the end-user in advance. Please see how it is done for [jsonnet_binary here](https://github.com/perses/perses-operator/blob/main/Makefile#L316-L320.
There was a problem hiding this comment.
Hi thanks for the feedback, will try to pick up this work again
| - name: Check Helm release | ||
| run: | | ||
| helm status perses-operator --namespace perses-operator-system |
There was a problem hiding this comment.
We also need to have a workflow to release this helm chart. Same like this one
| type: application | ||
| version: 0.1.0 | ||
| appVersion: "0.1.0" | ||
| icon: "https://example.com/icon.png" |
There was a problem hiding this comment.
| icon: "https://example.com/icon.png" | |
| icon: "https://github.com/perses/perses/blob/main/docs/images/perses_logo_lt.png" |
|
I think it would be cool to provide a "Installing Perses in Kubernetes" documentation in perses docs? |
Signed-off-by: Hélia Barroso <helia_barroso@hotmail.com>
| metrics: | ||
| enable: true | ||
|
|
||
| # [PROMETHEUS]: To enable a ServiceMonitor to export metrics to Prometheus set true | ||
| prometheus: | ||
| enable: false | ||
|
|
There was a problem hiding this comment.
Naming this metrics.enable and prometheus.enable might be really confusing. I would just nest it similar to other helm charts that I have used.
| metrics: | |
| enable: true | |
| # [PROMETHEUS]: To enable a ServiceMonitor to export metrics to Prometheus set true | |
| prometheus: | |
| enable: false | |
| metrics: | |
| enable: true | |
| # To enable a ServiceMonitor to export metrics to Prometheus set true | |
| serviceMonitor: | |
| enable: false |
|
Personally I would put the helm chart in |
| container: | ||
| image: | ||
| repository: controller | ||
| tag: latest |
There was a problem hiding this comment.
This should remain empty and the actual deployment.yaml Template should fallback to the AppVersion defined in Chart.yaml.
Thats also how helm recommend it when you run helm create xy
| {{- end }} | ||
| command: | ||
| - /manager | ||
| image: {{ .Values.controllerManager.container.image.repository }}:{{ .Values.controllerManager.container.image.tag }} |
There was a problem hiding this comment.
As mentioned below/above. The image.tag should use .Chart.AppVersion by default and values should be empty.
This example is from helm create xy
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
| replicas: 1 | ||
| container: | ||
| image: | ||
| repository: controller |
There was a problem hiding this comment.
| repository: controller | |
| repository: docker.io/persesdev/perses-operator |
| command: | ||
| - /manager |
There was a problem hiding this comment.
/manager doesn't exist in docker.io/persesdev/perses-operator:latest. I would suggest to allow users to overwrite it but keep it empty by default. No need to overwrite the default entrypoint in the container image.
Something like this (untested)
| command: | |
| - /manager | |
| {{ with .Values.controllerManager.container.command }} | |
| command: {{ . | toYaml | nindent 8 }} | |
| {{ end }} |
| resources: | ||
| limits: | ||
| cpu: 500m | ||
| memory: 128Mi | ||
| requests: | ||
| cpu: 10m | ||
| memory: 64Mi |
There was a problem hiding this comment.
This should not be set by default. Keep it empty:
resources: {}
|
I just noticed that there is already a dedicated helm chart repository here: https://github.com/perses/helm-charts So it would make sense to put the operator helm chart there as well. |
Please have a look at this comment for context - #79 (comment) |
4 participants
From issue #79
Add helm chart using kubebuilder helm plugin