Miretpl commented on code in PR #60751:
URL: https://github.com/apache/airflow/pull/60751#discussion_r2702545764
##########
chart/files/pod-template-file.kubernetes-helm-yaml:
##########
@@ -42,7 +42,7 @@ metadata:
{{- end }}
annotations:
{{- toYaml $podAnnotations | nindent 4 }}
- {{- if .Values.workers.kerberosInitContainer.enabled }}
+ {{- if or .Values.workers.kubernetes.kerberosInitContainer.enabled
.Values.workers.kerberosInitContainer.enabled }}
Review Comment:
I'm on a fance here with the logic..., cause I believe the below
configuration:
```
workers:
kerberosInitContainer:
resources:
limits:
cpu: 100m
memory: 128Mi
```
should be normally supported (not deprecated due to having
`kerberosInitContainer` in `pod-template-file` and `workers-deployment`), but
this leads to not working intuitively configuration like:
```
workers:
kerberosInitContainer:
enabled: True
celery:
kerberosInitContainer:
enabled: False
```
We could make it work, but it would mean going with `deprecation` logic for
the `enabled` field (which doesn't really make sense to only deprecate one
field in this section 🤔) and not deprecating the whole
`workers.kerberosInitContainer` makes sense as it will be likely the same
configuration for every kerberos-init container across workers.
WDYT?
And question from maybe another perspective, maybe there is no sense in
splitting this particular section into `celery/kubernetes`?
--
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]
