Skip to content

Make cncf.kubernetes model deserialization picklable in-cluster#68848

Open
ephraimbuddy wants to merge 1 commit into
apache:mainfrom
astronomer:fix-v1pod-unpicklable-config-provider
Open

Make cncf.kubernetes model deserialization picklable in-cluster#68848
ephraimbuddy wants to merge 1 commit into
apache:mainfrom
astronomer:fix-v1pod-unpicklable-config-provider

Conversation

@ephraimbuddy

Copy link
Copy Markdown
Contributor

The kubernetes client (v36) attaches the process-global in-cluster Configuration to every model it deserializes, and that Configuration's refresh_api_key_hook is an unpicklable local closure. Any deserialized model that later gets pickled -- for example a pod_override placed on the KubernetesExecutor multiprocessing queue -- then crashes.

Deserialize through an ApiClient built with a fresh Configuration in the provider's model-deserialization paths so the resulting models (and every nested object) stay picklable regardless of the kubernetes client version: PodGenerator.deserialize_model_dict, KubernetesJobOperator.deserialize_job_template_file, and the backcompat _convert_from_dict converter.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

claude opus 4.8


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

The kubernetes client (v36) attaches the process-global in-cluster Configuration
to every model it deserializes, and that Configuration's refresh_api_key_hook is
an unpicklable local closure. Any deserialized model that later gets pickled --
for example a pod_override placed on the KubernetesExecutor multiprocessing
queue -- then crashes.

Deserialize through an ApiClient built with a fresh Configuration in the
provider's model-deserialization paths so the resulting models (and every nested
object) stay picklable regardless of the kubernetes client version:
PodGenerator.deserialize_model_dict, KubernetesJobOperator.deserialize_job_template_file,
and the backcompat _convert_from_dict converter.

@amoghrajesh amoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something's up with the diff here, its same as: #68831, requesting changes on this one too to be careful

Comment on lines +112 to +114
import pickle

from kubernetes.client import Configuration

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Top level import pls.

Comment on lines +39 to +41
# A fresh Configuration() keeps the deserialized model picklable: in-cluster, the kubernetes
# client (v36) would otherwise stamp the global Configuration, whose refresh_api_key_hook is
# an unpicklable local closure, onto the model and every nested object.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This same comment seems to repeat 3 times - in here, in job.py and in pod_generator.py, one place is enough.

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 (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants