-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fix airflow-webserver cannot create resource pods for k8s exceutor deployed using helm chart #29012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix airflow-webserver cannot create resource pods for k8s exceutor deployed using helm chart #29012
Conversation
be3cf47 to
4cef0f7
Compare
|
Alright, so I attempted this locally where I have no restrictions. As discussed on #28394, scheduled tasks run just fine, but there is an exception when running a task manually: Which is strange, since this is the same service account that is spinning up pods to execute a scheduled task, here is a yaml of that pod definition: I verified whether this SA is able to spin up a pod, and that checks out too: |
|
@arjunanan6 please share the output of |
|
@snjypl Sure, here you go: |
@arjunanan6 it seems like you have not applied the patch in this PR correctly, you can try to manually apply the bellow gist reason why the scheduled task are not having issue is, the scheduled task pod are launched by the in case of manual trigger, the k8s pod is launched by the how are you deploying the helm chart from this PR? if you can share the steps i might be able to help you with it. |
|
@arjunanan6 also, |
f6a4c33 to
ce886c7
Compare
|
@snjypl Yeah, you're right. Not sure what happened there, but now the patch has been added in properly and I can confirm that the the change works. Running a task individually works. :) |
|
LGTM. But would love @jedcunningham or @dstandish to verify it. |
ce886c7 to
6648f1c
Compare
jedcunningham
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this PR does fix the chart, I can't argue there.
However, I'd never actually looked at the manual run code, and it turns out the webserver endpoint kicks off an executor and abandons it! KE pods don't even get cleaned up by it, that falls on a scheduler that ends up adopting the pod. That's the happy path! If you have something happen that causes your worker to not be successful, it may not ever be adopted?
So, in short, I'm a little hesitant to "fix" this chart issue when the underlying feature it relies on is itself flawed. Meaning, I think it's correct in the big picture that the webserver isn't able to launch pods. I'd rather explore fixing the manual run behavior first.
| }, | ||
| show_only=["templates/rbac/pod-launcher-rolebinding.yaml"], | ||
| ) | ||
| print(docs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| print(docs) |
|
So @snjypl are you going to work on it as explained by @jedcunningham ? Or maybe you would like to create theissue to fix the manual behaviour if you are not sure how to fix it? |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
we see the fellowing error in the
webserver logon trying to run the task manually from the UI for k8s executor.in case of k8s executor, the
webserver roleshould be able to launch the pod for manual task trigger. we need to add theairflow-webserver service accounttopod-launcher role.^ 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.