Miretpl commented on issue #67261:
URL: https://github.com/apache/airflow/issues/67261#issuecomment-4502896554
@benrifkind could you please add the Helm configuration? It can be hard to
find an issue without it, as it is connected to Helm.
> The RBAC role bindings (pod-launcher, job-launcher) referencing that
service account
It should, and in general it does, but not always, as I checked. I will
create a fix (if someone is not quicker than me) in the upcoming days. I'm not
sure if that will make it to the 1.22 release, as we are already two days past
the cutoff. @potiuk wdyt as the current release manager for that release?
---
For the rest points from expected behaviour, so:
> The Celery worker deployment using that service account
It should work correctly (we have tests for it), as I checked locally:
*command:*
```shell
helm template test chart/ --show-only
templates/workers/worker-deployment.yaml --set
workers.celery.serviceAccount.create=false,workers.celery.serviceAccount.name=test-test-test
| grep serv
iceAccount
```
*results in:*
```yaml
serviceAccountName: "test-test-test"
```
Also, the `workers-serviceaccount.yaml` template is not rendered here, so as
expected.
> The worker service account resource being created with that name (if
create = true)
Same as above:
*command:*
```shell
helm template test chart/ --show-only
templates/workers/worker-serviceaccount.yaml --set
workers.celery.serviceAccount.create=true,workers.celery.serviceAccount.name=t
est-test-test
```
*result:*
```yaml
# Source: airflow/templates/workers/worker-serviceaccount.yaml
apiVersion: v1
kind: ServiceAccount
automountServiceAccountToken: true
metadata:
name: "test-test-test"
labels:
tier: airflow
component: worker
release: test
chart: "airflow-1.21.0"
heritage: Helm
```
So the account is created as expected and:
```shell
helm template test chart/ --show-only
templates/workers/worker-deployment.yaml --set
workers.celery.serviceAccount.create=true,workers.celery.serviceAccount.name=test-test-test
| grep serviceAccount
```
```yaml
serviceAccountName: "test-test-test"
```
---
As for mechanism implementation clarification:
> All Celery worker templates use the worker.serviceAccountName helper,
which reads from workers.serviceAccount (the deprecated path) and never checks
workers.celery.serviceAccount. When a user only sets the new path, the helper
falls back to generating a default name ({{ .Release.Name }}-worker), ignoring
the user's configuration.
In checkes it, but not directly. The key is in the content, which is in
context passed to the function `worker.serviceAccountName`. All of the values
from `workers.celery.*` override the `workers.*` values with a couple of lines
in every worker-related template file. It basically uses the custom merge
function to handle that logic. It was done that way to make Workers Sets and
deprecations work in #60420. The #64730 introduces the new
`worker.kubernetes.serviceAccountName` to fully separate the Kubernetes SA from
the Celery SA. Old was retained unchanged to preserve backward compatibility.
--
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]
