You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "shuan1026 (via GitHub)" <gi...@apache.org> on 2026/07/29 17:05:10 UTC
[PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
shuan1026 opened a new pull request, #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53
## What changes were proposed in this pull request?
`charts/ozone/values.yaml` listed several `helm.*` keys that templates never consumed. Users could set them and reasonably expect an effect; nothing happened. This change aligns values with real behavior.
### Wire (keep + consume)
* `helm.resources` and `helm.podAnnotations` are now applied to both helm-manager Jobs:
* `om-leader-transfer-job` (`pre-upgrade`)
* `om-decommission-job` (`post-upgrade`)
* Use same pattern used by OM/SCM/DN/S3G StatefulSets.
### Remove (unused + misleading)
* Drop unused `helm.persistence` keys that never had template consumers: `enabled`, `accessModes`, `size`, `storageClassName`.
* Remove the comment claiming helm-manager persistence is enabled automatically when datanode/scm/om persistence is on (that logic does not exist).
* Fix SCM copy-paste comments on `helm.resources` / `helm.podAnnotations`.
### Keep
* `helm.persistence.path` — still used to build `HELM_MANAGER_PATH` for the OM bootstrap stamp.
## Why no dedicated helm-manager PVC?
Helm Manager is not a long-running workload. It is ephemeral Helm hook Jobs (`pre-upgrade` leader-transfer, `post-upgrade` decommission; deleted after `hook-succeeded` / `hook-failed`). They do not own durable storage:
* Decommission mounts the **existing OM PVC** to wipe content.
* The bootstrap stamp is written on the **OM volume**, not a helm-manager volume.
So `enabled` / `accessModes` / `size` / `storageClassName` had nothing to attach to and implied a PVC API the chart never implemented.
## Why keep `helm.persistence.path`?
`path` is not a PVC knob. It only names a **subdirectory under the OM volume** for chart-managed markers:
```text
HELM_MANAGER_PATH = om.persistence.path + helm.persistence.path
default: /data + /data → /data/data
stamp file: /data/data/bootstrapped
```
The OM bootstrap initContainer uses that path so the stamp stays next to OM data but outside the Ozone metadata tree. Deleting `path` would break (or force hard-coding of) that layout; keeping it is intentional and documented in the values comment.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15993
## How was this patch tested?
* Grep: removed persistence keys absent; `resources` / `podAnnotations` referenced from both Job templates; `helm.persistence.path` still used by bootstrap ConfigMap
* kind smoke: persistence + scale-down still works
* Green CI
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya merged PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "shuan1026 (via GitHub)" <gi...@apache.org>.
shuan1026 commented on PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#issuecomment-5281112625
Done, thanks @Tejaskriya @adoroszlai!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "shuan1026 (via GitHub)" <gi...@apache.org>.
shuan1026 commented on PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#issuecomment-5121088878
PTAL @adoroszlai Thank you!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "shuan1026 (via GitHub)" <gi...@apache.org>.
shuan1026 commented on code in PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#discussion_r3701016577
##########
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:
Agreed, removed helm.persistence entirely — see 3638069.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "shuan1026 (via GitHub)" <gi...@apache.org>.
shuan1026 commented on code in PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#discussion_r3705895164
##########
charts/ozone/values.yaml:
##########
@@ -286,9 +286,9 @@ helm:
env: []
# Additional Helm Manager envFrom items to set up environment variables (templated)
envFrom: []
- # Storage Container Manager resource requests and limits
+ # Helm Manager Job resource requests and limits
resources: {}
- # Extra annotations for Storage Container Manager pods
+ # Extra annotations for Helm Manager Job pods
podAnnotations: {}
# Constrain Storage Container Manager pods to nodes with specific node labels
Review Comment:
Done, thanks @Tejaskriya
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "dnskr (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#discussion_r3703641295
##########
charts/ozone/values.yaml:
##########
@@ -286,9 +286,9 @@ helm:
env: []
# Additional Helm Manager envFrom items to set up environment variables (templated)
envFrom: []
- # Storage Container Manager resource requests and limits
+ # Helm Manager Job resource requests and limits
resources: {}
- # Extra annotations for Storage Container Manager pods
+ # Extra annotations for Helm Manager Job pods
podAnnotations: {}
# Constrain Storage Container Manager pods to nodes with specific node labels
Review Comment:
nit: same fix as above here
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "shuan1026 (via GitHub)" <gi...@apache.org>.
shuan1026 commented on code in PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#discussion_r3701019753
##########
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:
Done, thanks
##########
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:
Done, thanks
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15993. Unused values in ozone helm chart values.yaml [ozone-helm-charts]
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #53:
URL: https://github.com/apache/ozone-helm-charts/pull/53#issuecomment-5677496521
Thanks for the contribution @shuan1026 and review @dnskr
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org