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 when
using KubernetesExecutor, 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) -}}
```
--
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]