[Monkeytesting]

fix: delete failed pod unless restartPolicy is Never

What this PR does / why we need it:

When a job is finished (Succeeded or Failed) with CleanPodPolicy: Running, pods in Failed state with RestartPolicy set to OnFailure or Always are not cleaned up properly. These pods keep cycling between Running → Failed → Running states indefinitely, causing resource leaks.

This PR fixes the cleanup logic to also delete Failed pods that are configured to restart (RestartPolicy != Never) when the job has finished. Since the job is already completed, these pods serve no purpose and should be cleaned up to release resources.

Changes:

Modified the cleanup condition in DeletePodsAndServices() to include Failed pods with restartable policies
Added comments explaining the edge case

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):
Fixes #

Checklist:

• Docs included if any changes are user facing

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign terrytangyuan for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

🎉 Welcome to the Kubeflow Trainer! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

• If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards.
• Our team will review your PR soon! cc @kubeflow/kubeflow-trainer-team

Join the community:

• Slack: Join our #kubeflow-trainer Slack channel.
• Meetings: Attend the Kubeflow AutoML and Training Working Group bi-weekly meetings.

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

[rishabhsai]

I tried this against the existing DeletePodsAndServices test setup. With a failed job and cleanPodPolicy: Running, an evicted pod with restartPolicy: OnFailure is now deleted, where release-1.9 keeps it. Failed pods with Never and succeeded pods are kept in both.

I'm not sure the Running → Failed → Running cycle can happen, though. Kubernetes only moves a pod to Failed when its containers are not set to restart automatically, and Failed is terminal, so the kubelet won't bring that pod back (pod phase docs). With OnFailure, a crashing container restarts in place and the pod stays Running, which the current check already deletes. That leaves pods the system terminated, like evictions, whose containers have already stopped.

Could you share the status of one of the pods you saw cycling, for example status.phase from kubectl get pod -o yaml? If there's a real case, a TestDeletePodsAndServices case for it would help, since the existing cases only cover a running and a succeeded pod.