Upgrade Helm Chart #43
Conversation
chart/templates/deployment.yaml
Outdated
| @@ -38,8 +38,34 @@ spec: | |||
| value: "{{ .Values.kafka.keystore }}" | |||
| - name: JVM_OPTS | |||
| value: "{{ .Values.jvm.opts }}" | |||
| {{- if .Values.jmx.port }} | |||
There was a problem hiding this comment.
Why do we need the if and end conditional blocks? Can't we just set the value, assuming there is a default in values.yaml?
There was a problem hiding this comment.
Threre is already default values in kafdrop application.yaml and docker entrypoint (kafdrop.sh), so i prefer not overriding them with some others/null values. It seems cleaner to me.
If i leave it as is, it's not so clean, but if i remove them, we need to synchronize default values (values.yaml and application.yaml/kafdrop.sh)
What is your opinion ? Doesn’t matter for me ;)
There was a problem hiding this comment.
I would prefer to avoid conditional macros and just deal with defaults that are clearly stated in values.yaml, so that anyone who wants to know what an unspecified value will default to can check values.yaml.
There was a problem hiding this comment.
No problemo, i'm making changes.
chart/values.yaml
Outdated
| @@ -10,14 +10,22 @@ kafka: | |||
| properties: "" | |||
| truststore: "" | |||
| keystore: "" | |||
| properties_file: "" | |||
There was a problem hiding this comment.
Suggest we use camel-casing here to be consistent with the other entries in values.yaml. I.e. propertiesFile.
There was a problem hiding this comment.
You're right.
| - name: SERVER_SERVLET_CONTEXTPATH | ||
| value: "{{ .Values.server.servlet.contextPath }}" | ||
| {{- end }} | ||
| {{- if .Values.kafka.properties_file }} | ||
| - name: KAFKA_PROPERTIES_FILE |
There was a problem hiding this comment.
Out of curiosity, is there a particular reason why the file location needs to be configurable? When you're using Helm (or any Docker setup), you would be passing in the contents of the properties/truststore/keystore files. Does it matter where they end up being placed inside the container?
I can see how configuring these locations may be useful or necessary when running directly from a Jar. But in what context would these be overridden in Docker?
There was a problem hiding this comment.
In my case, i'm running Kafdrop in a very secure environment, i can't write files anywhere i want to. I need this location to be configurable, so kafdrop.sh can write this file somewhere else than / (root).
Hi folks, I have added some environment variables to the Helm Chart.
Is it OK for you ?