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

BugFix: CLI 'kubernetes cleanup-pods' should only clean up Airflow-created Pods #15204

Merged
merged 4 commits into from
Apr 8, 2021

Conversation

XD-DENG
Copy link
Member

@XD-DENG XD-DENG commented Apr 5, 2021

closes: #15193

Currently condition if the pod is created by Airflow is not considered. This commit fixes this.

We decide if the Pod is created by Airflow via checking if it has all the labels added in PodGenerator.construct_pod() or KubernetesPodOperator.create_labels_for_pod().

Test added.

@XD-DENG XD-DENG added area:CLI type:bug-fix Changelog: Bug Fixes provider:cncf-kubernetes Kubernetes provider related issues labels Apr 5, 2021
@XD-DENG XD-DENG added this to the Airflow 2.0.2 milestone Apr 5, 2021
@XD-DENG XD-DENG requested review from potiuk, kaxil and mik-laj April 5, 2021 14:01
@github-actions
Copy link

github-actions bot commented Apr 5, 2021

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

@XD-DENG XD-DENG requested a review from mik-laj April 5, 2021 15:49
@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Apr 5, 2021
@github-actions
Copy link

github-actions bot commented Apr 5, 2021

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease.

Currently condition 'if the pod is created by Airflow' is not considered.
This commit fixes this.

We decide if the Pod is created by Airflow via checking if it has all
the labels added in PodGenerator.construct_pod().
@XD-DENG
Copy link
Member Author

XD-DENG commented Apr 8, 2021

Thanks @mik-laj and @kaxil .

I have added one more commit, c1218d5, to update the help string of this CLI (to highlight we only clean up pods created by KubernetesExecutor/KubernetesPodOperator), so that the scope is clearer to users.

If you have no objection about it, I will merge later today. Cheers

@XD-DENG XD-DENG merged commit c594d9c into apache:master Apr 8, 2021
@XD-DENG XD-DENG deleted the issue-15193 branch April 8, 2021 10:12
ashb pushed a commit that referenced this pull request Apr 15, 2021
…eated Pods (#15204)

closes: #15193

Currently condition if the pod is created by Airflow is not considered. This commit fixes this.

We decide if the Pod is created by Airflow via checking if it has all the labels added in PodGenerator.construct_pod() or KubernetesPodOperator.create_labels_for_pod().

(cherry picked from commit c594d9c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI okay to merge It's ok to merge this PR as it does not require more tests provider:cncf-kubernetes Kubernetes provider related issues type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI 'kubernetes cleanup-pods' should only clean up Airflow-created Pods
3 participants