dnskr commented on code in PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#discussion_r3695502263
##########
charts/ozone/templates/helm/om-decommission-job.yaml:
##########
@@ -27,6 +27,10 @@ spec:
labels:
{{- include "ozone.selectorLabels" $ | nindent 8 }}
app.kubernetes.io/component: helm-manager
+ {{- with $.Values.helm.podAnnotations }}
+ annotations:
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
Review Comment:
```suggestion
{{- with $.Values.helm.podAnnotations }}
annotations: {{- toYaml . | nindent 8 }}
{{- end }}
```
I would suggest to inline template values [similar to other
templates](https://github.com/apache/ozone-helm-charts/blob/edc68c92058fe34dfffd1eaf49ef5f19d8d6004a/charts/ozone/templates/datanode/datanode-service.yaml#L29-L31).
##########
charts/ozone/values.yaml:
##########
@@ -303,20 +303,11 @@ helm:
# This can happen if PVC has been deleted or is not reachable.
# This is used for decommissioning OM
backoffLimit: 5
- # Helm Manager persistence (this is enabled automatically if al least one
- # of datanode, scm or om is enabled)
persistence:
- # Enable persistence
- enabled: false
- # Persistence access modes
- accessModes:
- - ReadWriteOnce
- # Path for Storage Container Manager volume mount
+ # Subdirectory under om.persistence.path used for chart-managed markers
+ # (e.g. the OM bootstrap "bootstrapped" stamp). Not a separate PVC —
+ # this path lives on the OM's own persistent volume.
path: /data
Review Comment:
What do you think about renaming `helm.persistence.path` to something
clearer and remove `helm.persistence` completely? Since it isn't related to PVC
or persistence configurations, renaming it would prevent confusion with the
other `*.persistence.*` settings.
##########
charts/ozone/templates/helm/om-leader-transfer-job.yaml:
##########
@@ -26,6 +26,10 @@ spec:
labels:
{{- include "ozone.selectorLabels" $ | nindent 8 }}
app.kubernetes.io/component: helm-manager
+ {{- with $.Values.helm.podAnnotations }}
+ annotations:
+ {{- toYaml . | nindent 8 }}
+ {{- end }}
Review Comment:
```suggestion
{{- with $.Values.helm.podAnnotations }}
annotations: {{- toYaml . | nindent 8 }}
{{- end }}
```
I would suggest to inline template values here as well.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
