-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
[AIRFLOW-6843] Add delete_option_kwargs to delete_namespaced_pod #7523
Conversation
@petedejoy can you fix the conflicts? |
V1DeleteOptions
of kube client delete_namespaced_pod
requestCo-authored-by: Daniel Imberman <daniel.imberman@gmail.com> Co-authored-by: Kaxil Naik <kaxilnaik@apache.org>
@petedejoy if you want a unit test for this, you can set the timeout to 15 seconds and make sure that once a pod completes it stays up for 15 seconds. @kaxil do you think a test is necessary here? |
5279255
to
b6a11c1
Compare
Codecov Report
@@ Coverage Diff @@
## master #7523 +/- ##
=========================================
Coverage ? 86.85%
=========================================
Files ? 896
Lines ? 42638
Branches ? 0
=========================================
Hits ? 37035
Misses ? 5603
Partials ? 0
Continue to review full report at Codecov.
|
airflow/config_templates/config.yml
Outdated
https://github.com/kubernetes-client/python/blob/41f11a09995efcd0142e25946adc7591431bfb2f/kubernetes/client/models/v1_delete_options.py#L19 | ||
version_added: ~ | ||
type: string | ||
example: "{{\"grace_period_seconds\": 10}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't affect the output, but could be done as:
example: "{{\"grace_period_seconds\": 10}}" | |
example: '{{"grace_period_seconds": 10}}' |
Also: OH GOD. Do we show this as {
or {{
when we render the docs. Becasuse it needs to be {
-- {{
is only needed in default_airflow.cfg because that is a template, but we should show in our docs is not templated/formated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to update the pre-commit script to replace {
with {{
and }
with }}
so that this and the default_airflow.cfg are correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is tricky because of task_log_prefix_template = {{ti.dag_id}}-{{ti.task_id}}-{{execution_date}}-{{try_number}}
. It would replcae it too task_log_prefix_template = {{{{ti.dag_id}}}}-{{{{ti.task_id}}}}-{{{{execution_date}}}}-{{{{try_number}}}}
Maybe we need to update the pre-commit script to replace
{
with{{
and}
with}}
so that this and the default_airflow.cfg are correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated !
This reverts commit a4438f7.
Codecov Report
@@ Coverage Diff @@
## master #7523 +/- ##
=========================================
Coverage ? 86.85%
=========================================
Files ? 896
Lines ? 42638
Branches ? 0
=========================================
Hits ? 37035
Misses ? 5603
Partials ? 0
Continue to review full report at Codecov.
|
(cherry picked from commit 676c851)
(cherry picked from commit 676c851)
(cherry picked from commit 676c851)
(cherry picked from commit 676c851)
Issue link: AIRFLOW-6843
If you're using the Kubernetes executor, Amazon EKS deletes pods very quickly after tasks complete. This can be an issue if you're scraping Airflow logs from a service like FluentD, as it means that the task pod gets deleted before FluentD can pick up logs for fast-running (or fast-failing) tasks.
The
kube_client_request_args
environment variable is passed to thedelete_namespaced_pod
client request, but we need a way to pass thegrace_period_seconds
param (and potentially others) to theV1DeleteOptions
included in the body of the request. Towards that objective, I've created adelete_option_kwargs
object that can be set via an Airflow environment variable. This is object is then passed to theV1DeleteOptions
of thedelete_namespaced_pods
request.This can be set via an
AIRFLOW__KUBERNETES__DELETE_OPTION_KWARGS
env var. The value should be JSON and can contain any of the options available in the Kube clientV1DeleteOptions
class.Make sure to mark the boxes below before creating PR: [x]
[AIRFLOW-NNNN]
. AIRFLOW-NNNN = JIRA ID** For document-only changes commit message can start with
[AIRFLOW-XXXX]
.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.
Read the Pull Request Guidelines for more information.