Skip to content

KubernetesPodOperator: use randomized name to get the failure status#12171

Merged
ashb merged 5 commits intoapache:masterfrom
szczeles:use_random_name_to_get_pod_failure_status
Nov 9, 2020
Merged

KubernetesPodOperator: use randomized name to get the failure status#12171
ashb merged 5 commits intoapache:masterfrom
szczeles:use_random_name_to_get_pod_failure_status

Conversation

@szczeles
Copy link
Contributor

@szczeles szczeles commented Nov 7, 2020

This PR fixes the issue #12151, introduced by my commit here: #12117. It looks I didn't check the pod failure scenario, as none of my pods failed so far, sorry! 🙈

closes: #12151


^ 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.

@boring-cyborg boring-cyborg bot added the k8s label Nov 7, 2020
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

I think it would be great to add a unit test for this scenario (and possibly for success case as well ?)

@szczeles szczeles requested a review from potiuk November 8, 2020 18:56
@szczeles
Copy link
Contributor Author

szczeles commented Nov 8, 2020

That's great idea, Jarek, thanks! Just pushed two unit tests

@XD-DENG XD-DENG added the full tests needed We need to run full set of tests for this PR to merge label Nov 9, 2020
@XD-DENG
Copy link
Member

XD-DENG commented Nov 9, 2020

Thanks for the fix. I just added full-test label and will re-trigger the CI

@potiuk
Copy link
Member

potiuk commented Nov 9, 2020

Just one test failing : test_describes_pod_on_failure

@ashb
Copy link
Member

ashb commented Nov 9, 2020

That may have been my suggested change that I added to this.

@ashb
Copy link
Member

ashb commented Nov 9, 2020

Yup, that was my bad.

@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Nov 9, 2020

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@ashb
Copy link
Member

ashb commented Nov 9, 2020

One test job failed with exitcode 137 in backfill tests. Merging.

@ashb ashb merged commit 3f59e75 into apache:master Nov 9, 2020
@szczeles
Copy link
Contributor Author

szczeles commented Nov 9, 2020

@ashb @potiuk Thanks, guys!

@szczeles szczeles deleted the use_random_name_to_get_pod_failure_status branch October 5, 2021 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kubernetes tests are failing in master

4 participants