atharvalade commented on code in PR #3070:
URL: https://github.com/apache/iggy/pull/3070#discussion_r3066710863
##########
helm/charts/iggy/templates/servicemonitor.yaml:
##########
@@ -22,9 +22,9 @@ kind: ServiceMonitor
metadata:
labels:
{{- include "iggy.labels" . | nindent 4 }}
- {{- if .Values.server.serviceMonitor.additionalLabels }}
- {{ toYaml .Values.server.serviceMonitor.additionalLabels }}
- {{- end }}
+ {{- if .Values.server.serviceMonitor.additionalLabels }}
+ {{- toYaml .Values.server.serviceMonitor.additionalLabels }}
+ {{- end }}
name: {{ template "iggy.fullname" . }}
Review Comment:
`helmfmt` introduced a regression here. The change from `{{ toYaml to {{-
toYaml `eats the newline+indent between the `iggy.labels` output and the
`additionalLabels`. When a user sets `server.serviceMonitor.additionalLabels`,
the rendered `ServiceMonitor` produces invalid YAML (labels get concatenated
onto the previous line). I verified this locally -- helm template fails with a
YAML parse error. The fix is to use nindent here, e.g.:
```
{{- if .Values.server.serviceMonitor.additionalLabels }}
{{- toYaml .Values.server.serviceMonitor.additionalLabels | nindent 4 }}
{{- end }}
```
##########
helm/charts/iggy/README.md.gotmpl:
##########
Review Comment:
The `gotmpl` template drops the `Troubleshooting`, `Testing`, and `Port
Forward` sections that were in the previous README. The Troubleshooting content
is especially valuable -- it documents specific error messages (Out of memory
(os error 12), Invalid argument panic, ServiceMonitor CRD not found) with
concrete solutions that users running Iggy on Kubernetes will hit due to
`io_uring` requirements. The Testing section with smoke test instructions is
also important for contributors. Please add these sections back into the
`README.md.gotmpl` template so they're preserved in the generated output.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
