Google Cloud + CNCF Kubernetes OnFinishAction equality test

[miloszszymczak]

Apache Airflow Provider(s)

cncf-kubernetes, google

Versions of Apache Airflow Providers

I guess you can take the latest ones, should be reproducible there, but definitely those included in official Docker image for 2.10.5.

Apache Airflow version

2.10.5

Operating System

Linux

Deployment

Other Docker-based deployment

Deployment details

Using official Docker image in k8s (issue is related to usage of GKEStartPodOperator)

What happened

Starting at this line

airflow/providers/google/src/airflow/providers/google/cloud/operators/kubernetes_engine.py

Line 641 in 2b1c275

if is_delete_operator_pod is not None:

a new approach to pod deletion is utilized so we are on par with CNCF k8s package enum OnFinishAction. Unfortunately the values set there don't compare with the values withing CNCF k8s package (maybe it's because differences in imports) resulting in failing equality tests here

airflow/providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py

Line 1047 in 2b1c275

should_delete_pod = (self.on_finish_action == OnFinishAction.DELETE_POD) or (

What you think should happen instead

I guess another way to compare the enums could be used here so the pods are deleted if operator is properly configured.

How to reproduce

Use GKEStartPodOperator to run a job in k8s environment with on_finish_action='DELETE_POD' - the pod will be kept with message "Skipping deleting pod" instead of deletion.

Anything else

I changed the equality test to (str(self.on_finish_action) == str(OnFinishAction.DELETE_POD)) and it worked properly although it's not the best way to fix it. Looks like the property self.on_finish_action is of type str and cannot be directly compared to enum.

Skipping deleting pod

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[boring-cyborg]

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

[sepandar-sepehr]

Any update on this issue? We noticed the same problem after upgrading our google composer to version 2.12.1

[neko1437]

Apache Airflow Version

• 2.11.0

Apache Airflow Providers Versions

• apache-airflow-providers-cncf-kubernetes==10.5.0
• apache-airflow-providers-google==15.1.0

---

Issue Description

I encountered the same issue when using the GKEStartPodOperator with on_finish_action="delete_pod". Despite this configuration, the pod does not get deleted at the end of a task run. The task logs contain messages like:

"Skipping deleting pod ..." 

---

Diagnostics

To isolate the cause, I downgraded the apache-airflow-providers-google package step by step. I found that the issue was introduced in version 12.0.0. Everything works as expected when using:

• Airflow Version 2.10.4
• apache-airflow-providers-cncf-kubernetes==10.5.0
• apache-airflow-providers-google==11.0.0

With this combination, the pod is correctly deleted at the end of the task.

---

Impact

This issue significantly affects task reliability. When a task fails and retries are enabled, the retry attempt gets skipped because it detects an existing pod (with the same labels) that was not cleaned up. This behavior undermines Airflow’s retry mechanism and can lead to missed or incomplete processing.

Given the potential for skipped retries and unintended side effects, I believe this is a critical issue that should be addressed.

[agomez-etsy]

👋 We also faced this error and found a workaround. Our setup:

• Airflow Version 2.10.5
• apache-airflow-providers-cncf-kubernetes==10.1.0
• apache-airflow-providers-google==12.0.0

The bug was introduced in the google provider 12.0.0, specifically in this commit.

on_finish_action was added to the list of template_fields of the GKEStartPodOperator, as shown in the screenshot

This causes self.on_finish_action to be rendered as a string (OnFinishAction.DELETE_POD) when the task is executed, instead of the original enum instance (<OnFinishAction.DELETE_POD: 'delete_pod'>), and hence the should_delete_pod equality returns False.

self.on_finish_action == OnFinishAction.DELETE_POD

Some ideas for whoever wants to submit the PR:

1. Remove on_finish_action from template_fields

Simple, yet effective :).

2. Override process_pod_deletion

def process_pod_deletion(self, pod: k8s.V1Pod, *, reraise=True):
    from airflow.providers.cncf.kubernetes.utils.pod_manager import OnFinishAction

    if isinstance(self.on_finish_action, str):
        # Handle the case where the enum was casted using `str(on_finish_action)`
        # which results in a string like `OnFinishAction.DELETE_POD`
        if self.on_finish_action.startswith("OnFinishAction."):
            self.on_finish_action = OnFinishAction[self.on_finish_action.split(".")[-1]]
        else:
            # Handle the case where `on_finish_action = on_finish_action.value`
            # which results in a string like `delete_pod`
            try:
                self.on_finish_action = OnFinishAction(self.on_finish_action)
            except KeyError:
                self.log.error(f"Invalid on_finish_action {self.on_finish_action}")

    super().process_pod_deletion(pod, reraise)

3. Override render_template_fields

def render_template_fields(self, context: Context, jinja_env: Environment | None = None):
    # Remove "on_finish_action" from template fields if it's not a jinja template string
    # to prevent the enum from being casted as a string
    if not (isinstance(self.on_finish_action, str) and "{{" in self.on_finish_action):
        self.template_fields = [f for f in self.template_fields if f != "on_finish_action"]

    super().render_template_fields(context, jinja_env)

We are using approach 2, but any of these should work.

cc - @miloszszymczak

[olegkachur-e]

1. Remove on_finish_action from template_fields

Simple, yet effective :).

Seems it's already been done in #49637 and released in apache-airflow-providers-google==16.0.0.

[olegkachur-e]

I've checked it again and it is not reproducible now.

@potiuk Can we close it, pls?