Skip to content
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

Kubernetes decorator #21077

Closed
wants to merge 95 commits into from
Closed

Conversation

subkanthi
Copy link
Contributor

@subkanthi subkanthi commented Jan 24, 2022

Decorator to execute functions in k8s using KubernetesPodOperator

closes: #19135

@task.kubernetes(image='python:3.8-slim-buster', name='k8s_test', namespace='default')
def k8s_decorator_func():
       print("decorator func")

If no parameters passed, defaults are set in kubernetes.py

        # Image, name and namespace are all required.
        if not 'image' in kwargs:
            kwargs['image'] = 'python:3.8-slim-buster'

        if not 'name' in kwargs:
            kwargs['name'] = f'k8s_airflow_pod_{uuid.uuid4().hex}'

        if not 'namespace' in kwargs:
            kwargs['namespace'] = 'default'

This version sets the function as an environment variable - PYTHON_SCRIPT and in the command , the logic is to retrieve the environment variable and write to '/tmp/script.py' to execute the function(very similar to Docker)

  1. Initially proceeded with trying to mount a local file using the hostPath, the problem is that requires Airflow also to run in the same node. For example running airflow locally and connecting to the kind cluster would need a mount.

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

subkanthi and others added 30 commits November 22, 2021 13:36
airflow/providers/cncf/kubernetes/decorators/kubernetes.py Outdated Show resolved Hide resolved
airflow/decorators/__init__.pyi Outdated Show resolved Hide resolved
airflow/decorators/__init__.pyi Outdated Show resolved Hide resolved
@subkanthi subkanthi requested a review from uranusjr April 11, 2022 13:23
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need to add image to the argument list of _KubernetesDecoratedOperator

airflow/decorators/__init__.pyi Outdated Show resolved Hide resolved
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@@ -0,0 +1,44 @@
{#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be in the kube provider folder, else it won't get released with the provider, and it won't work.

@subkanthi
Copy link
Contributor Author

subkanthi commented Apr 27, 2022

@ashb , currently the decorator uses the write_python_script function thats in the utils/python_virtualenv.py file and thats why the template(python_kubernetes_script.jinja2) was also placed in the same folder. If we want to remove the dependency on the utils folder, I guess we can make a copy of the write_python_script in the cncf.providers.kubernetes folder. Question is if there a benefit of having a central place for the function and all templates, Docker decorator and python operator also call this function.

def write_python_script(
    jinja_context: dict,
    filename: str,
    render_template_as_native_obj: bool = False,
    template_file: str = 'python_virtualenv_script.jinja2',
):

@subkanthi subkanthi requested a review from uranusjr April 27, 2022 16:08
@Asciotti
Copy link

Bump? :)

@potiuk potiuk mentioned this pull request May 23, 2022
2 tasks
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also rebase your PR?

@@ -0,0 +1,44 @@
{#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this @subkanthi to the cncf kube providers please?

pod_runtime_info_envs: Optional[List[PodRuntimeInfoEnv]] = None,
termination_grace_period: Optional[int] = None,
configmaps: Optional[List[str]] = None,
**kwargs,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What can go into kwargs here? It seems unnecessary.

Comment on lines +76 to +81
# Set defaults for name and namespace.
if 'name' not in kwargs:
kwargs['name'] = f'k8s_airflow_pod_{uuid.uuid4().hex}'

if 'namespace' not in kwargs:
kwargs['namespace'] = 'default'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, it’s easier to use setdefault for these

return res


T = TypeVar("T", bound=Callable)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused?

@potiuk
Copy link
Member

potiuk commented Sep 1, 2022

Closing, as this has been already implemented and merged in #25663

@potiuk potiuk closed this Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:cncf-kubernetes Kubernetes provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PythonKubernetesOperator and kubernetes taskflow decorator
7 participants