Initial Helm Chart implementation #3036

ekazakas · 2024-04-11T19:57:40Z

Thank you for contributing your time to the Mosquitto project!

Before you go any further, please note that we cannot accept contributions if
you haven't signed the Eclipse Contributor Agreement.
If you aren't able to do that, or just don't want to, please describe your bug
fix/feature change in an issue. For simple bug fixes it is can be just as easy
for us to be told about the problem and then go fix it directly.

Then please check the following list of things we ask for in your pull request:

Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
If you are contributing a new feature, is your work based off the develop branch?
If you are contributing a bugfix, is your work based off the fixes branch?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you successfully run make test with your changes locally?

Greetings, I would like to contribute this Helm Chart for Mosquitto project. I would love if anyone could review it and write down necessary fixed, if there have to be any. It would be nice and useful if the project has it's own officially supported Helm Chart to speed up deployments on Kubernetes.

Signed-off-by: kamiKAZIK <eduardas.kazakas@gmail.com>

hardillb · 2024-04-18T09:03:00Z

deploy/chart/templates/configmap.yaml

+    {{ .Values.mosquitto.configFile }}
+    {{- if .Values.mosquitto.authentication.passwordFile }}
+    password_file {{ required ".Values.mosquitto.authentication.passwordFilePath is required!" .Values.mosquitto.authentication.passwordFilePath | quote }}
+    {{- end }}


If there is no password file then you should probably add allow_anonymous true since I don't see a way to enable an auth plugin with this chart.

@hardillb this is actually possible to set-up by adding necessary values using:

{{ .Values.mosquitto.configFile }}

deploy/chart/templates/persistencevolumeclaim.yaml

jlpedrosa · 2024-05-04T00:27:02Z

deploy/chart/templates/_helpers.tpl

+{{- end -}}
+
+{{- define "mosquitto.capabilities.ingress.apiVersion" -}}
+{{- if semverCompare "<1.14-0" (include "mosquitto.capabilities.kubeVersion" .) -}}


the last supported version of kube (not EOL) is 1.27, I don't think you need to support 10 year old versions...

I am more inclined to keep this, as it is already implemented and maybe there are some unlucky guys that are stuck with some very old k8s versions

Up to you! I feel it keeps complexity for something very, very old that is EOL.

jlpedrosa

I indeed would keep the release process in a different PR.
I'd suggest adding a Makefile / similar (and run it in this PR) with https://github.com/norwoodj/helm-docs

.PHONY: helm-docs
helm-docs:
	go run github.com/norwoodj/helm-docs/cmd/helm-docs@latest

Side note: I'm not a maintainer at all.

jlpedrosa · 2024-05-17T00:49:13Z

@hardillb do you know if we can merge this in? it would be highly appreciated.

npdgm · 2024-06-19T14:01:20Z

Hi. I understand you went for a Statefullset instead of a Deployment because of persistence, however this comes with severe drawbacks for apps that run a single replica such as Mosquitto. I would suggest to give the user the choice to run a Deployment with strategy.type: Recreate.
The problem with StatefullSet is the k8s scheduler wont restart Mosquitto on a node crash until it comes back online or is deleted. That's bad for high availability. With a Deployment, the Pod is recreated quickly. Having the right strategy type will prevent rollout issues of concurrent requests to a RWO PersistentVolume, and it is the responsibility of the CSI to prevent edge cases with crashing nodes and ensure no multiple attachments occur.

ekazakas · 2024-06-21T17:39:59Z

Hi. I understand you went for a Statefullset instead of a Deployment because of persistence, however this comes with severe drawbacks for apps that run a single replica such as Mosquitto. I would suggest to give the user the choice to run a Deployment with strategy.type: Recreate. The problem with StatefullSet is the k8s scheduler wont restart Mosquitto on a node crash until it comes back online or is deleted. That's bad for high availability. With a Deployment, the Pod is recreated quickly. Having the right strategy type will prevent rollout issues of concurrent requests to a RWO PersistentVolume, and it is the responsibility of the CSI to prevent edge cases with crashing nodes and ensure no multiple attachments occur.

Maybe if I will have time this/next month, I can remove the SS and create Deployment instead.

ekazakas · 2024-09-23T13:56:31Z

I might need a bit more explanation regarding Deployment vs StatefulSet @npdgm. And preferably some input from project maintainers to decide on what is best for the project. I will have some time in the upcoming 1-1.5 months, so please use this time wisely.

Marck · 2025-02-13T14:55:55Z

@ekazakas any progress on this PR? This would be highly appreciated to have (and have it supported by eclipse-mosquitto)

Marck · 2025-02-22T14:05:01Z

deploy/chart/templates/statefulset.yaml

+        {{- with .Values.mosquitto.persistentVolumeClaim.existingVolume }}
+        volumeName: {{ . }}
+        {{- end }}
+        {{- if or .Values.mosquitto.persistentVolumeClaim.matchLabels .Values.mosquitto.persistentVolumeClaim.matchExpressions) }}


This has to be without the trailing ')':
{{- if or .Values.mosquitto.persistentVolumeClaim.matchLabels .Values.mosquitto.persistentVolumeClaim.matchExpressions }}

Marck · 2025-02-22T14:10:53Z

deploy/chart/values.yaml

+
+  ## Identity Set Management image tag.
+  ##
+  imageTag: 2.0.18-openssl


Latest version is 2.0.20-openssl

Marck · 2025-02-22T14:12:55Z

deploy/chart/values.yaml

+  ## Configure resource requests and limits.
+  ## Ref: https://kubernetes.io/docs/user-guide/compute-resources/
+  ##
+  resources:


I would also configure default limits, for example:

limits: cpu: 200m memory: 512Mi

Marck · 2025-02-22T14:36:30Z

deploy/chart/templates/statefulset.yaml

+      containers:
+        - name: mosquitto
+          {{- if .Values.mosquitto.imageDigest }}
+          image: {{ printf "%s@%s" (required ".Values.mosquitto.image is required!" .Values.mosquitto.image) .Values.mosquitto.imageDigest }}


I think this has to be:

image: {{ printf "%s@%s" (required ".Values.mosquitto.imageDigest is required!" .Values.mosquitto.imageDigest) .Values.mosquitto.imageDigest }}

Marck · 2025-02-22T15:44:16Z

deploy/chart/templates/statefulset.yaml

+        {{- with .Values.mosquitto.pod.volumes }}
+        {{- toYaml . | trim | nindent 8 }}
+        {{- end }}
+  {{- if .Values.mosquitto.persistence.enabled -}}


This is removing one whitespace too many and causing a template error.
Remove the trailing '-':

{{- if .Values.mosquitto.persistence.enabled }}

Marck · 2025-02-22T21:01:57Z

deploy/chart/templates/secret.yaml

+  {{- end }}
+type: Opaque
+data:
+  {{- with .Values.mosquitto.mosquitto.passwordFile }}


This has to be:
{{- with .Values.mosquitto.authentication.passwordFile }}

Marck · 2025-02-25T20:39:52Z

deploy/chart/templates/statefulset.yaml

+            - name: certs
+              mountPath: {{ required ".Values.mosquitto.certificateLocation is required!" .Values.mosquitto.certificateLocation }}
+            {{- end }}
+            {{- if and .Values.mosquitto.authentication.passworFile }}


Typo, has to be:
{{- if and .Values.mosquitto.authentication.passwordFile }}

Marck · 2025-02-25T21:39:35Z

deploy/chart/templates/configmap.yaml

+    log_dest stdout
+    {{ .Values.mosquitto.configFile }}
+    {{- if .Values.mosquitto.authentication.passwordFile }}
+    password_file {{ required ".Values.mosquitto.authentication.passwordFilePath is required!" .Values.mosquitto.authentication.passwordFilePath | quote }}


for password_file the | quote should be removed, it fills the configmap with: "/localtion/of-file" which mqqt doesn't get. Has to be without the double quotes.
Should be:

{{- if .Values.mosquitto.authentication.passwordFile }} password_file {{ required ".Values.mosquitto.authentication.passwordFilePath is required!" .Values.mosquitto.authentication.passwordFilePath }}

ekazakas · 2025-02-28T07:50:07Z

@ekazakas any progress on this PR? This would be highly appreciated to have (and have it supported by eclipse-mosquitto)

@Marck I haven't changed anything since the last time I worked on this PR as the person who wanted some changes never replied, so the chart is still based on StatefulSet. If there is really the need to convert this into a Deployment, I can try to find time for that. I will also go through all your review comments and fix all those issues. Let me know your opinion about StatefulSet vs Deployment.

Marck · 2025-03-02T16:06:37Z

@Marck I haven't changed anything since the last time I worked on this PR as the person who wanted some changes never replied, so the chart is still based on StatefulSet. If there is really the need to convert this into a Deployment, I can try to find time for that. I will also go through all your review comments and fix all those issues. Let me know your opinion about StatefulSet vs Deployment.

@ekazakas No worries, time is precious.
I agree with the points @npdgm makes about the deployment, I also think it's better for a single replica application like mqtt to use a Deployment.

Initial Helm Chart implementation

f0b115c

Signed-off-by: kamiKAZIK <eduardas.kazakas@gmail.com>

hardillb reviewed Apr 18, 2024

View reviewed changes

jlpedrosa reviewed May 4, 2024

View reviewed changes

deploy/chart/templates/persistencevolumeclaim.yaml Outdated Show resolved Hide resolved

jlpedrosa reviewed May 4, 2024

View reviewed changes

deploy/chart/templates/persistencevolumeclaim.yaml Outdated Show resolved Hide resolved

jlpedrosa reviewed May 4, 2024

View reviewed changes

ekazakas added 6 commits May 5, 2024 16:59

Merge branch 'eclipse:develop' into develop

af568ee

Delete deploy/chart/templates/persistencevolumeclaim.yaml

833067a

Added volumeClaimTemplates to statefulset.yaml

f848cee

Update statefulset.yaml

1f9d01c

Update statefulset.yaml

6c02752

Update values.yaml

9c8f107

jlpedrosa reviewed May 5, 2024

View reviewed changes

NigelVanHattum mentioned this pull request May 18, 2024

[FR] Adding Mosquitto as a separate chart with a dependency Koenkk/zigbee2mqtt-chart#2

Closed

Merge branch 'eclipse:develop' into develop

0c4843a

ralight force-pushed the develop branch 3 times, most recently from b7e2d3d to e715b04 Compare October 29, 2024 10:40

Marck reviewed Feb 22, 2025

View reviewed changes

deploy/chart/values.yaml

## Identity Set Management image tag.

##

imageTag: 2.0.18-openssl

Copy link

Marck Feb 22, 2025

Choose a reason for hiding this comment

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

Latest version is 2.0.20-openssl

Marck reviewed Feb 22, 2025

View reviewed changes

Marck reviewed Feb 25, 2025

View reviewed changes

Initial Helm Chart implementation #3036

Are you sure you want to change the base?

Initial Helm Chart implementation #3036

Uh oh!

Conversation

ekazakas commented Apr 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlpedrosa left a comment

Choose a reason for hiding this comment

Uh oh!

jlpedrosa commented May 17, 2024

Uh oh!

npdgm commented Jun 19, 2024

Uh oh!

ekazakas commented Jun 21, 2024

Uh oh!

ekazakas commented Sep 23, 2024

Uh oh!

Marck commented Feb 13, 2025

Uh oh!

Marck Feb 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Marck Feb 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ekazakas commented Feb 28, 2025

Uh oh!

Marck commented Mar 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Marck Feb 22, 2025 •

edited

Loading

Marck Feb 22, 2025 •

edited

Loading

Marck commented Mar 2, 2025 •

edited

Loading