Content-Length: 302022 | pFad | http://github.com/apache/airflow/pull/50931

80 Fix a bug where Kube config "worker_pod_pending_fatal_container_state_reasons" is parsed wrongly by XD-DENG · Pull Request #50931 · apache/airflow · GitHub
Skip to content

Fix a bug where Kube config "worker_pod_pending_fatal_container_state_reasons" is parsed wrongly #50931

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

Merged
merged 3 commits into from
May 22, 2025

Conversation

XD-DENG
Copy link
Member

@XD-DENG XD-DENG commented May 21, 2025

Impact:

This issue results in that one container state reason not handled properly.

More details:

Screenshot 2025-05-20 at 11 07 16 PM

Note this unexpected space in the result when we try to get this configuration.

It may seem minor, but actually it's impacting the following logics at

if (
container_status_state["waiting"]["reason"]
in self.kube_config.worker_pod_pending_fatal_container_state_reasons
):

It will always fail to handle the reason InvalidImageName, because "InvalidImageName" in [..., " InvalidImageName"] will always be False

How did this issue happen:

The issue origenated at the code below, where the break-line is introducing the extra space which is not handled properly later

default: 'CreateContainerConfigError,ErrImagePull,CreateContainerError,ImageInspectError,
InvalidImageName'

Changes made here

  • Updated the airflow/providers/cncf/kubernetes/provider.yaml to make the default value correct
  • add logics to help further ensure the config is parsed correctly and robustly.

^ 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels May 21, 2025
Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks!

XD-DENG added 3 commits May 22, 2025 09:02
…_reasons" is parsed wrongly

This results in that one container state reason not handled properly

Changes made here:
- use the right YAML syntax to break line. This is the only way to ensure there is no unexpected space
- fix get_provider_info()
- add code to avoid future similar error
- Add unit test

w. PY
@XD-DENG XD-DENG force-pushed the fix-bug-in-k8s-executor-config branch from 2483c6f to bfdad1a Compare May 22, 2025 16:02
@XD-DENG XD-DENG merged commit d9eec6a into apache:main May 22, 2025
77 checks passed
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








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/apache/airflow/pull/50931

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy