mbaeck5 commented on code in PR #53358:
URL: https://github.com/apache/airflow/pull/53358#discussion_r2355513860
##########
chart/templates/dag-processor/dag-processor-deployment.yaml:
##########
@@ -46,8 +46,10 @@ metadata:
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service }}
- {{- with .Values.labels }}
- {{- toYaml . | nindent 4 }}
+ {{- if or (.Values.labels) (.Values.dagProcessor.labels) }}
+ {{- $global := default (dict) .Values.labels -}}
+ {{- $component := default (dict) .Values.dagProcessor.labels -}}
+ {{- mustMerge $global $component | toYaml | nindent 4 }}
Review Comment:
Was trying out this PR because we needed to label the dagProcessor, but
worker pods were being spawned with the labels meant for the dagProcessor. The
above code seems to be causing it.
From [helm
docs](https://helm.sh/docs/chart_template_guide/function_list/#merge-mustmerge):
mustMerge does not create a deep copy of the dictionaries, so this will
inject the dagProcessor labels into the global values. I think line 50 should
be:
```
{{- $global := deepCopy (default (dict) .Values.labels) -}}
```
##########
chart/templates/dag-processor/dag-processor-serviceaccount.yaml:
##########
@@ -37,8 +37,10 @@ metadata:
release: {{ .Release.Name }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
heritage: {{ .Release.Service }}
- {{- with .Values.labels }}
- {{- toYaml . | nindent 4 }}
+ {{- if or (.Values.labels) (.Values.dagProcessor.labels) }}
+ {{- $global := default (dict) .Values.labels -}}
+ {{- $component := default (dict) .Values.dagProcessor.labels -}}
+ {{- mustMerge $component $global | toYaml | nindent 4 }}
Review Comment:
same as comment above to prevent merging global values
--
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]
