search

elyra-ai / elyra

Elyra extends JupyterLab with an AI centric approach.
https://elyra.readthedocs.io/en/stable/
Apache License 2.0
1.81k stars 337 forks source link

Data Volume Mounts to Custom Operators in Apache Airflow to be propagated to the pods not only executors #2874

Open el-aasi opened 1 year ago

el-aasi commented 1 year ago

Is your feature request related to a problem? Please describe.

So it is really nice to see the initiative of adding support for using PVC as common data storages that can be shared between different tasks. The issue is - at least in my case, but maybe I am doing something wrong - that when having a custom KubernetesPodOperator, the data storage is mounted to the executer rather then the pod created by the executer, which in that case is not very useful.

Describe the solution you'd like

I want to be able to attach the PVC to the pods that are created by the executor, as in the below image: image

Describe alternatives you've considered I have tried to pass the volumes/volume_mounts directly as parameters, unfortunately it cannot convert them from dict to V1 volumes, and I am not sure if there is a way I can do that before passing them to the KubernetesPodExecutor

Broken DAG: [/opt/airflow/dags/repo/wip-0810093318.py] Traceback (most recent call last): File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py", line 63, in convert_volume_mount return _convert_kube_model_object(volume_mount, k8s.V1VolumeMount) File "/home/airflow/.local/lib/python3.8/site-packages/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py", line 33, in _convert_kube_model_object raise AirflowException(f"Expected {new_class}, got {type(obj)}") airflow.exceptions.AirflowException: Expected <class 'kubernetes.client.models.v1_volume_mount.V1VolumeMount'>, got <class 'dict'>

kiersten-stokes commented 1 year ago

@el-aasi this was just recently fixed by #2927 and will be included in our 3.12 release in the coming days 🙂

el-aasi commented 1 year ago

@el-aasi this was just recently fixed by #2927 and will be included in our 3.12 release in the coming days 🙂

Awesome, thank you ☺️☺️☺️

kiersten-stokes commented 1 year ago

My apologies @el-aasi, I completely misread your issue last week. Our most recent release actually fixes a bug related to generic operator volumes/mounts (which use the KubernetesPodOperator) rather than changing the processing for a custom KubernetesPodOperator. I'll re-open this issue such that we might be able to look into future solutions for this. Again, so sorry for the misunderstanding!

el-aasi commented 1 year ago

My apologies @el-aasi, I completely misread your issue last week. Our most recent release actually fixes a bug related to generic operator volumes/mounts (which use the KubernetesPodOperator) rather than changing the processing for a custom KubernetesPodOperator. I'll re-open this issue such that we might be able to look into future solutions for this. Again, so sorry for the misunderstanding!

No worries, and thank you for letting me know / re-opening the issue.