Remove scheduler automate serviceaccount token #44173
Conversation
romsharon98
requested review from
dstandish,
jedcunningham and
hussein-awala
as code owners
|
Hmm. I wonder.. Looking at #43464 - yes, when you have K8S executor, you will not be able to work without serviceAutomount set to true. But is it generally true for LocalExecutor and CeleryExecutor for example? I believe should work without automount for those executors? |
|
@potiuk you'd still need it for LocalExecutor and KubernetesPodOperator too. But yes, with CeleryExecutor it should be fine. Or if you don't care about LE+KPO. Not sure removing completely is the right call. |
Yeah. For me it looks like maybe we should detect it when we need it and it is not set and provide a better diagnostics. |
|
didn't fully understand what are the cases that we will need it. |
There was a problem hiding this comment.
I agree with @jedcunningham and @potiuk comments.
Summarising the cases I think we should cover
| @@ -23,7 +23,6 @@ | |||
| {{- if and .Values.scheduler.enabled .Values.scheduler.serviceAccount.create }} | |||
| apiVersion: v1 | |||
| kind: ServiceAccount | |||
| automountServiceAccountToken: {{ .Values.scheduler.serviceAccount.automountServiceAccountToken }} | |||
There was a problem hiding this comment.
These are the possible cases:
- We can set it to true for cases when it is not CeleryExecutor
- For other cases it has to be true or the key can be removed entirely.
Two possible approaches. Either control addition or removal of key based on the executor, otherwise allow to override only for CeleryExecutor
There was a problem hiding this comment.
Looks good! Sorry for the delay @romsharon98
One small nit.
romsharon98
force-pushed
the
remove-scheduler-automate-serviceaccount-token
branch
from
0ea1f3f to
e423f77
Compare
eladkal
force-pushed
the
remove-scheduler-automate-serviceaccount-token
branch
from
0558838 to
7018a19
Compare
closes: #43464
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.