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]

Reply via email to