You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "atrbgithub (via GitHub)" <gi...@apache.org> on 2025/11/07 15:07:32 UTC
[PR] k8s executor - ensure pods cleaned up [airflow]
atrbgithub opened a new pull request, #58047:
URL: https://github.com/apache/airflow/pull/58047
This commit ensures that completed pods are eventually cleaned up.
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
This relates to https://github.com/apache/airflow/issues/57553
Previously, in say `2.9.3`, `self.job.executor.try_adopt_task_instances` was always called here:
https://github.com/apache/airflow/blob/81845de9d95a733b4eb7826aaabe23ba9813eba3/airflow/jobs/scheduler_job_runner.py#L1641
It was called unconditionally, even if it found no TaskInstances to adopt.
This meant that in the kubernetes executor, we would always call this line:
https://github.com/apache/airflow/blob/81845de9d95a733b4eb7826aaabe23ba9813eba3/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py#L601
This was triggered at startup by calling `adopt_or_reset_orphaned_tasks`, here:
https://github.com/apache/airflow/blob/81845de9d95a733b4eb7826aaabe23ba9813eba3/airflow/jobs/scheduler_job_runner.py#L928
It was also then called frequently, configurable with`orphaned_tasks_check_interval`.
https://github.com/apache/airflow/blob/d9ed7b94d033dc61ee1c05aafb9f1c1f7b82cfb2/airflow/jobs/scheduler_job_runner.py#L1995
The result of this is that if the query that is run to detect adoptable tasks does not find any tasks to adopt, we no longer make a call to `_adopt_completed_pods`, and as a result completed pods are left hanging around. This happens when an old scheduler instance is stopped and a new one takes its place.
This PR partially restores the old behaviour (frequent calls to `_adopt_completed_pods`), but it does not appear to run directly after startup. Even if a call is made to `_adopt_completed_pods` on startup, due to race conditions, you may not catch all pods which enter a completed state but which have not been captured by the query to detect any adoptable tasks, and so there is a requirement to run it again at some point later.
Tests pass with:
```
breeze testing providers-tests --test-type "Providers[cncf]"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #58047: k8s executor - ensure pods cleaned up
URL: https://github.com/apache/airflow/pull/58047
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on PR #58047:
URL: https://github.com/apache/airflow/pull/58047#issuecomment-3680455093
Does this apply to main?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "atrbgithub (via GitHub)" <gi...@apache.org>.
atrbgithub commented on PR #58047:
URL: https://github.com/apache/airflow/pull/58047#issuecomment-3871932899
> Looks great now
Yep, thankfully 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "Nataneljpwd (via GitHub)" <gi...@apache.org>.
Nataneljpwd commented on PR #58047:
URL: https://github.com/apache/airflow/pull/58047#issuecomment-3866502733
@atrbgithub the tests seem to fail, could you fix them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #58047:
URL: https://github.com/apache/airflow/pull/58047#issuecomment-3884407140
Note, that if issues are solved in latest cncf.kubernetes, there is no point in making a change to `v2-11-test`. Providers (with exception of FAB provider that is now merged in v2-11-test and it only works for Airlfow 2 with 1.5.* version) are released from main - so 2.11.1 will use latest released cncf.kubernetes. the latest cncf.kubernetes provider has:
```
"apache-airflow>=2.11.0",
```
And it's already marked in constraints for v2-11-test as the one that will be used in image and "snapshotted" in constraints for 2.11.1
https://github.com/apache/airflow/commit/9e91e3cad7a5c5ebeca043ec05a7f4ccd40bc161#diff-5f68b2472499bba603e952b4530b73920f18ce4b62caaaf28e9f7b5b00605e18R125
You might see there that 2.11.1 will use latest cncf.kubernetes provider (10.12.3)
I will close that PR, but there are two things you will be able to do:
1) now -> if you already use airflow 2.11.0, you can upgrade to latest cncf.kubernetes provider version (10.12.3) and see if it solves your problem
2) when I release 2.11.1rc1 (this week) -> I will send message to devlist and ask people to test it and report back. Please do so. 2.11.1 image will have the latest cncf.kubernetes provider installed and should be ready to test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] k8s executor - ensure pods cleaned up [airflow]
Posted by "atrbgithub (via GitHub)" <gi...@apache.org>.
atrbgithub commented on PR #58047:
URL: https://github.com/apache/airflow/pull/58047#issuecomment-3723315582
Hi @uranusjr , this does still seem to be broken with the latest release, 3.1.5.
To test I did the following:
1. I span up a cluster locally with unmodified code.
2. Span up tasks via the executor
3. stopped the scheduler before they finished.
4. Pods go to a completed state
5. Start the scheduler
6. Pods are never cleaned up
However it seems that in main, the event_scheduler which this change relies on has been removed. To fix the issue in 3.1.5, a diff to `kubernetes_executor.py` looks as follows:
```74a75
> from airflow.utils.event_scheduler import EventScheduler
161a163
> self.event_scheduler: EventScheduler | None = None
252a255,262
> self.event_scheduler = EventScheduler()
>
> self.event_scheduler.call_regular_interval(
> conf.getfloat("scheduler", "orphaned_tasks_check_interval", fallback=300.0),
> self._adopt_completed_pods,
> (self.kube_client,),
> )
>
419a430,432
> if self.event_scheduler:
> self.event_scheduler.run(blocking=False)
>
```
That ensures completed pods are cleaned up. I'm not sure if re-added the scheduler is the right approach for main given that it has been removed, there might be a better way to resolve it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org