Skip to content

Add helm chart - #1

Open
giggio wants to merge 4 commits into
marcinbudny:masterfrom
giggio:helmchart
Open

Add helm chart#1
giggio wants to merge 4 commits into
marcinbudny:masterfrom
giggio:helmchart

Conversation

@giggio

@giggio giggio commented Nov 11, 2019

Copy link
Copy Markdown

This is a first iteration to try to create a Helm chart for the service bus exporter. We can discuss it and I'll make the changes you need.

This is self hosted on Github pages, I reached out to the Kubeapps people and they did not reply. See the index file at: https://giggio.github.io/servicebus_exporter/index.yaml

A release was created to make it work: https://github.com/giggio/servicebus_exporter/releases/tag/servicebusexporter-0.0.1

I used the chart releaser from the Helm project to create the index and the release on Github: https://github.com/helm/chart-releaser

Please let me know what you think and we can move it from there.

@marcinbudny

Copy link
Copy Markdown
Owner

Hi giggio,
Thank you for this contribution. I'll take some time to review it, because first I need to learn about creating helm charts a little bit :)

@giggio

giggio commented Nov 11, 2019

Copy link
Copy Markdown
Author

No problem, it is working from my fork. Whenever you have the time!

| `image.repository` | Image name | `marcinbudny/servicebus_exporter` |
| `image.tag` | Image tag | `{TAG_NAME}` |
| `image.pullPolicy` | Image pull policy | `IfNotPresent` |
| `image.pullSecrets` | Specify docker-registry secret names as an array | `[]` (does not add image pull secrets to deployed pods) |

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from imagePullSecrets?

@@ -0,0 +1,2 @@
# fake value so the chart installs

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain purpose of this file?

labels:
{{ include "servicebusexporter.labels" . | indent 4 }}
spec:
replicas: {{ .Values.replicaCount }}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes little sense to use more than one replica, since all of them will be scraped separately. That will result in multiple copies of the metrics and they will be difficult to separate.

app.kubernetes.io/instance: {{ .Release.Name }}
{{- if .Values.addPromAnnotations }}
annotations:
prometheus.io/scrape: "true"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing any custom annotations may be reasonable. The prometheus.io/* annotations are the default for discovery, but it can also be based on other annotations depending on the config.

| `tolerations` | List of node taints to tolerate | `[]` |
| `affinity` | Map of node/pod affinities | `{}` |

The first three parameters and `service.port` map to the command line arguments for the binary.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are mapped to env variables

@marcinbudny

Copy link
Copy Markdown
Owner

Hi giggio,
The chart looks good to me overall. I left some comments on the PR.

However I think it would make more sense if you contributed this chart to the official Helm repo at https://github.com/helm/charts They have several exporter charts already.
It will be difficult for me to take responsibility for maintaining this chart, because I don't use it in my deployments and I'm not an expert on Helm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants