HDDS-13992. Support additional annotations for pods#22
HDDS-13992. Support additional annotations for pods#22adoroszlai merged 3 commits intoapache:mainfrom
Conversation
peterxcli
left a comment
There was a problem hiding this comment.
Thanks for this great improvement. LGTM +1
|
@dnskr @Tejaskriya would you like to take a look? |
dnskr
left a comment
There was a problem hiding this comment.
Looks good overall!
I've suggested a couple of minor formatting points that would help with consistency and readability. Also, based on the use case described, I would suggest adding a common podAnnotations property similar to env and envFrom.
| {{- with .Values.datanode.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
| {{- with .Values.datanode.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.datanode.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} |
nit: I suggest adding two spaces to follow the project's formatting approach.
| {{- with .Values.om.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
| {{- with .Values.om.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.om.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} |
nit: I suggest adding two spaces to follow the project's formatting approach.
| {{- with .Values.s3g.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
| {{- with .Values.s3g.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.s3g.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} |
nit: I suggest adding two spaces to follow the project's formatting approach.
| {{- with .Values.scm.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
| {{- with .Values.scm.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} | |
| {{- with .Values.scm.podAnnotations }} | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} |
nit: I suggest adding two spaces to follow the project's formatting approach.
|
Test the precedence with this command instead: $ helm template charts/ozone \
--set "podAnnotations.sidecar\\.istio\\.io/proxyCPU=100m" \
--set "datanode.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=200m" \
--set "om.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=210m" \
--set "s3g.podAnnotations.sidecar\\.istio\\.io/proxyCPU=120m" \
--set "s3g.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=220m" \
--set "scm.podAnnotations.sidecar\\.istio\\.io/proxyCPU=130m" \
--set "scm.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=230m" \
> /tmp/ozone.v1.yamlOutput: --- /tmp/ozone.v0.yaml 2025-11-23 17:43:24
+++ /tmp/ozone.v1.yaml 2025-11-23 21:31:24
@@ -234,16 +234,18 @@
matchLabels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: datanode
template:
metadata:
annotations:
checksum/config: 6d1d3f4f11b1bab096dd5d48cebba6471332987371961847c439d6a9cb229678
+ sidecar.istio.io/proxyCPU: 100m
+ sidecar.istio.io/proxyCPULimit: 200m
labels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: datanode
spec:
containers:
- name: datanode
image: "apache/ozone:2.0.0"
@@ -313,16 +315,18 @@
matchLabels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: om
template:
metadata:
annotations:
checksum/config: 6d1d3f4f11b1bab096dd5d48cebba6471332987371961847c439d6a9cb229678
+ sidecar.istio.io/proxyCPU: 100m
+ sidecar.istio.io/proxyCPULimit: 210m
labels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: om
spec:
containers:
- name: om
image: "apache/ozone:2.0.0"
@@ -398,16 +402,18 @@
matchLabels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: s3g
template:
metadata:
annotations:
checksum/config: 6d1d3f4f11b1bab096dd5d48cebba6471332987371961847c439d6a9cb229678
+ sidecar.istio.io/proxyCPU: 120m
+ sidecar.istio.io/proxyCPULimit: 220m
labels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: s3g
spec:
containers:
- name: s3g
image: "apache/ozone:2.0.0"
@@ -479,16 +485,18 @@
matchLabels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: scm
template:
metadata:
annotations:
checksum/config: 6d1d3f4f11b1bab096dd5d48cebba6471332987371961847c439d6a9cb229678
+ sidecar.istio.io/proxyCPU: 130m
+ sidecar.istio.io/proxyCPULimit: 230m
labels:
app.kubernetes.io/name: ozone
app.kubernetes.io/instance: release-name
app.kubernetes.io/component: scm
spec:
initContainers:
- name: init
image: "apache/ozone:2.0.0" |
dnskr
left a comment
There was a problem hiding this comment.
The changes look great. Thanks for the contribution and for addressing the comments!
|
Thanks @chehsunliu for the patch, @dnskr, @peterxcli for the review. |
What changes were proposed in this pull request?
Support configuring additional pod annotations via values.yaml. This allows users to customize annotations such as those used to control Istio sidecar resources:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13992
How was this patch tested?
$ helm template charts/ozone \ --set "datanode.podAnnotations.sidecar\\.istio\\.io/proxyCPU=100m" \ --set "datanode.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=200m" \ --set "om.podAnnotations.sidecar\\.istio\\.io/proxyCPU=110m" \ --set "om.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=210m" \ --set "s3g.podAnnotations.sidecar\\.istio\\.io/proxyCPU=120m" \ --set "s3g.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=220m" \ --set "scm.podAnnotations.sidecar\\.istio\\.io/proxyCPU=130m" \ --set "scm.podAnnotations.sidecar\\.istio\\.io/proxyCPULimit=230m" \ > /tmp/ozone.v1.yaml $ diff -u /tmp/ozone.*.yamlOutput