-
Notifications
You must be signed in to change notification settings - Fork 16.4k
AIP-72: Swap KubernetesExecutor to use taskSDK for execution #46860
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
AIP-72: Swap KubernetesExecutor to use taskSDK for execution #46860
Conversation
We shouldn't really be changing it to be a union, we want to make it a totally different interface -- what I did in CeleryExecutor was this: airflow/airflow/jobs/scheduler_job_runner.py Lines 635 to 640 in b715909
So we should instead add a new impl like this airflow/providers/celery/src/airflow/providers/celery/executors/celery_executor.py Lines 507 to 513 in b715909
And we can have different paths for using the KE on v2 vs for v3 |
...cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py
Outdated
Show resolved
Hide resolved
...iders/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py
Outdated
Show resolved
Hide resolved
Yeah you are right, I already implemented the change you suggested from the very beginning. But looks like for some reason I got driven to make this change, probably it was mypy that made me do it! Fixing it |
dea6605 to
34968ec
Compare
...iders/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py
Outdated
Show resolved
Hide resolved
...cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py
Show resolved
Hide resolved
...cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor_utils.py
Show resolved
Hide resolved
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/pod_generator.py
Show resolved
Hide resolved
|
Waiting for a green CI now! |
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/pod_generator.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_pod_generator.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_pod_generator.py
Outdated
Show resolved
Hide resolved
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/test_pod_generator.py
Show resolved
Hide resolved
|
I think i fixed the static check now, lets see how it goes! |
|
Thanks for the approvals, @ashb and @o-nikolas! Merging this one |
closes: #45427
Why?
Approach
Changes to base executor
The interface for "CommandType" has been changed to accept either a sequence string or workloads.ExecuteTask which will be useful for KubernetesExecutor.No interface change for
CommandType, we want to make it a totally different interface -- what was done for CeleryExecutor was this:airflow/airflow/jobs/scheduler_job_runner.py
Lines 635 to 640 in b715909
So we should instead add a new impl like this
airflow/providers/celery/src/airflow/providers/celery/executors/celery_executor.py
Lines 507 to 513 in b715909
And we can have different paths for using the KE on v2 vs for v3.
Changes to K8sExecutor
queue_workloadand_process_workloadsin lieu of what was done for celery executor: Swap CeleryExecutor over to use TaskSDK for execution. #46265The queue_workload is simple, it just enqueues the "workload"
_process_workloadscallsexecute_asyncfor every workload present. And it also pops it fromqueued_tasksand adds it torunningexecute_asyncdoes the usual as it did earlier.Changes to K8sExecutor utils
run_nextwhich is supposed to run the "next" task in the queue in a pod.Where
execute_workloadis a new module (discussed later)Changes to the pod generator
content_json_for_volumeEmptyDir volume
A volume mount at:
/tmp/execute/input.jsonDump the content into this file as received from the K8sExecutor utils
/entrypointNew module:
airflow.sdk.execution_time.execute_workloadTesting Setup (useful setup for anyone wanting to run a hybrid K8s Exec development)
airflowservices locally, but also install it on K8s, maybe using breeze.Install on K8s but do not use that airflow. Use local airflow.
We will leverage and try and connect to the database present on K8s so that we interact with one Db, locally and for workers on K8s.
To do this, port forward the postgres service on K8s to local machine, on 5432.
Export these set of env variables:
Run 3 services:

airflow scheduler,airflow dag-processor,airflow fastapi-api --apps allin 3 terminalsCheck if you can access to the web UI on
localhost:9091orlocalhost:29091Note:
Testing results
DAG being used:
DAG Run

The supervisor and client logs are present on stdout of pod, but the task runner logs are written to a file and we dont get it as of now by design of KubeExecutor: a) Currently kube executor when a pod is running streams out the stdout to logs b) Also logs it to a file inside filesystem: /opt/airflow/logs which can later be sent to either PVC or s3 etc once pod dies.
Logs example:
Once a job is complete, you will not see any logs:

What's next?
serverin supervisor (can do in github comments)executor_configfor various executors using task sdk #46892 for K8sExecutor and then fix the xfail marked tests.^ 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.